Skip to content

Extend InlineVariable to support local variable assignments #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 22, 2025

Conversation

timtebeek
Copy link
Member

@timtebeek timtebeek self-assigned this Jul 21, 2025
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Jul 21, 2025
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Jul 21, 2025
Comment on lines 49 to 53
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.Block visitBlock(J.Block block, ExecutionContext ctx) {
J.Block b = super.visitBlock(block, ctx);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part that needs review.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions could not be made:

  • src/main/java/org/openrewrite/staticanalysis/MinimumSwitchCases.java
    • lines 260-260

@timtebeek
Copy link
Member Author

Turns out we already have a recipe that does something similar, but with not exactly overlapping coverage:

public class InlineVariable extends Recipe {
@Override
public String getDisplayName() {
return "Inline variable";
}
@Override
public String getDescription() {
return "Inline variables when they are immediately used to return or throw.";
}

@Stephan202
Copy link

For inspiration around possible edge cases, see the DirectReturn Error Prone check and its tests.

@timtebeek timtebeek changed the title Recipe to inline returned value Extend InlineVariable to support local variable assignments Jul 22, 2025
@timtebeek
Copy link
Member Author

For inspiration around possible edge cases, see the DirectReturn Error Prone check and its tests.

Thanks! Added a few similar ones just now in 406c195 ; with that I think we're good to merge this PR. 🙏🏻

@timtebeek timtebeek marked this pull request as ready for review July 22, 2025 09:49
Comment on lines 273 to 274
String result;
return "hello";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should trigger RemoveUnusedLocalVariables as an after visitor?

@timtebeek timtebeek merged commit 7e5fa84 into main Jul 22, 2025
2 checks passed
@timtebeek timtebeek deleted the inline-returned-value branch July 22, 2025 10:34
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants