Skip to content

Hoist PowerMock Whitebox calls used in expression position#1015

Closed
MBoegers wants to merge 3 commits into
mainfrom
MBoegers/powermock-whitebox-reflection-hoisting
Closed

Hoist PowerMock Whitebox calls used in expression position#1015
MBoegers wants to merge 3 commits into
mainfrom
MBoegers/powermock-whitebox-reflection-hoisting

Conversation

@MBoegers

@MBoegers MBoegers commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Ports the expression-position hoisting logic onto the current split Whitebox reflection recipes.

Adds a second pass in the shared WhiteboxToReflectionVisitor that handles Whitebox result calls nested inside an enclosing statement (return, method arguments, if conditions, …), which the statement/variable-declaration pass cannot rewrite in place because reflection needs a multi-statement preamble.

For each such call, the reflection preamble and a temp local are hoisted before the enclosing statement, and the call is replaced by a reference to the temp. Calls in short-circuit (&&/||) right operands or ternary branches are left untouched, since hoisting would change whether the reflective call executes.

Keeps the split per-API recipe structure now on main, updates the aggregate recipe metadata to describe the hoisting behavior, and retains 6 unit tests covering: a return, a method-call argument, an if condition, a for-loop body (hoisted into the loop body, not the method block), two nested calls in one statement (preserved order), and a short-circuit-guarded call left unchanged.

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Jun 8, 2026
@MBoegers MBoegers marked this pull request as draft June 8, 2026 13:57
@MBoegers MBoegers force-pushed the MBoegers/powermock-whitebox-reflection-coverage branch 2 times, most recently from 5fc96ec to 4ad8941 Compare June 8, 2026 15:29
@MBoegers MBoegers force-pushed the MBoegers/powermock-whitebox-reflection-hoisting branch from 6296e0a to bc38b91 Compare June 8, 2026 15:31
Adds migration of getField, getMethod, invokeConstructor, static
invokeMethod(Class,..), the where-Class get/setInternalState overloads,
and non-literal field/method names, alongside the existing
setInternalState/getInternalState/invokeMethod support. Primitive results
are cast through their wrapper type, and the explicit Class[]/varargs-array
overloads of invokeMethod/invokeConstructor are deliberately left untouched
(no faithful mechanical translation).
@MBoegers MBoegers force-pushed the MBoegers/powermock-whitebox-reflection-coverage branch from 4ad8941 to dfdc2d0 Compare June 12, 2026 10:35
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.
@MBoegers MBoegers force-pushed the MBoegers/powermock-whitebox-reflection-hoisting branch from bc38b91 to a2f486c Compare June 12, 2026 10:36
@timtebeek

Copy link
Copy Markdown
Member

Given the work we've done on this front in the past week, is there anything left to do here?

@timtebeek timtebeek changed the base branch from MBoegers/powermock-whitebox-reflection-coverage to main June 19, 2026 13:38
@timtebeek

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved in 024ffcf. The merge now keeps main’s split Whitebox recipe structure, moves the hoisting logic into the shared visitor, and preserves the added hoisting coverage.

Copilot AI requested a review from timtebeek June 19, 2026 13:48
@MBoegers

Copy link
Copy Markdown
Collaborator Author

I close this PR without merging. The PRs from last week should deliver the main value, the proposed changes here are huge in terms of code changes with questionable value.

@MBoegers MBoegers closed this Jun 22, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Jun 22, 2026
@timtebeek timtebeek deleted the MBoegers/powermock-whitebox-reflection-hoisting branch June 22, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants