Improve UseCorrectAssertInTests handling for primitive equality compa…

…risons

Recommended via[]

assert a == 1; // usage to fix
assertThat(a).isSameInstanceAs(1); // old transform
assertThat(a).isEqualTo(1); // new behavior

Performed a blaze_error_prone run on a sample directory to validate my own CUJ: unknown commit.

I also started going down other related cleanups to tackle some other primitive comparison cases like "assert a == false" (to use isFalse) or "assert a > 0" (to use isGreaterThan), but there are so few usages that I don't think it's worth polishing and complicating this logic for such edge cases.

PiperOrigin-RevId: 518859546

core/src/main/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTests.java

@@ -58,11 +58,16 @@ public class UseCorrectAssertInTests extends BugChecker implements MethodTreeMat
 
   private static final String IS_TRUE = "isTrue();";
   private static final String IS_FALSE = "isFalse();";
-  private static final String IS_SAME_AS = "isSameInstanceAs(%s);";
-  private static final String IS_NOT_SAME_AS = "isNotSameInstanceAs(%s);";
+
   private static final String IS_NULL = "isNull();";
   private static final String IS_NOT_NULL = "isNotNull();";
 
+  private static final String IS_EQUAL_TO = "isEqualTo(%s);";
+  private static final String IS_NOT_EQUAL_TO = "isNotEqualTo(%s);";
+
+  private static final String IS_SAME_AS = "isSameInstanceAs(%s);";
+  private static final String IS_NOT_SAME_AS = "isNotSameInstanceAs(%s);";
+
   public UseCorrectAssertInTests() {}
 
   @Override
@@ -113,13 +118,13 @@ private static void replaceAssert(
           expr1,
           foundAssert,
           state,
-          String.format("isEqualTo(%s);", normalizedSourceForExpression(expr2, state)));
+          String.format(IS_EQUAL_TO, normalizedSourceForExpression(expr2, state)));
       return;
     }
 
     // case: "assert expr1 == expr2" or "assert expr1 != expr2"
     if (expr.getKind().equals(Kind.EQUAL_TO) || expr.getKind().equals(Kind.NOT_EQUAL_TO)) {
-      suggestFixForSameReference(fix, foundAssert, state, expr.getKind().equals(Kind.EQUAL_TO));
+      suggestFixForEquality(fix, foundAssert, state, expr.getKind().equals(Kind.EQUAL_TO));
       return;
     }
 
@@ -152,7 +157,7 @@ private static void addFix(
   }
 
   /** Handles the case "expr1 == expr2" */
-  private static void suggestFixForSameReference(
+  private static void suggestFixForEquality(
       SuggestedFix.Builder fix, AssertTree foundAssert, VisitorState state, boolean isEqual) {
 
     BinaryTree equalityTree = (BinaryTree) TreeInfo.skipParens((JCTree) foundAssert.getCondition());
@@ -165,6 +170,14 @@ private static void suggestFixForSameReference(
     } else if (expr2.getKind() == NULL_LITERAL) {
       // case: "assert expr [op] null"
       addFix(fix, (JCExpression) expr1, foundAssert, state, isEqual ? IS_NULL : IS_NOT_NULL);
+    } else if (ASTHelpers.getType(expr1).isPrimitive() || ASTHelpers.getType(expr2).isPrimitive()) {
+      // case: eg. "assert expr == 1"
+      addFix(
+          fix,
+          (JCExpression) expr1,
+          foundAssert,
+          state,
+          String.format(isEqual ? IS_EQUAL_TO : IS_NOT_EQUAL_TO, state.getSourceForNode(expr2)));
     } else {
       // case: "assert expr1 [op] expr2"
       addFix(

core/src/test/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTestsTest.java

@@ -326,13 +326,15 @@ public void wrongAssertReferenceSameCase() {
             INPUT,
             inputWithExpressions(
                 "Integer a = 1;", //
-                "assert a == 1;"))
+                "Integer b = 1;",
+                "assert a == b;"))
         .addOutputLines(
             OUTPUT,
             inputWithExpressionsAndImport(
                 ASSERT_THAT_IMPORT, //
                 "Integer a = 1;",
-                "assertThat(a).isSameInstanceAs(1);"))
+                "Integer b = 1;",
+                "assertThat(a).isSameInstanceAs(b);"))
         .doTest();
   }
 
@@ -343,18 +345,56 @@ public void wrongAssertReferenceWithParensCase() {
             INPUT,
             inputWithExpressions(
                 "Integer a = 1;", //
-                "assert (a == 1);"))
+                "Integer b = 1;",
+                "assert (a == b);"))
         .addOutputLines(
             OUTPUT,
             inputWithExpressionsAndImport(
                 ASSERT_THAT_IMPORT, //
                 "Integer a = 1;",
-                "assertThat(a).isSameInstanceAs(1);"))
+                "Integer b = 1;",
+                "assertThat(a).isSameInstanceAs(b);"))
         .doTest();
   }
 
   @Test
   public void wrongAssertReferenceNotSameCase() {
+    refactoringHelper
+        .addInputLines(
+            INPUT,
+            inputWithExpressions(
+                "Integer a = 1;", //
+                "Integer b = 1;",
+                "assert a != b;"))
+        .addOutputLines(
+            OUTPUT,
+            inputWithExpressionsAndImport(
+                ASSERT_THAT_IMPORT, //
+                "Integer a = 1;",
+                "Integer b = 1;",
+                "assertThat(a).isNotSameInstanceAs(b);"))
+        .doTest();
+  }
+
+  @Test
+  public void wrongAssertEqualsPrimitiveCase() {
+    refactoringHelper
+        .addInputLines(
+            INPUT,
+            inputWithExpressions(
+                "Integer a = 1;", //
+                "assert a == 1;"))
+        .addOutputLines(
+            OUTPUT,
+            inputWithExpressionsAndImport(
+                ASSERT_THAT_IMPORT, //
+                "Integer a = 1;",
+                "assertThat(a).isEqualTo(1);"))
+        .doTest();
+  }
+
+  @Test
+  public void wrongAssertNotEqualsPrimitiveCase() {
     refactoringHelper
         .addInputLines(
             INPUT,
@@ -366,7 +406,7 @@ public void wrongAssertReferenceNotSameCase() {
             inputWithExpressionsAndImport(
                 ASSERT_THAT_IMPORT, //
                 "Integer a = 1;",
-                "assertThat(a).isNotSameInstanceAs(1);"))
+                "assertThat(a).isNotEqualTo(1);"))
         .doTest();
   }
 
@@ -383,7 +423,7 @@ public void wrongAssertReferenceSameCaseWithDetailCase() {
             inputWithExpressionsAndImport(
                 ASSERT_WITH_MESSAGE_IMPORT, //
                 "int a = 1;",
-                "assertWithMessage(\"detail\").that(a).isSameInstanceAs(1);"))
+                "assertWithMessage(\"detail\").that(a).isEqualTo(1);"))
         .doTest();
   }