Add PowerMockWhiteboxToJavaReflection recipe#944
Conversation
Replace `org.powermock.reflect.Whitebox` calls with plain Java reflection as part of the ReplacePowerMockito recipe chain. Handles setInternalState, getInternalState, and invokeMethod by expanding each call into getDeclaredField/getDeclaredMethod + setAccessible + get/set/invoke. Uses VariableNameUtils to avoid name collisions when multiple Whitebox calls target the same field.
bmuschko
force-pushed
the
bmuschko/whitebox-migration
branch
from
07a6548 to
c77ea54
Compare
The classpath.tsv.gz type table is what CI uses for type resolution in tests. The previously committed JAR is unnecessary when the type table includes the artifact.
- Inline the named WhiteboxVisitor into an anonymous class - Use immediate return for maybeAutoFormat - Remove unnecessary powermock-reflect-1 classpath from integration test
| class MyServiceTest { | ||
| void testInvokeWithArgs() throws Exception { | ||
| MyService service = new MyService(); | ||
| Method greetMethod = service.getClass().getDeclaredMethod("greet", "World".getClass()); |
There was a problem hiding this comment.
the .getClass() looks a bit odd; would it help to use type information where available, and only fall back where needed?
Also: would this correctly handle sub classes? E.g. if the argument is defined as an interface on the method declaration, but the argument passed in is a concrete instance, do we then still retrieve the correct method?
There was a problem hiding this comment.
the .getClass() looks a bit odd; would it help to use type information where available, and only fall back where needed?
It looks like you already relaxed the type information via #{any(java.lang.Object)}. Thanks for that!
Also: would this correctly handle sub classes?
Good point. This will likely be an issue. I can create a follow up PR though I believe covering this use case can be handled later when we actually run into it.
Untyped #{any()} placeholders caused the template parser to infer the
argument's actual type (e.g. MyService), which is not on the template
parser's classpath. This broke type resolution for the entire method
chain (.getClass(), .getDeclaredField(), .setAccessible(), etc.).
Using #{any(java.lang.Object)} ensures the substitution always resolves
against a JDK type, fixing type attribution. Also inlines the JAVA_PARSER
field and removes the now-unnecessary methodInvocations(false) from the
unit test.
github-project-automation
Bot
moved this from In Progress
to Ready to Review
in OpenRewrite
2 participants
Summary
PowerMockWhiteboxToJavaReflectionrecipe that replacesorg.powermock.reflect.Whiteboxcalls (setInternalState,getInternalState,invokeMethod) with plainjava.lang.reflectequivalentsReplacePowerMockitoYAML chain (beforeCleanupPowerMockImports)parserClasspath("org.powermock:powermock-reflect:1.6.5")tobuild.gradle.ktssoMethodMatchercan resolveWhiteboxtypespowermock-reflect-1toReplacePowerMockitoIntegrationTestclasspath so the integration test parser can resolveWhiteboxmethod calls now that the recipe chain includes this new recipeWhy
typeValidationOptions(methodInvocations(false))is neededThe replacement templates generate
java.lang.reflect.Field/java.lang.reflect.Methodcalls via string-basedJavaTemplate.builder(...). During template application, the parser doesn't have the full JDK reflection types on its classpath, so the generated method invocations (field.setAccessible(true),field.set(...),method.invoke(...)) lack fully resolved method types. Disabling method invocation validation suppresses errors for these unresolved types. This is consistent with the existing pattern inReplacePowerMockitoIntegrationTest.