Fix binary operations in MathTargetType (#9)

Author: jtraglia

README.md

@@ -3,7 +3,6 @@ errorprone-checks
 
 A collection of custom [errorprone] checks.
 
-
 ## Usage
 - Add Consensys maven dependency
 ```groovy

src/main/java/tech/pegasys/tools/epchecks/MathTargetType.java

@@ -15,6 +15,7 @@
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.Matchers.staticMethod;
 
+import java.util.List;
 import javax.lang.model.type.TypeKind;
 
 import com.google.auto.service.AutoService;
@@ -34,6 +35,7 @@
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.TypeCastTree;
 import com.sun.source.tree.VariableTree;
+import com.sun.source.util.TreePath;
 import com.sun.tools.javac.code.Type;
 
 @AutoService(BugChecker.class)
@@ -52,11 +54,20 @@ public class MathTargetType extends BugChecker implements MethodInvocationTreeMa
           .namedAnyOf(
               "min", "max", "addExact", "subtractExact", "multiplyExact", "floorDiv", "floorMod");
 
+  private static final List<Tree.Kind> COMPARISONS =
+      List.of(
+          Tree.Kind.EQUAL_TO,
+          Tree.Kind.NOT_EQUAL_TO,
+          Tree.Kind.GREATER_THAN,
+          Tree.Kind.GREATER_THAN_EQUAL,
+          Tree.Kind.LESS_THAN,
+          Tree.Kind.LESS_THAN_EQUAL);
+
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
     if (POTENTIALLY_CONFUSED_MATH_FUNC.matches(tree, state)) {
       Tree parent = state.getPath().getParentPath().getLeaf();
-      Type type = getTargetType(tree, parent, state);
+      Type type = getTargetType(tree, parent, state, state.getPath().getParentPath());
       if (type != null && !anyMethodArgIs(tree, state, type.getKind())) {
         String typeName = type.getKind().toString().toLowerCase();
         return buildDescription(tree)
@@ -79,21 +90,32 @@ private boolean anyMethodArgIs(MethodInvocationTree tree, VisitorState state, Ty
     return false;
   }
 
-  private Type getTargetType(Tree original, Tree tree, VisitorState state) {
+  private Type getTargetType(Tree prev, Tree tree, VisitorState state, TreePath path) {
     if (tree instanceof AssignmentTree) {
       AssignmentTree assignmentTree = (AssignmentTree) tree;
       return ASTHelpers.getType(assignmentTree.getVariable());
     }
 
     if (tree instanceof BinaryTree) {
+      // In the case of comparisons, the target type will be the
+      // other side of the operation.
+      BinaryTree binaryTree = (BinaryTree) tree;
+      if (COMPARISONS.contains(tree.getKind())) {
+        if (binaryTree.getLeftOperand().equals(prev)) {
+          return ASTHelpers.getType(binaryTree.getRightOperand());
+        }
+        return ASTHelpers.getType(binaryTree.getLeftOperand());
+      }
+
       // Need more information. Call again on parent.
-      Tree parent = state.getPath().getParentPath().getParentPath().getLeaf();
-      return getTargetType(original, parent, state);
+      TreePath parentPath = path.getParentPath();
+      Tree parent = parentPath.getLeaf();
+      return getTargetType(tree, parent, state, parentPath);
     }
 
     if (tree instanceof MethodInvocationTree) {
       MethodInvocationTree methodInvocationTree = (MethodInvocationTree) tree;
-      int index = methodInvocationTree.getArguments().indexOf(original);
+      int index = methodInvocationTree.getArguments().indexOf(prev);
       return ASTHelpers.getSymbol(methodInvocationTree).params.get(index).type;
     }
 

src/test/resources/tech/pegasys/tools/epchecks/MathTargetTypePositiveCases.java

@@ -81,6 +81,23 @@ public void expectedLongAllFuncs() {
         long l = Math.min(0, 1) * 1;
     }
 
+    public void binaryOperations() {
+        // BUG: Diagnostic contains: Neither of the function arguments are long types
+        long a = Math.min(0, 1) * 1;
+        // BUG: Diagnostic contains: Neither of the function arguments are long types
+        long b = Math.min(0, 1) * 1L;
+        // BUG: Diagnostic contains: Neither of the function arguments are long types
+        long c = 1 * Math.min(0, 1) * 1L;
+        // BUG: Diagnostic contains: Neither of the function arguments are long types
+        long d = 1 * 1 * 1 * 1 * Math.min(0, 1) * 1L;
+        // BUG: Diagnostic contains: Neither of the function arguments are double types
+        takesDouble(0, Math.min(1 / 2, Math.max(0, 1) - 1), 0);
+        // BUG: Diagnostic contains: Neither of the function arguments are long types
+        long e = Math.min(0, 1) & 27;
+        // BUG: Diagnostic contains: Neither of the function arguments are long types
+        long f = Math.min(0, 1) | 0x80;
+    }
+
     public void expectedLongVars() {
         int a = 0;
         int b = 1;
@@ -92,4 +109,11 @@ public void expectedLongArgs(int a, int b) {
         // BUG: Diagnostic contains: Neither of the function arguments are long types
         long c = Math.min(a, b);
     }
+
+    public void comparison(int a, int b) {
+        // BUG: Diagnostic contains: Neither of the function arguments are long types
+        boolean c = 27L == Math.min(a, b);
+        // BUG: Diagnostic contains: Neither of the function arguments are long types
+        boolean d = Math.min(a, b) == 27L;
+    }
 }