From 6c2116406dc41ef30bf5d7ca69248e4ad99dbc9a Mon Sep 17 00:00:00 2001 From: Niels de Bruin Date: Wed, 2 Oct 2024 20:03:05 +0200 Subject: [PATCH 1/3] Fix: CatchClauseOnlyRethrows does not handle multi catch correctly --- .../CatchClauseOnlyRethrows.java | 11 ++++++-- .../CatchClauseOnlyRethrowsTest.java | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java index 9b05e49e8..e947b8a21 100755 --- a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java +++ b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java @@ -77,8 +77,7 @@ 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 (!onlyRethrows(next) && hasWiderExceptionType(aCatch, next)) { return aCatch; } } @@ -88,6 +87,14 @@ 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(); + return multiCatch.getThrowableTypes().stream().anyMatch(alt -> TypeUtils.isAssignableTo(alt, aCatch.getParameter().getType())); + } + 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 index 0a8b24315..5a483f971 100755 --- 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() { From 771e21deba886d2efd42e3168914bc2e5570706f Mon Sep 17 00:00:00 2001 From: Niels de Bruin Date: Thu, 3 Oct 2024 14:27:12 +0200 Subject: [PATCH 2/3] Change stream to foreach loop --- .../staticanalysis/CatchClauseOnlyRethrows.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java index e947b8a21..87eb10a8d 100755 --- a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java +++ b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java @@ -90,7 +90,12 @@ 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(); - return multiCatch.getThrowableTypes().stream().anyMatch(alt -> TypeUtils.isAssignableTo(alt, aCatch.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()); } From 7bffbcae65dea13c8b6926053f738bcc70397671 Mon Sep 17 00:00:00 2001 From: Niels de Bruin Date: Sat, 5 Oct 2024 14:42:42 +0200 Subject: [PATCH 3/3] Fix check order of if statement --- .../openrewrite/staticanalysis/CatchClauseOnlyRethrows.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java index 87eb10a8d..d6a577d9b 100755 --- a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java +++ b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java @@ -77,7 +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) && hasWiderExceptionType(aCatch, next)) { + if (hasWiderExceptionType(aCatch, next)) { + if (onlyRethrows(next)) { + return null; + } return aCatch; } }