Only drop a literal zero delta in AssertEqualsIntegralDeltaToAssertEquals (#1212)

The recipe removed the delta from any `assertEquals(int|long, int|long, delta)`, so a tolerance check such as `assertEquals(100, elapsed, 50)` became the exact `assertEquals(100, elapsed)` and started failing. Only a literal zero delta is redundant, so the recipe now leaves any other delta in place, and the description no longer claims the delta is meaningless.

Author: timtebeek

src/main/java/org/openrewrite/java/testing/cleanup/AssertEqualsIntegralDeltaToAssertEquals.java

@@ -40,9 +40,7 @@ public class AssertEqualsIntegralDeltaToAssertEquals extends Recipe {
     final String displayName = "Remove unnecessary `assertEquals` delta argument for integral types";
 
     @Getter
-    final String description = "Remove the delta argument from `assertEquals()` when both expected and actual are " +
-            "`int` or `long` types, since the delta is meaningless for exact integer comparison. " +
-            "Integer arguments get unnecessarily upcasted to `double` when a delta is provided.";
+    final String description = "Remove a literal zero delta argument from `assertEquals()` when both expected and actual are `int` or `long` types, since a zero delta already asks for an exact comparison. Integer arguments get unnecessarily upcasted to `double` when a delta is provided; a non-zero delta is a tolerance and is kept.";
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
@@ -56,7 +54,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
                 }
 
                 List<Expression> args = mi.getArguments();
-                if (args.size() == 3 && isIntegralType(args.get(0)) && isIntegralType(args.get(1)) && isNumericType(args.get(2))) {
+                if (args.size() == 3 && isIntegralType(args.get(0)) && isIntegralType(args.get(1)) && isLiteralZero(args.get(2))) {
                     // assertEquals(expected, actual, delta) -> assertEquals(expected, actual)
                     StringBuilder sb = new StringBuilder();
                     if (mi.getSelect() != null) {
@@ -80,7 +78,7 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
                     return t.apply(updateCursor(mi), mi.getCoordinates().replace(), args.get(0), args.get(1));
                 }
 
-                if (args.size() == 4 && isIntegralType(args.get(0)) && isIntegralType(args.get(1)) && isNumericType(args.get(2))) {
+                if (args.size() == 4 && isIntegralType(args.get(0)) && isIntegralType(args.get(1)) && isLiteralZero(args.get(2))) {
                     // assertEquals(expected, actual, delta, message) -> assertEquals(expected, actual, message)
                     StringBuilder sb = new StringBuilder();
                     if (mi.getSelect() != null) {
@@ -117,18 +115,12 @@ private boolean isIntegralType(Expression expression) {
                 return p == JavaType.Primitive.Long || p == JavaType.Primitive.Int;
             }
 
-            private boolean isNumericType(Expression expression) {
-                JavaType.FullyQualified fq = TypeUtils.asFullyQualified(expression.getType());
-                if (fq != null) {
-                    String typeName = fq.getFullyQualifiedName();
-                    return "java.lang.Double".equals(typeName) || "java.lang.Float".equals(typeName) ||
-                            "java.lang.Long".equals(typeName) || "java.lang.Integer".equals(typeName) ||
-                            "java.lang.Short".equals(typeName) || "java.lang.Byte".equals(typeName);
+            private boolean isLiteralZero(Expression expression) {
+                if (!(expression instanceof J.Literal)) {
+                    return false;
                 }
-                JavaType.Primitive p = TypeUtils.asPrimitive(expression.getType());
-                return p == JavaType.Primitive.Double || p == JavaType.Primitive.Float ||
-                        p == JavaType.Primitive.Long || p == JavaType.Primitive.Int ||
-                        p == JavaType.Primitive.Short || p == JavaType.Primitive.Byte;
+                Object value = ((J.Literal) expression).getValue();
+                return value instanceof Number && ((Number) value).doubleValue() == 0;
             }
         });
     }

src/main/resources/META-INF/rewrite/recipes.csv

@@ -84,7 +84,7 @@ maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.tes
 maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.assertj.StaticImports,Statically import AssertJ's `assertThat`,Consistently use a static import rather than inlining the `Assertions` class name in tests.,3,AssertJ,Testing,Java,,,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.byteman.BytemanJUnit4ToBytemanJUnit5,Use Byteman JUnit 5 dependency,Migrates Byteman JUnit 4 to JUnit 5.,3,Byteman,Testing,Java,Recipes for [Byteman](https://byteman.jboss.org/) bytecode injection in tests.,,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.cleanup.AssertEqualsBooleanToAssertBoolean,"Replace JUnit `assertEquals(false, <boolean>)` to `assertFalse(<boolean>)` / `assertTrue(<boolean>)`",Using `assertFalse` or `assertTrue` is simpler and more clear.,1,Test cleanup,Testing,Java,Recipes to simplify assertions and clean up test code.,,Basic building blocks for transforming Java code.,,
-maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.cleanup.AssertEqualsIntegralDeltaToAssertEquals,Remove unnecessary `assertEquals` delta argument for integral types,"Remove the delta argument from `assertEquals()` when both expected and actual are `int` or `long` types, since the delta is meaningless for exact integer comparison. Integer arguments get unnecessarily upcasted to `double` when a delta is provided.",1,Test cleanup,Testing,Java,Recipes to simplify assertions and clean up test code.,,Basic building blocks for transforming Java code.,,
+maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.cleanup.AssertEqualsIntegralDeltaToAssertEquals,Remove unnecessary `assertEquals` delta argument for integral types,"Remove a literal zero delta argument from `assertEquals()` when both expected and actual are `int` or `long` types, since a zero delta already asks for an exact comparison. Integer arguments get unnecessarily upcasted to `double` when a delta is provided; a non-zero delta is a tolerance and is kept.",1,Test cleanup,Testing,Java,Recipes to simplify assertions and clean up test code.,,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.cleanup.AssertEqualsNullToAssertNull,"`assertEquals(a, null)` to `assertNull(a)`",Using `assertNull(a)` is simpler and more clear.,1,Test cleanup,Testing,Java,Recipes to simplify assertions and clean up test code.,,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.cleanup.AssertFalseEqualsToAssertNotEquals,"Replace JUnit `assertFalse(a.equals(b))` to `assertNotEquals(a,b)`","Using `assertNotEquals(a,b)` is simpler and more clear.",1,Test cleanup,Testing,Java,Recipes to simplify assertions and clean up test code.,,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.cleanup.AssertFalseNegationToAssertTrue,Replace JUnit `assertFalse(!<boolean>)` to `assertTrue(<boolean>)`,Using `assertTrue` is simpler and more clear.,1,Test cleanup,Testing,Java,Recipes to simplify assertions and clean up test code.,,Basic building blocks for transforming Java code.,,

src/test/java/org/openrewrite/java/testing/cleanup/AssertEqualsIntegralDeltaToAssertEqualsTest.java

@@ -185,6 +185,57 @@ void test() {
         );
     }
 
+    @Test
+    void noChangeForNonZeroDelta() {
+        rewriteRun(
+          java(
+            """
+              import static org.junit.jupiter.api.Assertions.assertEquals;
+
+              class Test {
+                  void test(long elapsed) {
+                      assertEquals(100, elapsed, 50);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void noChangeForNonZeroDeltaWithMessage() {
+        rewriteRun(
+          java(
+            """
+              import static org.junit.jupiter.api.Assertions.assertEquals;
+
+              class Test {
+                  void test(long elapsed) {
+                      assertEquals(100L, elapsed, 0.5, "elapsed");
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void noChangeForDeltaVariable() {
+        rewriteRun(
+          java(
+            """
+              import static org.junit.jupiter.api.Assertions.assertEquals;
+
+              class Test {
+                  void test(long elapsed, double tolerance) {
+                      assertEquals(100, elapsed, tolerance);
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @Test
     void noChangeForMessageArg() {
         rewriteRun(