Skip to content

A7 + B6 follow-up: side effects, comments and formatting scope - #1123

Merged
timtebeek merged 16 commits into
mainfrom
tim/a7-b6-follow-up
Oct 25, 2026
Merged

timtebeek merged 16 commits into
mainfrom
tim/a7-b6-follow-up

Conversation

@timtebeek

Copy link
Copy Markdown
Member

Follow-up to #1122 for items A7 and B6 of #1116, split off for a closer review; it is stacked on #1122 and should be retargeted to main once that merges.

B6: RemoveUnusedLocalVariables removed unused locals whose initializer or reassignment had a ++, -- or compound-assignment side effect, e.g. int unused = counter++;. It also parked the comments of a removed declaration on the enclosing class, where they landed on the next member, and threw when there was no class (Groovy scripts). Comments now stay in their own block.

A7: three more recipes reformatted or cleaned up more than they changed.

  • SimplifyConstantIfBranchExecution ran RemoveUnneededBlock and EmptyBlock over every block enclosing a change. It now inlines or removes only the statement a simplified if leaves behind, keeps comment-only blocks, and drops an else emptied by a simplified else if.
  • UnwrapElseAfterReturn auto-formatted the whole block. It now shifts the unwrapped statements with ShiftFormat, so they keep their own formatting. This may conflict with UnwrapElseAfterReturn: only unwrap when else declarations do not collide #979.
  • UnnecessaryPrimitiveAnnotations auto-formatted the whole method. It now hands the removed annotation's prefix to the next element.

Each commit adds a test that failed before the fix. ./gradlew test passes locally on this branch.

… running `RemoveUnusedLocalVariables` file-wide

Refs #1116 (A7)
…Super`, `RemoveUnusedPrivateMethods` and `UseLambdaForFunctionalInterface`

They ran `RemoveUnusedImports` over the whole file, removing unrelated unused imports too.

Refs #1116 (A7)
…uldNotBeNested` produce

Both ran `RemoveUnneededBlock` over the whole file, flattening unrelated blocks too.

Refs #1116 (A7)
It ran `SimplifyBooleanExpressionVisitor` over the whole file.

Refs #1116 (A7)
…` call

It ran `EqualsAvoidsNull` over the whole file to flip its own invocations, which flipped unrelated ones too.

Refs #1116 (A7)
It ran `EmptyBlock` over the whole file, deleting unrelated empty blocks and statements. An `if` whose condition may have side effects is now kept rather than reduced to its condition.

Refs #1116 (A7)
…eters` replaced

It ran `SimplifyBooleanExpression` and `SimplifyConstantIfBranchExecution` over the whole file.

Refs #1116 (A7)
…mpound-assigns in `RemoveUnusedLocalVariables`

Refs #1116 (B6)
…d of auto-formatting the whole block

Refs #1116 (A7)
…aryPrimitiveAnnotations` instead of auto-formatting the whole method

Refs #1116 (A7)
…n `SimplifyConstantIfBranchExecution`

It ran `RemoveUnneededBlock` and `EmptyBlock` over every block enclosing a change, flattening and deleting unrelated blocks in the same method.

Refs #1116 (A7)
# Conflicts:
#	src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java
#	src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java
@timtebeek
timtebeek merged commit 1812dc5 into main Oct 25, 2026
2 checks passed
@timtebeek
timtebeek deleted the tim/a7-b6-follow-up branch October 25, 2026 18:02
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant