Add TryWithResources recipe (RSPEC-S2093) by timtebeek · Pull Request #831 · openrewrite/rewrite-static-analysis

timtebeek · 2026-03-16T21:16:06Z

Summary

Adds a new TryWithResources recipe that converts try/finally blocks to try-with-resources when the finally block only closes an AutoCloseable resource
Covers SonarQube rule S2093, one of the gap rules from https://github.com/moderneinc/customer-requests/issues/2009
Requires Java 9+ via precondition check
Incorporates test cases and improvements suggested in PR Try with resources #599

Patterns handled

Recognizes close in finally as any of:

Direct: in.close()
Null-guarded: if (in != null) { in.close(); } (with or without braces)
Try-catch wrapped: try { in.close(); } catch (...) {}
Null-guarded + try-catch

Features

Complex finally blocks: When the finally block contains close plus other statements, the recipe strips just the close and preserves remaining statements in finally
Java 9+ try(var) syntax: When the resource variable is used after the try block, uses try (varName) { ... } syntax keeping the declaration in place

Safety guards (skips transformation when)

Resource is reassigned in the try body (including +=, ++, etc.)
Resource is closed in a catch block
Type does not implement AutoCloseable
Close is via a helper method (not a direct .close() call)
Variable is initialized to null
Close is on a qualified expression (e.g. wrapper.getStream().close())
Resource is a field assignment, not a local variable declaration

Test plan

5 positive tests (direct close, null-guarded, try-catch wrapped, null-guarded+try-catch, preserves catch blocks)
2 feature tests (complex finally with extra logic, Java 9+ syntax for resource used after try)
9 negative tests (reassigned, non-AutoCloseable, already try-with-resources, helper method close, closed in catch, qualified close call, field-based resource, null-initialized with catch close)
Validated at scale against 87 Apache Maven repositories (61,833 source files) — 0 errors, 0 false positives

Converts try/finally blocks to try-with-resources when the finally block only closes an AutoCloseable resource. Handles direct close, null-guarded close, and try-catch wrapped close patterns in the finally block.

timtebeek · 2026-03-16T21:18:24Z

This replaces Try with resources #599, which saw some earlier feedback not addressed.

timtebeek · 2026-03-16T21:53:39Z

At-scale validation results

Ran TryWithResources against 850 Apache repositories using the Moderne CLI. Zero errors.

Summary

Metric	Value
Repos scanned	850
Repos with changes	26
Files changed	130
Errors	0

Repos with changes (sorted by file count)

Repository	Files changed
apache/activemq	42
apache/lucene	25
apache/ctakes	10
apache/jclouds	8
apache/qpid-jms	7
apache/qpid-broker-j	7
apache/royale-compiler	5
apache/cxf	4
apache/incubator-hugegraph	3
apache/orc	2
apache/kafka	2
apache/systemds	1
apache/spark	1
apache/sling-org-apache-sling-junit-remote	1
apache/sling-org-apache-sling-discovery-commons	1
apache/sling-org-apache-sling-commons-osgi	1
apache/sling-org-apache-sling-commons-compiler	1
apache/shiro	1
apache/santuario-xml-security-java	1
apache/roller	1
apache/netbeans-mavenutils-nbm-maven-plugin	1
apache/karaf-decanter	1
apache/jmeter	1
apache/freemarker	1
apache/camel-kafka-connector	1
apache/activemq-artemis-native	1

Patch review

Reviewed patches from Kafka, Lucene, JMeter, ActiveMQ, and CXF. All transformations are correct:

Direct close (finally { x.close(); }) — correctly moved to try-with-resources
Null-guarded close (finally { if (x != null) x.close(); }) — correctly handled
Try-catch wrapped close (finally { try { x.close(); } catch (...) {} }) — correctly handled
Catch blocks are preserved; only the finally block is removed
No false positives observed

- Replace manual boolean+ArrayList loop with ListUtils.map for idiomatic OpenRewrite style (returns same list reference when unchanged) - Handle single-statement null-guarded close without braces: if (x != null) x.close(); - Add test for braceless null-guard pattern

timtebeek · 2026-03-16T22:29:41Z

src/main/java/org/openrewrite/staticanalysis/TryWithResources.java

+                                J.VariableDeclarations varDecl = (J.VariableDeclarations) stmt;
+                                J.Try tryStmt = (J.Try) stmts.get(i + 1);
+                                if (canTransform(varDecl, tryStmt, stmts, i)) {
+                                    return transform(varDecl, tryStmt);


Instead of transforming the variable declaration, does it not make more sense to return null here and add the variable to the existing try? That saves us having to call canTransform twice.

knutwannheden · 2026-03-17T04:25:45Z

We can also compare against #599 . Perhaps we find some more corner cases or tests covered there.

Support resources used after the try block via Java 9+ try(varName) syntax. Handle finally blocks with extra logic by stripping just the close statement and keeping remaining statements. Add negative tests for qualified close calls, field-based resources, and null-initialized resources closed in catch blocks. Based on review feedback from PR #599.

Handle the transform in the try branch (which looks back at the preceding varDecl), and use the varDecl branch only for removal. This makes the try branch the single source of truth for the transformation decision.

Replace custom isNullLiteral helper with the existing J.Literal.isLiteralValue API. Add @nullable to stripCloseFromFinally parameter. Remove redundant parentheses in null-init check.

timtebeek · 2026-03-17T08:47:56Z

We can also compare against #599 . Perhaps we find some more corner cases or tests covered there.

Thanks! yes had that one in mind as well; now did the explicit comparison and cover a few more cases already.

src/main/java/org/openrewrite/staticanalysis/TryWithResources.java

steve-aom-elliott · 2026-03-17T13:37:08Z

src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java

+                            if (stmt instanceof J.Try && i > 0
+                                    && stmts.get(i - 1) instanceof J.VariableDeclarations) {


Is the implication here (and below), that the variable declaration has to happen directly before the try block? It feels like it may miss a number of scenarios where you have the variable declaration for the AutoCloseable, one or more other statements, and then the try block.

I'd indeed kept that for simplicity for now; we can extend it of course, but figured this first set was a safe default to merge already.

src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java

steve-aom-elliott

Approved so far as an initial version goes, having already noted about the variable / try block detection that could be extended in future.

Add TryWithResources recipe (RSPEC-S2093)

d10ed42

Converts try/finally blocks to try-with-resources when the finally block only closes an AutoCloseable resource. Handles direct close, null-guarded close, and try-catch wrapped close patterns in the finally block.

moderne-meeseeks bot added this to OpenRewrite Mar 16, 2026

moderne-meeseeks bot assigned timtebeek Mar 16, 2026

github-project-automation bot moved this to In Progress in OpenRewrite Mar 16, 2026

timtebeek marked this pull request as ready for review March 16, 2026 21:55

timtebeek added 2 commits March 16, 2026 22:55

Merge branch 'main' into tim/sonar-rule-gaps

eeefb41

timtebeek commented Mar 16, 2026

View reviewed changes

timtebeek added 4 commits March 17, 2026 09:29

Extract shared resource-building logic into addResource helper

4add688

Move transformation logic to try branch per review feedback

ec15b02

Handle the transform in the try branch (which looks back at the preceding varDecl), and use the varDecl branch only for removal. This makes the try branch the single source of truth for the transformation decision.

Use J.Literal.isLiteralValue and @nullable annotation

0df0c38

Replace custom isNullLiteral helper with the existing J.Literal.isLiteralValue API. Add @nullable to stripCloseFromFinally parameter. Remove redundant parentheses in null-init check.

timtebeek requested a review from steve-aom-elliott March 17, 2026 09:45

steve-aom-elliott reviewed Mar 17, 2026

View reviewed changes

src/main/java/org/openrewrite/staticanalysis/TryWithResources.java Outdated Show resolved Hide resolved

steve-aom-elliott reviewed Mar 17, 2026

View reviewed changes

src/main/java/org/openrewrite/staticanalysis/TryWithResources.java Outdated Show resolved Hide resolved

steve-aom-elliott reviewed Mar 17, 2026

View reviewed changes

Rename TryWithResources to UseTryWithResources

99b9743

steve-aom-elliott reviewed Mar 17, 2026

View reviewed changes

src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java Outdated Show resolved Hide resolved

Add resource to existing try-with-resources instead of skipping

cfc9a7d

steve-aom-elliott reviewed Mar 18, 2026

View reviewed changes

src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java Outdated Show resolved Hide resolved

steve-aom-elliott reviewed Mar 18, 2026

View reviewed changes

src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java Show resolved Hide resolved

Use static emptyList import and check for duplicate resources

ede229e

steve-aom-elliott approved these changes Mar 18, 2026

View reviewed changes

github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Mar 18, 2026

Use SemanticallyEqual for duplicate resource check

69d5c82

timtebeek merged commit eced538 into main Mar 18, 2026
1 check passed

timtebeek deleted the tim/sonar-rule-gaps branch March 18, 2026 16:51

github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Mar 18, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TryWithResources recipe (RSPEC-S2093)#831

Add TryWithResources recipe (RSPEC-S2093)#831
timtebeek merged 11 commits intomainfrom
tim/sonar-rule-gaps

timtebeek commented Mar 16, 2026 •

edited

Loading

Uh oh!

timtebeek commented Mar 16, 2026

Uh oh!

timtebeek commented Mar 16, 2026

Uh oh!

timtebeek Mar 16, 2026

Uh oh!

knutwannheden commented Mar 17, 2026

Uh oh!

timtebeek commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

steve-aom-elliott Mar 17, 2026

Uh oh!

timtebeek Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steve-aom-elliott left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if (stmt instanceof J.Try && i > 0
		&& stmts.get(i - 1) instanceof J.VariableDeclarations) {

Conversation

timtebeek commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Patterns handled

Features

Safety guards (skips transformation when)

Test plan

Uh oh!

timtebeek commented Mar 16, 2026

Uh oh!

timtebeek commented Mar 16, 2026

At-scale validation results

Summary

Repos with changes (sorted by file count)

Patch review

Uh oh!

timtebeek Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

knutwannheden commented Mar 17, 2026

Uh oh!

timtebeek commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

steve-aom-elliott Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

timtebeek Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steve-aom-elliott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

timtebeek commented Mar 16, 2026 •

edited

Loading