From 8075b7c2d129bea61ac0462188088190fa0c937d Mon Sep 17 00:00:00 2001 From: mdepaula Date: Wed, 5 Aug 2026 12:40:14 -0400 Subject: [PATCH 1/3] Skip assertEquals/assertNotEquals conversion when equals() argument is null AssertFalseEqualsToAssertNotEquals and AssertTrueEqualsToAssertEquals were converting assertFalse(a.equals(null)) and assertTrue(a.equals(null)) to assertNotEquals(a, null) / assertEquals(a, null). AssertionsArgumentOrder then hoisted null to the expected position, producing assertNotEquals(null, a). Assertions.objectsAreEqual short-circuits on a null first argument, returning a == null without ever invoking a.equals(null). This silently disabled the null-contract test for custom equals() implementations. Fix: return false from isEquals() when the argument to .equals() is a null literal, leaving the original assertion unchanged. Fixes https://github.com/openrewrite/rewrite-testing-frameworks/issues/1071 --- .../AssertFalseEqualsToAssertNotEquals.java | 11 +++++++-- .../AssertTrueEqualsToAssertEquals.java | 11 +++++++-- ...AssertFalseEqualToAssertNotEqualsTest.java | 24 +++++++++++++++++++ .../AssertTrueEqualsToAssertEqualsTest.java | 24 +++++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualsToAssertNotEquals.java b/src/main/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualsToAssertNotEquals.java index 9f3840ba9..ef0bb25cf 100644 --- a/src/main/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualsToAssertNotEquals.java +++ b/src/main/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualsToAssertNotEquals.java @@ -87,8 +87,15 @@ private boolean isEquals(Expression expr) { J.MethodInvocation methodInvocation = (J.MethodInvocation) expr; - return "equals".equals(methodInvocation.getName().getSimpleName()) && - methodInvocation.getArguments().size() == 1; + if (!"equals".equals(methodInvocation.getName().getSimpleName()) || + methodInvocation.getArguments().size() != 1) { + return false; + } + Expression arg = methodInvocation.getArguments().get(0); + if (arg instanceof J.Literal && ((J.Literal) arg).getValue() == null) { + return false; + } + return true; } }); } diff --git a/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEquals.java b/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEquals.java index be899ab5d..331d08fcd 100644 --- a/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEquals.java +++ b/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEquals.java @@ -89,8 +89,15 @@ private boolean isEquals(Expression expr) { J.MethodInvocation methodInvocation = (J.MethodInvocation) expr; - return "equals".equals(methodInvocation.getName().getSimpleName()) && - methodInvocation.getArguments().size() == 1; + if (!"equals".equals(methodInvocation.getName().getSimpleName()) || + methodInvocation.getArguments().size() != 1) { + return false; + } + Expression arg = methodInvocation.getArguments().get(0); + if (arg instanceof J.Literal && ((J.Literal) arg).getValue() == null) { + return false; + } + return true; } }); } diff --git a/src/test/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java b/src/test/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java index ce658c043..72690bc9b 100644 --- a/src/test/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java +++ b/src/test/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java @@ -101,6 +101,30 @@ void test() { ); } + @SuppressWarnings({"ConstantConditions", "SimplifiableAssertion"}) + @Test + void doNotConvertWhenArgumentToEqualsIsNull() { + // Converting assertFalse(a.equals(null)) to assertNotEquals(null, a) changes semantics: + // assertNotEquals calls objectsAreEqual(null, a) which short-circuits to (a == null) + // without ever invoking a.equals(null), silently disabling the null-contract test. + //language=java + rewriteRun( + java( + """ + import static org.junit.jupiter.api.Assertions.assertFalse; + + public class Test { + void test() { + String a = "a"; + assertFalse(a.equals(null)); + assertFalse(a.equals(null), "message"); + } + } + """ + ) + ); + } + @SuppressWarnings("ConstantConditions") @Test void retainEqualsAndedWithSomethingElse() { diff --git a/src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java b/src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java index 46e9d2e37..b44186118 100644 --- a/src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java +++ b/src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java @@ -126,6 +126,30 @@ void test() { ); } + @SuppressWarnings({"ConstantConditions", "SimplifiableAssertion"}) + @Test + void doNotConvertWhenArgumentToEqualsIsNull() { + // Converting assertTrue(a.equals(null)) to assertEquals(null, a) changes semantics: + // assertEquals calls objectsAreEqual(null, a) which short-circuits to (a == null) + // without ever invoking a.equals(null), silently disabling the null-contract test. + //language=java + rewriteRun( + java( + """ + import static org.junit.jupiter.api.Assertions.assertTrue; + + public class Test { + void test() { + String a = "a"; + assertTrue(a.equals(null)); + assertTrue(a.equals(null), "message"); + } + } + """ + ) + ); + } + @SuppressWarnings("ConstantConditions") @Test void retainEqualsAndedWithSomethingElse() { From 6440f62da1b781f3b7a79520a99bd16694758d4d Mon Sep 17 00:00:00 2001 From: mdepaula Date: Wed, 5 Aug 2026 12:47:44 -0400 Subject: [PATCH 2/3] Use J.Literal.isLiteralValue idiom for null literal check in isEquals() --- .../testing/cleanup/AssertFalseEqualsToAssertNotEquals.java | 6 +----- .../testing/cleanup/AssertTrueEqualsToAssertEquals.java | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualsToAssertNotEquals.java b/src/main/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualsToAssertNotEquals.java index ef0bb25cf..8e9e0888c 100644 --- a/src/main/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualsToAssertNotEquals.java +++ b/src/main/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualsToAssertNotEquals.java @@ -91,11 +91,7 @@ private boolean isEquals(Expression expr) { methodInvocation.getArguments().size() != 1) { return false; } - Expression arg = methodInvocation.getArguments().get(0); - if (arg instanceof J.Literal && ((J.Literal) arg).getValue() == null) { - return false; - } - return true; + return !J.Literal.isLiteralValue(methodInvocation.getArguments().get(0), null); } }); } diff --git a/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEquals.java b/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEquals.java index 331d08fcd..4ea7f7698 100644 --- a/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEquals.java +++ b/src/main/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEquals.java @@ -93,11 +93,7 @@ private boolean isEquals(Expression expr) { methodInvocation.getArguments().size() != 1) { return false; } - Expression arg = methodInvocation.getArguments().get(0); - if (arg instanceof J.Literal && ((J.Literal) arg).getValue() == null) { - return false; - } - return true; + return !J.Literal.isLiteralValue(methodInvocation.getArguments().get(0), null); } }); } From d9756bb6eaf0fa36239ebf5291a49721b8115d09 Mon Sep 17 00:00:00 2001 From: mdepaula Date: Wed, 5 Aug 2026 12:54:11 -0400 Subject: [PATCH 3/3] Fix test formatting to match file conventions --- .../testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java | 3 --- .../testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java b/src/test/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java index 72690bc9b..29e51b77e 100644 --- a/src/test/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java +++ b/src/test/java/org/openrewrite/java/testing/cleanup/AssertFalseEqualToAssertNotEqualsTest.java @@ -104,9 +104,6 @@ void test() { @SuppressWarnings({"ConstantConditions", "SimplifiableAssertion"}) @Test void doNotConvertWhenArgumentToEqualsIsNull() { - // Converting assertFalse(a.equals(null)) to assertNotEquals(null, a) changes semantics: - // assertNotEquals calls objectsAreEqual(null, a) which short-circuits to (a == null) - // without ever invoking a.equals(null), silently disabling the null-contract test. //language=java rewriteRun( java( diff --git a/src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java b/src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java index b44186118..f4a20229c 100644 --- a/src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java +++ b/src/test/java/org/openrewrite/java/testing/cleanup/AssertTrueEqualsToAssertEqualsTest.java @@ -129,9 +129,6 @@ void test() { @SuppressWarnings({"ConstantConditions", "SimplifiableAssertion"}) @Test void doNotConvertWhenArgumentToEqualsIsNull() { - // Converting assertTrue(a.equals(null)) to assertEquals(null, a) changes semantics: - // assertEquals calls objectsAreEqual(null, a) which short-circuits to (a == null) - // without ever invoking a.equals(null), silently disabling the null-contract test. //language=java rewriteRun( java(