From d10ed42a1ed8ce0f73788e54a1b3eba92f6c84a7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 16 Mar 2026 22:15:48 +0100 Subject: [PATCH 01/10] Add TryWithResources recipe (RSPEC-S2093) 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. --- .../staticanalysis/TryWithResources.java | 305 ++++++++++++++ .../resources/META-INF/rewrite/recipes.csv | 1 + .../staticanalysis/TryWithResourcesTest.java | 394 ++++++++++++++++++ 3 files changed, 700 insertions(+) create mode 100644 src/main/java/org/openrewrite/staticanalysis/TryWithResources.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java new file mode 100644 index 000000000..9f9924e8c --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java @@ -0,0 +1,305 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import lombok.Getter; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.UsesJavaVersion; +import org.openrewrite.java.tree.*; +import org.openrewrite.marker.Markers; +import org.openrewrite.staticanalysis.java.JavaFileChecker; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; + +public class TryWithResources extends Recipe { + + private static final MethodMatcher CLOSE = new MethodMatcher("java.lang.AutoCloseable close()", true); + private static final JavaType.ShallowClass AUTO_CLOSEABLE = JavaType.ShallowClass.build("java.lang.AutoCloseable"); + + @Getter + final String displayName = "Use try-with-resources"; + + @Getter + final String description = "Refactor try/finally blocks to use try-with-resources when the finally block only closes an `AutoCloseable` resource."; + + @Getter + final Duration estimatedEffortPerOccurrence = Duration.ofMinutes(5); + + @Getter + final Set tags = singleton("RSPEC-S2093"); + + @Override + public TreeVisitor getVisitor() { + return Preconditions.check( + Preconditions.and(new UsesJavaVersion<>(9), new JavaFileChecker<>()), + new JavaIsoVisitor() { + @Override + public J.Block visitBlock(J.Block block, ExecutionContext ctx) { + J.Block b = super.visitBlock(block, ctx); + List stmts = b.getStatements(); + + boolean changed = false; + List newStmts = new ArrayList<>(stmts.size()); + + for (int i = 0; i < stmts.size(); i++) { + if (i + 1 < stmts.size() + && stmts.get(i) instanceof J.VariableDeclarations + && stmts.get(i + 1) instanceof J.Try) { + + J.VariableDeclarations varDecl = (J.VariableDeclarations) stmts.get(i); + J.Try tryStmt = (J.Try) stmts.get(i + 1); + + if (canTransform(varDecl, tryStmt, stmts, i)) { + newStmts.add(transform(varDecl, tryStmt)); + i++; // skip the try statement + changed = true; + continue; + } + } + newStmts.add(stmts.get(i)); + } + + return changed ? b.withStatements(newStmts) : b; + } + }); + } + + private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStmt, + List stmts, int varDeclIndex) { + // Single variable only + if (varDecl.getVariables().size() != 1) { + return false; + } + J.VariableDeclarations.NamedVariable namedVar = varDecl.getVariables().get(0); + String varName = namedVar.getSimpleName(); + + // Must have a non-null initializer + Expression init = namedVar.getInitializer(); + if (init == null || (init instanceof J.Literal && ((J.Literal) init).getValue() == null)) { + return false; + } + + // Must implement AutoCloseable + JavaType.FullyQualified type = TypeUtils.asFullyQualified(varDecl.getType()); + if (type == null || !TypeUtils.isAssignableTo(AUTO_CLOSEABLE, type)) { + return false; + } + + // Must have a finally block + if (tryStmt.getFinally() == null) { + return false; + } + + // Must not already have resources + if (tryStmt.getResources() != null && !tryStmt.getResources().isEmpty()) { + return false; + } + + // Finally block must only close this variable + if (!isFinallyOnlyClosing(tryStmt.getFinally(), varName)) { + return false; + } + + // Variable must not be used after the try + if (isUsedAfter(varName, stmts, varDeclIndex + 1)) { + return false; + } + + // Variable must not be reassigned in try body + if (isReassigned(varName, tryStmt.getBody())) { + return false; + } + + // Variable must not be closed in catch blocks + for (J.Try.Catch aCatch : tryStmt.getCatches()) { + if (containsClose(varName, aCatch.getBody())) { + return false; + } + } + + return true; + } + + private static J.Try transform(J.VariableDeclarations varDecl, J.Try tryStmt) { + J.VariableDeclarations resourceDecl = varDecl.withPrefix(Space.EMPTY); + + J.Try.Resource resource = new J.Try.Resource( + Tree.randomId(), + Space.EMPTY, + Markers.EMPTY, + resourceDecl, + false + ); + + JContainer resources = JContainer.build( + Space.SINGLE_SPACE, + singletonList(JRightPadded.build(resource)), + Markers.EMPTY + ); + + return tryStmt.getPadding() + .withResources(resources) + .withFinally(null) + .withPrefix(varDecl.getPrefix()); + } + + private static boolean isFinallyOnlyClosing(J.Block finallyBlock, String varName) { + List stmts = finallyBlock.getStatements(); + return stmts.size() == 1 && isCloseStatement(stmts.get(0), varName); + } + + private static boolean isCloseStatement(Statement stmt, String varName) { + // Direct: var.close() + if (isDirectClose(stmt, varName)) { + return true; + } + + // Null-guarded: if (var != null) { ... close ... } + if (stmt instanceof J.If) { + J.If ifStmt = (J.If) stmt; + if (ifStmt.getElsePart() != null) { + return false; + } + if (!isNullCheck(ifStmt.getIfCondition().getTree(), varName)) { + return false; + } + if (!(ifStmt.getThenPart() instanceof J.Block)) { + return false; + } + J.Block thenBlock = (J.Block) ifStmt.getThenPart(); + if (thenBlock.getStatements().size() != 1) { + return false; + } + Statement inner = thenBlock.getStatements().get(0); + return isDirectClose(inner, varName) || isTryCatchClose(inner, varName); + } + + // Try-catch wrapped: try { var.close(); } catch (...) {} + return isTryCatchClose(stmt, varName); + } + + private static boolean isDirectClose(Statement stmt, String varName) { + return stmt instanceof J.MethodInvocation && isCloseInvocation((J.MethodInvocation) stmt, varName); + } + + private static boolean isCloseInvocation(J.MethodInvocation mi, String varName) { + return CLOSE.matches(mi) + && mi.getSelect() instanceof J.Identifier + && ((J.Identifier) mi.getSelect()).getSimpleName().equals(varName); + } + + private static boolean isTryCatchClose(Statement stmt, String varName) { + if (!(stmt instanceof J.Try)) { + return false; + } + J.Try innerTry = (J.Try) stmt; + if (innerTry.getFinally() != null) { + return false; + } + List body = innerTry.getBody().getStatements(); + return body.size() == 1 && isDirectClose(body.get(0), varName); + } + + private static boolean isNullCheck(Expression condition, String varName) { + if (!(condition instanceof J.Binary)) { + return false; + } + J.Binary binary = (J.Binary) condition; + if (binary.getOperator() != J.Binary.Type.NotEqual) { + return false; + } + return (isIdentifier(binary.getLeft(), varName) && isNullLiteral(binary.getRight())) + || (isNullLiteral(binary.getLeft()) && isIdentifier(binary.getRight(), varName)); + } + + private static boolean isIdentifier(Expression expr, String name) { + return expr instanceof J.Identifier && ((J.Identifier) expr).getSimpleName().equals(name); + } + + private static boolean isNullLiteral(Expression expr) { + return expr instanceof J.Literal && ((J.Literal) expr).getValue() == null; + } + + private static boolean isUsedAfter(String varName, List stmts, int tryIndex) { + for (int i = tryIndex + 1; i < stmts.size(); i++) { + if (new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, AtomicBoolean f) { + if (identifier.getSimpleName().equals(varName)) { + f.set(true); + } + return super.visitIdentifier(identifier, f); + } + }.reduce(stmts.get(i), new AtomicBoolean()).get()) { + return true; + } + } + return false; + } + + private static boolean isReassigned(String varName, J tree) { + return new JavaIsoVisitor() { + @Override + public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean f) { + if (isIdentifier(assignment.getVariable(), varName)) { + f.set(true); + } + return super.visitAssignment(assignment, f); + } + + @Override + public J.AssignmentOperation visitAssignmentOperation(J.AssignmentOperation assignOp, AtomicBoolean f) { + if (isIdentifier(assignOp.getVariable(), varName)) { + f.set(true); + } + return super.visitAssignmentOperation(assignOp, f); + } + + @Override + public J.Unary visitUnary(J.Unary unary, AtomicBoolean f) { + if (unary.getOperator().isModifying() && isIdentifier(unary.getExpression(), varName)) { + f.set(true); + } + return super.visitUnary(unary, f); + } + }.reduce(tree, new AtomicBoolean()).get(); + } + + private static boolean containsClose(String varName, J tree) { + return new JavaIsoVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, AtomicBoolean f) { + if (isCloseInvocation(mi, varName)) { + f.set(true); + } + return super.visitMethodInvocation(mi, f); + } + }.reduce(tree, new AtomicBoolean()).get(); + } +} diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index f6cd70142..973e99906 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -147,6 +147,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.StaticMethodNotFinal,Static methods need not be final,Static methods do not need to be declared final because they cannot be overridden.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.StringLiteralEquality,Use `String.equals()` on `String` literals,"`String.equals()` should be used when checking value equality on String literals. Using `==` or `!=` compares object references, not the actual value of the Strings. This only modifies code where at least one side of the binary operation (`==` or `!=`) is a String literal, such as `""someString"" == someVariable;`. This is to prevent inadvertently changing code where referential equality is the user's intent.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.TernaryOperatorsShouldNotBeNested,Ternary operators should not be nested,"Nested ternary operators can be hard to read quickly. Prefer simpler constructs for improved readability. If supported, this recipe will try to replace nested ternaries with switch expressions.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.TryWithResources,Use try-with-resources,Refactor try/finally blocks to use try-with-resources when the finally block only closes an `AutoCloseable` resource.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.TypecastParenPad,Typecast parenthesis padding,"Fixes whitespace padding between a typecast type identifier and the enclosing left and right parentheses. For example, when configured to remove spacing, `( int ) 0L;` becomes `(int) 0L;`.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.URLEqualsHashCodeRecipes,URL Equals and Hash Code,"Uses of `equals()` and `hashCode()` cause `java.net.URL` to make blocking internet connections. Instead, use `java.net.URI`.",3,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.URLEqualsHashCodeRecipes$URLEqualsRecipe,URL Equals,"Uses of `equals()` cause `java.net.URL` to make blocking internet connections. Instead, use `java.net.URI`.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., diff --git a/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java b/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java new file mode 100644 index 000000000..943cace8d --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java @@ -0,0 +1,394 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.javaVersion; + +@SuppressWarnings({"ConstantConditions", "TryFinallyCanBeTryWithResources", "unused"}) +class TryWithResourcesTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new TryWithResources()) + .allSources(s -> s.markers(javaVersion(9))); + } + + @DocumentExample + @Test + void simpleResourceWithDirectClose() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } finally { + in.close(); + } + } + } + """, + """ + import java.io.*; + + class Test { + void method() throws IOException { + try (InputStream in = new FileInputStream("file.txt")) { + int data = in.read(); + } + } + } + """ + ) + ); + } + + @Test + void nullGuardedClose() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } finally { + if (in != null) { + in.close(); + } + } + } + } + """, + """ + import java.io.*; + + class Test { + void method() throws IOException { + try (InputStream in = new FileInputStream("file.txt")) { + int data = in.read(); + } + } + } + """ + ) + ); + } + + @Test + void tryCatchWrappedClose() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } finally { + try { + in.close(); + } catch (IOException ignored) { + } + } + } + } + """, + """ + import java.io.*; + + class Test { + void method() throws IOException { + try (InputStream in = new FileInputStream("file.txt")) { + int data = in.read(); + } + } + } + """ + ) + ); + } + + @Test + void nullGuardedWithTryCatchClose() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } finally { + if (in != null) { + try { + in.close(); + } catch (IOException ignored) { + } + } + } + } + } + """, + """ + import java.io.*; + + class Test { + void method() throws IOException { + try (InputStream in = new FileInputStream("file.txt")) { + int data = in.read(); + } + } + } + """ + ) + ); + } + + @Test + void preservesCatchBlocks() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } catch (IOException e) { + e.printStackTrace(); + } finally { + in.close(); + } + } + } + """, + """ + import java.io.*; + + class Test { + void method() { + try (InputStream in = new FileInputStream("file.txt")) { + int data = in.read(); + } catch (IOException e) { + e.printStackTrace(); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeResourceUsedAfterTry() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } finally { + in.close(); + } + System.out.println(in); + } + } + """ + ) + ); + } + + @Test + void doNotChangeResourceReassignedInTry() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file1.txt"); + try { + in = new FileInputStream("file2.txt"); + int data = in.read(); + } finally { + in.close(); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeNonAutoCloseable() { + rewriteRun( + //language=java + java( + """ + class Test { + static class CustomResource { + public void close() {} + public void doSomething() {} + } + + void method() { + CustomResource resource = new CustomResource(); + try { + resource.doSomething(); + } finally { + resource.close(); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeAlreadyTryWithResources() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + try (InputStream in = new FileInputStream("file.txt")) { + int data = in.read(); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeCloseViaHelperMethod() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + static void closeQuietly(InputStream stream) { + try { + if (stream != null) { + stream.close(); + } + } catch (IOException ignored) {} + } + + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } finally { + closeQuietly(in); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeResourceClosedInCatch() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } catch (IOException e) { + in.close(); + throw e; + } finally { + in.close(); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeFinallyWithExtraLogic() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } finally { + System.out.println("closing"); + in.close(); + } + } + } + """ + ) + ); + } +} From 0c8bba42cd1ce5f91aee59f9a7ee9c47531c1b2b Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 16 Mar 2026 23:03:05 +0100 Subject: [PATCH 02/10] Use ListUtils.map and handle braceless null-guarded close - 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 --- .../staticanalysis/TryWithResources.java | 49 +++++++++---------- .../staticanalysis/TryWithResourcesTest.java | 35 +++++++++++++ 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java index 9f9924e8c..96087ec65 100644 --- a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java @@ -24,12 +24,12 @@ import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.search.UsesJavaVersion; +import org.openrewrite.internal.ListUtils; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; import org.openrewrite.staticanalysis.java.JavaFileChecker; import java.time.Duration; -import java.util.ArrayList; import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -63,29 +63,24 @@ public TreeVisitor getVisitor() { public J.Block visitBlock(J.Block block, ExecutionContext ctx) { J.Block b = super.visitBlock(block, ctx); List stmts = b.getStatements(); - - boolean changed = false; - List newStmts = new ArrayList<>(stmts.size()); - - for (int i = 0; i < stmts.size(); i++) { - if (i + 1 < stmts.size() - && stmts.get(i) instanceof J.VariableDeclarations + return b.withStatements(ListUtils.map(stmts, (i, stmt) -> { + // Transform varDecl into try-with-resources + if (stmt instanceof J.VariableDeclarations && i + 1 < stmts.size() && stmts.get(i + 1) instanceof J.Try) { - - J.VariableDeclarations varDecl = (J.VariableDeclarations) stmts.get(i); + J.VariableDeclarations varDecl = (J.VariableDeclarations) stmt; J.Try tryStmt = (J.Try) stmts.get(i + 1); - if (canTransform(varDecl, tryStmt, stmts, i)) { - newStmts.add(transform(varDecl, tryStmt)); - i++; // skip the try statement - changed = true; - continue; + return transform(varDecl, tryStmt); } } - newStmts.add(stmts.get(i)); - } - - return changed ? b.withStatements(newStmts) : b; + // Remove the try statement that was merged into the preceding varDecl + if (stmt instanceof J.Try && i > 0 + && stmts.get(i - 1) instanceof J.VariableDeclarations + && canTransform((J.VariableDeclarations) stmts.get(i - 1), (J.Try) stmt, stmts, i - 1)) { + return null; + } + return stmt; + })); } }); } @@ -189,14 +184,16 @@ private static boolean isCloseStatement(Statement stmt, String varName) { if (!isNullCheck(ifStmt.getIfCondition().getTree(), varName)) { return false; } - if (!(ifStmt.getThenPart() instanceof J.Block)) { - return false; - } - J.Block thenBlock = (J.Block) ifStmt.getThenPart(); - if (thenBlock.getStatements().size() != 1) { - return false; + Statement inner; + if (ifStmt.getThenPart() instanceof J.Block) { + J.Block thenBlock = (J.Block) ifStmt.getThenPart(); + if (thenBlock.getStatements().size() != 1) { + return false; + } + inner = thenBlock.getStatements().get(0); + } else { + inner = ifStmt.getThenPart(); } - Statement inner = thenBlock.getStatements().get(0); return isDirectClose(inner, varName) || isTryCatchClose(inner, varName); } diff --git a/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java b/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java index 943cace8d..1c5f78f4e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java @@ -103,6 +103,41 @@ void method() throws IOException { ); } + @Test + void nullGuardedCloseWithoutBraces() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try { + int data = in.read(); + } finally { + if (in != null) + in.close(); + } + } + } + """, + """ + import java.io.*; + + class Test { + void method() throws IOException { + try (InputStream in = new FileInputStream("file.txt")) { + int data = in.read(); + } + } + } + """ + ) + ); + } + @Test void tryCatchWrappedClose() { rewriteRun( From 297057b83cab1e3ef78475a900aa206d0aa3fbda Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 17 Mar 2026 09:29:15 +0100 Subject: [PATCH 03/10] Add Java 9+ try(var) syntax, complex finally handling, and missing tests 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. --- .../staticanalysis/TryWithResources.java | 104 ++++++++++++--- .../staticanalysis/TryWithResourcesTest.java | 121 +++++++++++++++++- 2 files changed, 204 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java index 96087ec65..1a8bed4e9 100644 --- a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java @@ -30,6 +30,7 @@ import org.openrewrite.staticanalysis.java.JavaFileChecker; import java.time.Duration; +import java.util.Collections; import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; @@ -69,15 +70,29 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { && stmts.get(i + 1) instanceof J.Try) { J.VariableDeclarations varDecl = (J.VariableDeclarations) stmt; J.Try tryStmt = (J.Try) stmts.get(i + 1); - if (canTransform(varDecl, tryStmt, stmts, i)) { + if (canTransform(varDecl, tryStmt)) { + boolean usedAfter = isUsedAfter(varDecl.getVariables().get(0).getSimpleName(), stmts, i + 1); + if (usedAfter) { + // Keep the varDecl; the try will be transformed below + return stmt; + } return transform(varDecl, tryStmt); } } - // Remove the try statement that was merged into the preceding varDecl + // Transform or remove the try statement that follows a transformable varDecl if (stmt instanceof J.Try && i > 0 - && stmts.get(i - 1) instanceof J.VariableDeclarations - && canTransform((J.VariableDeclarations) stmts.get(i - 1), (J.Try) stmt, stmts, i - 1)) { - return null; + && stmts.get(i - 1) instanceof J.VariableDeclarations) { + J.VariableDeclarations prevDecl = (J.VariableDeclarations) stmts.get(i - 1); + J.Try tryStmt = (J.Try) stmt; + if (canTransform(prevDecl, tryStmt)) { + boolean usedAfter = isUsedAfter(prevDecl.getVariables().get(0).getSimpleName(), stmts, i); + if (usedAfter) { + // Use Java 9+ try(varName) syntax + return transformJava9(prevDecl, tryStmt); + } + // Merged into varDecl above, remove this try + return null; + } } return stmt; })); @@ -85,8 +100,7 @@ && canTransform((J.VariableDeclarations) stmts.get(i - 1), (J.Try) stmt, stmts, }); } - private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStmt, - List stmts, int varDeclIndex) { + private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStmt) { // Single variable only if (varDecl.getVariables().size() != 1) { return false; @@ -116,13 +130,8 @@ private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStm return false; } - // Finally block must only close this variable - if (!isFinallyOnlyClosing(tryStmt.getFinally(), varName)) { - return false; - } - - // Variable must not be used after the try - if (isUsedAfter(varName, stmts, varDeclIndex + 1)) { + // Finally block must contain a close for this variable + if (!finallyContainsClose(tryStmt.getFinally(), varName)) { return false; } @@ -158,15 +167,72 @@ private static J.Try transform(J.VariableDeclarations varDecl, J.Try tryStmt) { Markers.EMPTY ); - return tryStmt.getPadding() + String varName = varDecl.getVariables().get(0).getSimpleName(); + J.Try result = tryStmt.getPadding() .withResources(resources) - .withFinally(null) .withPrefix(varDecl.getPrefix()); + return result.getPadding() + .withFinally(stripCloseFromFinally(result.getPadding().getFinally(), varName)); + } + + private static J.Try transformJava9(J.VariableDeclarations varDecl, J.Try tryStmt) { + J.VariableDeclarations.NamedVariable namedVar = varDecl.getVariables().get(0); + J.Identifier resourceRef = new J.Identifier( + Tree.randomId(), + Space.EMPTY, + Markers.EMPTY, + Collections.emptyList(), + namedVar.getSimpleName(), + namedVar.getType(), + null + ); + + J.Try.Resource resource = new J.Try.Resource( + Tree.randomId(), + Space.EMPTY, + Markers.EMPTY, + resourceRef, + false + ); + + JContainer resources = JContainer.build( + Space.SINGLE_SPACE, + singletonList(JRightPadded.build(resource)), + Markers.EMPTY + ); + + String varName = namedVar.getSimpleName(); + J.Try result = tryStmt.getPadding() + .withResources(resources); + return result.getPadding() + .withFinally(stripCloseFromFinally(result.getPadding().getFinally(), varName)); } - private static boolean isFinallyOnlyClosing(J.Block finallyBlock, String varName) { - List stmts = finallyBlock.getStatements(); - return stmts.size() == 1 && isCloseStatement(stmts.get(0), varName); + private static boolean finallyContainsClose(J.Block finallyBlock, String varName) { + for (Statement stmt : finallyBlock.getStatements()) { + if (isCloseStatement(stmt, varName)) { + return true; + } + } + return false; + } + + /** + * Remove the close statement from the finally block. Returns null if the finally block + * becomes empty (so it can be removed entirely), otherwise returns the pruned finally block. + */ + @SuppressWarnings("DataFlowIssue") + private static JLeftPadded stripCloseFromFinally(JLeftPadded finallyPadded, String varName) { + if (finallyPadded == null) { + return null; + } + J.Block finallyBlock = finallyPadded.getElement(); + List remaining = ListUtils.map(finallyBlock.getStatements(), + stmt -> isCloseStatement(stmt, varName) ? null : stmt); + if (remaining.isEmpty()) { + return null; + } + return finallyPadded.withElement(finallyBlock.withStatements(remaining)); } private static boolean isCloseStatement(Statement stmt, String varName) { diff --git a/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java b/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java index 1c5f78f4e..8c019a3b3 100644 --- a/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java @@ -253,7 +253,7 @@ void method() { } @Test - void doNotChangeResourceUsedAfterTry() { + void resourceUsedAfterTryUsesJava9Syntax() { rewriteRun( //language=java java( @@ -271,6 +271,19 @@ void method() throws IOException { System.out.println(in); } } + """, + """ + import java.io.*; + + class Test { + void method() throws IOException { + InputStream in = new FileInputStream("file.txt"); + try (in) { + int data = in.read(); + } + System.out.println(in); + } + } """ ) ); @@ -404,7 +417,7 @@ void method() throws IOException { } @Test - void doNotChangeFinallyWithExtraLogic() { + void finallyWithExtraLogicKeepsRemainingStatements() { rewriteRun( //language=java java( @@ -422,6 +435,110 @@ void method() throws IOException { } } } + """, + """ + import java.io.*; + + class Test { + void method() throws IOException { + try (InputStream in = new FileInputStream("file.txt")) { + int data = in.read(); + } finally { + System.out.println("closing"); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeQualifiedCloseMethodCall() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + static class Wrapper { + InputStream stream; + Wrapper(InputStream s) { this.stream = s; } + InputStream getStream() { return stream; } + } + + void method() throws IOException { + Wrapper wrapper = new Wrapper(new FileInputStream("file.txt")); + try { + int data = wrapper.getStream().read(); + } finally { + wrapper.getStream().close(); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeResourceAssignedToField() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + private InputStream fieldStream; + + void method() throws IOException { + fieldStream = new FileInputStream("file.txt"); + try { + int data = fieldStream.read(); + } finally { + fieldStream.close(); + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeNullInitializedResourceClosedInCatch() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() { + InputStream in = null; + try { + in = new FileInputStream("file.txt"); + int data = in.read(); + } catch (IOException e) { + if (in != null) { + try { + in.close(); + } catch (IOException ignored) { + } + } + throw new RuntimeException(e); + } finally { + if (in != null) { + try { + in.close(); + } catch (IOException ignored) { + } + } + } + } + } """ ) ); From 4add68878f5b5b831282fae58ab88fcfae9d33cf Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 17 Mar 2026 09:33:39 +0100 Subject: [PATCH 04/10] Extract shared resource-building logic into addResource helper --- .../staticanalysis/TryWithResources.java | 44 +++++-------------- 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java index 1a8bed4e9..77c822cff 100644 --- a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java @@ -151,28 +151,9 @@ private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStm } private static J.Try transform(J.VariableDeclarations varDecl, J.Try tryStmt) { - J.VariableDeclarations resourceDecl = varDecl.withPrefix(Space.EMPTY); - - J.Try.Resource resource = new J.Try.Resource( - Tree.randomId(), - Space.EMPTY, - Markers.EMPTY, - resourceDecl, - false - ); - - JContainer resources = JContainer.build( - Space.SINGLE_SPACE, - singletonList(JRightPadded.build(resource)), - Markers.EMPTY - ); - - String varName = varDecl.getVariables().get(0).getSimpleName(); - J.Try result = tryStmt.getPadding() - .withResources(resources) - .withPrefix(varDecl.getPrefix()); - return result.getPadding() - .withFinally(stripCloseFromFinally(result.getPadding().getFinally(), varName)); + return addResource(varDecl.withPrefix(Space.EMPTY), + varDecl.getVariables().get(0).getSimpleName(), + tryStmt.withPrefix(varDecl.getPrefix())); } private static J.Try transformJava9(J.VariableDeclarations varDecl, J.Try tryStmt) { @@ -186,24 +167,23 @@ private static J.Try transformJava9(J.VariableDeclarations varDecl, J.Try tryStm namedVar.getType(), null ); + return addResource(resourceRef, namedVar.getSimpleName(), tryStmt); + } + private static J.Try addResource(TypedTree resourceExpr, String varName, J.Try tryStmt) { J.Try.Resource resource = new J.Try.Resource( Tree.randomId(), Space.EMPTY, Markers.EMPTY, - resourceRef, + resourceExpr, false ); - - JContainer resources = JContainer.build( - Space.SINGLE_SPACE, - singletonList(JRightPadded.build(resource)), - Markers.EMPTY - ); - - String varName = namedVar.getSimpleName(); J.Try result = tryStmt.getPadding() - .withResources(resources); + .withResources(JContainer.build( + Space.SINGLE_SPACE, + singletonList(JRightPadded.build(resource)), + Markers.EMPTY + )); return result.getPadding() .withFinally(stripCloseFromFinally(result.getPadding().getFinally(), varName)); } From ec15b029d4f5567a63fc8374e6c1bb2ec57a42ef Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 17 Mar 2026 09:35:55 +0100 Subject: [PATCH 05/10] Move transformation logic to try branch per review feedback 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. --- .../staticanalysis/TryWithResources.java | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java index 77c822cff..2b9e8dc54 100644 --- a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java @@ -65,21 +65,6 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { J.Block b = super.visitBlock(block, ctx); List stmts = b.getStatements(); return b.withStatements(ListUtils.map(stmts, (i, stmt) -> { - // Transform varDecl into try-with-resources - if (stmt instanceof J.VariableDeclarations && i + 1 < stmts.size() - && stmts.get(i + 1) instanceof J.Try) { - J.VariableDeclarations varDecl = (J.VariableDeclarations) stmt; - J.Try tryStmt = (J.Try) stmts.get(i + 1); - if (canTransform(varDecl, tryStmt)) { - boolean usedAfter = isUsedAfter(varDecl.getVariables().get(0).getSimpleName(), stmts, i + 1); - if (usedAfter) { - // Keep the varDecl; the try will be transformed below - return stmt; - } - return transform(varDecl, tryStmt); - } - } - // Transform or remove the try statement that follows a transformable varDecl if (stmt instanceof J.Try && i > 0 && stmts.get(i - 1) instanceof J.VariableDeclarations) { J.VariableDeclarations prevDecl = (J.VariableDeclarations) stmts.get(i - 1); @@ -87,13 +72,18 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { if (canTransform(prevDecl, tryStmt)) { boolean usedAfter = isUsedAfter(prevDecl.getVariables().get(0).getSimpleName(), stmts, i); if (usedAfter) { - // Use Java 9+ try(varName) syntax return transformJava9(prevDecl, tryStmt); } - // Merged into varDecl above, remove this try - return null; + return transform(prevDecl, tryStmt); } } + // Remove varDecl that was merged into the following try-with-resources + if (stmt instanceof J.VariableDeclarations && i + 1 < stmts.size() + && stmts.get(i + 1) instanceof J.Try + && canTransform((J.VariableDeclarations) stmt, (J.Try) stmts.get(i + 1)) + && !isUsedAfter(((J.VariableDeclarations) stmt).getVariables().get(0).getSimpleName(), stmts, i + 1)) { + return null; + } return stmt; })); } From 0df0c38def04e2e68551bdbc59e9d7fd48d6cddc Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 17 Mar 2026 09:44:27 +0100 Subject: [PATCH 06/10] Use J.Literal.isLiteralValue and @Nullable annotation Replace custom isNullLiteral helper with the existing J.Literal.isLiteralValue API. Add @Nullable to stripCloseFromFinally parameter. Remove redundant parentheses in null-init check. --- .../staticanalysis/TryWithResources.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java index 2b9e8dc54..04db058f7 100644 --- a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java @@ -16,6 +16,7 @@ package org.openrewrite.staticanalysis; import lombok.Getter; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; import org.openrewrite.Recipe; @@ -100,7 +101,7 @@ private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStm // Must have a non-null initializer Expression init = namedVar.getInitializer(); - if (init == null || (init instanceof J.Literal && ((J.Literal) init).getValue() == null)) { + if (init == null || init instanceof J.Literal && ((J.Literal) init).getValue() == null) { return false; } @@ -192,7 +193,7 @@ private static boolean finallyContainsClose(J.Block finallyBlock, String varName * becomes empty (so it can be removed entirely), otherwise returns the pruned finally block. */ @SuppressWarnings("DataFlowIssue") - private static JLeftPadded stripCloseFromFinally(JLeftPadded finallyPadded, String varName) { + private static JLeftPadded stripCloseFromFinally(@Nullable JLeftPadded finallyPadded, String varName) { if (finallyPadded == null) { return null; } @@ -267,18 +268,16 @@ private static boolean isNullCheck(Expression condition, String varName) { if (binary.getOperator() != J.Binary.Type.NotEqual) { return false; } - return (isIdentifier(binary.getLeft(), varName) && isNullLiteral(binary.getRight())) - || (isNullLiteral(binary.getLeft()) && isIdentifier(binary.getRight(), varName)); + if (isIdentifier(binary.getLeft(), varName) && J.Literal.isLiteralValue(binary.getRight(), null)) { + return true; + } + return J.Literal.isLiteralValue(binary.getLeft(), null) && isIdentifier(binary.getRight(), varName); } private static boolean isIdentifier(Expression expr, String name) { return expr instanceof J.Identifier && ((J.Identifier) expr).getSimpleName().equals(name); } - private static boolean isNullLiteral(Expression expr) { - return expr instanceof J.Literal && ((J.Literal) expr).getValue() == null; - } - private static boolean isUsedAfter(String varName, List stmts, int tryIndex) { for (int i = tryIndex + 1; i < stmts.size(); i++) { if (new JavaIsoVisitor() { From 99b974332ae13c84b2eb0cf9e9d08b3800482128 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 17 Mar 2026 14:39:02 +0100 Subject: [PATCH 07/10] Rename TryWithResources to UseTryWithResources --- .../{TryWithResources.java => UseTryWithResources.java} | 4 ++-- src/main/resources/META-INF/rewrite/recipes.csv | 2 +- ...yWithResourcesTest.java => UseTryWithResourcesTest.java} | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) rename src/main/java/org/openrewrite/staticanalysis/{TryWithResources.java => UseTryWithResources.java} (99%) rename src/test/java/org/openrewrite/staticanalysis/{TryWithResourcesTest.java => UseTryWithResourcesTest.java} (99%) diff --git a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java similarity index 99% rename from src/main/java/org/openrewrite/staticanalysis/TryWithResources.java rename to src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java index 04db058f7..20bbc39e4 100644 --- a/src/main/java/org/openrewrite/staticanalysis/TryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2026 the original author or authors. *

* Licensed under the Moderne Source Available License (the "License"); * you may not use this file except in compliance with the License. @@ -39,7 +39,7 @@ import static java.util.Collections.singleton; import static java.util.Collections.singletonList; -public class TryWithResources extends Recipe { +public class UseTryWithResources extends Recipe { private static final MethodMatcher CLOSE = new MethodMatcher("java.lang.AutoCloseable close()", true); private static final JavaType.ShallowClass AUTO_CLOSEABLE = JavaType.ShallowClass.build("java.lang.AutoCloseable"); diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index db26fcdca..80e656b09 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -148,7 +148,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.StaticMethodNotFinal,Static methods need not be final,Static methods do not need to be declared final because they cannot be overridden.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.StringLiteralEquality,Use `String.equals()` on `String` literals,"`String.equals()` should be used when checking value equality on String literals. Using `==` or `!=` compares object references, not the actual value of the Strings. This only modifies code where at least one side of the binary operation (`==` or `!=`) is a String literal, such as `""someString"" == someVariable;`. This is to prevent inadvertently changing code where referential equality is the user's intent.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.TernaryOperatorsShouldNotBeNested,Ternary operators should not be nested,"Nested ternary operators can be hard to read quickly. Prefer simpler constructs for improved readability. If supported, this recipe will try to replace nested ternaries with switch expressions.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., -maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.TryWithResources,Use try-with-resources,Refactor try/finally blocks to use try-with-resources when the finally block only closes an `AutoCloseable` resource.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.UseTryWithResources,Use try-with-resources,Refactor try/finally blocks to use try-with-resources when the finally block only closes an `AutoCloseable` resource.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.TypecastParenPad,Typecast parenthesis padding,"Fixes whitespace padding between a typecast type identifier and the enclosing left and right parentheses. For example, when configured to remove spacing, `( int ) 0L;` becomes `(int) 0L;`.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.URLEqualsHashCodeRecipes,URL Equals and Hash Code,"Uses of `equals()` and `hashCode()` cause `java.net.URL` to make blocking internet connections. Instead, use `java.net.URI`.",3,,Static analysis and remediation,,Remediations for issues identified by SAST tools., maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.URLEqualsHashCodeRecipes$URLEqualsRecipe,URL Equals,"Uses of `equals()` cause `java.net.URL` to make blocking internet connections. Instead, use `java.net.URI`.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools., diff --git a/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java b/src/test/java/org/openrewrite/staticanalysis/UseTryWithResourcesTest.java similarity index 99% rename from src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java rename to src/test/java/org/openrewrite/staticanalysis/UseTryWithResourcesTest.java index 8c019a3b3..0a8ddf5c9 100644 --- a/src/test/java/org/openrewrite/staticanalysis/TryWithResourcesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UseTryWithResourcesTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2025 the original author or authors. + * Copyright 2026 the original author or authors. *

* Licensed under the Moderne Source Available License (the "License"); * you may not use this file except in compliance with the License. @@ -24,11 +24,11 @@ import static org.openrewrite.java.Assertions.javaVersion; @SuppressWarnings({"ConstantConditions", "TryFinallyCanBeTryWithResources", "unused"}) -class TryWithResourcesTest implements RewriteTest { +class UseTryWithResourcesTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new TryWithResources()) + spec.recipe(new UseTryWithResources()) .allSources(s -> s.markers(javaVersion(9))); } From cfc9a7db69186cd7bcb954d3715dd73b76697a35 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 18 Mar 2026 13:17:58 +0100 Subject: [PATCH 08/10] Add resource to existing try-with-resources instead of skipping --- .../staticanalysis/UseTryWithResources.java | 24 +++++++++---- .../UseTryWithResourcesTest.java | 34 +++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java index 20bbc39e4..3a0ebab28 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java @@ -31,6 +31,7 @@ import org.openrewrite.staticanalysis.java.JavaFileChecker; import java.time.Duration; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Set; @@ -116,11 +117,6 @@ private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStm return false; } - // Must not already have resources - if (tryStmt.getResources() != null && !tryStmt.getResources().isEmpty()) { - return false; - } - // Finally block must contain a close for this variable if (!finallyContainsClose(tryStmt.getFinally(), varName)) { return false; @@ -169,10 +165,24 @@ private static J.Try addResource(TypedTree resourceExpr, String varName, J.Try t resourceExpr, false ); + List> existingResources = tryStmt.getPadding().getResources() != null ? + tryStmt.getPadding().getResources().getPadding().getElements() : Collections.emptyList(); + List> newResources; + if (existingResources.isEmpty()) { + newResources = singletonList(JRightPadded.build(resource)); + } else { + newResources = new ArrayList<>(existingResources); + // Set semicolon on the previous last resource + int lastIdx = newResources.size() - 1; + JRightPadded prev = newResources.get(lastIdx); + newResources.set(lastIdx, prev.withElement(prev.getElement().withTerminatedWithSemicolon(true))); + newResources.add(JRightPadded.build(resource.withPrefix(Space.SINGLE_SPACE))); + } J.Try result = tryStmt.getPadding() .withResources(JContainer.build( - Space.SINGLE_SPACE, - singletonList(JRightPadded.build(resource)), + tryStmt.getPadding().getResources() != null ? + tryStmt.getPadding().getResources().getBefore() : Space.SINGLE_SPACE, + newResources, Markers.EMPTY )); return result.getPadding() diff --git a/src/test/java/org/openrewrite/staticanalysis/UseTryWithResourcesTest.java b/src/test/java/org/openrewrite/staticanalysis/UseTryWithResourcesTest.java index 0a8ddf5c9..7664feb8b 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UseTryWithResourcesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UseTryWithResourcesTest.java @@ -507,6 +507,40 @@ void method() throws IOException { ); } + @Test + void addResourceToExistingTryWithResources() { + rewriteRun( + //language=java + java( + """ + import java.io.*; + + class Test { + void method() throws IOException { + OutputStream out = new FileOutputStream("out.txt"); + try (InputStream in = new FileInputStream("in.txt")) { + out.write(in.read()); + } finally { + out.close(); + } + } + } + """, + """ + import java.io.*; + + class Test { + void method() throws IOException { + try (InputStream in = new FileInputStream("in.txt"); OutputStream out = new FileOutputStream("out.txt")) { + out.write(in.read()); + } + } + } + """ + ) + ); + } + @Test void doNotChangeNullInitializedResourceClosedInCatch() { rewriteRun( From ede229eca1dc103d5e2902c610ca1fa0edac1895 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 18 Mar 2026 15:11:17 +0100 Subject: [PATCH 09/10] Use static emptyList import and check for duplicate resources --- .../staticanalysis/UseTryWithResources.java | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java index 3a0ebab28..ad97e6f06 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java @@ -32,11 +32,11 @@ import java.time.Duration; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import static java.util.Collections.emptyList; import static java.util.Collections.singleton; import static java.util.Collections.singletonList; @@ -117,6 +117,22 @@ private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStm return false; } + // Must not already be a resource in the try + if (tryStmt.getResources() != null) { + for (J.Try.Resource res : tryStmt.getResources()) { + if (res.getVariableDeclarations() instanceof J.VariableDeclarations) { + J.VariableDeclarations resDecl = (J.VariableDeclarations) res.getVariableDeclarations(); + if (resDecl.getVariables().get(0).getSimpleName().equals(varName)) { + return false; + } + } else if (res.getVariableDeclarations() instanceof J.Identifier) { + if (((J.Identifier) res.getVariableDeclarations()).getSimpleName().equals(varName)) { + return false; + } + } + } + } + // Finally block must contain a close for this variable if (!finallyContainsClose(tryStmt.getFinally(), varName)) { return false; @@ -149,7 +165,7 @@ private static J.Try transformJava9(J.VariableDeclarations varDecl, J.Try tryStm Tree.randomId(), Space.EMPTY, Markers.EMPTY, - Collections.emptyList(), + emptyList(), namedVar.getSimpleName(), namedVar.getType(), null @@ -166,7 +182,7 @@ private static J.Try addResource(TypedTree resourceExpr, String varName, J.Try t false ); List> existingResources = tryStmt.getPadding().getResources() != null ? - tryStmt.getPadding().getResources().getPadding().getElements() : Collections.emptyList(); + tryStmt.getPadding().getResources().getPadding().getElements() : emptyList(); List> newResources; if (existingResources.isEmpty()) { newResources = singletonList(JRightPadded.build(resource)); From 69d5c8200026b40a2e238305f941e7861ddbe9fd Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 18 Mar 2026 15:17:53 +0100 Subject: [PATCH 10/10] Use SemanticallyEqual for duplicate resource check --- .../staticanalysis/UseTryWithResources.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java b/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java index ad97e6f06..dcd328ab6 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseTryWithResources.java @@ -24,6 +24,7 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.SemanticallyEqual; import org.openrewrite.java.search.UsesJavaVersion; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.tree.*; @@ -119,16 +120,10 @@ private static boolean canTransform(J.VariableDeclarations varDecl, J.Try tryStm // Must not already be a resource in the try if (tryStmt.getResources() != null) { + J.VariableDeclarations normalized = varDecl.withPrefix(Space.EMPTY); for (J.Try.Resource res : tryStmt.getResources()) { - if (res.getVariableDeclarations() instanceof J.VariableDeclarations) { - J.VariableDeclarations resDecl = (J.VariableDeclarations) res.getVariableDeclarations(); - if (resDecl.getVariables().get(0).getSimpleName().equals(varName)) { - return false; - } - } else if (res.getVariableDeclarations() instanceof J.Identifier) { - if (((J.Identifier) res.getVariableDeclarations()).getSimpleName().equals(varName)) { - return false; - } + if (SemanticallyEqual.areEqual(res.getVariableDeclarations(), normalized)) { + return false; } } }