Skip to content

Add UnnecessaryFinalInTryWithResources - #1046

Open
sullis wants to merge 1 commit into
openrewrite:mainfrom
sullis:cleanup-try-with-resources
Open

Add UnnecessaryFinalInTryWithResources#1046
sullis wants to merge 1 commit into
openrewrite:mainfrom
sullis:cleanup-try-with-resources

Conversation

@sullis

@sullis sullis commented Sep 1, 2026

Copy link
Copy Markdown

Adds an UnnecessaryFinalInTryWithResources recipe that removes the redundant final modifier from resources declared in a try-with-resources statement. Such resources are implicitly final (JLS 14.20.3), so the modifier adds no meaning.

// before
try (final var input = new ByteArrayInputStream(new byte[0])) {
    int x = input.read();
}

// after
try (var input = new ByteArrayInputStream(new byte[0])) {
    int x = input.read();
}

Relationship to NoFinalizedLocalVariables

Per @greg-at-moderne's feedback, the two overlap but are not the same thing:

  • NoFinalizedLocalVariables is a stylistic preference — it strips final from every local variable (and optionally parameters), where the modifier is meaningful. It is opt-in and belongs to no composite recipe.
  • This recipe only touches the one position where final is provably redundant by the language spec, so it changes nothing about what the code means.

Also per that feedback, the recipe was renamed from the original CleanupTryWithResources — "cleanup" didn't say what it did — to UnnecessaryFinalInTryWithResources, matching the neighbouring UnnecessaryCloseInTryWithResources.

Implementation

  • Java-only: the visitor is gated behind Preconditions.check(new JavaFileChecker<>(), ...).
  • Only J.Try.Resource declarations are visited, so final on ordinary local variables is untouched.
  • A final modifier carrying its own annotations is left alone, since dropping it would lose them.
  • Formatting is preserved deliberately, since final owns whitespace and can carry comments:
    • comments in the removed modifier's prefix are concatenated onto the following token (type expression, or the variable name when the type is implicit) in source order;
    • a declaration that was wrapped across lines around final is collapsed back onto one line, but only the gap before each variable name — the initializer keeps its own layout, so a multi-line new ...(...) is not reflowed;
    • when final leads the declaration its whitespace is dropped entirely; behind a leading annotation the gap belongs to the annotation and survives.
  • estimatedEffortPerOccurrence is 1 minute. No RSPEC tag — this is not backed by a SonarQube rule.
  • Registered in META-INF/rewrite/recipes.csv; intentionally not added to common-static-analysis.yml, per the repo convention that the file holds proven recipes only.

Tests

UnnecessaryFinalInTryWithResourcesTest, 12 cases:

Case Covers
removeFinalModifier (@DocumentExample) single resource with var
removeFinalWithExplicitType explicit type expression instead of var
cleanupMultiline declaration wrapped across lines is collapsed
removeFinalFromEachOfMultipleResources several resources, all final
removeFinalOnlyFromTheResourceThatHasIt mixed list, only one final
retainLeadingAnnotationWhenRemovingFinal leading annotation keeps its prefix
retainCommentBetweenFinalAndType comment between final and the type
retainCommentBetweenAnnotationAndFinal comment between an annotation and final
retainLineCommentAndItsLineBreak line comment keeps its trailing newline
doNotReflowInitializerAcrossLines multi-line initializer left as written
doNotChangeResourceWithoutFinal no-op
doNotChangeResourceReferencingExistingVariable no-op on try (existingVar)

🤖 Generated with Claude Code

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Sep 1, 2026
@sullis
sullis marked this pull request as ready for review September 1, 2026 19:46
@sullis
sullis force-pushed the cleanup-try-with-resources branch 2 times, most recently from 3dba993 to e14f36f Compare September 2, 2026 12:43
@sullis

sullis commented Sep 2, 2026

Copy link
Copy Markdown
Author

Ready for review / feedback
@greg-at-moderne

@sullis

sullis commented Sep 7, 2026

Copy link
Copy Markdown
Author

Ready for review / feedback: CleanupTryWithResources recipe

@greg-at-moderne

Copy link
Copy Markdown
Contributor

BTW, we already have NoFinalizedLocalVariables which does something similar. Or the same thing, but with broader scope. I am not sure yet, how to proceed. Probably just have the new recipe as you suggest.

The name is too generic though. "Cleanup" doesn't necessarily tell the user what it does.

@greg-at-moderne
greg-at-moderne self-requested a review September 8, 2026 07:58
@sullis
sullis force-pushed the cleanup-try-with-resources branch 2 times, most recently from e1a17e7 to 19ab130 Compare September 8, 2026 20:08
@sullis
sullis force-pushed the cleanup-try-with-resources branch from 19ab130 to c9a41de Compare September 8, 2026 20:11
@sullis sullis changed the title Add CleanupTryWithResources Add UnnecessaryFinalInTryWithResources Sep 8, 2026
@sullis

sullis commented Sep 8, 2026

Copy link
Copy Markdown
Author

BTW, we already have NoFinalizedLocalVariables which does something similar. Or the same thing, but with broader scope. I am not sure yet, how to proceed. Probably just have the new recipe as you suggest.

The name is too generic though. "Cleanup" doesn't necessarily tell the user what it does.

I pushed an update. Recipe has been renamed to UnnecessaryFinalInTryWithResources

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

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants