From 1e6ad980176dec05075e11fa3ad7e9cae00aac9a Mon Sep 17 00:00:00 2001 From: Adam Birem Date: Tue, 5 Nov 2024 17:01:31 +0100 Subject: [PATCH 1/4] Add test case for issue https://github.com/openrewrite/rewrite-testing-frameworks/issues/634 --- .../SimplifyMockitoVerifyWhenGivenTest.java | 66 ++++++++++++++----- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java b/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java index 69815db02..df11132ec 100644 --- a/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java +++ b/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java @@ -42,7 +42,7 @@ void shouldRemoveUnneccesaryEqFromVerify() { import static org.mockito.Mockito.verify; import static org.mockito.Mockito.mock; import static org.mockito.ArgumentMatchers.eq; - + class Test { void test() { var mockString = mock(String.class); @@ -52,7 +52,7 @@ void test() { """, """ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.mock; - + class Test { void test() { var mockString = mock(String.class); @@ -64,6 +64,38 @@ void test() { ); } + @Test + void shouldRemoveUnneccesaryEqFromVerify_withMockitoStarImport() { + rewriteRun( + //language=Java + java( + """ + import static org.mockito.Mockito.eq; + import static org.mockito.Mockito.mock; + import static org.mockito.Mockito.verify; + + class Test { + void test() { + var mockString = mock(String.class); + verify(mockString).replace(eq("foo"), eq("bar")); + } + } + """, """ + import static org.mockito.Mockito.mock; + import static org.mockito.Mockito.verify; + + class Test { + void test() { + var mockString = mock(String.class); + verify(mockString).replace("foo", "bar"); + } + } + """ + ) + ); + } + + @Test void shouldRemoveUnneccesaryEqFromWhen() { rewriteRun( @@ -73,7 +105,7 @@ void shouldRemoveUnneccesaryEqFromWhen() { import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.mockito.ArgumentMatchers.eq; - + class Test { void test() { var mockString = mock(String.class); @@ -83,7 +115,7 @@ void test() { """, """ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; - + class Test { void test() { var mockString = mock(String.class); @@ -105,7 +137,7 @@ void shouldNotRemoveEqWhenMatchersAreMixed() { import static org.mockito.Mockito.when; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.anyString; - + class Test { void test() { var mockString = mock(String.class); @@ -125,7 +157,7 @@ void shouldRemoveUnneccesaryEqFromStubber() { """ import static org.mockito.Mockito.doThrow; import static org.mockito.ArgumentMatchers.eq; - + class Test { void test() { doThrow(new RuntimeException()).when("foo").substring(eq(1)); @@ -133,7 +165,7 @@ void test() { } """, """ import static org.mockito.Mockito.doThrow; - + class Test { void test() { doThrow(new RuntimeException()).when("foo").substring(1); @@ -152,7 +184,7 @@ void shouldRemoveUnneccesaryEqFromBDDGiven() { """ import static org.mockito.BDDMockito.given; import static org.mockito.ArgumentMatchers.eq; - + class Test { void test() { given("foo".substring(eq(1))); @@ -160,7 +192,7 @@ void test() { } """, """ import static org.mockito.BDDMockito.given; - + class Test { void test() { given("foo".substring(1)); @@ -181,13 +213,13 @@ void shouldNotRemoveEqImportWhenStillNeeded() { import static org.mockito.Mockito.when; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.anyString; - + class Test { void testRemoveEq() { var mockString = mock(String.class); when(mockString.replace(eq("foo"), eq("bar"))).thenReturn("bar"); } - + void testKeepEq() { var mockString = mock(String.class); when(mockString.replace(eq("foo"), anyString())).thenReturn("bar"); @@ -198,13 +230,13 @@ void testKeepEq() { import static org.mockito.Mockito.when; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.anyString; - + class Test { void testRemoveEq() { var mockString = mock(String.class); when(mockString.replace("foo", "bar")).thenReturn("bar"); } - + void testKeepEq() { var mockString = mock(String.class); when(mockString.replace(eq("foo"), anyString())).thenReturn("bar"); @@ -227,7 +259,7 @@ void shouldFixSonarExamples() { import static org.mockito.Mockito.doThrow; import static org.mockito.BDDMockito.given; import static org.mockito.ArgumentMatchers.eq; - + class Test { void test(Object v1, Object v2, Object v3, Object v4, Object v5, Foo foo) { given(foo.bar(eq(v1), eq(v2), eq(v3))).willReturn(null); @@ -236,7 +268,7 @@ void test(Object v1, Object v2, Object v3, Object v4, Object v5, Foo foo) { verify(foo).bar(eq(v1), eq(v2), eq(v3)); } } - + class Foo { Object bar(Object v1, Object v2, Object v3) { return null; } String baz(Object v4, Object v5) { return ""; } @@ -248,7 +280,7 @@ void quux(int x) {} import static org.mockito.Mockito.verify; import static org.mockito.Mockito.doThrow; import static org.mockito.BDDMockito.given; - + class Test { void test(Object v1, Object v2, Object v3, Object v4, Object v5, Foo foo) { given(foo.bar(v1, v2, v3)).willReturn(null); @@ -257,7 +289,7 @@ void test(Object v1, Object v2, Object v3, Object v4, Object v5, Foo foo) { verify(foo).bar(v1, v2, v3); } } - + class Foo { Object bar(Object v1, Object v2, Object v3) { return null; } String baz(Object v4, Object v5) { return ""; } From f90149e1dd2d4306b58f16c818e44ca57ad8ba28 Mon Sep 17 00:00:00 2001 From: Adam Birem Date: Tue, 5 Nov 2024 17:08:02 +0100 Subject: [PATCH 2/4] Add handling of org.mockito.Mockito.eq --- .../SimplifyMockitoVerifyWhenGiven.java | 50 ++++++++++--------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java b/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java index 76260a777..2102c455c 100644 --- a/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java +++ b/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java @@ -39,6 +39,7 @@ public class SimplifyMockitoVerifyWhenGiven extends Recipe { private static final MethodMatcher VERIFY_MATCHER = new MethodMatcher("org.mockito.Mockito verify(..)"); private static final MethodMatcher STUBBER_MATCHER = new MethodMatcher("org.mockito.stubbing.Stubber when(..)"); private static final MethodMatcher EQ_MATCHER = new MethodMatcher("org.mockito.ArgumentMatchers eq(..)"); + private static final MethodMatcher MOCKITO_EQ_MATCHER = new MethodMatcher("org.mockito.Mockito eq(..)"); @Override public String getDisplayName() { @@ -57,32 +58,35 @@ public Set getTags() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new UsesMethod<>(EQ_MATCHER), new JavaIsoVisitor() { - @Override - public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, ExecutionContext ctx) { - J.MethodInvocation mi = super.visitMethodInvocation(methodInvocation, ctx); + return Preconditions.check(Preconditions.or(new UsesMethod<>(EQ_MATCHER), new UsesMethod<>(MOCKITO_EQ_MATCHER)), + new JavaIsoVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, ExecutionContext ctx) { + J.MethodInvocation mi = super.visitMethodInvocation(methodInvocation, ctx); - if ((WHEN_MATCHER.matches(mi) || GIVEN_MATCHER.matches(mi)) && mi.getArguments().get(0) instanceof J.MethodInvocation) { - List updatedArguments = new ArrayList<>(mi.getArguments()); - updatedArguments.set(0, checkAndUpdateEq((J.MethodInvocation) mi.getArguments().get(0))); - mi = mi.withArguments(updatedArguments); - } else if (VERIFY_MATCHER.matches(mi.getSelect()) || - STUBBER_MATCHER.matches(mi.getSelect())) { - mi = checkAndUpdateEq(mi); - } + if ((WHEN_MATCHER.matches(mi) || GIVEN_MATCHER.matches(mi)) && mi.getArguments().get(0) instanceof J.MethodInvocation) { + List updatedArguments = new ArrayList<>(mi.getArguments()); + updatedArguments.set(0, checkAndUpdateEq((J.MethodInvocation) mi.getArguments().get(0))); + mi = mi.withArguments(updatedArguments); + } else if (VERIFY_MATCHER.matches(mi.getSelect()) || + STUBBER_MATCHER.matches(mi.getSelect())) { + mi = checkAndUpdateEq(mi); + } - maybeRemoveImport("org.mockito.ArgumentMatchers.eq"); - return mi; - } + maybeRemoveImport("org.mockito.ArgumentMatchers.eq"); + maybeRemoveImport("org.mockito.Mockito.eq"); + return mi; + } - private J.MethodInvocation checkAndUpdateEq(J.MethodInvocation methodInvocation) { - if (methodInvocation.getArguments().stream().allMatch(EQ_MATCHER::matches)) { - return methodInvocation.withArguments(ListUtils.map(methodInvocation.getArguments(), invocation -> - ((MethodCall) invocation).getArguments().get(0).withPrefix(invocation.getPrefix()))); - } - return methodInvocation; - } - }); + private J.MethodInvocation checkAndUpdateEq(J.MethodInvocation methodInvocation) { + if (methodInvocation.getArguments().stream().allMatch(arg -> EQ_MATCHER.matches(arg) || + MOCKITO_EQ_MATCHER.matches(arg))) { + return methodInvocation.withArguments(ListUtils.map(methodInvocation.getArguments(), invocation -> + ((MethodCall) invocation).getArguments().get(0).withPrefix(invocation.getPrefix()))); + } + return methodInvocation; + } + }); } } From f16729ae5e5d3fe048ffa896f943fe091a365ed6 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 5 Nov 2024 17:14:38 +0100 Subject: [PATCH 3/4] Apply formatter to minimize diff --- .../SimplifyMockitoVerifyWhenGiven.java | 4 ++-- .../SimplifyMockitoVerifyWhenGivenTest.java | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java b/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java index 2102c455c..e837aa9be 100644 --- a/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java +++ b/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java @@ -69,7 +69,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat updatedArguments.set(0, checkAndUpdateEq((J.MethodInvocation) mi.getArguments().get(0))); mi = mi.withArguments(updatedArguments); } else if (VERIFY_MATCHER.matches(mi.getSelect()) || - STUBBER_MATCHER.matches(mi.getSelect())) { + STUBBER_MATCHER.matches(mi.getSelect())) { mi = checkAndUpdateEq(mi); } @@ -80,7 +80,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat private J.MethodInvocation checkAndUpdateEq(J.MethodInvocation methodInvocation) { if (methodInvocation.getArguments().stream().allMatch(arg -> EQ_MATCHER.matches(arg) || - MOCKITO_EQ_MATCHER.matches(arg))) { + MOCKITO_EQ_MATCHER.matches(arg))) { return methodInvocation.withArguments(ListUtils.map(methodInvocation.getArguments(), invocation -> ((MethodCall) invocation).getArguments().get(0).withPrefix(invocation.getPrefix()))); } diff --git a/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java b/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java index df11132ec..0f04f8c96 100644 --- a/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java +++ b/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java @@ -64,12 +64,12 @@ void test() { ); } - @Test - void shouldRemoveUnneccesaryEqFromVerify_withMockitoStarImport() { - rewriteRun( - //language=Java - java( - """ + @Test + void shouldRemoveUnneccesaryEqFromVerify_withMockitoStarImport() { + rewriteRun( + //language=Java + java( + """ import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -91,9 +91,9 @@ void test() { } } """ - ) - ); - } + ) + ); + } @Test From 5a994126da26cc4b552b0c11c4ac3702b5a762b9 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 5 Nov 2024 17:16:34 +0100 Subject: [PATCH 4/4] Add issue reference to document the change --- .../testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java b/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java index 0f04f8c96..baa688cdf 100644 --- a/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java +++ b/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java @@ -18,6 +18,7 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.Issue; import org.openrewrite.java.JavaParser; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -65,6 +66,7 @@ void test() { } @Test + @Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/634") void shouldRemoveUnneccesaryEqFromVerify_withMockitoStarImport() { rewriteRun( //language=Java @@ -95,7 +97,6 @@ void test() { ); } - @Test void shouldRemoveUnneccesaryEqFromWhen() { rewriteRun(