SONAR-1832:

* Improve performance for "Architecture rule"
* Reduce number of violations whose line number is 0

6 files changed, 55 insertions, 48 deletions

diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/PatternUtils.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/PatternUtils.java
index 048e39a0a00..9f2d927677f 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/PatternUtils.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/PatternUtils.java
@@ -12,7 +12,7 @@ public final class PatternUtils {
   private PatternUtils() {
   }
 
-  public static List<WildcardPattern> createMatchers(String pattern) {
+  public static WildcardPattern[] createMatchers(String pattern) {
     List<WildcardPattern> matchers = Lists.newArrayList();
     if (StringUtils.isNotEmpty(pattern)) {
       String[] patterns = pattern.split(",");
@@ -21,10 +21,10 @@ public final class PatternUtils {
         matchers.add(WildcardPattern.create(p));
       }
     }
-    return matchers;
+    return matchers.toArray(new WildcardPattern[matchers.size()]);
   }
 
-  public static boolean matches(String text, List<WildcardPattern> matchers) {
+  public static boolean matches(String text, WildcardPattern[] matchers) {
     for (WildcardPattern matcher : matchers) {
       if (matcher.match(text)) {
         return true;
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/UndocumentedApiCheck.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/UndocumentedApiCheck.java
index c508ad95388..0a8aa111b04 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/UndocumentedApiCheck.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/UndocumentedApiCheck.java
@@ -28,7 +28,7 @@ public class UndocumentedApiCheck extends JavaAstCheck {
   @RuleProperty(description = "Optional. If this property is not defined, all classes should adhere to this constraint. Ex : **.api.**")
   private String forClasses = new String();
 
-  private List<WildcardPattern> matchers;
+  private WildcardPattern[] matchers;
 
   @Override
   public List<Integer> getWantedTokens() {
@@ -49,7 +49,7 @@ public class UndocumentedApiCheck extends JavaAstCheck {
     }
   }
 
-  private List<WildcardPattern> getMatchers() {
+  private WildcardPattern[] getMatchers() {
     if (matchers == null) {
       matchers = PatternUtils.createMatchers(StringUtils.defaultIfEmpty(forClasses, "**"));
     }
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/check/ArchitectureCheck.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/check/ArchitectureCheck.java
index ee89cf2100f..c4a165d07ed 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/check/ArchitectureCheck.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/check/ArchitectureCheck.java
@@ -25,12 +25,13 @@ import org.sonar.check.*;
 import org.sonar.java.PatternUtils;
 import org.sonar.java.bytecode.asm.AsmClass;
 import org.sonar.java.bytecode.asm.AsmEdge;
+import org.sonar.java.bytecode.asm.AsmMethod;
 import org.sonar.squid.api.CheckMessage;
 import org.sonar.squid.api.SourceFile;
+import org.sonar.squid.api.SourceMethod;
 
 import com.google.common.collect.Maps;
 
-import java.util.List;
 import java.util.Map;
 
 @Rule(key = "ArchitecturalConstraint", name = "Architectural constraint", cardinality = Cardinality.MULTIPLE, isoCategory = IsoCategory.Portability, priority = Priority.MAJOR, description = "<p>A source code comply to an architectural model when it fully adheres to a set of architectural constraints. " +
@@ -47,8 +48,8 @@ public class ArchitectureCheck extends BytecodeCheck {
   @RuleProperty(description = "Mandatory. Ex : java.util.Vector, java.util.Hashtable, java.util.Enumeration")
   private String toClasses = new String();
 
-  private List<WildcardPattern> fromMatchers;
-  private List<WildcardPattern> toMatchers;
+  private WildcardPattern[] fromMatchers;
+  private WildcardPattern[] toMatchers;
   private AsmClass asmClass;
   private Map<String, CheckMessage> internalNames;
 
@@ -70,49 +71,70 @@ public class ArchitectureCheck extends BytecodeCheck {
 
   @Override
   public void visitClass(AsmClass asmClass) {
-    this.asmClass = asmClass;
-    this.internalNames = Maps.newHashMap();
+    String nameAsmClass = asmClass.getInternalName();
+    if (PatternUtils.matches(nameAsmClass, getFromMatchers())) {
+      this.asmClass = asmClass;
+      this.internalNames = Maps.newHashMap();
+    } else {
+      this.asmClass = null;
+    }
   }
 
   @Override
   public void leaveClass(AsmClass asmClass) {
-    for (CheckMessage message : internalNames.values()) {
-      SourceFile sourceFile = getSourceFile(asmClass);
-      sourceFile.log(message);
+    if (this.asmClass != null) {
+      for (CheckMessage message : internalNames.values()) {
+        SourceFile sourceFile = getSourceFile(asmClass);
+        sourceFile.log(message);
+      }
     }
   }
 
   @Override
   public void visitEdge(AsmEdge edge) {
-    if (edge != null) {
+    if (asmClass != null && edge != null) {
       String internalNameTargetClass = edge.getTargetAsmClass().getInternalName();
       if ( !internalNames.containsKey(internalNameTargetClass)) {
-        String nameAsmClass = asmClass.getInternalName();
-        if (PatternUtils.matches(nameAsmClass, getFromMatchers()) && PatternUtils.matches(internalNameTargetClass, getToMatchers())) {
-          logMessage(edge);
+        if (PatternUtils.matches(internalNameTargetClass, getToMatchers())) {
+          int sourceLineNumber = getSourceLineNumber(edge);
+          logMessage(asmClass.getInternalName(), internalNameTargetClass, sourceLineNumber);
+        }
+      } else {
+        int sourceLineNumber = getSourceLineNumber(edge);
+        // we log only first occurrence with non-zero line number if exists
+        if (internalNames.get(internalNameTargetClass).getLine() == 0 && sourceLineNumber != 0) {
+          logMessage(asmClass.getInternalName(), internalNameTargetClass, sourceLineNumber);
+        }
+      }
+    }
+  }
+
+  private int getSourceLineNumber(AsmEdge edge) {
+    if (edge.getSourceLineNumber() == 0) {
+      if (edge.getFrom() instanceof AsmMethod) {
+        SourceMethod sourceMethod = getSourceMethod((AsmMethod) edge.getFrom());
+        if (sourceMethod != null) {
+          return sourceMethod.getStartAtLine();
         }
-      } else if (internalNames.get(internalNameTargetClass).getLine() == 0 && edge.getSourceLineNumber() != 0) {
-        logMessage(edge);
       }
     }
+    return edge.getSourceLineNumber();
   }
 
-  private void logMessage(AsmEdge edge) {
-    String fromClass = asmClass.getInternalName();
-    String toClass = edge.getTargetAsmClass().getInternalName();
+  private void logMessage(String fromClass, String toClass, int sourceLineNumber) {
     CheckMessage message = new CheckMessage(this, fromClass + " must not use " + toClass);
-    message.setLine(edge.getSourceLineNumber());
+    message.setLine(sourceLineNumber);
     internalNames.put(toClass, message);
   }
 
-  private List<WildcardPattern> getFromMatchers() {
+  private WildcardPattern[] getFromMatchers() {
     if (fromMatchers == null) {
       fromMatchers = PatternUtils.createMatchers(StringUtils.defaultIfEmpty(fromClasses, "**"));
     }
     return fromMatchers;
   }
 
-  private List<WildcardPattern> getToMatchers() {
+  private WildcardPattern[] getToMatchers() {
     if (toMatchers == null) {
       toMatchers = PatternUtils.createMatchers(toClasses);
     }
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/check/ArchitectureCheckTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/check/ArchitectureCheckTest.java
index 1a93c99244b..f4c140d72e1 100644
--- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/check/ArchitectureCheckTest.java
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/bytecode/check/ArchitectureCheckTest.java
@@ -52,9 +52,9 @@ public class ArchitectureCheckTest {
 
     SourceFile file = (SourceFile) squid.search("ArchitectureCheckDateForbidden.java");
     assertThat(file.getCheckMessages().size(), is(2));
-    // for (CheckMessage message : file.getCheckMessages()) {
-    // System.out.println(message.getDefaultMessage());
-    // }
+    for (CheckMessage message : file.getCheckMessages()) {
+      System.out.println(message.getDefaultMessage() + " at line " + message.getLine());
+    }
   }
 
   @Test
@@ -62,10 +62,9 @@ public class ArchitectureCheckTest {
     check("*UI", "java.sql.*");
 
     SourceFile file = (SourceFile) squid.search("ArchitectureCheckToSqlFromUI.java");
-    assertThat(file.getCheckMessages().size(), is(4));
-    // for (CheckMessage message : file.getCheckMessages()) {
-    // System.out.println(message.getDefaultMessage() + " at line " + message.getLine());
-    // }
+    assertThat(file.getCheckMessages().size(), is(1));
+    CheckMessage message = file.getCheckMessages().iterator().next();
+    assertThat(message.getLine(), is(4));
   }
 
   @Test
diff --git a/plugins/sonar-squid-java-plugin/test-resources/bytecode/architecture/bin/ArchitectureCheckToSqlFromUI.class b/plugins/sonar-squid-java-plugin/test-resources/bytecode/architecture/bin/ArchitectureCheckToSqlFromUI.class
index 03cfe2219d7..bf8f8c0c33d 100644
--- a/plugins/sonar-squid-java-plugin/test-resources/bytecode/architecture/bin/ArchitectureCheckToSqlFromUI.class
+++ b/plugins/sonar-squid-java-plugin/test-resources/bytecode/architecture/bin/ArchitectureCheckToSqlFromUI.class
diff --git a/plugins/sonar-squid-java-plugin/test-resources/bytecode/architecture/src/ArchitectureCheckToSqlFromUI.java b/plugins/sonar-squid-java-plugin/test-resources/bytecode/architecture/src/ArchitectureCheckToSqlFromUI.java
index 18f58ba4801..eaa2d7f462f 100644
--- a/plugins/sonar-squid-java-plugin/test-resources/bytecode/architecture/src/ArchitectureCheckToSqlFromUI.java
+++ b/plugins/sonar-squid-java-plugin/test-resources/bytecode/architecture/src/ArchitectureCheckToSqlFromUI.java
@@ -1,21 +1,7 @@
-import java.sql.Connection;
-import java.sql.ResultSet;
-import java.sql.SQLException;
 import java.sql.Statement;
 
-
 public class ArchitectureCheckToSqlFromUI {
-
-  ResultSet result;
-
-  Connection connection;
-
-  public ArchitectureCheckToSqlFromUI(Statement statement, String requete) {
-    try {
-      connection = statement.getConnection();
-      result = statement.executeQuery(requete);
-    } catch (SQLException sql) {
-      sql.printStackTrace();
-    }
+  public ArchitectureCheckToSqlFromUI(Statement statement) {
+    System.out.println(statement);
   }
 }