Added transform to merge stacked try statements. (#350)

Also added this a a last step to ResourceLeakFixer transform.

Closes #341.

Author: andrecsilva

core-codemods/src/main/java/io/codemodder/codemods/ResourceLeakFixer.java

@@ -51,12 +51,20 @@ public static Optional<TryStmt> checkAndFix(final Expression expr) {
       // finds the root expression in a chain of new AutoCloseable objects
       ObjectCreationExpr root = findRootExpression(expr.asObjectCreationExpr());
       if (isFixable(root)) {
-        return tryToFix(root);
+        var maybeFixed = tryToFix(root);
+        // try to merge with an encompassing try stmt and return
+        return maybeFixed
+            .flatMap(ts -> ASTTransforms.mergeStackedTryStmts(ts))
+            .or(() -> maybeFixed);
       }
     }
     if (expr instanceof MethodCallExpr) {
       if (isFixable(expr)) {
-        return tryToFix(expr.asMethodCallExpr());
+        var maybeFixed = tryToFix(expr.asMethodCallExpr());
+        // try to merge with an encompassing try stmt and return
+        return maybeFixed
+            .flatMap(ts -> ASTTransforms.mergeStackedTryStmts(ts))
+            .or(() -> maybeFixed);
       }
     }
     return Optional.empty();

core-codemods/src/test/resources/database-resource-leak/Test.java.after

@@ -57,10 +57,9 @@ public class Test {
   }
 
   public void simple(String query) throws SQLException {
-    try (Statement stmt = conn.createStatement()) {
-        try (ResultSet rs = stmt.executeQuery(query)) {
-        }
-    }
+    try (Statement stmt = conn.createStatement();
+    ResultSet rs = stmt.executeQuery(query)) {
+}
   }
 
   public void assignedToParameter(String query, Statement stmt) throws SQLException {

core-codemods/src/test/resources/prevent-filewriter-leak-with-nio/Test.java.after

@@ -9,13 +9,12 @@ public class Test
 {
     private Response response;
     void foo(File f) {
-        try (Writer change1 = Files.newBufferedWriter(f.toPath())) {
-            try (Writer change2 = Files.newBufferedWriter(f.toPath())) {
-                Writer dontChange1 = new BufferedWriter(w);
-                Writer dontChange2 = new BufferedWriter(w, 256);
-                Writer dontChange3 = new BufferedWriter(response.getWriter(), 256);
-                Writer dontChange3 = Files.newBufferedWriter(f.toPath());
-            }
+        try (Writer change1 = Files.newBufferedWriter(f.toPath());
+        Writer change2 = Files.newBufferedWriter(f.toPath())) {
+            Writer dontChange1 = new BufferedWriter(w);
+            Writer dontChange2 = new BufferedWriter(w, 256);
+            Writer dontChange3 = new BufferedWriter(response.getWriter(), 256);
+            Writer dontChange3 = Files.newBufferedWriter(f.toPath());
         }
 
     }

core-codemods/src/test/resources/resource-leak/Test.java.after

@@ -38,6 +38,13 @@ public class Test {
     }
   }
 
+  public void mergeFixes(File f) throws IOException {
+    try (FileReader fr = new FileReader(f);
+    BufferedReader br = new BufferedReader(fr)) {
+        System.out.println(br.readLine());
+    }
+  }
+
   public Reader noFixReturned() throws IOException {
     BufferedReader br = new BufferedReader(new FileReader(""));
     BufferedReader br2 = new BufferedReader(br);

core-codemods/src/test/resources/resource-leak/Test.java.before

@@ -32,6 +32,12 @@ public class Test {
     BufferedReader br = new BufferedReader(new BufferedReader(new FileReader("~/test.txt")));
   }
 
+  public void mergeFixes(File f) throws IOException {
+    FileReader fr = new FileReader(f);
+    BufferedReader br = new BufferedReader(fr);
+    System.out.println(br.readLine());
+  }
+
   public Reader noFixReturned() throws IOException {
     BufferedReader br = new BufferedReader(new FileReader(""));
     BufferedReader br2 = new BufferedReader(br);

framework/codemodder-base/src/main/java/io/codemodder/ast/ASTTransforms.java

@@ -30,13 +30,13 @@ public final class ASTTransforms {
   public static void addImportIfMissing(final CompilationUnit cu, final String className) {
     final NodeList<ImportDeclaration> imports = cu.getImports();
     final ImportDeclaration newImport = new ImportDeclaration(className, false, false);
-    if (addInOrdrerIfNeeded(className, imports, newImport)) {
+    if (addInOrderIfNeeded(className, imports, newImport)) {
       return;
     }
     cu.addImport(className);
   }
 
-  private static boolean addInOrdrerIfNeeded(
+  private static boolean addInOrderIfNeeded(
       final String className,
       final NodeList<ImportDeclaration> imports,
       final ImportDeclaration newImport) {
@@ -70,7 +70,7 @@ public static void addImportIfMissing(final CompilationUnit cu, final Class<?> c
   public static void addStaticImportIfMissing(final CompilationUnit cu, final String method) {
     final NodeList<ImportDeclaration> imports = cu.getImports();
     final ImportDeclaration newMethodImport = new ImportDeclaration(method, true, false);
-    if (addInOrdrerIfNeeded(method, imports, newMethodImport)) {
+    if (addInOrderIfNeeded(method, imports, newMethodImport)) {
       return;
     }
     cu.addImport(method, true, false);
@@ -371,4 +371,29 @@ private static Optional<ResolvedType> calculateResolvedType(final Expression e)
       return Optional.empty();
     }
   }
+
+  /**
+   * Tries to merge the given try stmt with an enveloping one. Returns the merged try stmts if
+   * sucessfull.
+   */
+  public static Optional<TryStmt> mergeStackedTryStmts(final TryStmt tryStmt) {
+    // is the parent a try statement whose single statment in its block is tryStmt?
+    var maybeTryParent =
+        tryStmt
+            .getParentNode()
+            .flatMap(p -> p.getParentNode())
+            .map(p -> p instanceof TryStmt ? (TryStmt) p : null)
+            .filter(
+                ts ->
+                    ts.getTryBlock().getStatements().size() == 1
+                        && ts.getTryBlock().getStatement(0) == tryStmt);
+    if (maybeTryParent.isPresent()) {
+      tryStmt.remove();
+      var parent = maybeTryParent.get();
+      parent.getResources().addAll(tryStmt.getResources());
+      parent.getTryBlock().getStatements().addAll(tryStmt.getTryBlock().getStatements());
+      return Optional.of(parent);
+    }
+    return Optional.empty();
+  }
 }

framework/codemodder-base/src/test/java/io/codemodder/ast/MergeStackedTryStmtTest.java

@@ -0,0 +1,68 @@
+package io.codemodder.ast;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import com.github.javaparser.ast.CompilationUnit;
+import com.github.javaparser.ast.stmt.ExpressionStmt;
+import com.github.javaparser.ast.stmt.TryStmt;
+import io.codemodder.javaparser.JavaParserFactory;
+import java.io.IOException;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+
+final class MergeStackedTryStmtTest {
+
+  @Test
+  void it_merges_try_stmts() throws IOException {
+    String code =
+        """
+	    class A{
+		    void f(File f){
+			    try(var fr = new FileReader(f)){
+				    try(var br = new BufferedReader(fr)){
+					    System.out.println(br.readLine());
+				    }
+			    }
+		    }
+	    }
+    """;
+    CompilationUnit cu =
+        JavaParserFactory.newFactory().create(List.of()).parse(code).getResult().get();
+
+    var tryStmt = cu.findAll(TryStmt.class).get(1);
+    var call = cu.findAll(ExpressionStmt.class).get(0);
+
+    var merged = ASTTransforms.mergeStackedTryStmts(tryStmt);
+
+    assertThat(merged.isEmpty(), equalTo(false));
+    assertThat(merged.get().getResources().size(), equalTo(2));
+    assertThat(merged.get().getTryBlock().getStatements().isEmpty(), equalTo(false));
+    assertThat(merged.get().getTryBlock().getStatements().get(0), equalTo(call));
+  }
+
+  @Test
+  void it_wont_merge_because_not_stacked() throws IOException {
+    String code =
+        """
+	    class A{
+		    void f(File f){
+			    try(var fr = new FileReader(f)){
+				    try(var br = new BufferedReader(fr)){
+					    System.out.println(br.readLine());
+				    }
+				    System.out.println(fr.readLine());
+			    }
+		    }
+	    }
+    """;
+    CompilationUnit cu =
+        JavaParserFactory.newFactory().create(List.of()).parse(code).getResult().get();
+
+    var tryStmt = cu.findAll(TryStmt.class).get(1);
+
+    var merged = ASTTransforms.mergeStackedTryStmts(tryStmt);
+
+    assertThat(merged.isEmpty(), equalTo(true));
+  }
+}