Skip to content

Commit 6296e0a

Browse files
committed
Hoist PowerMock Whitebox calls used in expression position
Adds a second pass to PowerMockWhiteboxToJavaReflection that handles Whitebox result calls nested inside an enclosing statement (return, method arguments, if-conditions, ...): the reflection preamble and a temp local are hoisted before the statement and the call is replaced by a reference to the temp. Calls in short-circuit (&&/||) or ternary positions are left untouched to preserve evaluation semantics.
1 parent 1782300 commit 6296e0a

3 files changed

Lines changed: 392 additions & 2 deletions

File tree

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

Lines changed: 149 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@
2222
import org.openrewrite.java.JavaIsoVisitor;
2323
import org.openrewrite.java.JavaParser;
2424
import org.openrewrite.java.JavaTemplate;
25+
import org.openrewrite.java.JavaVisitor;
2526
import org.openrewrite.java.MethodMatcher;
2627
import org.openrewrite.java.search.UsesType;
2728
import org.openrewrite.java.tree.*;
2829
import org.openrewrite.marker.Markers;
2930

3031
import java.util.ArrayList;
3132
import java.util.List;
33+
import java.util.UUID;
3234

3335
import static java.util.Collections.emptyList;
3436
import static org.openrewrite.Tree.randomId;
@@ -66,7 +68,8 @@ public class PowerMockWhiteboxToJavaReflection extends Recipe {
6668
final String description = "Replace `org.powermock.reflect.Whitebox` calls (`setInternalState`, " +
6769
"`getInternalState`, `invokeMethod`, static `invokeMethod`, `invokeConstructor`, `getField` and " +
6870
"`getMethod`, including the `Class where` overloads) with plain Java reflection using " +
69-
"`java.lang.reflect.Field`, `Method` and `Constructor`. Field/constructor lookups " +
71+
"`java.lang.reflect.Field`, `Method` and `Constructor`. Calls nested in expression position " +
72+
"(returns, arguments, conditions) are hoisted into preceding statements. Field/constructor lookups " +
7073
"use `getDeclaredField`/`getDeclaredConstructor` on the named class, which differs from PowerMock " +
7174
"for members inherited from a superclass.";
7275

@@ -149,6 +152,28 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) {
149152
}
150153
}
151154

155+
// Phase B: hoist Whitebox calls nested in expression position (return/arguments/conditions/...),
156+
// introducing a temp local before the enclosing statement and referencing it in place.
157+
List<UUID> originalIds = new ArrayList<>();
158+
for (Statement s : b.getStatements()) {
159+
originalIds.add(s.getId());
160+
}
161+
for (UUID id : originalIds) {
162+
Statement s = findStatementById(b, id);
163+
while (s != null) {
164+
J.MethodInvocation nested = findNestedWhiteboxResultCall(s, ctx);
165+
if (nested == null) {
166+
break;
167+
}
168+
J.Block hoisted = hoistNestedCall(b, s, nested, ctx);
169+
if (hoisted == b) {
170+
break; // could not migrate this nested call; leave it for the comment recipe
171+
}
172+
b = hoisted;
173+
s = findStatementById(b, id);
174+
}
175+
}
176+
152177
return b;
153178
}
154179

@@ -205,6 +230,129 @@ private ResultSink sinkFromStatement(Statement statement) {
205230
return new ResultSink(null, null);
206231
}
207232

233+
private @Nullable Statement findStatementById(J.Block b, UUID id) {
234+
for (Statement s : b.getStatements()) {
235+
if (s.getId().equals(id)) {
236+
return s;
237+
}
238+
}
239+
return null;
240+
}
241+
242+
/**
243+
* Find the first result-producing Whitebox call nested in expression position within
244+
* {@code enclosing}, excluding the call Phase A targets directly, calls in short-circuit
245+
* or ternary positions (hoisting would change evaluation), and calls inside nested
246+
* blocks/lambdas/anonymous classes (handled by their own block).
247+
*/
248+
private J.@Nullable MethodInvocation findNestedWhiteboxResultCall(Statement enclosing, ExecutionContext ctx) {
249+
J.MethodInvocation primary = extractWhiteboxInvocation(enclosing);
250+
UUID primaryId = primary != null ? primary.getId() : null;
251+
J.MethodInvocation[] holder = {null};
252+
new JavaIsoVisitor<ExecutionContext>() {
253+
@Override
254+
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext c) {
255+
if (holder[0] == null &&
256+
!method.getId().equals(primaryId) &&
257+
isMigratableWhiteboxResult(method) &&
258+
!inCarveOut(getCursor())) {
259+
holder[0] = method;
260+
return method;
261+
}
262+
return super.visitMethodInvocation(method, c);
263+
}
264+
265+
@Override
266+
public J.Block visitBlock(J.Block nestedBlock, ExecutionContext c) {
267+
return nestedBlock; // nested blocks have their own scope and are handled separately
268+
}
269+
270+
@Override
271+
public J.Lambda visitLambda(J.Lambda lambda, ExecutionContext c) {
272+
return lambda;
273+
}
274+
275+
@Override
276+
public J.NewClass visitNewClass(J.NewClass newClass, ExecutionContext c) {
277+
return newClass;
278+
}
279+
}.visit(enclosing, ctx);
280+
return holder[0];
281+
}
282+
283+
// True when the call sits in a short-circuit (&&/||) right operand or a ternary branch,
284+
// where hoisting would change whether the reflective call executes.
285+
private boolean inCarveOut(Cursor callCursor) {
286+
Object child = callCursor.getValue();
287+
for (Cursor c = callCursor.getParent(); c != null; c = c.getParent()) {
288+
Object val = c.getValue();
289+
if (val instanceof J.Binary) {
290+
J.Binary bin = (J.Binary) val;
291+
if ((bin.getOperator() == J.Binary.Type.And || bin.getOperator() == J.Binary.Type.Or) &&
292+
bin.getRight() == child) {
293+
return true;
294+
}
295+
} else if (val instanceof J.Ternary) {
296+
J.Ternary ternary = (J.Ternary) val;
297+
if (ternary.getTruePart() == child || ternary.getFalsePart() == child) {
298+
return true;
299+
}
300+
}
301+
child = val;
302+
}
303+
return false;
304+
}
305+
306+
private J.Block hoistNestedCall(J.Block b, Statement enclosing, J.MethodInvocation nested, ExecutionContext ctx) {
307+
Cursor blockCursor = new Cursor(getCursor().getParentOrThrow(), b);
308+
JavaType.Method resolved = resolveFor(nested);
309+
String tempName = resultTempName(nested, blockCursor);
310+
ResultSink sink = new ResultSink(tempName, getCastType(nested.getType()));
311+
String template = buildReplacementTemplate(nested, sink, blockCursor, resolved);
312+
if (template == null) {
313+
return b;
314+
}
315+
J.Block rebuilt = JavaTemplate.builder(template)
316+
.contextSensitive()
317+
.javaParser(JavaParser.fromJavaVersion())
318+
.imports(templateImports(resolved).toArray(new String[0]))
319+
.build()
320+
.apply(blockCursor, enclosing.getCoordinates().before(), buildTemplateArgs(nested, resolved));
321+
322+
J.Identifier ref = new J.Identifier(randomId(), nested.getPrefix(), Markers.EMPTY, emptyList(),
323+
tempName, nested.getType(), null);
324+
rebuilt = (J.Block) new JavaVisitor<ExecutionContext>() {
325+
@Override
326+
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext c) {
327+
if (method.getId().equals(nested.getId())) {
328+
return ref;
329+
}
330+
return super.visitMethodInvocation(method, c);
331+
}
332+
}.visitNonNull(rebuilt, ctx);
333+
334+
recordReplacement(nested);
335+
return rebuilt;
336+
}
337+
338+
private String resultTempName(J.MethodInvocation mi, Cursor scope) {
339+
String base;
340+
if (GET_FIELD.matches(mi)) {
341+
base = "reflectField";
342+
} else if (GET_METHOD.matches(mi)) {
343+
base = "reflectMethod";
344+
} else if (INVOKE_CONSTRUCTOR_ARGS.matches(mi) || INVOKE_CONSTRUCTOR_EXPLICIT.matches(mi)) {
345+
base = "reflectInstance";
346+
} else if (INVOKE_METHOD.matches(mi) || INVOKE_METHOD_STATIC.matches(mi)) {
347+
String literal = extractStringLiteral(mi.getArguments().get(1));
348+
base = literal != null ? literal + "Result" : "reflectResult";
349+
} else {
350+
String literal = extractStringLiteral(mi.getArguments().get(1));
351+
base = literal != null ? literal + "Value" : "reflectValue";
352+
}
353+
return generateVariableName(base, scope, INCREMENT_NUMBER);
354+
}
355+
208356
private @Nullable String buildReplacementTemplate(J.MethodInvocation mi, ResultSink sink,
209357
Cursor scope, JavaType.@Nullable Method resolvedMethod) {
210358
List<Expression> args = mi.getArguments();

src/main/resources/META-INF/rewrite/recipes.csv

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.tes
201201
maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.mockito.RemoveInitMocksIfRunnersSpecified,Remove `MockitoAnnotations.initMocks(this)` and `openMocks(this)` if JUnit runners specified,Remove `MockitoAnnotations.initMocks(this)` and `MockitoAnnotations.openMocks(this)` if class-level JUnit runners `@RunWith(MockitoJUnitRunner.class)` or `@ExtendWith(MockitoExtension.class)` are specified. These manual initialization calls are redundant when using Mockito's JUnit integration. Note that the `@Mock` fields will then be initialized by the strict mocking session of the extension or runner; tests that relied on the lenient mocks created by an explicit `openMocks(this)` call inside `@BeforeEach` may surface `UnnecessaryStubbingException`. Add `@MockitoSettings(strictness = Strictness.LENIENT)` to opt out.,1,Mockito,Testing,Java,,,Basic building blocks for transforming Java code.,,
202202
maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.mockito.AddMockitoSettingsWithWarnStrictness,Add `@MockitoSettings(strictness = Strictness.WARN)` to `@ExtendWith(MockitoExtension.class)` classes,Adds `@MockitoSettings(strictness = Strictness.WARN)` to test classes that have `@ExtendWith(MockitoExtension.class)` but do not already have a `@MockitoSettings` annotation. This preserves the lenient stubbing behavior from Mockito 1.x/2.x migrations and prevents `UnnecessaryStubbingException` from strict stubbing defaults.,1,Mockito,Testing,Java,,,Basic building blocks for transforming Java code.,,
203203
maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.mockito.VerifyZeroToNoMoreInteractions,Replace `verifyZeroInteractions()` with `verifyNoMoreInteractions()`,Replaces `verifyZeroInteractions()` with `verifyNoMoreInteractions()` in Mockito tests when migration when using a Mockito version < 3.x.,1,Mockito,Testing,Java,,,Basic building blocks for transforming Java code.,,
204-
maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.mockito.PowerMockWhiteboxToJavaReflection,Replace PowerMock `Whitebox` with Java reflection,"Replace `org.powermock.reflect.Whitebox` calls (`setInternalState`, `getInternalState`, `invokeMethod`, static `invokeMethod`, `invokeConstructor`, `getField` and `getMethod`, including the `Class where` overloads) with plain Java reflection using `java.lang.reflect.Field`, `Method` and `Constructor`. Field/constructor lookups use `getDeclaredField`/`getDeclaredConstructor` on the named class, which differs from PowerMock for members inherited from a superclass.",1,Mockito,Testing,Java,,,Basic building blocks for transforming Java code.,,
204+
maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.mockito.PowerMockWhiteboxToJavaReflection,Replace PowerMock `Whitebox` with Java reflection,"Replace `org.powermock.reflect.Whitebox` calls (`setInternalState`, `getInternalState`, `invokeMethod`, static `invokeMethod`, `invokeConstructor`, `getField` and `getMethod`, including the `Class where` overloads) with plain Java reflection using `java.lang.reflect.Field`, `Method` and `Constructor`. Calls nested in expression position (returns, arguments, conditions) are hoisted into preceding statements. Field/constructor lookups use `getDeclaredField`/`getDeclaredConstructor` on the named class, which differs from PowerMock for members inherited from a superclass.",1,Mockito,Testing,Java,,,Basic building blocks for transforming Java code.,,
205205
maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.mockito.ArgumentMatcherToLambda,Convert `ArgumentMatcher<T>` anonymous class to lambda,"Converts anonymous `ArgumentMatcher<T>` implementations with `matches(Object)` to lambda expressions with the correct parameter type. In Mockito 1.x, `ArgumentMatcher<T>` extended Hamcrest's `BaseMatcher` and `matches` always took `Object`. In Mockito 2+, `ArgumentMatcher<T>` is a functional interface where `matches` takes `T`.",1,Mockito,Testing,Java,,,Basic building blocks for transforming Java code.,,
206206
maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.mockito.MockConstructionToTryWithResources,Wrap `MockedConstruction` in try-with-resources,"Wraps `MockedConstruction` variable declarations that have explicit `.close()` calls into try-with-resources blocks, removing the explicit close call. This ensures proper resource management and makes the code cleaner.",1,Mockito,Testing,Java,,,Basic building blocks for transforming Java code.,,
207207
maven,org.openrewrite.recipe:rewrite-testing-frameworks,org.openrewrite.java.testing.mockito.RemoveDoNothingForDefaultMocks,Remove `doNothing()` for void methods on `@Mock` fields,"Remove unnecessary `doNothing()` stubbings for void methods on `@Mock` fields. Mockito mocks already do nothing for void methods by default, making these stubbings redundant and triggering strict stubbing violations in Mockito 3+.",1,Mockito,Testing,Java,,,Basic building blocks for transforming Java code.,,

0 commit comments

Comments
 (0)