From a30cba1d846062dac47f6298ebf702c3c9d28c05 Mon Sep 17 00:00:00 2001 From: Jammy Louie Date: Fri, 17 Jul 2026 16:33:49 -0400 Subject: [PATCH 1/2] Fix infinite loop unwrapping non-Matchers not() in Hamcrest JUnit5 recipes --- .../hamcrest/HamcrestInstanceOfToJUnit5.java | 2 +- .../hamcrest/HamcrestMatcherToJUnit5.java | 2 +- .../HamcrestInstanceOfToJUnit5Test.java | 29 +++++++++++++++++++ .../hamcrest/HamcrestMatcherToJUnit5Test.java | 29 +++++++++++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5.java b/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5.java index f8ceb435b..4ccc1fff0 100644 --- a/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5.java +++ b/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5.java @@ -70,7 +70,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, Execution } J.MethodInvocation matcherInvocation = (J.MethodInvocation) hamcrestMatcher; - while ("not".equals(matcherInvocation.getSimpleName())) { + while (RemoveNotMatcherVisitor.NOT_MATCHER.matches(matcherInvocation)) { maybeRemoveImport("org.hamcrest.Matchers.not"); maybeRemoveImport("org.hamcrest.CoreMatchers.not"); matcherInvocation = (J.MethodInvocation) new RemoveNotMatcherVisitor().visit(matcherInvocation, ctx); diff --git a/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5.java b/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5.java index cd4594b70..2117fbe81 100644 --- a/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5.java +++ b/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5.java @@ -137,7 +137,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu J.MethodInvocation matcherInvocation = (J.MethodInvocation) hamcrestMatcher; maybeRemoveImport("org.hamcrest.MatcherAssert.assertThat"); - while ("not".equals(matcherInvocation.getSimpleName())) { + while (RemoveNotMatcherVisitor.NOT_MATCHER.matches(matcherInvocation)) { maybeRemoveImport("org.hamcrest.Matchers.not"); maybeRemoveImport("org.hamcrest.CoreMatchers.not"); matcherInvocation = (J.MethodInvocation) new RemoveNotMatcherVisitor().visit(matcherInvocation, ctx); diff --git a/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5Test.java b/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5Test.java index 226fccfb7..84ea53efe 100644 --- a/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5Test.java +++ b/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5Test.java @@ -16,12 +16,15 @@ package org.openrewrite.java.testing.hamcrest; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.openrewrite.DocumentExample; import org.openrewrite.InMemoryExecutionContext; import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import java.util.concurrent.TimeUnit; + import static org.openrewrite.java.Assertions.java; import static org.openrewrite.test.TypeValidation.all; @@ -81,6 +84,32 @@ void testInstance() { ); } + @Test + @Timeout(value = 30, unit = TimeUnit.SECONDS, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) + void notFromCoreMatchersDoesNotHang() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + import java.util.List; + + import static org.hamcrest.MatcherAssert.assertThat; + import static org.hamcrest.Matchers.instanceOf; + import static org.hamcrest.CoreMatchers.not; + + class ATest { + private static final List list = List.of(); + @Test + void testInstance() { + assertThat(list, not(instanceOf(Integer.class))); + } + } + """ + ) + ); + } + @Test void assertionsWithReason() { //language=java diff --git a/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5Test.java b/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5Test.java index 40499b6f4..d9704c026 100644 --- a/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5Test.java +++ b/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5Test.java @@ -16,12 +16,15 @@ package org.openrewrite.java.testing.hamcrest; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.openrewrite.DocumentExample; import org.openrewrite.InMemoryExecutionContext; import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import java.util.concurrent.TimeUnit; + import static org.openrewrite.java.Assertions.java; import static org.openrewrite.test.TypeValidation.all; @@ -158,6 +161,32 @@ void testEquals() { ); } + @Test + @Timeout(value = 30, unit = TimeUnit.SECONDS, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) + void notFromCoreMatchersDoesNotHang() { + //language=java + rewriteRun( + spec -> spec.typeValidationOptions(all().immutableExecutionContext(false)), + java( + """ + import org.junit.jupiter.api.Test; + import static org.hamcrest.MatcherAssert.assertThat; + import static org.hamcrest.CoreMatchers.equalTo; + import static org.hamcrest.CoreMatchers.not; + + class ATest { + @Test + void testEquals() { + String str1 = "Hello world!"; + String str2 = "Hello world!"; + assertThat(str1, not(equalTo(str2))); + } + } + """ + ) + ); + } + @Test void greaterThan() { //language=java From 2e95cd796fea1adda16fc518108081dab7ac1127 Mon Sep 17 00:00:00 2001 From: Jammy Louie Date: Fri, 17 Jul 2026 16:37:41 -0400 Subject: [PATCH 2/2] Migrate CoreMatchers not()/instanceOf()/isA() in Hamcrest JUnit5 recipes --- .../hamcrest/HamcrestInstanceOfToJUnit5.java | 4 +- .../hamcrest/RemoveNotMatcherVisitor.java | 2 +- .../HamcrestInstanceOfToJUnit5Test.java | 59 ++++++++++++++++++- .../hamcrest/HamcrestMatcherToJUnit5Test.java | 16 ++++- 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5.java b/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5.java index 4ccc1fff0..be55d700d 100644 --- a/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5.java +++ b/src/main/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5.java @@ -38,8 +38,8 @@ public class HamcrestInstanceOfToJUnit5 extends Recipe { @Getter final String description = "Migrate from Hamcrest `instanceOf` and `isA` matcher to JUnit5 `assertInstanceOf` assertion."; - private static final MethodMatcher INSTANCE_OF_MATCHER = new MethodMatcher("org.hamcrest.Matchers instanceOf(..)"); - private static final MethodMatcher IS_A_MATCHER = new MethodMatcher("org.hamcrest.Matchers isA(..)"); + private static final MethodMatcher INSTANCE_OF_MATCHER = new MethodMatcher("org.hamcrest.*Matchers instanceOf(..)"); + private static final MethodMatcher IS_A_MATCHER = new MethodMatcher("org.hamcrest.*Matchers isA(..)"); private static final MethodMatcher ASSERT_THAT_MATCHER = new MethodMatcher("org.hamcrest.MatcherAssert assertThat(.., org.hamcrest.Matcher)"); @Override diff --git a/src/main/java/org/openrewrite/java/testing/hamcrest/RemoveNotMatcherVisitor.java b/src/main/java/org/openrewrite/java/testing/hamcrest/RemoveNotMatcherVisitor.java index 8a5a9507d..5d3aebfb4 100644 --- a/src/main/java/org/openrewrite/java/testing/hamcrest/RemoveNotMatcherVisitor.java +++ b/src/main/java/org/openrewrite/java/testing/hamcrest/RemoveNotMatcherVisitor.java @@ -26,7 +26,7 @@ import java.util.Objects; class RemoveNotMatcherVisitor extends JavaIsoVisitor { - static final MethodMatcher NOT_MATCHER = new MethodMatcher("org.hamcrest.Matchers not(..)"); + static final MethodMatcher NOT_MATCHER = new MethodMatcher("org.hamcrest.*Matchers not(..)"); public static boolean getLogicalContext(J.MethodInvocation mi, ExecutionContext ctx) throws InvalidParameterException { Object msg = ctx.getMessage(mi.toString()); diff --git a/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5Test.java b/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5Test.java index 84ea53efe..da7fbf9f0 100644 --- a/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5Test.java +++ b/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestInstanceOfToJUnit5Test.java @@ -86,7 +86,7 @@ void testInstance() { @Test @Timeout(value = 30, unit = TimeUnit.SECONDS, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) - void notFromCoreMatchersDoesNotHang() { + void notFromCoreMatchers() { //language=java rewriteRun( java( @@ -95,7 +95,7 @@ void notFromCoreMatchersDoesNotHang() { import java.util.List; import static org.hamcrest.MatcherAssert.assertThat; - import static org.hamcrest.Matchers.instanceOf; + import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.not; class ATest { @@ -105,6 +105,61 @@ void testInstance() { assertThat(list, not(instanceOf(Integer.class))); } } + """, + """ + import org.junit.jupiter.api.Test; + import java.util.List; + + import static org.junit.jupiter.api.Assertions.assertFalse; + + class ATest { + private static final List list = List.of(); + @Test + void testInstance() { + assertFalse(Integer.class.isAssignableFrom(list.getClass())); + } + } + """ + ) + ); + } + + @Test + void instanceOfAndIsAFromCoreMatchers() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + import java.util.List; + + import static org.hamcrest.MatcherAssert.assertThat; + import static org.hamcrest.CoreMatchers.instanceOf; + import static org.hamcrest.CoreMatchers.isA; + + class ATest { + private static final List list = List.of(); + @Test + void testInstance() { + assertThat(list, instanceOf(Iterable.class)); + assertThat(list, isA(Iterable.class)); + } + } + """, + """ + import org.junit.jupiter.api.Test; + import java.util.List; + + import static org.junit.jupiter.api.Assertions.assertInstanceOf; + + class ATest { + private static final List list = List.of(); + @Test + void testInstance() { + assertInstanceOf(Iterable.class, list); + assertInstanceOf(Iterable.class, list); + } + } """ ) ); diff --git a/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5Test.java b/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5Test.java index d9704c026..5082341da 100644 --- a/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5Test.java +++ b/src/test/java/org/openrewrite/java/testing/hamcrest/HamcrestMatcherToJUnit5Test.java @@ -163,7 +163,7 @@ void testEquals() { @Test @Timeout(value = 30, unit = TimeUnit.SECONDS, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) - void notFromCoreMatchersDoesNotHang() { + void notFromCoreMatchers() { //language=java rewriteRun( spec -> spec.typeValidationOptions(all().immutableExecutionContext(false)), @@ -182,6 +182,20 @@ void testEquals() { assertThat(str1, not(equalTo(str2))); } } + """, + """ + import org.junit.jupiter.api.Test; + + import static org.junit.jupiter.api.Assertions.assertNotEquals; + + class ATest { + @Test + void testEquals() { + String str1 = "Hello world!"; + String str2 = "Hello world!"; + assertNotEquals(str1, str2); + } + } """ ) );