SONARJAVA-1460 symplify recognition of methods from Objects

Author: Wohops

java-squid/src/main/java/org/sonar/java/se/ConstraintManager.java

@@ -104,11 +104,11 @@ public SymbolicValue createMethodSymbolicValue(MethodInvocationTree syntaxNode,
       SymbolicValue leftOp = values.get(0);
       SymbolicValue rightOp = values.get(1);
       result.computedFrom(ImmutableList.of(rightOp, leftOp));
-    } else if (ExplodedGraphWalker.isObjectsMethod(syntaxNode, "isNull", 1)) {
+    } else if (isObjectsMethod(syntaxNode.symbol(), "isNull")) {
       result = new NullCheckSymbolicValue(counter, true);
       SymbolicValue operand = values.get(1);
       result.computedFrom(ImmutableList.of(operand));
-    } else if (ExplodedGraphWalker.isObjectsMethod(syntaxNode, "nonNull", 1)) {
+    } else if (isObjectsMethod(syntaxNode.symbol(), "nonNull")) {
       result = new NullCheckSymbolicValue(counter, false);
       SymbolicValue operand = values.get(1);
       result.computedFrom(ImmutableList.of(operand));
@@ -119,6 +119,10 @@ public SymbolicValue createMethodSymbolicValue(MethodInvocationTree syntaxNode,
     return result;
   }
 
+  private static boolean isObjectsMethod(Symbol symbol, String methodName) {
+    return symbol.owner().type().is("java.util.Objects") && methodName.equals(symbol.name());
+  }
+
   private static boolean isEqualsMethod(MethodInvocationTree syntaxNode) {
     if (syntaxNode.arguments().size() == 1) {
       ExpressionTree methodSelect = syntaxNode.methodSelect();

java-squid/src/main/java/org/sonar/java/se/ExplodedGraphWalker.java

@@ -81,7 +81,6 @@ public class ExplodedGraphWalker extends BaseTreeVisitor {
   private static final int MAX_STEPS = 10000;
   private static final Logger LOG = LoggerFactory.getLogger(ExplodedGraphWalker.class);
   private static final Set<String> THIS_SUPER = ImmutableSet.of("this", "super");
-  public static final String OBJECT_CLASS_NAME = "java.lang.Object";
 
   private static final boolean DEBUG_MODE_ACTIVATED = false;
   private static final int MAX_EXEC_PROGRAM_POINT = 2;
@@ -575,17 +574,6 @@ private static boolean isLocalMethodInvocation(MethodInvocationTree tree) {
     return false;
   }
 
-  public static boolean isObjectsMethod(MethodInvocationTree syntaxNode, String methodName, int numberOfParameter) {
-    Symbol symbol = syntaxNode.symbol();
-    if (symbol.isMethodSymbol()) {
-      Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) symbol;
-      if (methodSymbol.owner().type().is("java.util.Objects") && methodName.equals(methodSymbol.name())) {
-        return methodSymbol.parameterTypes().size() == numberOfParameter;
-      }
-    }
-    return false;
-  }
-
   private void resetFieldValues() {
     programState = programState.resetFieldValues(constraintManager);
   }

java-squid/src/main/java/org/sonar/java/se/checks/NullDereferenceCheck.java

@@ -26,12 +26,12 @@
 import org.sonar.check.Rule;
 import org.sonar.java.model.DefaultJavaFileScannerContext;
 import org.sonar.java.se.CheckerContext;
-import org.sonar.java.se.ExplodedGraphWalker;
 import org.sonar.java.se.ObjectConstraint;
 import org.sonar.java.se.ProgramState;
 import org.sonar.java.se.symbolicvalues.SymbolicValue;
 import org.sonar.plugins.java.api.JavaFileScanner;
 import org.sonar.plugins.java.api.JavaFileScannerContext;
+import org.sonar.plugins.java.api.semantic.Symbol;
 import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
 import org.sonar.plugins.java.api.tree.MethodInvocationTree;
 import org.sonar.plugins.java.api.tree.Tree;
@@ -52,8 +52,6 @@
 @ActivatedByDefault
 public class NullDereferenceCheck extends SECheck implements JavaFileScanner {
 
-  private static final String REQUIRE_NON_NULL_METHOD_NAME = "requireNonNull";
-
   @Override
   public void scanFile(JavaFileScannerContext context) {
     Multimap<Tree, String> issues = ((DefaultJavaFileScannerContext) context).getSEIssues(NullDereferenceCheck.class);
@@ -73,13 +71,10 @@ public ProgramState checkPreStatement(CheckerContext context, Tree syntaxNode) {
     if (syntaxNode.is(Tree.Kind.METHOD_INVOCATION)) {
       MethodInvocationTree methodInvocation = (MethodInvocationTree) syntaxNode;
       toCheck = methodInvocation.methodSelect();
-      if (ExplodedGraphWalker.isObjectsMethod(methodInvocation, REQUIRE_NON_NULL_METHOD_NAME, 1)) {
-        final List<SymbolicValue> values = context.getState().peekValues(2);
-        return context.getState().addConstraint(values.get(1), ObjectConstraint.NOT_NULL);
-      } else if (ExplodedGraphWalker.isObjectsMethod(methodInvocation, REQUIRE_NON_NULL_METHOD_NAME, 2)
-        || ExplodedGraphWalker.isObjectsMethod(methodInvocation, REQUIRE_NON_NULL_METHOD_NAME, 2)) {
-        final List<SymbolicValue> values = context.getState().peekValues(3);
-        return context.getState().addConstraint(values.get(2), ObjectConstraint.NOT_NULL);
+      if (isObjectsRequireNonNullMethod(methodInvocation.symbol())) {
+        int numberArguments = methodInvocation.arguments().size();
+        List<SymbolicValue> values = context.getState().peekValues(numberArguments + 1);
+        return context.getState().addConstraint(values.get(numberArguments), ObjectConstraint.NOT_NULL);
       }
     }
     if (toCheck.is(Tree.Kind.MEMBER_SELECT)) {
@@ -88,6 +83,10 @@ public ProgramState checkPreStatement(CheckerContext context, Tree syntaxNode) {
     return context.getState();
   }
 
+  private static boolean isObjectsRequireNonNullMethod(Symbol symbol) {
+    return symbol.owner().type().is("java.util.Objects") && "requireNonNull".equals(symbol.name());
+  }
+
   private ProgramState checkMemberSelect(CheckerContext context, MemberSelectExpressionTree syntaxNode, SymbolicValue currentVal) {
     final ProgramState programState = context.getState();
     if ("class".equals(syntaxNode.identifier().name())) {

java-squid/src/main/java/org/sonar/java/se/symbolicvalues/NullCheckSymbolicValue.java

@@ -38,11 +38,9 @@ public NullCheckSymbolicValue(int id, boolean isNull) {
 
   @Override
   public List<ProgramState> setConstraint(ProgramState programState, BooleanConstraint booleanConstraint) {
-    final ObjectConstraint constraint;
-    if (BooleanConstraint.TRUE.equals(booleanConstraint)) {
-      constraint = isNull ? ObjectConstraint.NULL : ObjectConstraint.NOT_NULL;
-    } else {
-      constraint = isNull ? ObjectConstraint.NOT_NULL : ObjectConstraint.NULL;
+    ObjectConstraint constraint = ObjectConstraint.NULL;
+    if (BooleanConstraint.TRUE.equals(booleanConstraint) ^ isNull) {
+      constraint = ObjectConstraint.NOT_NULL;
     }
     return ImmutableList.of(programState.addConstraint(operand, constraint));
   }

java-squid/src/test/files/se/NullDereferenceCheck.java

@@ -1,10 +1,7 @@
 @SomeAnnotation(name = value)
 package javax.annotation;
 
-import java.util.function.Supplier;
-
 import java.util.List;
-import java.util.Objects;
 
 @interface CheckForNull {}
 

java-squid/src/test/files/se/ObjectsMethodsTest.java

@@ -42,7 +42,7 @@ public void testRequireNull(Supplier<String> supplier) {
   }
 }
 
-// Needed to ensure that all methods of Objects are declared (for tests with Java version prior to 1.8
+// Needed to ensure that all methods of Objects are declared (for tests with Java version prior to 1.8)
 
 class Objects {
   public static boolean isNull(Object obj) {
@@ -70,4 +70,4 @@ public static <T> T  requireNonNull(T obj, Supplier<String> supplier) {
       throw new NullPointerException(supplier.get());
     }
   }
-}
+}