Merge pull request #348 from nielsdebruin/catch-clause-only-rethrows-fix

Fix: `CatchClauseOnlyRethrows` does not handle multi catch correctly
openrewrite · Oct 5, 2024 · d523fc1 · d523fc1
2 parents 7b2c565 + 7bffbca
commit d523fc1
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 2 deletions.
diff --git a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java
@@ -77,8 +77,10 @@ public J.Try visitTry(J.Try tryable, ExecutionContext ctx) {
                         // keep this one
                         for (int j = i + 1; j < tryable.getCatches().size(); j++) {
                             J.Try.Catch next = tryable.getCatches().get(j);
-                            if (!onlyRethrows(next) && TypeUtils.isAssignableTo(next.getParameter().getType(),
-                                    aCatch.getParameter().getType())) {
+                            if (hasWiderExceptionType(aCatch, next)) {
+                                if (onlyRethrows(next)) {
+                                    return null;
+                                }
                                 return aCatch;
                             }
                         }
@@ -88,6 +90,19 @@ public J.Try visitTry(J.Try tryable, ExecutionContext ctx) {
                 }));
             }
 
+            private boolean hasWiderExceptionType(J.Try.Catch aCatch, J.Try.Catch next) {
+                if (next.getParameter().getType() instanceof JavaType.MultiCatch) {
+                    JavaType.MultiCatch multiCatch = (JavaType.MultiCatch) next.getParameter().getType();
+                    for (JavaType throwableType : multiCatch.getThrowableTypes()) {
+                        if (TypeUtils.isAssignableTo(throwableType, aCatch.getParameter().getType())) {
+                            return true;
+                        }
+                    }
+                    return false;
+                }
+                return TypeUtils.isAssignableTo(next.getParameter().getType(), aCatch.getParameter().getType());
+            }
+
             private boolean onlyRethrows(J.Try.Catch aCatch) {
                 if (aCatch.getBody().getStatements().size() != 1 ||
                         !(aCatch.getBody().getStatements().get(0) instanceof J.Throw)) {

diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java
@@ -82,6 +82,31 @@ void foo() throws IOException {
         );
     }
 
+    @Test
+    void catchShouldBePreservedBecauseLessSpecificCatchFollowsWithMultiCast() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import java.io.FileReader;
+              import java.io.IOException;
+              
+              class A {
+                  void foo() throws IOException {
+                      try {
+                          new FileReader("").read();
+                      } catch (IOException e) {
+                          throw e;
+                      } catch(Exception | Throwable t) {
+                          t.printStackTrace();
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @DocumentExample
     @Test
     void tryCanBeRemoved() {