From 3e8d04aa1e27553bf81fb3a3405a6a97d6094ba5 Mon Sep 17 00:00:00 2001 From: Niels de Bruin Date: Mon, 7 Oct 2024 11:58:23 +0200 Subject: [PATCH 1/2] Fix CatchCauseOnlyRethrows rethrows check for multi catch statements --- .../CatchClauseOnlyRethrows.java | 5 --- .../CatchClauseOnlyRethrowsTest.java | 39 +++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java index d6a577d9b4..3868313296 100755 --- a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java +++ b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java @@ -110,11 +110,6 @@ private boolean onlyRethrows(J.Try.Catch aCatch) { } Expression exception = ((J.Throw) aCatch.getBody().getStatements().get(0)).getException(); - JavaType.FullyQualified catchType = TypeUtils.asFullyQualified(aCatch.getParameter().getType()); - if (catchType == null || !catchType.equals(exception.getType())) { - return false; - } - if (exception instanceof J.Identifier) { return ((J.Identifier) exception).getSimpleName().equals(aCatch.getParameter().getTree().getVariables().get(0).getSimpleName()); } diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index 5a483f9710..dc58664ade 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -141,6 +141,45 @@ void foo() throws IOException { ); } + @Test + void tryCanBeRemovedWithMultiCatch() { + rewriteRun( + //language=java + java( + """ + import java.io.FileReader; + import java.io.IOException; + import java.io.FileNotFoundException; + + class A { + void foo() throws IOException { + try { + new FileReader("").read(); + } catch (FileNotFoundException e) { + throw e; + } catch(IOException | ArrayIndexOutOfBoundsException e) { + throw e; + } catch(Exception e) { + throw e; + } + } + } + """, + """ + import java.io.FileReader; + import java.io.IOException; + import java.io.FileNotFoundException; + + class A { + void foo() throws IOException { + new FileReader("").read(); + } + } + """ + ) + ); + } + @Test void tryShouldBePreservedBecauseFinally() { rewriteRun( From 6b39457a151e4d78beb9f17b234fb677f9e8d81e Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 22 Oct 2024 10:23:10 +0200 Subject: [PATCH 2/2] Specifically check for `JavaType.MultiCatch` --- .../CatchClauseOnlyRethrows.java | 16 ++++++++-- .../CatchClauseOnlyRethrowsTest.java | 32 +++++++++++++++++-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java index 3868313296..53b978fcea 100755 --- a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java +++ b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java @@ -20,7 +20,10 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; -import org.openrewrite.java.tree.*; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.TypeUtils; import java.time.Duration; import java.util.Set; @@ -37,7 +40,7 @@ public String getDisplayName() { @Override public String getDescription() { return "A `catch` clause that only rethrows the caught exception is unnecessary. " + - "Letting the exception bubble up as normal achieves the same result with less code."; + "Letting the exception bubble up as normal achieves the same result with less code."; } @Override @@ -105,11 +108,18 @@ private boolean hasWiderExceptionType(J.Try.Catch aCatch, J.Try.Catch next) { private boolean onlyRethrows(J.Try.Catch aCatch) { if (aCatch.getBody().getStatements().size() != 1 || - !(aCatch.getBody().getStatements().get(0) instanceof J.Throw)) { + !(aCatch.getBody().getStatements().get(0) instanceof J.Throw)) { return false; } Expression exception = ((J.Throw) aCatch.getBody().getStatements().get(0)).getException(); + JavaType catchParameterType = aCatch.getParameter().getType(); + if (!(catchParameterType instanceof JavaType.MultiCatch)) { + JavaType.FullyQualified catchType = TypeUtils.asFullyQualified(catchParameterType); + if (catchType == null || !catchType.equals(exception.getType())) { + return false; + } + } if (exception instanceof J.Identifier) { return ((J.Identifier) exception).getSimpleName().equals(aCatch.getParameter().getTree().getVariables().get(0).getSimpleName()); } diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index dc58664ade..c6c93ebf10 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -165,14 +165,40 @@ void foo() throws IOException { } } """, - """ + """ + import java.io.FileReader; + import java.io.IOException; + import java.io.FileNotFoundException; + + class A { + void foo() throws IOException { + new FileReader("").read(); + } + } + """ + ) + ); + } + + @Test + void multiCatchPreservedOnDifferentThrow() { + rewriteRun( + //language=java + java( + """ import java.io.FileReader; import java.io.IOException; import java.io.FileNotFoundException; - + class A { void foo() throws IOException { - new FileReader("").read(); + try { + new FileReader("").read(); + } catch (FileNotFoundException e) { + throw e; + } catch(IOException | ArrayIndexOutOfBoundsException e) { + throw new IOException("another message", e); + } } } """