SONARJAVA-1372 only consider ternary operators if java version >= 8

Author: Wohops

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

@@ -20,6 +20,7 @@
 package org.sonar.java.checks;
 
 import com.google.common.collect.ImmutableList;
+import org.apache.commons.lang.ArrayUtils;
 import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
@@ -39,6 +40,7 @@
 import org.sonar.plugins.java.api.tree.ParameterizedTypeTree;
 import org.sonar.plugins.java.api.tree.ReturnStatementTree;
 import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.Tree.Kind;
 import org.sonar.plugins.java.api.tree.TypeCastTree;
 import org.sonar.plugins.java.api.tree.TypeTree;
 import org.sonar.plugins.java.api.tree.VariableTree;
@@ -61,8 +63,20 @@
 @SqaleConstantRemediation("1min")
 public class DiamondOperatorCheck extends SubscriptionBaseVisitor implements JavaVersionAwareVisitor {
 
+  private static final Tree.Kind[] JAVA_7_KINDS = new Tree.Kind[] {
+    Tree.Kind.VARIABLE,
+    Tree.Kind.TYPE_CAST,
+    Tree.Kind.RETURN_STATEMENT,
+    Tree.Kind.ASSIGNMENT};
+  private static final Tree.Kind[] JAVA_8_KINDS = (Tree.Kind[]) ArrayUtils.addAll(JAVA_7_KINDS, new Tree.Kind[] {
+    Tree.Kind.CONDITIONAL_EXPRESSION});
+  private Tree.Kind[] expressionKindsToCheck = JAVA_7_KINDS;
+
   @Override
   public boolean isCompatibleWithJavaVersion(@Nullable Integer version) {
+    if (JavaVersionHelper.java8Compatible(version)) {
+      expressionKindsToCheck = JAVA_8_KINDS;
+    }
     return JavaVersionHelper.java7Compatible(version);
   }
 
@@ -76,7 +90,7 @@ public void visitNode(Tree tree) {
     NewClassTree newClassTree = (NewClassTree) tree;
     TypeTree newTypeTree = newClassTree.identifier();
     if (newClassTree.classBody() == null && isParameterizedType(newTypeTree)) {
-      TypeTree type = getTypeFromExpression(tree.parent());
+      TypeTree type = getTypeFromExpression(tree.parent(), expressionKindsToCheck);
       if (type != null && isParameterizedType(type)) {
         reportIssue(
           ((ParameterizedTypeTree) newTypeTree).typeArguments(),
@@ -87,9 +101,9 @@ public void visitNode(Tree tree) {
   }
 
   @CheckForNull
-  private static TypeTree getTypeFromExpression(Tree expression) {
-    if (expression.is(Tree.Kind.VARIABLE, Tree.Kind.TYPE_CAST, Tree.Kind.RETURN_STATEMENT, Tree.Kind.ASSIGNMENT, Tree.Kind.CONDITIONAL_EXPRESSION)) {
-      TypeTreeLocator visitor = new TypeTreeLocator();
+  private static TypeTree getTypeFromExpression(Tree expression, Tree.Kind[] kinds) {
+    if (expression.is(kinds)) {
+      TypeTreeLocator visitor = new TypeTreeLocator(kinds);
       expression.accept(visitor);
       return visitor.type;
     }
@@ -105,8 +119,15 @@ private static boolean isParameterizedType(TypeTree type) {
 
   private static class TypeTreeLocator extends BaseTreeVisitor {
 
+    private final Tree.Kind[] kinds;
+
+    @Nullable
     private TypeTree type = null;
 
+    public TypeTreeLocator(Kind[] kinds) {
+      this.kinds = kinds;
+    }
+
     @Override
     public void visitReturnStatement(ReturnStatementTree tree) {
       type = getMethodReturnType(tree);
@@ -121,7 +142,7 @@ public void visitTypeCast(TypeCastTree tree) {
     public void visitAssignmentExpression(AssignmentExpressionTree tree) {
       Tree assignedVariable = getAssignedVariable(tree.variable());
       if (assignedVariable != null) {
-        type = getTypeFromExpression(assignedVariable);
+        type = getTypeFromExpression(assignedVariable, kinds);
       }
     }
 
@@ -132,7 +153,7 @@ public void visitVariable(VariableTree tree) {
 
     @Override
     public void visitConditionalExpression(ConditionalExpressionTree tree) {
-      type = getTypeFromExpression(tree.parent());
+      type = getTypeFromExpression(tree.parent(), kinds);
     }
 
     private static TypeTree getMethodReturnType(ReturnStatementTree returnStatementTree) {

java-checks/src/test/files/checks/DiamondOperatorCheck_java_7.java

@@ -0,0 +1,51 @@
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
+class A {
+  List<Object> myList1 = new ArrayList<>(); // Compliant
+  List<Object> myList2 = new ArrayList<Object>(); // Noncompliant [[sc=39;ec=47]] {{Replace the type specification in this constructor call with the diamond operator ("<>").}}
+
+  void foo() {
+    List<Object> myList;
+    myList = new ArrayList<>(); // Compliant
+    myList = new ArrayList<Object>(); // Noncompliant [[sc=27;ec=35]]
+
+    List<String> strings1 = new ArrayList<>(); // Compliant
+    List<String> strings2 = new ArrayList<String>(); // Noncompliant [[sc=42;ec=50]]
+    Map<String,List<Integer>> map1 = new HashMap<>(); // Compliant
+    Map<String,List<Integer>> map2 = new HashMap<String,List<Integer>>(); // Noncompliant [[sc=49;ec=71]]
+
+    List myOtherList = new ArrayList<Object>(); // Compliant
+    new A().myList1 = new ArrayList<>(); // Compliant
+    new A().myList1 = new ArrayList<Object>(); // Noncompliant [[sc=36;ec=44]]
+
+    List<Object>[] myArrayOfList = new List[10];
+    myArrayOfList[0] = new ArrayList<>(); // Compliant
+    myArrayOfList[1] = new ArrayList<Object>(); // Noncompliant [[sc=37;ec=45]]
+
+    new ArrayList<Object>().add(new Object()); // Compliant
+
+    ((List<Object>) new ArrayList<Object>()).isEmpty(); // Noncompliant [[sc=34;ec=42]]
+    ((List<Object>) new ArrayList<>()).isEmpty(); // Compliant
+
+    Iterator<Object> iterator = new Iterator<Object>() { // Compliant - anonymous classes requires to be typed
+    };
+
+    MyUnknownVariable = new ArrayList<Object>(); // Compliant
+  }
+
+  List<Object> qix(boolean test) {
+    List<Object> myList = test ?
+      new ArrayList<>() : // Compliant
+        new ArrayList<Object>(); // Compliant - using diamond operator only works with java 8
+    
+    myList = new ArrayList<>(test ? new ArrayList<>() : new ArrayList<Object>(5)); // Compliant
+    if (test) {
+      return new ArrayList<>(); // Compliant
+    }
+    return new ArrayList<Object>(); // Noncompliant [[sc=25;ec=33]]
+  }
+}

…t/files/checks/DiamondOperatorCheck.java …/checks/DiamondOperatorCheck_java_8.javajava-checks/src/test/files/checks/DiamondOperatorCheck.java renamed to java-checks/src/test/files/checks/DiamondOperatorCheck_java_8.java java-checks/src/test/files/checks/DiamondOperatorCheck.java renamed to java-checks/src/test/files/checks/DiamondOperatorCheck_java_8.java

@@ -31,7 +31,13 @@ public void test_no_version() {
 
   @Test
   public void test_with_java_7() {
-    JavaCheckVerifier.verify("src/test/files/checks/DiamondOperatorCheck.java", new DiamondOperatorCheck(), 7);
+    JavaCheckVerifier.verify("src/test/files/checks/DiamondOperatorCheck_java_7.java", new DiamondOperatorCheck(), 7);
+  }
+
+  @Test
+  public void test_with_java_8() {
+    // take into account ternary operators
+    JavaCheckVerifier.verify("src/test/files/checks/DiamondOperatorCheck_java_8.java", new DiamondOperatorCheck(), 8);
   }
 
 }