From 25479625125651582714359deefd3d53b8c2fd06 Mon Sep 17 00:00:00 2001 From: Peter Streef Date: Fri, 5 May 2023 12:58:29 +0200 Subject: [PATCH 1/4] issue: Recipe `CleanupMockitoImports` cleans too much. My argument matchers are being cleaned up while used. I have reproduced the problem with a new failing test. I needed to add junit-pioneer as it was not yet present, I added with a fixed version as it looks like it is not part of the platform. --- build.gradle.kts | 1 + .../mockito/CleanupMockitoImportsTest.java | 54 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index 69500c229..d2f318d0e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -40,4 +40,5 @@ dependencies { testImplementation("org.openrewrite:rewrite-java-17") testImplementation("org.openrewrite:rewrite-groovy") + testImplementation("org.junit-pioneer:junit-pioneer:2.0.1") } diff --git a/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java b/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java index 60bfbd46e..d98330f68 100755 --- a/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java +++ b/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java @@ -16,8 +16,10 @@ package org.openrewrite.java.testing.mockito; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; 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; @@ -207,6 +209,58 @@ public class MockitoArgumentMatchersTest { ) ); } + + @Issue("#3111") //maybe not exactly the same issue though? + @ExpectedToFail + @Test + void removeUnusedArgumentMatchersOnly() { + //language=java + rewriteRun( + java( + """ + class MyObject { + String doAThing(Object other, String value){return value;} + } + """ + ), + java( + """ + import static org.mockito.Mockito.when; + import static org.mockito.Mockito.after; + import static org.mockito.ArgumentMatchers.any; + import static org.mockito.ArgumentMatchers.anyString; + import static org.mockito.ArgumentMatchers.eq; + import org.junit.jupiter.api.Test; + import org.mockito.Mock; + + class MyObjectTest { + @Mock + MyObject myObject; + + void test() { + when(myObject.doAThing(any(), eq("testValue"))).thenReturn("testValue"); + } + } + """, + """ + import static org.mockito.Mockito.when; + import static org.mockito.ArgumentMatchers.any; + import static org.mockito.ArgumentMatchers.eq; + import org.junit.jupiter.api.Test; + import org.mockito.Mock; + + class MyObjectTest { + @Mock + MyObject myObject; + + void test() { + when(myObject.doAThing(any(), eq("testValue"))).thenReturn("testValue"); + } + } + """ + ) + ); + } } From 8185f7d2e62bc088bf6dd5b36fbc1a4a9d5fd9c2 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Fri, 5 May 2023 13:02:18 +0200 Subject: [PATCH 2/4] Update issue link --- .../java/testing/mockito/CleanupMockitoImportsTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java b/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java index d98330f68..29f4debb9 100755 --- a/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java +++ b/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java @@ -210,9 +210,9 @@ public class MockitoArgumentMatchersTest { ); } - @Issue("#3111") //maybe not exactly the same issue though? - @ExpectedToFail @Test + @ExpectedToFail + @Issue("https://github.com/openrewrite/rewrite/issues/3111") // maybe not exactly the same issue void removeUnusedArgumentMatchersOnly() { //language=java rewriteRun( From c8c0e8a29022b18b792b6cf1cb7a01e87490b29d Mon Sep 17 00:00:00 2001 From: Peter Streef Date: Fri, 5 May 2023 15:02:55 +0200 Subject: [PATCH 3/4] update mockito version used in `CleanupMockitoImportsTest` and reproduce issue with lombok builder --- .../mockito/CleanupMockitoImportsTest.java | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java b/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java index 29f4debb9..9e3db6bef 100755 --- a/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java +++ b/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java @@ -32,7 +32,7 @@ class CleanupMockitoImportsTest implements RewriteTest { public void defaults(RecipeSpec spec) { spec .parser(JavaParser.fromJavaVersion() - .classpathFromResources(new InMemoryExecutionContext(), "mockito-all-1.10.19")) + .classpathFromResources(new InMemoryExecutionContext(), "mockito-core-3.12.4")) .recipe(new CleanupMockitoImports()); } @@ -218,8 +218,11 @@ void removeUnusedArgumentMatchersOnly() { rewriteRun( java( """ + import lombok.Builder; + @Builder class MyObject { - String doAThing(Object other, String value){return value;} + String field; + String doAThing(Object other, MyObject myObject){return value;} } """ ), @@ -234,12 +237,13 @@ class MyObject { import org.mockito.Mock; class MyObjectTest { - @Mock - MyObject myObject; - - void test() { - when(myObject.doAThing(any(), eq("testValue"))).thenReturn("testValue"); - } + @Mock + MyObject myObject; + + void test() { + var testObject = MyObject.builder().field("field").build(); + when(myObject.doAThing(any(), eq(testObject))).thenReturn("testValue"); + } } """, """ @@ -252,9 +256,10 @@ void test() { class MyObjectTest { @Mock MyObject myObject; - + void test() { - when(myObject.doAThing(any(), eq("testValue"))).thenReturn("testValue"); + MyObject testObject = MyObject.builder().field("field").build(); + when(myObject.doAThing(any(), eq(testObject))).thenReturn("testValue"); } } """ From acb258b2f337bd29f2aefd48d665b32678df6bff Mon Sep 17 00:00:00 2001 From: Peter Streef Date: Fri, 5 May 2023 15:09:59 +0200 Subject: [PATCH 4/4] add failing and succesful test with/without lombok --- .../mockito/CleanupMockitoImportsTest.java | 80 ++++++++++++++++--- 1 file changed, 70 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java b/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java index 9e3db6bef..05ced1916 100755 --- a/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java +++ b/src/test/java/org/openrewrite/java/testing/mockito/CleanupMockitoImportsTest.java @@ -213,15 +213,16 @@ public class MockitoArgumentMatchersTest { @Test @ExpectedToFail @Issue("https://github.com/openrewrite/rewrite/issues/3111") // maybe not exactly the same issue - void removeUnusedArgumentMatchersOnly() { + void removeUnusedArgumentMatchersOnlyWithLombok() { //language=java rewriteRun( java( """ - import lombok.Builder; - @Builder + import lombok.RequiredArgsConstructor; + @RequiredArgsConstructor class MyObject { - String field; + private final String field; + String doAThing(Object other, MyObject myObject){return value;} } """ @@ -237,13 +238,72 @@ class MyObject { import org.mockito.Mock; class MyObjectTest { - @Mock - MyObject myObject; + @Mock + MyObject myObject; + + void test() { + MyObject testObject = new MyObject("field"); + when(myObject.doAThing(any(), eq(testObject))).thenReturn("testValue"); + } + } + """, + """ + import static org.mockito.Mockito.when; + import static org.mockito.ArgumentMatchers.any; + import static org.mockito.ArgumentMatchers.eq; + import org.junit.jupiter.api.Test; + import org.mockito.Mock; + + class MyObjectTest { + @Mock + MyObject myObject; + + void test() { + MyObject testObject = new MyObject("field"); + when(myObject.doAThing(any(), eq(testObject))).thenReturn("testValue"); + } + } + """ + ) + ); + } + + @Test + void removeUnusedArgumentMatchersOnlyWithoutLombok() { + //language=java + rewriteRun( + java( + """ + + class MyObject { + private final String field; - void test() { - var testObject = MyObject.builder().field("field").build(); - when(myObject.doAThing(any(), eq(testObject))).thenReturn("testValue"); + MyObject(String field){ + this.field = field; } + + String doAThing(Object other, MyObject myObject){return value;} + } + """ + ), + java( + """ + import static org.mockito.Mockito.when; + import static org.mockito.Mockito.after; + import static org.mockito.ArgumentMatchers.any; + import static org.mockito.ArgumentMatchers.anyString; + import static org.mockito.ArgumentMatchers.eq; + import org.junit.jupiter.api.Test; + import org.mockito.Mock; + + class MyObjectTest { + @Mock + MyObject myObject; + + void test() { + MyObject testObject = new MyObject("field"); + when(myObject.doAThing(any(), eq(testObject))).thenReturn("testValue"); + } } """, """ @@ -258,7 +318,7 @@ class MyObjectTest { MyObject myObject; void test() { - MyObject testObject = MyObject.builder().field("field").build(); + MyObject testObject = new MyObject("field"); when(myObject.doAThing(any(), eq(testObject))).thenReturn("testValue"); } }