SONARJAVA-1753 Handle empty final arrays (#941)

* SONARJAVA-1753 Handle empty final arrays

* fix based on review

its/ruling/src/test/resources/commons-beanutils/squid-S2386.json

@@ -1,6 +1,5 @@
 {
 'commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils/BeanMap.java':[
-59,
 78,
 ],
 }

its/ruling/src/test/resources/jdk6/squid-S2386.json

@@ -1,7 +1,4 @@
 {
-'jdk6:java/io/ObjectStreamClass.java':[
-53,
-],
 'jdk6:java/util/Collections.java':[
 2877,
 2935,

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

@@ -23,6 +23,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Multimap;
+
 import org.sonar.check.Rule;
 import org.sonar.java.checks.helpers.ExpressionsHelper;
 import org.sonar.java.matcher.MethodMatcher;
@@ -33,16 +34,20 @@
 import org.sonar.plugins.java.api.JavaFileScannerContext;
 import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.semantic.Type;
+import org.sonar.plugins.java.api.tree.ArrayDimensionTree;
 import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
 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.LiteralTree;
 import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.MethodInvocationTree;
+import org.sonar.plugins.java.api.tree.NewArrayTree;
 import org.sonar.plugins.java.api.tree.Tree;
 import org.sonar.plugins.java.api.tree.VariableTree;
 
 import javax.annotation.Nullable;
+
 import java.text.MessageFormat;
 import java.util.HashSet;
 import java.util.List;
@@ -117,7 +122,7 @@ public void visitNode(Tree tree) {
   private void preCheckVariable(Tree owner, VariableTree variableTree) {
     Symbol symbol = variableTree.symbol();
     if (symbol != null && isPublicStatic(symbol) && isForbiddenType(symbol.type())) {
-      if (isMutable(variableTree.initializer(), symbol.type())) {
+      if (isMutable(variableTree.initializer(), symbol)) {
         String message = "Make this member \"protected\".";
         if (owner.is(Tree.Kind.INTERFACE)) {
           message = MessageFormat.format("Move \"{0}\" to a class and lower its visibility", variableTree.simpleName().name());
@@ -138,7 +143,7 @@ private void checkAssignment(AssignmentExpressionTree node) {
     if (variable.is(Tree.Kind.IDENTIFIER)) {
       IdentifierTree identifierTree = (IdentifierTree) variable;
       Symbol symbol = identifierTree.symbol();
-      if (IMMUTABLE_CANDIDATES.contains(symbol) && isMutable(node.expression(), symbol.type())) {
+      if (IMMUTABLE_CANDIDATES.contains(symbol) && isMutable(node.expression(), symbol)) {
         reportIssue(identifierTree, "Make member \"" + symbol.name() + "\" \"protected\".");
         IMMUTABLE_CANDIDATES.remove(symbol);
       }
@@ -153,24 +158,53 @@ public void leaveNode(Tree tree) {
     }
   }
 
-  static boolean isMutable(@Nullable ExpressionTree initializer, Type type) {
+  static boolean isMutable(@Nullable ExpressionTree initializer, Symbol symbol) {
+    Type type = symbol.type();
     if (initializer == null) {
       return ALWAYS_MUTABLE_TYPES.stream().anyMatch(type::isSubtypeOf);
     }
+    if (symbol.isFinal() && isEmptyArray(initializer)) {
+      return false;
+    }
     ExpressionTree expression = ExpressionsHelper.skipParentheses(initializer);
     if (expression.is(Tree.Kind.METHOD_INVOCATION)) {
-      MethodInvocationTree mit = (MethodInvocationTree) expression;
-      if (isAcceptedTypeOrUnmodifiableMethodCall(mit)) {
-        return false;
-      } else if (ARRAYS_AS_LIST.matches(mit)) {
-        return !mit.arguments().isEmpty();
-      }
+      return returnValueIsMutable((MethodInvocationTree) expression);
     } else if (expression.is(Tree.Kind.NEW_CLASS)) {
       return !isAcceptedType(expression.symbolType(), ACCEPTED_NEW_TYPES);
     }
     return true;
   }
 
+  private static boolean isEmptyArray(ExpressionTree expression) {
+    if (!expression.is(Tree.Kind.NEW_ARRAY)) {
+      return false;
+    }
+    NewArrayTree nat = (NewArrayTree) expression;
+    return hasEmptyInitializer(nat) || hasOnlyZeroDimensions(nat.dimensions());
+  }
+
+  private static boolean hasEmptyInitializer(NewArrayTree newArrayTree) {
+    return newArrayTree.openBraceToken() != null && newArrayTree.initializers().isEmpty();
+  }
+
+  private static boolean hasOnlyZeroDimensions(List<ArrayDimensionTree> dimensions) {
+    return !dimensions.isEmpty() && dimensions.stream().allMatch(PublicStaticMutableMembersCheck::isZeroDimension);
+  }
+
+  private static boolean isZeroDimension(ArrayDimensionTree dim) {
+    ExpressionTree expression = dim.expression();
+    return expression != null && expression.is(Tree.Kind.INT_LITERAL) && "0".equals(((LiteralTree) expression).value());
+  }
+
+  private static boolean returnValueIsMutable(MethodInvocationTree mit) {
+    if (isAcceptedTypeOrUnmodifiableMethodCall(mit)) {
+      return false;
+    } else if (ARRAYS_AS_LIST.matches(mit)) {
+      return !mit.arguments().isEmpty();
+    }
+    return true;
+  }
+
   private static boolean isAcceptedTypeOrUnmodifiableMethodCall(MethodInvocationTree mit) {
     Type type = mit.symbolType();
     return type.isUnknown() || isAcceptedType(type, ACCEPTED_TYPES) || UNMODIFIABLE_METHOD_CALLS.anyMatch(mit);

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

@@ -34,6 +34,17 @@ public class A {
   public static final Map h2 = new Hashtable(); // Noncompliant
 
   public static final int[] data = new int[5]; // Noncompliant
+  public static final int[] data_unknown_size = new int[getDim()]; // Noncompliant
+
+  public static int getDim() {
+    return 42;
+  }
+
+  public static final int[] EMPTY_DATA_1 = new int[0]; // Compliant
+  public static final int[] EMPTY_DATA_2 = {}; // Compliant
+  public static final int[] EMPTY_DATA_3 = new int[]{}; // Compliant
+  public static final int[] NON_EMPTY_DATA_1 = new int[]{ 0 }; // Noncompliant - dim 1 array
+  public static final int[][] NON_EMPTY_DATA_2 = {new int[0], {}}; // Noncompliant - you can still modify sub array
 
   public static int[] data2 = new int[5]; // Noncompliant