Fix quality flaw: Migrate to precise issue location

benzonico · benzonico · commit 37c91855fcbd · 2016-02-19T16:51:55.000+01:00
diff --git a/java-checks/src/main/java/org/sonar/java/checks/AbstractDeprecatedChecker.java b/java-checks/src/main/java/org/sonar/java/checks/AbstractDeprecatedChecker.java
@@ -21,6 +21,7 @@
 
 import com.google.common.collect.Lists;
 import org.sonar.java.ast.visitors.PublicApiChecker;
+import org.sonar.java.checks.helpers.ExpressionsHelper;
 import org.sonar.plugins.java.api.tree.AnnotationTree;
 import org.sonar.plugins.java.api.tree.ClassTree;
 import org.sonar.plugins.java.api.tree.IdentifierTree;
@@ -83,6 +84,18 @@ private static boolean hasDeprecatedAnnotation(Iterable<AnnotationTree> annotati
     return false;
   }
 
+  protected static Tree getReportTree(Tree tree) {
+    Tree reportTree = tree;
+    if(reportTree.is(PublicApiChecker.classKinds())) {
+      reportTree = ExpressionsHelper.reportOnClassTree(((ClassTree) reportTree));
+    } else if(reportTree.is(PublicApiChecker.methodKinds())) {
+      reportTree = ((MethodTree) reportTree).simpleName();
+    } else if(reportTree.is(Tree.Kind.VARIABLE)) {
+      reportTree = ((VariableTree) reportTree).simpleName();
+    }
+    return reportTree;
+  }
+
   public static boolean isDeprecated(AnnotationTree tree) {
     return tree.annotationType().is(Kind.IDENTIFIER) &&
       "Deprecated".equals(((IdentifierTree) tree.annotationType()).name());
diff --git a/java-checks/src/main/java/org/sonar/java/checks/AbstractSerializableInnerClassRule.java b/java-checks/src/main/java/org/sonar/java/checks/AbstractSerializableInnerClassRule.java
@@ -50,13 +50,13 @@ private void visitClassTree(ClassTree classTree) {
       if (owner.isTypeSymbol()) {
         Symbol.TypeSymbol ownerType = (Symbol.TypeSymbol) owner;
         if (isMatchingOuterClass(ownerType.type()) && !symbol.isStatic()) {
-          addIssue(classTree, "Make this inner class static");
+          reportIssue(classTree.simpleName(), "Make this inner class static");
         }
       } else if (owner.isMethodSymbol()) {
         Symbol.TypeSymbol methodOwner = (Symbol.TypeSymbol) owner.owner();
         if (isMatchingOuterClass(methodOwner.type()) && !owner.isStatic()) {
           String methodName = owner.name();
-          addIssue(classTree, "Make \"" + methodName + "\" static");
+          reportIssue(classTree.simpleName(), "Make \"" + methodName + "\" static");
         }
       }
     }
diff --git a/java-checks/src/main/java/org/sonar/java/checks/CustomCryptographicAlgorithmCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CustomCryptographicAlgorithmCheck.java
@@ -23,6 +23,7 @@
 import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.ExpressionsHelper;
 import org.sonar.java.tag.Tag;
 import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.tree.ClassTree;
@@ -51,8 +52,9 @@ public List<Kind> nodesToVisit() {
 
   @Override
   public void visitNode(Tree tree) {
-    if (hasSemantic() && isJavaSecurityMessageDigestSubClass((ClassTree) tree)) {
-      addIssue(tree, "Use a standard algorithm instead of creating a custom one.");
+    ClassTree classTree = (ClassTree) tree;
+    if (hasSemantic() && isJavaSecurityMessageDigestSubClass(classTree)) {
+      reportIssue(ExpressionsHelper.reportOnClassTree(classTree), "Use a standard algorithm instead of creating a custom one.");
     }
   }
 
diff --git a/java-checks/src/main/java/org/sonar/java/checks/DeprecatedTagPresenceCheck.java b/java-checks/src/main/java/org/sonar/java/checks/DeprecatedTagPresenceCheck.java
@@ -41,7 +41,7 @@ public class DeprecatedTagPresenceCheck extends AbstractDeprecatedChecker {
   @Override
   public void visitNode(Tree tree) {
     if (hasDeprecatedAnnotation(tree) || hasJavadocDeprecatedTag(tree)) {
-      addIssue(tree, "Do not forget to remove this deprecated code someday.");
+      reportIssue(getReportTree(tree), "Do not forget to remove this deprecated code someday.");
     }
   }
 
diff --git a/java-checks/src/main/java/org/sonar/java/checks/EqualsNotOverriddenInSubclassCheck.java b/java-checks/src/main/java/org/sonar/java/checks/EqualsNotOverriddenInSubclassCheck.java
@@ -58,7 +58,7 @@ public List<Kind> nodesToVisit() {
   public void visitNode(Tree tree) {
     ClassTree classTree = (ClassTree) tree;
     if (hasSemantic() && hasAtLeastOneField(classTree) && !implementsEquals(classTree) && parentClassImplementsEquals(classTree)) {
-      addIssue(classTree, "Override this superclass' \"equals\" method.");
+      reportIssue(classTree.simpleName(), "Override this superclass' \"equals\" method.");
     }
   }
 
diff --git a/java-checks/src/main/java/org/sonar/java/checks/InnerClassTooManyLinesCheck.java b/java-checks/src/main/java/org/sonar/java/checks/InnerClassTooManyLinesCheck.java
@@ -24,6 +24,7 @@
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;
+import org.sonar.java.checks.helpers.ExpressionsHelper;
 import org.sonar.java.syntaxtoken.FirstSyntaxTokenFinder;
 import org.sonar.java.syntaxtoken.LastSyntaxTokenFinder;
 import org.sonar.java.tag.Tag;
@@ -70,7 +71,7 @@ public void visitNode(Tree tree) {
       int last = LastSyntaxTokenFinder.lastSyntaxToken(node).line();
       int length = last - first + 1;
       if (length > max) {
-        addIssue(node, "Reduce this class from " + length + " to the maximum allowed " + max + " or externalize it in a public class.");
+        reportIssue(ExpressionsHelper.reportOnClassTree(node), "Reduce this class from " + length + " to the maximum allowed " + max + " or externalize it in a public class.");
       }
     }
   }
diff --git a/java-checks/src/main/java/org/sonar/java/checks/MissingDeprecatedCheck.java b/java-checks/src/main/java/org/sonar/java/checks/MissingDeprecatedCheck.java
@@ -62,9 +62,9 @@ public void visitNode(Tree tree) {
     boolean hasJavadocDeprecatedTag = hasJavadocDeprecatedTag(tree);
     if (currentClassNotDeprecated() && !isLocalVar) {
       if (hasDeprecatedAnnotation && !hasJavadocDeprecatedTag) {
-        addIssue(tree, "Add the missing @deprecated Javadoc tag.");
+        reportIssue(getReportTree(tree), "Add the missing @deprecated Javadoc tag.");
       } else if (hasJavadocDeprecatedTag && !hasDeprecatedAnnotation) {
-        addIssue(tree, "Add the missing @Deprecated annotation.");
+        reportIssue(getReportTree(tree), "Add the missing @Deprecated annotation.");
       }
     }
     if (tree.is(CLASS_KINDS)) {
diff --git a/java-checks/src/main/java/org/sonar/java/checks/SynchronizedClassUsageCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SynchronizedClassUsageCheck.java
@@ -25,6 +25,7 @@
 import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.ExpressionsHelper;
 import org.sonar.java.model.declaration.MethodTreeImpl;
 import org.sonar.java.tag.Tag;
 import org.sonar.plugins.java.api.semantic.Type;
@@ -78,7 +79,7 @@ private class DeprecatedTypeVisitor extends BaseTreeVisitor {
     public void visitClass(ClassTree tree) {
       TypeTree superClass = tree.superClass();
       if (superClass != null) {
-        reportIssueOnDeprecatedType(tree, superClass.symbolType());
+        reportIssueOnDeprecatedType(ExpressionsHelper.reportOnClassTree(tree), superClass.symbolType());
       }
 
       scan(tree.members());
@@ -110,7 +111,7 @@ public void visitVariable(VariableTree tree) {
 
     private boolean reportIssueOnDeprecatedType(Tree tree, Type type) {
       if (isDeprecatedType(type)) {
-        addIssue(tree, "Replace the synchronized class \"" + type.name() + "\" by an unsynchronized one such as " + REPLACEMENTS.get(type.fullyQualifiedName()) + ".");
+        reportIssue(tree, "Replace the synchronized class \"" + type.name() + "\" by an unsynchronized one such as " + REPLACEMENTS.get(type.fullyQualifiedName()) + ".");
         return true;
       }
       return false;
diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/ExpressionsHelper.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/ExpressionsHelper.java
@@ -19,9 +19,11 @@
  */
 package org.sonar.java.checks.helpers;
 
+import org.sonar.plugins.java.api.tree.ClassTree;
 import org.sonar.plugins.java.api.tree.ExpressionTree;
 import org.sonar.plugins.java.api.tree.IdentifierTree;
 import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
+import org.sonar.plugins.java.api.tree.NewClassTree;
 import org.sonar.plugins.java.api.tree.ParenthesizedTree;
 import org.sonar.plugins.java.api.tree.Tree;
 
@@ -67,5 +69,18 @@ public static String concatenate(@Nullable ExpressionTree tree) {
     return sb.toString();
   }
 
+  /**
+   * Return the correct tree to report on for class trees.
+   * @param classTree class tree raising an issue.
+   * @return simple name of class tree or identifier in parent expression for anonymous class.
+   */
+  public static Tree reportOnClassTree(ClassTree classTree) {
+    Tree reportTree = classTree.simpleName();
+    if(reportTree == null) {
+      reportTree = ((NewClassTree) classTree.parent()).identifier();
+    }
+    return reportTree;
+  }
+
 
 }
diff --git a/java-checks/src/test/files/checks/CustomCryptographicAlgorithmCheck.java b/java-checks/src/test/files/checks/CustomCryptographicAlgorithmCheck.java
@@ -2,9 +2,14 @@
 import java.util.Observable;
 
 class A { // Compliant
+  void foo() {
+    MessageDigest md = new MessageDigest() { // Noncompliant [[sc=28;ec=41]] {{Use a standard algorithm instead of creating a custom one.}}
+
+    };
+  }
 }
 
-abstract class B extends MessageDigest { // Noncompliant {{Use a standard algorithm instead of creating a custom one.}}
+abstract class B extends MessageDigest { // Noncompliant [[sc=16;ec=17]] {{Use a standard algorithm instead of creating a custom one.}}
   protected B(String algorithm) {
     super(algorithm);
   }
diff --git a/java-checks/src/test/files/checks/DeprecatedTagPresenceCheck.java b/java-checks/src/test/files/checks/DeprecatedTagPresenceCheck.java
@@ -1,7 +1,7 @@
 class Foo {
 
   @Deprecated
-  public int foo; // Noncompliant {{Do not forget to remove this deprecated code someday.}}
+  public int foo; // Noncompliant [[sc=14;ec=17]] {{Do not forget to remove this deprecated code someday.}}
 
   public void foo1() { // Compliant
   }
@@ -22,7 +22,7 @@ public void foo3() { // Noncompliant
    */
   @Ignore
   @Deprecated
-  public void foo4() { // Noncompliant
+  public void foo4() { // Noncompliant [[sc=15;ec=19]]
   }
 
   @Deprecated
diff --git a/java-checks/src/test/files/checks/EqualsNotOverriddenInSubclassCheck.java b/java-checks/src/test/files/checks/EqualsNotOverriddenInSubclassCheck.java
@@ -3,12 +3,18 @@ class A {
   static String staticMember = "static member";
   String s1;
 
+  void foo() {
+    Object o = new A() {
+      int inner;
+    };
+  }
+
   public boolean equals(Object obj) {
     return true;
   }
 }
 
-class B extends A { // Noncompliant {{Override this superclass' "equals" method.}}
+class B extends A { // Noncompliant [[sc=7;ec=8]]  {{Override this superclass' "equals" method.}}
   String s2;
 }
 
diff --git a/java-checks/src/test/files/checks/InnerClassOfNonSerializableCheck.java b/java-checks/src/test/files/checks/InnerClassOfNonSerializableCheck.java
@@ -13,12 +13,13 @@ class B {
   
   class B1 {}
   class B2 implements Cloneable {}
-  class B3 implements Serializable {} // Noncompliant {{Make this inner class static}}
+  class B3 implements Serializable {} // Noncompliant [[sc=9;ec=11]]{{Make this inner class static}}
   class B4 extends B3 {}
   static class B5 implements Serializable {}
   
   void nonStaticMethod() {
-    class X1 implements Serializable {} // Noncompliant {{Make "nonStaticMethod" static}}
+    class X1 implements Serializable {} // Noncompliant [[sc=11;ec=13]] {{Make "nonStaticMethod" static}}
+    Object o = new X1(){};
   }
   
   static void staticMethod() {
diff --git a/java-checks/src/test/files/checks/InnerClassOfSerializableCheck.java b/java-checks/src/test/files/checks/InnerClassOfSerializableCheck.java
@@ -13,12 +13,13 @@ class B implements Serializable {
   
   class B1 {}
   class B2 implements Cloneable {}
-  class B3 implements Serializable {} // Noncompliant {{Make this inner class static}}
+  class B3 implements Serializable {} // Noncompliant [[sc=9;ec=11]] {{Make this inner class static}}
   class B4 extends B3 {}
   static class B5 implements Serializable {}
   
   void nonStaticMethod() {
-    class X1 implements Serializable {} // Noncompliant {{Make "nonStaticMethod" static}}
+    class X1 implements Serializable {} // Noncompliant [[sc=11;ec=13]] {{Make "nonStaticMethod" static}}
+    Object o = new X1(){};
   }
   
   static void staticMethod() {
diff --git a/java-checks/src/test/files/checks/MissingDeprecatedCheck.java b/java-checks/src/test/files/checks/MissingDeprecatedCheck.java
@@ -1,7 +1,7 @@
 class Foo {
 
   @Deprecated
-  public int foo; // Noncompliant {{Add the missing @deprecated Javadoc tag.}}
+  public int foo; // Noncompliant [[sc=14;ec=17]] {{Add the missing @deprecated Javadoc tag.}}
 
   public void foo1() {
   }
diff --git a/java-checks/src/test/files/checks/SynchronizedClassUsageCheck.java b/java-checks/src/test/files/checks/SynchronizedClassUsageCheck.java
@@ -2,7 +2,7 @@
 import java.util.Vector;        // Compliant
 
 class A {
-  List a = new Vector();         // Noncompliant {{Replace the synchronized class "Vector" by an unsynchronized one such as "ArrayList" or "LinkedList".}}
+  List a = new Vector();         // Noncompliant [[sc=12;ec=24]] {{Replace the synchronized class "Vector" by an unsynchronized one such as "ArrayList" or "LinkedList".}}
   Vector a = new Vector();       // Noncompliant {{Replace the synchronized class "Vector" by an unsynchronized one such as "ArrayList" or "LinkedList".}}
   Hashtable a = new Hashtable(); // Noncompliant {{Replace the synchronized class "Hashtable" by an unsynchronized one such as "HashMap".}}
   Map a = new Hashtable();       // Noncompliant {{Replace the synchronized class "Hashtable" by an unsynchronized one such as "HashMap".}}
@@ -26,7 +26,7 @@ private void f() {
     List result = new java.util.Vector<Integer>(); // Noncompliant
   }
   
-  private Vector getVector() { // Noncompliant {{Replace the synchronized class "Vector" by an unsynchronized one such as "ArrayList" or "LinkedList".}}
+  private Vector getVector() { // Noncompliant [[sc=11;ec=17]]{{Replace the synchronized class "Vector" by an unsynchronized one such as "ArrayList" or "LinkedList".}}
     return new Vector();
   }
 
@@ -35,7 +35,7 @@ private Vector getVector() { // Noncompliant {{Replace the synchronized class "V
   public java.util.Stack f() { // Noncompliant {{Replace the synchronized class "Stack" by an unsynchronized one such as "Deque".}}
   }
 
-  public void f(Vector a) { // Noncompliant {{Replace the synchronized class "Vector" by an unsynchronized one such as "ArrayList" or "LinkedList".}}
+  public void f(Vector a) { // Noncompliant [[sc=17;ec=23]] {{Replace the synchronized class "Vector" by an unsynchronized one such as "ArrayList" or "LinkedList".}}
   }
 
   @Override
diff --git a/java-checks/src/test/java/org/sonar/java/checks/SynchronizedClassUsageCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/SynchronizedClassUsageCheckTest.java
@@ -24,11 +24,9 @@
 
 public class SynchronizedClassUsageCheckTest {
 
-  private final SynchronizedClassUsageCheck check = new SynchronizedClassUsageCheck();
-
   @Test
   public void detected() {
-    JavaCheckVerifier.verify("src/test/files/checks/SynchronizedClassUsageCheck.java", check);
+    JavaCheckVerifier.verify("src/test/files/checks/SynchronizedClassUsageCheck.java", new SynchronizedClassUsageCheck());
   }
 
 }

Original file line number	Diff line number	Diff line change
`@@ -50,13 +50,13 @@ private void visitClassTree(ClassTree classTree) {`
`50`	`50`	`if (owner.isTypeSymbol()) {`
`51`	`51`	`Symbol.TypeSymbol ownerType = (Symbol.TypeSymbol) owner;`
`52`	`52`	`if (isMatchingOuterClass(ownerType.type()) && !symbol.isStatic()) {`
`53`		`- addIssue(classTree, "Make this inner class static");`
	`53`	`+ reportIssue(classTree.simpleName(), "Make this inner class static");`
`54`	`54`	`}`
`55`	`55`	`} else if (owner.isMethodSymbol()) {`
`56`	`56`	`Symbol.TypeSymbol methodOwner = (Symbol.TypeSymbol) owner.owner();`
`57`	`57`	`if (isMatchingOuterClass(methodOwner.type()) && !owner.isStatic()) {`
`58`	`58`	`String methodName = owner.name();`
`59`		`- addIssue(classTree, "Make \"" + methodName + "\" static");`
	`59`	`+ reportIssue(classTree.simpleName(), "Make \"" + methodName + "\" static");`
`60`	`60`	`}`
`61`	`61`	`}`
`62`	`62`	`}`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ public class DeprecatedTagPresenceCheck extends AbstractDeprecatedChecker {`
`41`	`41`	`@Override`
`42`	`42`	`public void visitNode(Tree tree) {`
`43`	`43`	`if (hasDeprecatedAnnotation(tree) \|\| hasJavadocDeprecatedTag(tree)) {`
`44`		`- addIssue(tree, "Do not forget to remove this deprecated code someday.");`
	`44`	`+ reportIssue(getReportTree(tree), "Do not forget to remove this deprecated code someday.");`
`45`	`45`	`}`
`46`	`46`	`}`
`47`	`47`
Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ public List<Kind> nodesToVisit() {`
`58`	`58`	`public void visitNode(Tree tree) {`
`59`	`59`	`ClassTree classTree = (ClassTree) tree;`
`60`	`60`	`if (hasSemantic() && hasAtLeastOneField(classTree) && !implementsEquals(classTree) && parentClassImplementsEquals(classTree)) {`
`61`		`- addIssue(classTree, "Override this superclass' \"equals\" method.");`
	`61`	`+ reportIssue(classTree.simpleName(), "Override this superclass' \"equals\" method.");`
`62`	`62`	`}`
`63`	`63`	`}`
`64`	`64`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@`
`24`	`24`	`import org.sonar.check.Priority;`
`25`	`25`	`import org.sonar.check.Rule;`
`26`	`26`	`import org.sonar.check.RuleProperty;`
	`27`	`+import org.sonar.java.checks.helpers.ExpressionsHelper;`
`27`	`28`	`import org.sonar.java.syntaxtoken.FirstSyntaxTokenFinder;`
`28`	`29`	`import org.sonar.java.syntaxtoken.LastSyntaxTokenFinder;`
`29`	`30`	`import org.sonar.java.tag.Tag;`
`@@ -70,7 +71,7 @@ public void visitNode(Tree tree) {`
`70`	`71`	`int last = LastSyntaxTokenFinder.lastSyntaxToken(node).line();`
`71`	`72`	`int length = last - first + 1;`
`72`	`73`	`if (length > max) {`
`73`		`- addIssue(node, "Reduce this class from " + length + " to the maximum allowed " + max + " or externalize it in a public class.");`
	`74`	`+ reportIssue(ExpressionsHelper.reportOnClassTree(node), "Reduce this class from " + length + " to the maximum allowed " + max + " or externalize it in a public class.");`
`74`	`75`	`}`
`75`	`76`	`}`
`76`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,9 +62,9 @@ public void visitNode(Tree tree) {`
`62`	`62`	`boolean hasJavadocDeprecatedTag = hasJavadocDeprecatedTag(tree);`
`63`	`63`	`if (currentClassNotDeprecated() && !isLocalVar) {`
`64`	`64`	`if (hasDeprecatedAnnotation && !hasJavadocDeprecatedTag) {`
`65`		`- addIssue(tree, "Add the missing @deprecated Javadoc tag.");`
	`65`	`+ reportIssue(getReportTree(tree), "Add the missing @deprecated Javadoc tag.");`
`66`	`66`	`} else if (hasJavadocDeprecatedTag && !hasDeprecatedAnnotation) {`
`67`		`- addIssue(tree, "Add the missing @Deprecated annotation.");`
	`67`	`+ reportIssue(getReportTree(tree), "Add the missing @Deprecated annotation.");`
`68`	`68`	`}`
`69`	`69`	`}`
`70`	`70`	`if (tree.is(CLASS_KINDS)) {`
Original file line number	Diff line number	Diff line change
`@@ -2,9 +2,14 @@`
`2`	`2`	`import java.util.Observable;`
`3`	`3`
`4`	`4`	`class A { // Compliant`
	`5`	`+ void foo() {`
	`6`	`+ MessageDigest md = new MessageDigest() { // Noncompliant [[sc=28;ec=41]] {{Use a standard algorithm instead of creating a custom one.}}`
	`7`	`+`
	`8`	`+ };`
	`9`	`+ }`
`5`	`10`	`}`
`6`	`11`
`7`		`-abstract class B extends MessageDigest { // Noncompliant {{Use a standard algorithm instead of creating a custom one.}}`
	`12`	`+abstract class B extends MessageDigest { // Noncompliant [[sc=16;ec=17]] {{Use a standard algorithm instead of creating a custom one.}}`
`8`	`13`	`protected B(String algorithm) {`
`9`	`14`	`super(algorithm);`
`10`	`15`	`}`