From 8a95cb80a7633b15e28272d1449d4fc44f4bf1b0 Mon Sep 17 00:00:00 2001 From: Yurii Date: Fri, 11 Oct 2024 13:19:03 +0200 Subject: [PATCH] Migrate JMockit block to Mockito which contains redundant "this" prefix for arg matcher and migrate array as parameter for any (#610) * cope with redundant "this." parameters in expectation and verification block * fix incorrect logic made caused by carelessness * add test for redundant this modifer * add support for refactoring any() with array as parameter * add test to make sure OpenRewrite will not confuse identifiers with and without "this" * refactor to functional programming style * add tests in unit tests for expectations because the change in ArgumentMatchersRewrite.java affects Expectations too * Minor polish --------- Co-authored-by: Sheng Yu Co-authored-by: Tim te Beek --- .../jmockit/ArgumentMatchersRewriter.java | 84 ++++++++---- .../JMockitExpectationsToMockitoTest.java | 73 +++++++++- .../JMockitVerificationsToMockitoTest.java | 125 +++++++++++++++++- 3 files changed, 245 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/openrewrite/java/testing/jmockit/ArgumentMatchersRewriter.java b/src/main/java/org/openrewrite/java/testing/jmockit/ArgumentMatchersRewriter.java index ee5e86c59..71694a704 100644 --- a/src/main/java/org/openrewrite/java/testing/jmockit/ArgumentMatchersRewriter.java +++ b/src/main/java/org/openrewrite/java/testing/jmockit/ArgumentMatchersRewriter.java @@ -99,17 +99,23 @@ private J.MethodInvocation rewriteMethodInvocation(J.MethodInvocation invocation if (invocation.getSelect() instanceof J.MethodInvocation) { invocation = invocation.withSelect(rewriteMethodInvocation((J.MethodInvocation) invocation.getSelect())); } + // in mockito, argument matchers must be used for all arguments or none - boolean hasArgumentMatcher = false; List arguments = invocation.getArguments(); - for (Expression methodArgument : arguments) { - if (isJmockitArgumentMatcher(methodArgument)) { - hasArgumentMatcher = true; - break; + // replace this.matcher with matcher, otherwise it's ignored + arguments.replaceAll(arg -> { + if (arg instanceof J.FieldAccess) { + J.FieldAccess fieldAccess = (J.FieldAccess) arg; + if (fieldAccess.getTarget() instanceof J.Identifier && + "this".equals(((J.Identifier) fieldAccess.getTarget()).getSimpleName())) { + return fieldAccess.getName(); + } } - } + return arg; + }); + // if there are no argument matchers, return the invocation as-is - if (!hasArgumentMatcher) { + if (arguments.stream().noneMatch(ArgumentMatchersRewriter::isJmockitArgumentMatcher)) { return invocation; } // replace each argument with the appropriate argument matcher @@ -145,9 +151,9 @@ private Expression rewriteMethodArgument(Expression methodArgument) { // ((int) any) to anyInt(), ((long) any) to anyLong(), etc argumentMatcher = PRIMITIVE_TO_MOCKITO_ARGUMENT_MATCHER.get(type); template = argumentMatcher + "()"; - } else if (type instanceof JavaType.FullyQualified) { - // (() any) to any(.class) - return rewriteFullyQualifiedToArgumentMatcher(methodArgument, (JavaType.FullyQualified) type); + } else if (type instanceof JavaType.FullyQualified || type instanceof JavaType.Array) { + // (() any) to any(.class), type can also be simple array + return rewriteAnyWithClassParameterToArgumentMatcher(methodArgument, type); } if (template == null || argumentMatcher == null) { // unhandled type, return argument unchanged @@ -157,7 +163,7 @@ private Expression rewriteMethodArgument(Expression methodArgument) { } private Expression applyArgumentTemplate(Expression methodArgument, String argumentMatcher, String template, - List templateParams) { + List templateParams) { visitor.maybeAddImport("org.mockito.Mockito", argumentMatcher); return JavaTemplate.builder(template) .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "mockito-core-3.12")) @@ -166,8 +172,7 @@ private Expression applyArgumentTemplate(Expression methodArgument, String argum .apply( new Cursor(visitor.getCursor(), methodArgument), methodArgument.getCoordinates().replace(), - templateParams.toArray() - ); + templateParams.toArray()); } private Expression applyClassArgumentTemplate(Expression methodArgument, JavaType.FullyQualified type) { @@ -179,24 +184,32 @@ private Expression applyClassArgumentTemplate(Expression methodArgument, JavaTyp .apply( new Cursor(visitor.getCursor(), methodArgument), methodArgument.getCoordinates().replace(), - type.getClassName() - )) + type.getClassName())) .withType(type); } - private Expression rewriteFullyQualifiedToArgumentMatcher(Expression methodArgument, JavaType.FullyQualified type) { + private Expression rewriteAnyWithClassParameterToArgumentMatcher(Expression methodArgument, JavaType type) { String template; List templateParams = new ArrayList<>(); - String argumentMatcher = FQN_TO_MOCKITO_ARGUMENT_MATCHER.get(type.getFullyQualifiedName()); - if (argumentMatcher != null) { - // mockito has convenience argument matchers - template = argumentMatcher + "()"; - return applyArgumentTemplate(methodArgument, argumentMatcher, template, templateParams); + + if (type instanceof JavaType.FullyQualified) { + JavaType.FullyQualified fq = (JavaType.FullyQualified) type; + String argumentMatcher = FQN_TO_MOCKITO_ARGUMENT_MATCHER.get(fq.getFullyQualifiedName()); + if (argumentMatcher != null) { + // mockito has convenience argument matchers + template = argumentMatcher + "()"; + return applyArgumentTemplate(methodArgument, argumentMatcher, template, templateParams); + } } // mockito uses any(Class) for all other types - argumentMatcher = "any"; + String argumentMatcher = "any"; template = argumentMatcher + "(#{any(java.lang.Class)})"; - templateParams.add(applyClassArgumentTemplate(methodArgument, type)); + + if (type instanceof JavaType.FullyQualified) { + templateParams.add(applyClassArgumentTemplate(methodArgument, (JavaType.FullyQualified) type)); + } else if (type instanceof JavaType.Array) { + templateParams.add(applyArrayClassArgumentTemplate(methodArgument, ((JavaType.Array) type).getElemType())); + } J.MethodInvocation invocationArgument = (J.MethodInvocation) applyArgumentTemplate(methodArgument, argumentMatcher, template, templateParams); @@ -209,15 +222,34 @@ private Expression rewriteFullyQualifiedToArgumentMatcher(Expression methodArgum !(invocationArgument.getMethodType().getParameterTypes().get(0) instanceof JavaType.Parameterized)) { return invocationArgument; } - JavaType.Parameterized newParameterType = - ((JavaType.Parameterized) invocationArgument.getMethodType().getParameterTypes().get(0)) - .withTypeParameters(Collections.singletonList(classArgument.getType())); + JavaType.Parameterized newParameterType = ((JavaType.Parameterized) invocationArgument.getMethodType() + .getParameterTypes().get(0)) + .withTypeParameters(Collections.singletonList(classArgument.getType())); JavaType.Method newMethodType = invocationArgument.getMethodType() .withReturnType(classArgument.getType()) .withParameterTypes(Collections.singletonList(newParameterType)); return invocationArgument.withMethodType(newMethodType); } + private Expression applyArrayClassArgumentTemplate(Expression methodArgument, JavaType elementType) { + String newArrayElementClassName = ""; + if (elementType instanceof JavaType.FullyQualified) { + newArrayElementClassName = ((JavaType.FullyQualified) elementType).getClassName(); + } else if (elementType instanceof JavaType.Primitive) { + newArrayElementClassName = ((JavaType.Primitive) elementType).getKeyword(); + } else { + newArrayElementClassName = elementType.getClass().getName(); + } + + return JavaTemplate.builder("#{}[].class") + .javaParser(JavaParser.fromJavaVersion()) + .build() + .apply( + new Cursor(visitor.getCursor(), methodArgument), + methodArgument.getCoordinates().replace(), + newArrayElementClassName); + } + private static boolean isJmockitArgumentMatcher(Expression expression) { if (expression instanceof J.TypeCast) { expression = ((J.TypeCast) expression).getExpression(); diff --git a/src/test/java/org/openrewrite/java/testing/jmockit/JMockitExpectationsToMockitoTest.java b/src/test/java/org/openrewrite/java/testing/jmockit/JMockitExpectationsToMockitoTest.java index b2b529193..27c27c838 100644 --- a/src/test/java/org/openrewrite/java/testing/jmockit/JMockitExpectationsToMockitoTest.java +++ b/src/test/java/org/openrewrite/java/testing/jmockit/JMockitExpectationsToMockitoTest.java @@ -210,7 +210,7 @@ void test() { import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; - + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.when; @@ -668,6 +668,9 @@ public String getSomeField(List input) { public String getSomeOtherField(Object input) { return "Y"; } + public String getSomeArrayField(Object input) { + return "Z"; + } } """ ), @@ -694,9 +697,12 @@ void test() { result = null; myObject.getSomeOtherField((Object) any); result = null; + myObject.getSomeArrayField((byte[]) any); + result = null; }}; assertNull(myObject.getSomeField(new ArrayList<>())); assertNull(myObject.getSomeOtherField(new Object())); + assertNull(myObject.getSomeArrayField(new byte[0])); } } """, @@ -719,8 +725,10 @@ class MyTest { void test() { when(myObject.getSomeField(anyList())).thenReturn(null); when(myObject.getSomeOtherField(any(Object.class))).thenReturn(null); + when(myObject.getSomeArrayField(any(byte[].class))).thenReturn(null); assertNull(myObject.getSomeField(new ArrayList<>())); assertNull(myObject.getSomeOtherField(new Object())); + assertNull(myObject.getSomeArrayField(new byte[0])); } } """ @@ -1075,7 +1083,7 @@ void whenMinTimes() { class MyTest { @Mocked Object myObject; - + void test() { new Expectations() {{ myObject.wait(anyLong, anyInt); @@ -1096,7 +1104,7 @@ void test() { class MyTest { @Mock Object myObject; - + void test() { myObject.wait(10L, 10); verify(myObject, atLeast(2)).wait(anyLong(), anyInt()); @@ -1122,7 +1130,7 @@ void whenMaxTimes() { class MyTest { @Mocked Object myObject; - + void test() { new Expectations() {{ myObject.wait(anyLong, anyInt); @@ -1143,7 +1151,7 @@ void test() { class MyTest { @Mock Object myObject; - + void test() { myObject.wait(10L, 10); verify(myObject, atMost(5)).wait(anyLong(), anyInt()); @@ -1169,7 +1177,7 @@ void whenMinTimesMaxTimes() { class MyTest { @Mocked Object myObject; - + void test() { new Expectations() {{ myObject.wait(anyLong, anyInt); @@ -1191,7 +1199,7 @@ void test() { class MyTest { @Mock Object myObject; - + void test() { myObject.wait(10L, 10); verify(myObject, atLeast(1)).wait(anyLong(), anyInt()); @@ -1549,6 +1557,57 @@ void test() { ); } + @Test + void whenWithRedundantThisModifier() { + //language=java + rewriteRun( + java( + """ + import mockit.Expectations; + import mockit.Mocked; + import mockit.integration.junit5.JMockitExtension; + import org.junit.jupiter.api.extension.ExtendWith; + + import static org.junit.jupiter.api.Assertions.assertEquals; + import static org.junit.jupiter.api.Assertions.assertNull; + + @ExtendWith(JMockitExtension.class) + class MyTest { + @Mocked + Object myObject; + + void test() { + new Expectations() {{ + myObject.wait(this.anyLong, anyInt); + }}; + myObject.wait(); + } + } + """, + """ + import org.junit.jupiter.api.extension.ExtendWith; + import org.mockito.Mock; + import org.mockito.junit.jupiter.MockitoExtension; + + import static org.junit.jupiter.api.Assertions.assertEquals; + import static org.junit.jupiter.api.Assertions.assertNull; + import static org.mockito.Mockito.*; + + @ExtendWith(MockitoExtension.class) + class MyTest { + @Mock + Object myObject; + + void test() { + myObject.wait(); + verify(myObject).wait(anyLong(), anyInt()); + } + } + """ + ) + ); + } + @Disabled // comment migration not supported yet @Test void whenComments() { diff --git a/src/test/java/org/openrewrite/java/testing/jmockit/JMockitVerificationsToMockitoTest.java b/src/test/java/org/openrewrite/java/testing/jmockit/JMockitVerificationsToMockitoTest.java index 3c0717ad8..148d2a737 100644 --- a/src/test/java/org/openrewrite/java/testing/jmockit/JMockitVerificationsToMockitoTest.java +++ b/src/test/java/org/openrewrite/java/testing/jmockit/JMockitVerificationsToMockitoTest.java @@ -684,10 +684,8 @@ class MyTest { void test() { myObject.wait(); - new Verifications() {{ - - myObject.wait(); - + new Verifications() {{ + myObject.wait(); myObject.wait(anyLong, anyInt); }}; myObject.wait(1L); @@ -828,4 +826,123 @@ void test() { ) ); } + + @Test + void whenWithRedundantThisModifier() { + //language=java + rewriteRun( + java( + """ + import mockit.Verifications; + import mockit.Mocked; + import mockit.integration.junit5.JMockitExtension; + import org.junit.jupiter.api.extension.ExtendWith; + + @ExtendWith(JMockitExtension.class) + class MyTest { + @Mocked + Object myObject; + + void test() { + new Verifications() {{ + myObject.wait(this.anyLong, this.anyInt); + myObject.wait(anyLong, this.anyInt); + }}; + } + } + """, + """ + import org.junit.jupiter.api.extension.ExtendWith; + import org.mockito.Mock; + import org.mockito.junit.jupiter.MockitoExtension; + + import static org.mockito.Mockito.*; + + @ExtendWith(MockitoExtension.class) + class MyTest { + @Mock + Object myObject; + + void test() { + verify(myObject).wait(anyLong(), anyInt()); + verify(myObject).wait(anyLong(), anyInt()); + } + } + """ + ) + ); + } + + @Test + void whenArrayArgumentMatcher() { + //language=java + rewriteRun( + spec -> spec.afterTypeValidationOptions(TypeValidation.builder().methodInvocations(false).build()), + java( + """ + import java.util.List; + + class MyObject { + public String getSomeObject(Object input) { + return "Z"; + } + } + """ + ), + java( + """ + import java.util.ArrayList; + import java.util.List; + + import mockit.Mocked; + import mockit.Verifications; + import mockit.integration.junit5.JMockitExtension; + import org.junit.jupiter.api.extension.ExtendWith; + + @ExtendWith(JMockitExtension.class) + class MyTest { + @Mocked + MyObject myObject; + + void test() { + myObject.getSomeObject(new byte[0]); + myObject.getSomeObject(new int[0]); + myObject.getSomeObject(new Exception[0]); + new Verifications() {{ + myObject.getSomeObject((byte[]) any); + myObject.getSomeObject((int[]) any); + myObject.getSomeObject((Exception[]) any); + }}; + } + } + """, + """ + import java.util.ArrayList; + import java.util.List; + + import static org.mockito.Mockito.any; + import static org.mockito.Mockito.verify; + + import org.junit.jupiter.api.extension.ExtendWith; + import org.mockito.Mock; + import org.mockito.junit.jupiter.MockitoExtension; + + @ExtendWith(MockitoExtension.class) + class MyTest { + @Mock + MyObject myObject; + + void test() { + myObject.getSomeObject(new byte[0]); + myObject.getSomeObject(new int[0]); + myObject.getSomeObject(new Exception[0]); + verify(myObject).getSomeObject(any(byte[].class)); + verify(myObject).getSomeObject(any(int[].class)); + verify(myObject).getSomeObject(any(Exception[].class)); + } + } + """ + ) + ); + } }