Suggest Double.compare in ObjectEqualsForPrimitives

`Objects.equal(a_double, another_double)` and `a_double == another_double` have different semantics (different result when both compared values are NaN). Suggest replacement using `Double.compare` to avoid changing code semantics. This is especially relevant when implementing `equals` methods.
google · May 15, 2024 · 037b589 · 037b589
1 parent 2a06dd3
commit 037b589
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 3 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ObjectEqualsForPrimitives.java b/core/src/main/java/com/google/errorprone/bugpatterns/ObjectEqualsForPrimitives.java
@@ -29,7 +29,12 @@
 import com.google.errorprone.fixes.SuggestedFix;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
+import com.sun.tools.javac.code.Type;
+
+import javax.lang.model.type.TypeKind;
 
 /**
  * Check for usage of {@code Objects.equal} on primitive types.
@@ -53,11 +58,28 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
       return NO_MATCH;
     }
 
-    String arg1 = state.getSourceForNode(tree.getArguments().get(0));
-    String arg2 = state.getSourceForNode(tree.getArguments().get(1));
+    ExpressionTree expression1 = tree.getArguments().get(0);
+    ExpressionTree expression2 = tree.getArguments().get(1);
+    String arg1 = state.getSourceForNode(expression1);
+    String arg2 = state.getSourceForNode(expression2);
 
     // TODO: Rewrite to a != b if the original code has a negation (e.g. !Object.equals)
-    Fix fix = SuggestedFix.builder().replace(tree, "(" + arg1 + " == " + arg2 + ")").build();
+    Fix fix;
+    if (is(expression1, TypeKind.DOUBLE) || is(expression2, TypeKind.DOUBLE)) {
+      fix = SuggestedFix.builder().replace(tree, "(Double.doubleToLongBits(" + arg1 + ") == Double.doubleToLongBits(" + arg2 + "))").build();
+    } else if (is(expression1, TypeKind.FLOAT) || is(expression2, TypeKind.FLOAT)) {
+      fix = SuggestedFix.builder().replace(tree, "(Float.floatToIntBits(" + arg1 + ") == Float.floatToIntBits(" + arg2 + "))").build();
+    } else {
+      fix = SuggestedFix.builder().replace(tree, "(" + arg1 + " == " + arg2 + ")").build();
+    }
     return describeMatch(tree, fix);
   }
+
+  private static boolean is(ExpressionTree expression, TypeKind typeKind) {
+    Type type = ASTHelpers.getType(expression);
+    if (type == null) {
+      return false;
+    }
+    return type.getKind() == typeKind;
+  }
 }
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ObjectEqualsForPrimitivesTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ObjectEqualsForPrimitivesTest.java
@@ -179,4 +179,92 @@ public void intAndLong() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void doubles() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "import java.util.Objects;",
+            "public class Test {",
+            "  private static boolean testDoubles(double a, double b) {",
+            "    return Objects.equals(a, b);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import java.util.Objects;",
+            "public class Test {",
+            "  private static boolean testDoubles(double a, double b) {",
+            "    return (Double.doubleToLongBits(a) == Double.doubleToLongBits(b));",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void doubleAndFloat() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "import java.util.Objects;",
+            "public class Test {",
+            "  private static boolean testDoubles(double a, float b) {",
+            "    return Objects.equals(a, b);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import java.util.Objects;",
+            "public class Test {",
+            "  private static boolean testDoubles(double a, float b) {",
+            "    return (Double.doubleToLongBits(a) == Double.doubleToLongBits(b));",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void doubleAndLong() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "import java.util.Objects;",
+            "public class Test {",
+            "  private static boolean testDoubles(double a, long b) {",
+            "    return Objects.equals(a, b);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import java.util.Objects;",
+            "public class Test {",
+            "  private static boolean testDoubles(double a, long b) {",
+            "    return (Double.doubleToLongBits(a) == Double.doubleToLongBits(b));",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void floatAndLong() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "import java.util.Objects;",
+            "public class Test {",
+            "  private static boolean testDoubles(float a, long b) {",
+            "    return Objects.equals(a, b);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import java.util.Objects;",
+            "public class Test {",
+            "  private static boolean testDoubles(float a, long b) {",
+            "    return (Float.floatToIntBits(a) == Float.floatToIntBits(b));",
+            "  }",
+            "}")
+        .doTest();
+  }
 }