SONARJAVA-1437 Precise issue location for a bunch of rules

Author: benzonico

java-checks/src/main/java/org/sonar/java/checks/AbstractClassWithoutAbstractMethodCheck.java

@@ -64,11 +64,11 @@ public void visitNode(Tree tree) {
       }
       if (isExtendingObject(classTree) && nbAbstractMethod == nbOfMembers) {
         // emtpy abstract class or only abstract method
-        context.addIssue(tree, this, "Convert this \"" + typeSymbol + "\" class to an interface");
+        context.reportIssue(this, classTree.simpleName(), "Convert this \"" + typeSymbol + "\" class to an interface");
       }
       if (nbOfMembers > 0 && nbAbstractMethod == 0 && !isPartialImplementation(classTree)) {
         // Not empty abstract class with no abstract method
-        context.addIssue(tree, this, "Convert this \"" + typeSymbol + "\" class to a concrete class with a private constructor");
+        context.reportIssue(this, classTree.simpleName(), "Convert this \"" + typeSymbol + "\" class to a concrete class with a private constructor");
       }
     }
   }

java-checks/src/main/java/org/sonar/java/checks/ArrayEqualsCheck.java

@@ -54,7 +54,7 @@ public void visitMethodInvocation(MethodInvocationTree tree) {
     if (tree.methodSelect().is(Tree.Kind.MEMBER_SELECT)) {
       MemberSelectExpressionTree mset = (MemberSelectExpressionTree) tree.methodSelect();
       if ("equals".equals(mset.identifier().name()) && mset.expression().symbolType().isArray()) {
-        context.addIssue(tree, this, "Use the '==' operator instead of calling the equals() method to prevent any misunderstandings");
+        context.reportIssue(this, mset.identifier(), "Use the '==' operator instead of calling the equals() method to prevent any misunderstandings");
       }
     }
     super.visitMethodInvocation(tree);

java-checks/src/main/java/org/sonar/java/checks/BooleanInversionCheck.java

@@ -66,7 +66,7 @@ public void visitNode(Tree tree) {
         Tree.Kind.EQUAL_TO, Tree.Kind.NOT_EQUAL_TO,
         Tree.Kind.LESS_THAN, Tree.Kind.GREATER_THAN,
         Tree.Kind.LESS_THAN_OR_EQUAL_TO, Tree.Kind.GREATER_THAN_OR_EQUAL_TO)) {
-      context.addIssue(tree, this, "Use the opposite operator (\"" + OPERATORS.get(((BinaryExpressionTree) expression).operatorToken().text()) + "\") instead.");
+      context.reportIssue(this, tree, "Use the opposite operator (\"" + OPERATORS.get(((BinaryExpressionTree) expression).operatorToken().text()) + "\") instead.");
     }
   }
 

java-checks/src/main/java/org/sonar/java/checks/CatchUsesExceptionWithContextCheck.java

@@ -101,7 +101,7 @@ public void visitCatch(CatchTree tree) {
       super.visitCatch(tree);
       Collection<IdentifierTree> usages = validUsagesStack.pop();
       if (usages.isEmpty()) {
-        context.addIssue(tree, this, "Either log or rethrow this exception.");
+        context.reportIssue(this, tree.parameter(), "Either log or rethrow this exception.");
       }
     }
   }

java-checks/src/main/java/org/sonar/java/checks/CollapsibleIfCandidateCheck.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.java.checks;
 
+import com.google.common.collect.Lists;
 import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
@@ -48,24 +49,26 @@
 public class CollapsibleIfCandidateCheck extends BaseTreeVisitor implements JavaFileScanner {
 
   private JavaFileScannerContext context;
-  private Deque<Boolean> outerIf = new ArrayDeque<>();
+  private Deque<IfStatementTree> outerIf = new ArrayDeque<>();
 
   @Override
   public void scanFile(JavaFileScannerContext context) {
     this.context = context;
     scan(context.getTree());
+    outerIf.clear();
   }
 
   @Override
   public void visitIfStatement(IfStatementTree tree) {
 
     if (!outerIf.isEmpty() && !hasElseClause(tree)) {
-      context.addIssue(tree, this, "Merge this if statement with the enclosing one.");
+      context.reportIssue(this, tree.ifKeyword(), "Merge this if statement with the enclosing one.",
+          Lists.newArrayList(new JavaFileScannerContext.Location("", outerIf.peek().ifKeyword())), null);
     }
 
     if (!hasElseClause(tree) && hasBodySingleIfStatement(tree.thenStatement())) {
       // children of this if statement are eligible for issues
-      outerIf.push(Boolean.TRUE);
+      outerIf.push(tree);
       // recurse into sub-tree
       super.visitIfStatement(tree);
       if (!outerIf.isEmpty()) {

java-checks/src/main/java/org/sonar/java/checks/CollectionImplementationReferencedCheck.java

@@ -134,10 +134,7 @@ private void checkIfAllowed(@Nullable TypeTree tree, String messagePrefix) {
     String collectionInterface = MAPPING.get(collectionImplementation);
 
     if (collectionInterface != null) {
-      context.addIssue(
-        tree,
-        this,
-        messagePrefix + messageRemainder(collectionImplementation, collectionInterface));
+      context.reportIssue(this, tree, messagePrefix + messageRemainder(collectionImplementation, collectionInterface));
     }
   }
 

java-checks/src/main/java/org/sonar/java/checks/CollectionIsEmptyCheck.java

@@ -74,7 +74,7 @@ public void visitBinaryExpression(BinaryExpressionTree tree) {
     super.visitBinaryExpression(tree);
 
     if (hasCallToSizeMethod(tree) && isEmptyComparison(tree)) {
-      context.addIssue(tree, this, "Use isEmpty() to check whether the collection is empty or not.");
+      context.reportIssue(this, tree, "Use isEmpty() to check whether the collection is empty or not.");
     }
   }
 

java-checks/src/main/java/org/sonar/java/checks/CollectionsEmptyConstantsCheck.java

@@ -67,7 +67,7 @@ public void visitMemberSelectExpression(MemberSelectExpressionTree tree) {
     boolean isCollectionsCall = tree.expression().is(Kind.IDENTIFIER) && "Collections".equals(((IdentifierTree) tree.expression()).name());
     boolean callEmptyConstant = identifier.startsWith("EMPTY_");
     if (isCollectionsCall && callEmptyConstant) {
-      context.addIssue(tree, this, "Replace \"Collections."+identifier+"\" by \"Collections."+ IDENTIFIER_REPLACEMENT.get(identifier)+"\".");
+      context.reportIssue(this, tree, "Replace \"Collections." + identifier + "\" by \"Collections." + IDENTIFIER_REPLACEMENT.get(identifier) + "\".");
     }
   }
 

java-checks/src/main/java/org/sonar/java/checks/EnumMapCheck.java

@@ -63,7 +63,7 @@ public void visitVariable(VariableTree tree) {
     if (tree.type().symbolType().isSubtypeOf("java.util.Map")) {
       ExpressionTree initializer = tree.initializer();
       if (initializer != null) {
-        checkNewMap(tree, initializer, hasEnumKey(tree.type().symbolType()));
+        checkNewMap(initializer, hasEnumKey(tree.type().symbolType()));
       }
     } else {
       super.visitVariable(tree);
@@ -73,7 +73,7 @@ public void visitVariable(VariableTree tree) {
   @Override
   public void visitAssignmentExpression(AssignmentExpressionTree tree) {
     if (tree.variable().symbolType().isSubtypeOf("java.util.Map")) {
-      checkNewMap(tree, tree.expression(), hasEnumKey(tree.variable().symbolType()));
+      checkNewMap(tree.expression(), hasEnumKey(tree.variable().symbolType()));
     } else {
       super.visitAssignmentExpression(tree);
     }
@@ -88,12 +88,12 @@ public void visitNewClass(NewClassTree tree) {
     }
   }
 
-  private void checkNewMap(Tree tree, ExpressionTree given, boolean useEnumKey) {
+  private void checkNewMap(ExpressionTree given, boolean useEnumKey) {
     ExpressionTree expression = ExpressionsHelper.skipParentheses(given);
     if (expression.is(Tree.Kind.NEW_CLASS)) {
       NewClassTree newClassTree = (NewClassTree) expression;
       if (newClassTree.symbolType().isSubtypeOf("java.util.HashMap") && (useEnumKey || hasEnumKey(newClassTree.identifier().symbolType()))) {
-        addIssue(tree);
+        addIssue(newClassTree);
       }
     }
   }
@@ -110,7 +110,7 @@ private static boolean hasEnumKey(Type type) {
   }
 
   private void addIssue(Tree typeTree) {
-    context.addIssue(typeTree, this, "Convert this Map to an EnumMap.");
+    context.reportIssue(this, typeTree, "Convert this Map to an EnumMap.");
   }
 
 }

java-checks/src/main/java/org/sonar/java/checks/ErrorClassExtendedCheck.java

@@ -31,6 +31,7 @@
 import org.sonar.plugins.java.api.tree.IdentifierTree;
 import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.TypeTree;
 import org.sonar.squidbridge.annotations.ActivatedByDefault;
 import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
 import org.sonar.squidbridge.annotations.SqaleSubCharacteristic;
@@ -55,16 +56,17 @@ public void scanFile(JavaFileScannerContext context) {
 
   @Override
   public void visitClass(ClassTree tree) {
-    if (tree.is(Tree.Kind.CLASS) && tree.superClass() != null) {
-      if (tree.superClass().is(Tree.Kind.IDENTIFIER)) {
-        IdentifierTree idt = (IdentifierTree) tree.superClass();
+    TypeTree superClass = tree.superClass();
+    if (tree.is(Tree.Kind.CLASS) && superClass != null) {
+      if (superClass.is(Tree.Kind.IDENTIFIER)) {
+        IdentifierTree idt = (IdentifierTree) superClass;
         if ("Error".equals(idt.name())) {
-          context.addIssue(tree, this, "Extend \"java.lang.Exception\" or one of its subclasses.");
+          context.reportIssue(this, superClass, "Extend \"java.lang.Exception\" or one of its subclasses.");
         }
-      } else if (tree.superClass().is(Tree.Kind.MEMBER_SELECT)) {
-        MemberSelectExpressionTree mse = (MemberSelectExpressionTree) tree.superClass();
+      } else if (superClass.is(Tree.Kind.MEMBER_SELECT)) {
+        MemberSelectExpressionTree mse = (MemberSelectExpressionTree) superClass;
         if ("Error".equals(mse.identifier().name()) && isJavaLang(mse.expression())) {
-          context.addIssue(tree, this, "Extend \"java.lang.Exception\" or one of its subclasses.");
+          context.reportIssue(this, superClass, "Extend \"java.lang.Exception\" or one of its subclasses.");
         }
       }
     }