From 3c92c0f26aa1151699eaab5fbcee74d483097d58 Mon Sep 17 00:00:00 2001 From: Sander Knauff Date: Sun, 13 Oct 2024 16:24:26 +0200 Subject: [PATCH] Add recipe for replacing unnecesary Mockito#eq with direct parameters (#615) * Add recipe for unnecesary Mockito#eq This adds a basic recipe for replacing Mockito#eq with direct parameters in when this is supported. This change should resolve Sonar issue java:S6068 * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Drop unnecessary annotations * Rename class and apply some first best practices * Make the tests pass by retaining the prefix * Update recipe to also support `doThrow#when` and `BDDMockito#given` Also tests methods using Method Matchers. Also removes the `Mockito#eq` import if it is not required anymore. * Format code * Apply suggested formatting changes * Apply suggested formatting changes * Inline methods simplified through MethodMatcher * Cast to MethodCall & replace conditionally * Remove unused import * Collapse if's with similar blocks * Add SimplifyMockitoVerifyWhenGiven to MockitoBestPractices --------- Co-authored-by: Tim te Beek Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tim te Beek --- .../SimplifyMockitoVerifyWhenGiven.java | 88 ++++++ .../resources/META-INF/rewrite/mockito.yml | 1 + .../SimplifyMockitoVerifyWhenGivenTest.java | 270 ++++++++++++++++++ 3 files changed, 359 insertions(+) create mode 100644 src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java create mode 100644 src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java diff --git a/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java b/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java new file mode 100644 index 000000000..76260a777 --- /dev/null +++ b/src/main/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGiven.java @@ -0,0 +1,88 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.testing.mockito; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.UsesMethod; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.MethodCall; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +public class SimplifyMockitoVerifyWhenGiven extends Recipe { + + private static final MethodMatcher WHEN_MATCHER = new MethodMatcher("org.mockito.Mockito when(..)"); + private static final MethodMatcher GIVEN_MATCHER = new MethodMatcher("org.mockito.BDDMockito given(..)"); + 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(..)"); + + @Override + public String getDisplayName() { + return "Call to Mockito method \"verify\", \"when\" or \"given\" should be simplified"; + } + + @Override + public String getDescription() { + return "Fixes Sonar issue `java:S6068`: Call to Mockito method \"verify\", \"when\" or \"given\" should be simplified."; + } + + @Override + public Set getTags() { + return Collections.singleton("RSPEC-6068"); + } + + @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); + + 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; + } + + 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; + } + }); + } + +} diff --git a/src/main/resources/META-INF/rewrite/mockito.yml b/src/main/resources/META-INF/rewrite/mockito.yml index 7e60ad788..5e5c61f6f 100644 --- a/src/main/resources/META-INF/rewrite/mockito.yml +++ b/src/main/resources/META-INF/rewrite/mockito.yml @@ -25,6 +25,7 @@ recipeList: - org.openrewrite.java.testing.mockito.Mockito1to5Migration - org.openrewrite.java.RemoveAnnotation: annotationPattern: "@org.mockito.junit.jupiter.MockitoSettings(strictness=org.mockito.quality.Strictness.WARN)" + - org.openrewrite.java.testing.mockito.SimplifyMockitoVerifyWhenGiven --- type: specs.openrewrite.org/v1beta/recipe name: org.openrewrite.java.testing.mockito.Mockito1to5Migration diff --git a/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java b/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java new file mode 100644 index 000000000..69815db02 --- /dev/null +++ b/src/test/java/org/openrewrite/java/testing/mockito/SimplifyMockitoVerifyWhenGivenTest.java @@ -0,0 +1,270 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.java.testing.mockito; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class SimplifyMockitoVerifyWhenGivenTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new SimplifyMockitoVerifyWhenGiven()) + .parser(JavaParser.fromJavaVersion().classpathFromResources(new InMemoryExecutionContext(), "mockito-core")); + } + + @DocumentExample + @Test + void shouldRemoveUnneccesaryEqFromVerify() { + rewriteRun( + //language=Java + java( + """ + 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); + verify(mockString).replace(eq("foo"), eq("bar")); + } + } + """, """ + import static org.mockito.Mockito.verify; + import static org.mockito.Mockito.mock; + + class Test { + void test() { + var mockString = mock(String.class); + verify(mockString).replace("foo", "bar"); + } + } + """ + ) + ); + } + + @Test + void shouldRemoveUnneccesaryEqFromWhen() { + rewriteRun( + //language=Java + java( + """ + 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); + when(mockString.replace(eq("foo"), eq("bar"))).thenReturn("bar"); + } + } + """, """ + import static org.mockito.Mockito.mock; + import static org.mockito.Mockito.when; + + class Test { + void test() { + var mockString = mock(String.class); + when(mockString.replace("foo", "bar")).thenReturn("bar"); + } + } + """ + ) + ); + } + + @Test + void shouldNotRemoveEqWhenMatchersAreMixed() { + rewriteRun( + //language=Java + java( + """ + import static org.mockito.Mockito.mock; + 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); + when(mockString.replace(eq("foo"), anyString())).thenReturn("bar"); + } + } + """ + ) + ); + } + + @Test + void shouldRemoveUnneccesaryEqFromStubber() { + rewriteRun( + //language=Java + java( + """ + import static org.mockito.Mockito.doThrow; + import static org.mockito.ArgumentMatchers.eq; + + class Test { + void test() { + doThrow(new RuntimeException()).when("foo").substring(eq(1)); + } + } + """, """ + import static org.mockito.Mockito.doThrow; + + class Test { + void test() { + doThrow(new RuntimeException()).when("foo").substring(1); + } + } + """ + ) + ); + } + + @Test + void shouldRemoveUnneccesaryEqFromBDDGiven() { + rewriteRun( + //language=Java + java( + """ + import static org.mockito.BDDMockito.given; + import static org.mockito.ArgumentMatchers.eq; + + class Test { + void test() { + given("foo".substring(eq(1))); + } + } + """, """ + import static org.mockito.BDDMockito.given; + + class Test { + void test() { + given("foo".substring(1)); + } + } + """ + ) + ); + } + + @Test + void shouldNotRemoveEqImportWhenStillNeeded() { + rewriteRun( + //language=Java + java( + """ + import static org.mockito.Mockito.mock; + 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"); + } + } + """, """ + import static org.mockito.Mockito.mock; + 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"); + } + } + """ + ) + ); + } + + @Test + void shouldFixSonarExamples() { + rewriteRun( + //language=Java + java( + """ + import static org.mockito.Mockito.mock; + import static org.mockito.Mockito.when; + import static org.mockito.Mockito.verify; + 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); + when(foo.baz(eq(v4), eq(v5))).thenReturn("foo"); + doThrow(new RuntimeException()).when(foo).quux(eq(42)); + 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 ""; } + void quux(int x) {} + } + """, """ + import static org.mockito.Mockito.mock; + import static org.mockito.Mockito.when; + 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); + when(foo.baz(v4, v5)).thenReturn("foo"); + doThrow(new RuntimeException()).when(foo).quux(42); + 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 ""; } + void quux(int x) {} + } + """ + ) + ); + } +}