Skip to content

Fix RemoveUnusedLocalVariables to preserve assignments with side effects #741

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 19, 2025
Merged

Fix RemoveUnusedLocalVariables to preserve assignments with side effects #741

merged 4 commits into from
Nov 19, 2025

Conversation

@timtebeek
Copy link
Member

@timtebeek timtebeek commented Sep 26, 2025 •
edited
Loading

Summary

Changes

  • Added assignmentsMightSideEffect() to check if assignments could have side effects
  • Added expressionMightThrowException() to specifically detect method calls that might throw
  • Added isAssignmentInTryBlock() to identify assignments within try block scope
  • Added test case demonstrating the fix

Test plan

  • Added test doNotRemoveVariableAssignmentWithPotentialSideEffects for the reported issue
  • All existing RemoveUnusedLocalVariablesTest tests pass
  • Verified handleVariablesReadWithinTry test still works correctly

🤖 Generated with Claude Code

@timtebeek @claude
Fix RemoveUnusedLocalVariables to preserve assignments with side effe…
7d57f52 
…cts in try blocks

The recipe was incorrectly removing variable assignments in try blocks even when
those assignments could throw exceptions and change program behavior. This fix
ensures that method invocations within try blocks are preserved since they may
throw exceptions that affect control flow.

Fixes #740

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Sep 26, 2025
@timtebeek timtebeek self-assigned this Sep 26, 2025
@timtebeek timtebeek mentioned this pull request Sep 26, 2025
Closed
@timtebeek timtebeek added bug Something isn't working recipe labels Nov 19, 2025
@timtebeek timtebeek marked this pull request as ready for review November 19, 2025 22:54
@timtebeek timtebeek changed the title Fix RemoveUnusedLocalVariables to preserve assignments with side effects in try blocks Fix RemoveUnusedLocalVariables to preserve assignments with side effects Nov 19, 2025
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Nov 19, 2025
@timtebeek timtebeek merged commit 4d6fd83 into main Nov 19, 2025
2 checks passed
@timtebeek timtebeek deleted the fix-remove-unused-locals-side-effects branch November 19, 2025 23:04
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Nov 19, 2025
mergify bot added a commit to robfrank/linklift that referenced this pull request Nov 24, 2025
@mergify
chore(deps): bump org.openrewrite.recipe:rewrite-static-analysis from…
6f12ce4 
… 2.21.0 to 2.22.0 [skip ci]

Bumps [org.openrewrite.recipe:rewrite-static-analysis](https://github.com/openrewrite/rewrite-static-analysis) from 2.21.0 to 2.22.0.
Release notes

*Sourced from [org.openrewrite.recipe:rewrite-static-analysis's releases](https://github.com/openrewrite/rewrite-static-analysis/releases).*

> 2.22.0
> ------
>
> What's Changed
> --------------
>
> * Prevent `AnnotateNullableMethods` from failing on TypeScript sources by [`@​greg-at-moderne`](https://github.com/greg-at-moderne) in [openrewrite/rewrite-static-analysis#773](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/773)
> * Fix CombineSemanticallyEqualCatchBlocks for inner exception types by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#772](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/772)
> * Handle single quotes in ReplaceStringBuilderWithString by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#775](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/775)
> * Prevent `ReplaceLambdaWithMethodReference` in nested generic/overloaded contexts by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#776](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/776)
> * Fix ReplaceLambdaWithMethodReference for local records and classes by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#777](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/777)
> * Move nullable annotations to array types by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#778](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/778)
> * Annotate nullable array type parameters by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#779](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/779)
> * Annotate required parameters with `@NonNull` and remove throws by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#782](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/782)
> * Skip UnnecessaryCatch after any missing types via a precondition by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#784](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/784)
> * Run `UnnecessaryCatch` after `UnnecessaryThrows` to remove unused catch clauses by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#785](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/785)
> * Add JavascriptFileChecker to selectively skip JavaScript files by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#787](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/787)
> * Fix `RemoveUnusedLocalVariables` to preserve assignments with side effects by [`@​timtebeek`](https://github.com/timtebeek) in [openrewrite/rewrite-static-analysis#741](https://redirect.github.com/openrewrite/rewrite-static-analysis/pull/741)
>
> **Full Changelog**: <openrewrite/rewrite-static-analysis@v2.21.0...v2.22.0>


Commits

* [`4d6fd83`](openrewrite/rewrite-static-analysis@4d6fd83) Fix `RemoveUnusedLocalVariables` to preserve assignments with side effects (#...
* [`16dc1da`](openrewrite/rewrite-static-analysis@16dc1da) Add JavascriptFileChecker to selectively skip JavaScript files ([#787](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/787))
* [`c1712c2`](openrewrite/rewrite-static-analysis@c1712c2) Update documentation examples
* [`af53ce1`](openrewrite/rewrite-static-analysis@af53ce1) Run `UnnecessaryCatch` after `UnnecessaryThrows` to remove unused catch claus...
* [`b670b5a`](openrewrite/rewrite-static-analysis@b670b5a) Skip UnnecessaryCatch after any missing types via a precondition ([#784](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/784))
* [`0f1d5db`](openrewrite/rewrite-static-analysis@0f1d5db) Annotate required parameters with `@NonNull` and remove throws ([#782](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/782))
* [`02e09d0`](openrewrite/rewrite-static-analysis@02e09d0) Annotate nullable array type parameters ([#779](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/779))
* [`0f25051`](openrewrite/rewrite-static-analysis@0f25051) Move nullable annotations to array types ([#778](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/778))
* [`995c2d3`](openrewrite/rewrite-static-analysis@995c2d3) Fix `ReplaceLambdaWithMethodReference` for local records and classes ([#336](https://redirect.github.com/openrewrite/rewrite-static-analysis/issues/336)) (...
* [`0ea5975`](openrewrite/rewrite-static-analysis@0ea5975) Prevent `ReplaceLambdaWithMethodReference` in nested generic/overloaded conte...
* Additional commits viewable in [compare view](openrewrite/rewrite-static-analysis@v2.21.0...v2.22.0)
  
[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility\_score?dependency-name=org.openrewrite.recipe:rewrite-static-analysis&package-manager=maven&previous-version=2.21.0&new-version=2.22.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
Dependabot commands and options
  
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot show  ignore conditions` will show all of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greg-at-moderne greg-at-moderne Awaiting requested review from greg-at-moderne

Assignees

@timtebeek timtebeek

Labels

bug Something isn't working recipe

Projects

OpenRewrite
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RemoveUnusedLocalVariables removes code with side effects

2 participants

@timtebeek