From 2d82eec709d7076dc86ec51fce9e6cf22363075a Mon Sep 17 00:00:00 2001 From: Godin Date: Mon, 1 Nov 2010 15:18:10 +0000 Subject: [PATCH] SONAR-1832: * Improve performance for "Architecture rule" * Reduce number of violations whose line number is 0 --- .../java/org/sonar/java/PatternUtils.java | 6 +- .../java/ast/check/UndocumentedApiCheck.java | 4 +- .../bytecode/check/ArchitectureCheck.java | 62 ++++++++++++------ .../bytecode/check/ArchitectureCheckTest.java | 13 ++-- .../bin/ArchitectureCheckToSqlFromUI.class | Bin 846 -> 526 bytes .../src/ArchitectureCheckToSqlFromUI.java | 18 +---- 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 createMatchers(String pattern) { + public static WildcardPattern[] createMatchers(String pattern) { List 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 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 matchers; + private WildcardPattern[] matchers; @Override public List getWantedTokens() { @@ -49,7 +49,7 @@ public class UndocumentedApiCheck extends JavaAstCheck { } } - private List 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 = "

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 fromMatchers; - private List toMatchers; + private WildcardPattern[] fromMatchers; + private WildcardPattern[] toMatchers; private AsmClass asmClass; private Map 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 getFromMatchers() { + private WildcardPattern[] getFromMatchers() { if (fromMatchers == null) { fromMatchers = PatternUtils.createMatchers(StringUtils.defaultIfEmpty(fromClasses, "**")); } return fromMatchers; } - private List 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 03cfe2219d7766c0cf597015dc834e004cf1a9ec..bf8f8c0c33d1206a07d1c23f63f6d5c8e7ec7bfc 100644 GIT binary patch delta 300 zcmZvW%Syvg5Qe{##2lMOt)>_6;;vpG)}v{>_)V79~@fk%~^rmkry-2Eg?%z>iO%yv|D z^t3zrIm%+y3S8zj3qFf1X_kFfSk1oF-b1Au-9=$9O0UAs{f&wH3VU#C6t&}GlMKSs zVUn7T8OBk+tqA@ZEA(|9>ZQUp$6#^tp1dqUP{_Ui%!BBhgj0!Z@!^k mu(n>YpT%30M5nPuodPZk=83D1^OW($3CWb?^0PDIEGmEYiZ;Ul literal 846 zcmaJ<-A)rh7(LV0E-hPH3W#W1EsFF9XuPg5F^wc94F(sv->#E(VY^GGvj$(m7xCH) zHF)6z_)x|(71{NMZ1$V)f6jO2`}O^m7K?c7FZvixgXr#d=~WHE?kb(eSxAEg`xDcAB6%NNzUNL&7}NuKlJqrfp#m& zaI~xCREE0$*B7|qxaVl)hvR8^0gHu=e#=drD z2~1!!)&6++>e7=J1{opb^u(VFG=?vgH}N$Q$4U++(mOwioLO+FqUpQW%qz_Q!TN9H z1gwCio+8aWwpqF8zI&^K6{w_SH&s%@-b*Fwui8yy=qN;bOKvr)t{8NDIh zNzG$pAG-pz+nNhh5^!%uXPj6d+cL~nlcveArLok>qmjy1J0UX(kRn@NN-x&(IF{<8 zKYdM6I@kBJ#*g}m