Skip to content

Commit

Permalink
Migrate JMockit block to Mockito which contains redundant "this" pref…
Browse files Browse the repository at this point in the history
…ix 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 <sheng.yu@capgemini.com>
Co-authored-by: Tim te Beek <tim@moderne.io>
  • Loading branch information
3 people authored Oct 11, 2024
1 parent d81cc70 commit 8a95cb8
Show file tree
Hide file tree
Showing 3 changed files with 245 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<Expression> 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
Expand Down Expand Up @@ -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) {
// ((<type>) any) to any(<type>.class)
return rewriteFullyQualifiedToArgumentMatcher(methodArgument, (JavaType.FullyQualified) type);
} else if (type instanceof JavaType.FullyQualified || type instanceof JavaType.Array) {
// ((<type>) any) to any(<type>.class), type can also be simple array
return rewriteAnyWithClassParameterToArgumentMatcher(methodArgument, type);
}
if (template == null || argumentMatcher == null) {
// unhandled type, return argument unchanged
Expand All @@ -157,7 +163,7 @@ private Expression rewriteMethodArgument(Expression methodArgument) {
}

private Expression applyArgumentTemplate(Expression methodArgument, String argumentMatcher, String template,
List<Object> templateParams) {
List<Object> templateParams) {
visitor.maybeAddImport("org.mockito.Mockito", argumentMatcher);
return JavaTemplate.builder(template)
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "mockito-core-3.12"))
Expand All @@ -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) {
Expand All @@ -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<Object> 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);
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -668,6 +668,9 @@ public String getSomeField(List<String> input) {
public String getSomeOtherField(Object input) {
return "Y";
}
public String getSomeArrayField(Object input) {
return "Z";
}
}
"""
),
Expand All @@ -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]));
}
}
""",
Expand All @@ -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]));
}
}
"""
Expand Down Expand Up @@ -1075,7 +1083,7 @@ void whenMinTimes() {
class MyTest {
@Mocked
Object myObject;
void test() {
new Expectations() {{
myObject.wait(anyLong, anyInt);
Expand All @@ -1096,7 +1104,7 @@ void test() {
class MyTest {
@Mock
Object myObject;
void test() {
myObject.wait(10L, 10);
verify(myObject, atLeast(2)).wait(anyLong(), anyInt());
Expand All @@ -1122,7 +1130,7 @@ void whenMaxTimes() {
class MyTest {
@Mocked
Object myObject;
void test() {
new Expectations() {{
myObject.wait(anyLong, anyInt);
Expand All @@ -1143,7 +1151,7 @@ void test() {
class MyTest {
@Mock
Object myObject;
void test() {
myObject.wait(10L, 10);
verify(myObject, atMost(5)).wait(anyLong(), anyInt());
Expand All @@ -1169,7 +1177,7 @@ void whenMinTimesMaxTimes() {
class MyTest {
@Mocked
Object myObject;
void test() {
new Expectations() {{
myObject.wait(anyLong, anyInt);
Expand All @@ -1191,7 +1199,7 @@ void test() {
class MyTest {
@Mock
Object myObject;
void test() {
myObject.wait(10L, 10);
verify(myObject, atLeast(1)).wait(anyLong(), anyInt());
Expand Down Expand Up @@ -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() {
Expand Down
Loading

0 comments on commit 8a95cb8

Please sign in to comment.