Skip to content

Commit ab8ff8d

Browse files
authored
Preserve statement order in UseTryWithResources (#866)
Refuse to merge a `close()` into try-with-resources when non-close statements precede it in the finally block. Try-with-resources runs the auto-close before the finally, so those statements would otherwise move from before to after the close, changing semantics.
1 parent 90e4a60 commit ab8ff8d

2 files changed

Lines changed: 65 additions & 2 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,10 +306,18 @@ private static boolean isReassignedBetween(String varName, List<Statement> stmts
306306
}
307307

308308
private static boolean finallyContainsClose(J.Block finallyBlock, String varName) {
309+
// The target close must be reachable without any non-close statements before it.
310+
// Try-with-resources closes the resource before the finally block runs, so any
311+
// statement that originally ran before the close in the finally would otherwise
312+
// run after it. Other close statements (for variables that will also be merged
313+
// into the try-with-resources) are tolerated.
309314
for (Statement stmt : finallyBlock.getStatements()) {
310315
if (isCloseStatement(stmt, varName)) {
311316
return true;
312317
}
318+
if (extractVarNameFromCloseStatement(stmt) == null) {
319+
return false;
320+
}
313321
}
314322
return false;
315323
}

src/test/java/org/openrewrite/staticanalysis/UseTryWithResourcesTest.java

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import org.junit.jupiter.api.Test;
1919
import org.openrewrite.DocumentExample;
20+
import org.openrewrite.Issue;
2021
import org.openrewrite.test.RecipeSpec;
2122
import org.openrewrite.test.RewriteTest;
2223

@@ -343,7 +344,7 @@ void method() throws IOException {
343344
}
344345

345346
@Test
346-
void finallyWithExtraLogicKeepsRemainingStatements() {
347+
void finallyWithExtraLogicAfterCloseKeepsRemainingStatements() {
347348
rewriteRun(
348349
//language=java
349350
java(
@@ -356,8 +357,8 @@ void method() throws IOException {
356357
try {
357358
int data = in.read();
358359
} finally {
359-
System.out.println("closing");
360360
in.close();
361+
System.out.println("closed");
361362
}
362363
}
363364
}
@@ -369,8 +370,62 @@ class Test {
369370
void method() throws IOException {
370371
try (InputStream in = new FileInputStream("file.txt")) {
371372
int data = in.read();
373+
} finally {
374+
System.out.println("closed");
375+
}
376+
}
377+
}
378+
"""
379+
)
380+
);
381+
}
382+
383+
@Test
384+
@Issue("https://github.com/moderneinc/rewrite-java-application-server/issues/19")
385+
void doNotChangeWhenStatementsBeforeClose() {
386+
rewriteRun(
387+
//language=java
388+
java(
389+
"""
390+
import java.io.*;
391+
392+
class Test {
393+
void method() throws IOException {
394+
InputStream in = new FileInputStream("file.txt");
395+
try {
396+
int data = in.read();
372397
} finally {
373398
System.out.println("closing");
399+
in.close();
400+
}
401+
}
402+
}
403+
"""
404+
)
405+
);
406+
}
407+
408+
@Test
409+
@Issue("https://github.com/moderneinc/rewrite-java-application-server/issues/19")
410+
void doNotChangeWhenCleanupBeforeClose() {
411+
rewriteRun(
412+
//language=java
413+
java(
414+
"""
415+
import java.io.*;
416+
417+
class Test {
418+
interface CancellationTokenSource {
419+
void cancel();
420+
}
421+
422+
void method(CancellationTokenSource cancellation) throws IOException {
423+
InputStream in = new FileInputStream("file.txt");
424+
try {
425+
int data = in.read();
426+
} finally {
427+
cancellation.cancel();
428+
in.close();
374429
}
375430
}
376431
}

0 commit comments

Comments
 (0)