Visitar URL original
SimplifyConstantIfBranchExecution leaves unreachable code by steve-aom-elliott · Pull Request #546 · openrewrite/rewrite-static-analysis · GitHub
Skip to content

SimplifyConstantIfBranchExecution leaves unreachable code - #546

Merged
timtebeek merged 3 commits into
mainfrom
399-simplify-constant-if-branch-execution-unreachable-code
May 9, 2025
Merged

timtebeek merged 3 commits into
mainfrom
399-simplify-constant-if-branch-execution-unreachable-code

Conversation

@steve-aom-elliott

@steve-aom-elliott steve-aom-elliott commented May 7, 2025 •

Copy link
Copy Markdown
Contributor

Certain cases where there is an object assignment in an unneeded if block in the initial code can cause SimplifyConstantIfBranchExecution to leave scoping braces and unreachable lines in the code. The underlying cause is due to RemoveUnneededBlock deciding to not remove scoping braces if a block contains variable declarations, however, if the block also contains a return, then SimplifyConstantIfBranchExecution which also uses RemoveUnreachableCodeVisitor should be able to be unwrapped because there should be no conflicts after unreachable code is removed.

What's changed?

  • Test example from submitted issue has been added.
  • Deeper nested if examples included for SimplifyConstantIfBranchExecution tests and test example provided for RemoveUnneededBlock specifically covering change in logic to allow unwrapping if return statement is nested.

Checklist

  • I've added further unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite May 7, 2025

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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/resources/META-INF/rewrite/examples.yml
    • lines 474-473
    • lines 636-652
    • lines 698-697
    • lines 2311-2310
    • lines 2326-2338

… return statements. Modified condition on `RemoveUnneededBlock` to allow for block flattening even if it contains variable declarations as long as it also contains a return (which when combined with `RemoveUnreachableCodeVisitor` should be able to prevent issues that might crop up due to descoping the first block.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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/resources/META-INF/rewrite/examples.yml
    • lines 474-473
    • lines 636-652
    • lines 698-697
    • lines 2311-2310
    • lines 2326-2338

@steve-aom-elliott
steve-aom-elliott marked this pull request as ready for review May 7, 2025 20:41
@timtebeek timtebeek added the bug Something isn't working label May 8, 2025
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite May 8, 2025
@steve-aom-elliott
steve-aom-elliott force-pushed the 399-simplify-constant-if-branch-execution-unreachable-code branch from 525b4cb to 25c08d9 Compare May 8, 2025 19:54

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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/resources/META-INF/rewrite/examples.yml
    • lines 474-473
    • lines 636-652
    • lines 698-697
    • lines 2311-2310
    • lines 2326-2338

@timtebeek timtebeek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code changes look good already; let me know if you saw any results on a large run that gave you pause before a merge, or if we can already merge as is.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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/resources/META-INF/rewrite/examples.yml
    • lines 474-473
    • lines 636-652
    • lines 698-697
    • lines 2311-2310
    • lines 2326-2338

@timtebeek
timtebeek merged commit 649a92b into main May 9, 2025
@timtebeek
timtebeek deleted the 399-simplify-constant-if-branch-execution-unreachable-code branch May 9, 2025 13:17
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

SimplifyConstantIfBranchExecution leaves unreachable code

2 participants