Skip to content

Fix JavaTemplate expression replacement inside return statements#7096

Merged
timtebeek merged 2 commits intomainfrom
tim/fix-template-return-expr
Mar 24, 2026
Merged

Fix JavaTemplate expression replacement inside return statements#7096
timtebeek merged 2 commits intomainfrom
tim/fix-template-return-expr

Conversation

@timtebeek
Copy link
Copy Markdown
Member

@timtebeek timtebeek commented Mar 23, 2026

Summary

  • Fix JavaTemplate.apply() with mi.getCoordinates().replace() throwing IllegalArgumentException when the target method invocation is inside a J.Return (or other non-block expression context)
  • When the parent tree is not a J.Block, use parseExpression instead of parseBlockStatements — following the existing precedent in visitFieldAccess
  • Apply the same fix to visitNewClass which has the identical issue

Test plan

  • Added replaceMethodInvocationInsideReturn test in JavaTemplateTest8Test
  • All existing JavaTemplate* tests pass

@timtebeek
Fix JavaTemplate expression replacement inside return statements
692ed7e 
When a method invocation or new class expression is the direct expression
of a J.Return (or other non-block expression context), coordinates.replace()
produces STATEMENT_PREFIX but parseBlockStatements fails because the template
is an expression. Detect expression context by checking if the parent tree
is not a J.Block, and use parseExpression instead.
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Mar 23, 2026
@timtebeek timtebeek requested a review from knutwannheden March 23, 2026 14:16
@timtebeek
Copy link
Copy Markdown
Member Author

timtebeek commented Mar 23, 2026

We already use the same pattern for field access, so I'd think this is fairly safe and fixes similar issues elsewhere:

rewrite/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateJavaExtension.java

Lines 236 to 243 in 8a181c3

} else if (loc == STATEMENT_PREFIX && isScope(fa)) {
// NOTE: while `J.FieldAccess` inherits from `Statement` they can only ever be used as expressions
return autoFormat(unsubstitute(templateParser.parseExpression(
getCursor(),
substitutedTemplate,
substitutions.getTypeVariables(),
loc))
.withPrefix(fa.getPrefix()), p);

@timtebeek timtebeek added bug Something isn't working java labels Mar 23, 2026
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Mar 23, 2026
@timtebeek
Copy link
Copy Markdown
Member Author

timtebeek commented Mar 23, 2026

Since we already use the same pattern elsewhere I'm inclined to merge this with asynchronous review if needed.. It's specific enough and tested that I don't foresee any issues.

@timtebeek timtebeek merged commit 76c4321 into main Mar 24, 2026
1 check passed
@timtebeek timtebeek deleted the tim/fix-template-return-expr branch March 24, 2026 10:17
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Mar 24, 2026
@knutwannheden
Copy link
Copy Markdown
Contributor

knutwannheden commented Mar 24, 2026

Nice! Surprised this never surfaced before...

@timtebeek timtebeek mentioned this pull request Mar 24, 2026
Merged
2 tasks
@timtebeek
Copy link
Copy Markdown
Member Author

timtebeek commented Mar 24, 2026 •
edited
Loading

It needed a small tweak to fix an incorrect cursor in rewrite-testing-frameworks AssertThrowsOnLastStatement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@knutwannheden knutwannheden Awaiting requested review from knutwannheden

Assignees

@timtebeek timtebeek

Labels

bug Something isn't working java

Projects

OpenRewrite
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@timtebeek @knutwannheden