Refactor `Mockito.when` on static (non mock) to try with resource #601

Laurens-W · 2024-09-12T14:56:42Z

What's your motivation?

Mockito.when on static (non mock) is no longer allowed with Mockito 4

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

@timtebeek

Checklist

I've added unit tests to cover both positive and negative cases
I've read and applied the recipe conventions and best practices
I've used the IntelliJ IDEA auto-formatter on affected files

src/test/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStaticTest.java

timtebeek · 2024-09-12T16:30:13Z

I'm seeing failures with

java.lang.IllegalStateException: LST contains missing or invalid type information	
Identifier->ParameterizedType->VariableDeclarations->Resource->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/A	
Identifier->FieldAccess->MethodInvocation->NamedVariable->VariableDeclarations->Resource->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/A	
Identifier->MethodInvocation->MethodInvocation->Block->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/mockA	
MemberReference->MethodInvocation->MethodInvocation->Block->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(MemberReference type is missing or malformed)~~>*//*~~(Identifier type is missing or malformed)~~>*/A::getA	
Identifier->MemberReference->MethodInvocation->MethodInvocation->Block->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/A	
Identifier->MethodInvocation->Block->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/aMock

Would you already want me to have a look? Or are you still exploring those?

timtebeek · 2024-09-12T16:31:14Z

Also: could you hook this recipe into the Mockito migration already? That way we can't forget that step.

timtebeek · 2024-09-12T16:34:14Z

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java

+                                    String template = String.format("try(MockedStatic<%s> mock%s = mockStatic(%s.class)){\n" +
+                                                                    "mock%s.when(%s::%s).thenReturn(%s);\n" +
+                                                                    "}", arg_fq.getClassName(), arg_fq.getClassName(), arg_fq.getClassName(), arg_fq.getClassName(), arg_fq.getClassName(), ((J.MethodInvocation) when.getArguments().get(0)).getSimpleName(), ((J.MethodInvocation) stmt).getArguments().get(0));
+                                    m = JavaTemplate.builder(template)


Instead of String.format, can we use #{any()} in the template as much as possible, and pass the matching LST elements as arguments into the JavaTemplate.apply? That way the type attribution is already correct on those elements.

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java

src/test/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStaticTest.java

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java

Laurens-W · 2024-09-12T17:20:39Z

I'm seeing failures with

java.lang.IllegalStateException: LST contains missing or invalid type information	
Identifier->ParameterizedType->VariableDeclarations->Resource->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/A	
Identifier->FieldAccess->MethodInvocation->NamedVariable->VariableDeclarations->Resource->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/A	
Identifier->MethodInvocation->MethodInvocation->Block->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/mockA	
MemberReference->MethodInvocation->MethodInvocation->Block->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(MemberReference type is missing or malformed)~~>*//*~~(Identifier type is missing or malformed)~~>*/A::getA	
Identifier->MemberReference->MethodInvocation->MethodInvocation->Block->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/A	
Identifier->MethodInvocation->Block->Try->Block->MethodDeclaration->Block->ClassDeclaration->CompilationUnit	
/*~~(Identifier type is missing or malformed)~~>*/aMock

Would you already want me to have a look? Or are you still exploring those?

I'll have a look tomorrow, am aware of them :)

Add missing imports Fill out name and desc

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

timtebeek

I've pushed up a couple changes already, mostly to adopt ListUtils instead of the binary before/after lists that were there previously.

Also: can you point me to any resources where the non-MockStatic usage was valid with older Mockito version? I'm having a hard time finding any sources on whether this recipe would actually match any cases in practices, as it seems to have been newly introduced in 3.4.0.

timtebeek · 2024-09-16T21:03:35Z

src/test/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStaticTest.java

+                      try (MockedStatic<com.foo.A> mockA = mockStatic(com.foo.A.class)) {
+                          mockA.when(A.getNumber()).thenReturn(-1);
+                          assertEquals(A.getNumber(), -1);
+
+                          try (MockedStatic<com.foo.A> mockA2 = mockStatic(com.foo.A.class)) {
+                              mockA2.when(A.getNumber()).thenReturn(-2);
+                              assertEquals(A.getNumber(), -2);
+                          }
+                      }


I'm not sure if this is the exact snippet we ought to be generating, but I do think we should account for more than one static mocked method.

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java

timtebeek

Managed to get the types shortened and added some meaningful names to the extracted method and arguments to drive home what we're after there. Thanks again for the work done here! Let's hope this helps.

github-actions bot reviewed Sep 12, 2024

View reviewed changes

src/test/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStaticTest.java Outdated Show resolved Hide resolved

timtebeek assigned Laurens-W Sep 12, 2024

timtebeek added the enhancement New feature or request label Sep 12, 2024

timtebeek reviewed Sep 12, 2024

View reviewed changes

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java Show resolved Hide resolved

timtebeek reviewed Sep 12, 2024

View reviewed changes

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java Outdated Show resolved Hide resolved

timtebeek reviewed Sep 12, 2024

View reviewed changes

src/test/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStaticTest.java Outdated Show resolved Hide resolved

timtebeek reviewed Sep 12, 2024

View reviewed changes

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java Outdated Show resolved Hide resolved

Laurens-W added 3 commits September 13, 2024 15:48

WIP

d274c98

Format and suggestion

497252d

Add recipe to mockito recipes

3d4b4d8

Add missing imports Fill out name and desc

Laurens-W force-pushed the mockito-when-on-static branch from e690282 to 3d4b4d8 Compare September 13, 2024 13:49

timtebeek and others added 3 commits September 16, 2024 09:54

Merge branch 'main' into mockito-when-on-static

7a85e68

Update test with TypeValidation

8c9d1d4

Format and refactor to remove errors

c25c40d

timtebeek marked this pull request as ready for review September 16, 2024 14:57

Merge branch 'main' into mockito-when-on-static

760e451

Laurens-W requested a review from timtebeek September 16, 2024 15:34

timtebeek added 3 commits September 16, 2024 21:23

Merge branch 'main' into mockito-when-on-static

24defbe

Reduce and flatten ahead of further changes

903899e

Show problematic case with new unit test

92da6ad

timtebeek marked this pull request as draft September 16, 2024 19:43

github-actions bot reviewed Sep 16, 2024

View reviewed changes

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java Outdated Show resolved Hide resolved

timtebeek added 2 commits September 16, 2024 22:14

Generate a non-conflicting variable name of the right type

59b721a

Switch to ListUtils.flatMap without mock field in test

fa1ad4b

github-actions bot reviewed Sep 16, 2024

View reviewed changes

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java Outdated Show resolved Hide resolved

Apply suggestions from code review

011362c

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

timtebeek reviewed Sep 16, 2024

View reviewed changes

Make recursive in case multiple occurrences exist

04ca397

github-actions bot reviewed Sep 17, 2024

View reviewed changes

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java Outdated Show resolved Hide resolved

src/main/java/org/openrewrite/java/testing/mockito/MockitoWhenOnStaticToMockStatic.java Outdated Show resolved Hide resolved

Return immediately

69d2522

Laurens-W marked this pull request as ready for review September 19, 2024 15:12

timtebeek added 3 commits September 21, 2024 12:52

Reduce nesting and clarify intent through method rename

50c9e0c

Only add preceding statements before try, not all

1976e51

Shorten qualified class refs & enable method type validation

fdd26ac

timtebeek approved these changes Sep 21, 2024

View reviewed changes

timtebeek merged commit 4367149 into main Sep 21, 2024
2 checks passed

timtebeek deleted the mockito-when-on-static branch September 21, 2024 11:39

timtebeek mentioned this pull request Sep 23, 2024

Add JMockit's MockUp to Mockito migration #599

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `Mockito.when` on static (non mock) to try with resource #601

Refactor `Mockito.when` on static (non mock) to try with resource #601

Laurens-W commented Sep 12, 2024 •

edited

Loading

timtebeek commented Sep 12, 2024

timtebeek commented Sep 12, 2024

timtebeek Sep 12, 2024

Laurens-W commented Sep 12, 2024

timtebeek left a comment

timtebeek Sep 16, 2024

timtebeek left a comment

Refactor Mockito.when on static (non mock) to try with resource #601

Refactor Mockito.when on static (non mock) to try with resource #601

Conversation

Laurens-W commented Sep 12, 2024 • edited Loading

What's your motivation?

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Checklist

timtebeek commented Sep 12, 2024

timtebeek commented Sep 12, 2024

timtebeek Sep 12, 2024

Choose a reason for hiding this comment

Laurens-W commented Sep 12, 2024

timtebeek left a comment

Choose a reason for hiding this comment

timtebeek Sep 16, 2024

Choose a reason for hiding this comment

timtebeek left a comment

Choose a reason for hiding this comment

Refactor `Mockito.when` on static (non mock) to try with resource #601

Refactor `Mockito.when` on static (non mock) to try with resource #601

Laurens-W commented Sep 12, 2024 •

edited

Loading