From 896c89b78e6f39ed5c7419c4372dfb2b6b50f318 Mon Sep 17 00:00:00 2001 From: "jetbrains-junie[bot]" <201638009+jetbrains-junie[bot]@users.noreply.github.com> Date: Wed, 4 Jun 2025 19:58:15 +0200 Subject: [PATCH 01/19] =?UTF-8?q?Initialize=20JetBrains=20Junie=20?= =?UTF-8?q?=F0=9F=9A=80=20(#583)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(junie): added .junie workflow * feat(junie): added .devcontainer.json * Update devcontainer.json --------- Co-authored-by: jetbrains-junie[bot] <201638009+jetbrains-junie[bot]@users.noreply.github.com> Co-authored-by: Knut Wannheden --- .devcontainer/devcontainer.json | 10 ++++++++++ .github/workflows/junie.yml | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 .devcontainer/devcontainer.json create mode 100644 .github/workflows/junie.yml diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000000..edda91e19e --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,10 @@ +{ + "name": "Java", + "image": "mcr.microsoft.com/devcontainers/java:1-21", + "features": { + "ghcr.io/devcontainers/features/java:1": { + "version": "none", + "installGradle": "true" + } + } +} diff --git a/.github/workflows/junie.yml b/.github/workflows/junie.yml new file mode 100644 index 0000000000..66a8e4e64d --- /dev/null +++ b/.github/workflows/junie.yml @@ -0,0 +1,22 @@ +name: Junie +run-name: Junie run ${{ inputs.run_id }} + +permissions: + contents: write + pull-requests: write + +on: + workflow_dispatch: + inputs: + run_id: + description: "id of workflow process" + required: true + workflow_params: + description: "stringified params" + required: true + +jobs: + call-workflow-passing-data: + uses: jetbrains-junie/junie-workflows/.github/workflows/ej-issue.yml@main + with: + workflow_params: ${{ inputs.workflow_params }} From 9c9560f0e3e44ac2d2d6fb29e56f4ab9b56a8fd0 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Thu, 5 Jun 2025 23:56:46 +0200 Subject: [PATCH 02/19] =?UTF-8?q?Revert=20"Initialize=20JetBrains=20Junie?= =?UTF-8?q?=20=F0=9F=9A=80=20(#583)"=20(#585)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit ec88484ba4fb9ea868acbe1fd769721c5e0c2f08. --- .devcontainer/devcontainer.json | 10 ---------- .github/workflows/junie.yml | 22 ---------------------- 2 files changed, 32 deletions(-) delete mode 100644 .devcontainer/devcontainer.json delete mode 100644 .github/workflows/junie.yml diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json deleted file mode 100644 index edda91e19e..0000000000 --- a/.devcontainer/devcontainer.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "name": "Java", - "image": "mcr.microsoft.com/devcontainers/java:1-21", - "features": { - "ghcr.io/devcontainers/features/java:1": { - "version": "none", - "installGradle": "true" - } - } -} diff --git a/.github/workflows/junie.yml b/.github/workflows/junie.yml deleted file mode 100644 index 66a8e4e64d..0000000000 --- a/.github/workflows/junie.yml +++ /dev/null @@ -1,22 +0,0 @@ -name: Junie -run-name: Junie run ${{ inputs.run_id }} - -permissions: - contents: write - pull-requests: write - -on: - workflow_dispatch: - inputs: - run_id: - description: "id of workflow process" - required: true - workflow_params: - description: "stringified params" - required: true - -jobs: - call-workflow-passing-data: - uses: jetbrains-junie/junie-workflows/.github/workflows/ej-issue.yml@main - with: - workflow_params: ${{ inputs.workflow_params }} From c35f58285a937f1a642ed243052140e20e0197a7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sat, 14 Jun 2025 16:22:32 +0200 Subject: [PATCH 03/19] TypecastParenPad should not make incorrect changes for Groovy Fixes #561 --- .../resources/META-INF/rewrite/examples.yml | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 8039ad9def..e66bc26007 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -360,44 +360,44 @@ examples: - description: '' sources: - before: | + import java.io.FileReader; + import java.io.IOException; + class A { - void foo() throws IllegalAccessException { + void foo() throws IOException { try { - throw new IllegalAccessException(); - } catch (Exception e) { - throw e; // `e` is removed below + new FileReader("").read(); + } catch (IOException e) { + throw e; } } } after: | + import java.io.FileReader; + import java.io.IOException; + class A { - void foo() throws IllegalAccessException { - throw new IllegalAccessException(); + void foo() throws IOException { + new FileReader("").read(); } } language: java - description: '' sources: - before: | - import java.io.FileReader; - import java.io.IOException; - class A { - void foo() throws IOException { + void foo() throws IllegalAccessException { try { - new FileReader("").read(); - } catch (IOException e) { - throw e; + throw new IllegalAccessException(); + } catch (Exception e) { + throw e; // `e` is removed below } } } after: | - import java.io.FileReader; - import java.io.IOException; - class A { - void foo() throws IOException { - new FileReader("").read(); + void foo() throws IllegalAccessException { + throw new IllegalAccessException(); } } language: java @@ -2842,18 +2842,6 @@ examples: type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.staticanalysis.SimplifyBooleanExpression examples: -- description: '' - sources: - - before: | - fun getSymbol() : String? { - return null - } - language: kotlin - - before: | - val isPositive = getSymbol().equals("+") == true - after: | - val isPositive = getSymbol().equals("+") - language: kotlin - description: '' sources: - before: | @@ -2873,6 +2861,18 @@ examples: } } language: java +- description: '' + sources: + - before: | + fun getSymbol() : String? { + return null + } + language: kotlin + - before: | + val isPositive = getSymbol().equals("+") == true + after: | + val isPositive = getSymbol().equals("+") + language: kotlin --- type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.staticanalysis.SimplifyBooleanReturn From d6c83ca75d405246fdc98a52f11ba17b90086d78 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Mon, 16 Jun 2025 17:40:09 +0000 Subject: [PATCH 04/19] refactor: Extract documentation examples from tests Use this link to re-run the recipe: https://app.moderne.io/recipes/org.openrewrite.java.recipes.ExamplesExtractor?organizationId=ODQ2MGExMTUtNDg0My00N2EwLTgzMGMtNGE1NGExMTBmZDkw Co-authored-by: Moderne --- .../resources/META-INF/rewrite/examples.yml | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index e66bc26007..8039ad9def 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -360,44 +360,44 @@ examples: - description: '' sources: - before: | - import java.io.FileReader; - import java.io.IOException; - class A { - void foo() throws IOException { + void foo() throws IllegalAccessException { try { - new FileReader("").read(); - } catch (IOException e) { - throw e; + throw new IllegalAccessException(); + } catch (Exception e) { + throw e; // `e` is removed below } } } after: | - import java.io.FileReader; - import java.io.IOException; - class A { - void foo() throws IOException { - new FileReader("").read(); + void foo() throws IllegalAccessException { + throw new IllegalAccessException(); } } language: java - description: '' sources: - before: | + import java.io.FileReader; + import java.io.IOException; + class A { - void foo() throws IllegalAccessException { + void foo() throws IOException { try { - throw new IllegalAccessException(); - } catch (Exception e) { - throw e; // `e` is removed below + new FileReader("").read(); + } catch (IOException e) { + throw e; } } } after: | + import java.io.FileReader; + import java.io.IOException; + class A { - void foo() throws IllegalAccessException { - throw new IllegalAccessException(); + void foo() throws IOException { + new FileReader("").read(); } } language: java @@ -2842,6 +2842,18 @@ examples: type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.staticanalysis.SimplifyBooleanExpression examples: +- description: '' + sources: + - before: | + fun getSymbol() : String? { + return null + } + language: kotlin + - before: | + val isPositive = getSymbol().equals("+") == true + after: | + val isPositive = getSymbol().equals("+") + language: kotlin - description: '' sources: - before: | @@ -2861,18 +2873,6 @@ examples: } } language: java -- description: '' - sources: - - before: | - fun getSymbol() : String? { - return null - } - language: kotlin - - before: | - val isPositive = getSymbol().equals("+") == true - after: | - val isPositive = getSymbol().equals("+") - language: kotlin --- type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.staticanalysis.SimplifyBooleanReturn From 666b67791cadd5cc59ec7fd9d761169f8c18f379 Mon Sep 17 00:00:00 2001 From: stefanod Date: Tue, 15 Jul 2025 00:22:57 +0200 Subject: [PATCH 05/19] UnnecessaryCatch: Handling MultiCatch --- .../staticanalysis/UnnecessaryCatch.java | 149 ++++++++++++++++-- .../staticanalysis/UnnecessaryCatchTest.java | 149 ++++++++++++++---- 2 files changed, 254 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index 618328931e..066c1048c9 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -17,20 +17,28 @@ import lombok.EqualsAndHashCode; import lombok.Value; +import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; import org.openrewrite.Option; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.J.NewClass; import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.TypeTree; import org.openrewrite.java.tree.TypeUtils; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.List; +import java.util.Objects; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; @EqualsAndHashCode(callSuper = false) @Value @@ -66,6 +74,13 @@ public String getDescription() { public TreeVisitor getVisitor() { return new JavaIsoVisitor() { + private static final String JAVA_LANG_EXCEPTION = "java.lang.Exception"; + private static final String JAVA_LANG_ERROR = "java.lang.Error"; + private static final String JAVA_LANG_RUNTIME_EXCEPTION = "java.lang.RuntimeException"; + private static final String JAVA_LANG_THROWABLE = "java.lang.Throwable"; + private static final String TRY_CATCH_TEMPLATE = "try {} catch (%s %s) {}"; + private static final String MULTI_CATCH_SEPARATOR = "|"; + @Override public J.Block visitBlock(J.Block block, ExecutionContext ctx) { J.Block b = super.visitBlock(block, ctx); @@ -119,27 +134,133 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integ return t; } - //!e.isAssignableTo("java.lang.RuntimeException") + Set unnecessaryTypes = getUnnecessaryTypes(t, thrownExceptions); + if(unnecessaryTypes.isEmpty()) { + return t; + } + + for (JavaType type : unnecessaryTypes) { + maybeRemoveImport(TypeUtils.asFullyQualified(type)); + } + //For any checked exceptions being caught, if the exception is not thrown, remove the catch block. return t.withCatches(ListUtils.map(t.getCatches(), (i, aCatch) -> { JavaType parameterType = aCatch.getParameter().getType(); - if (parameterType == null || TypeUtils.isAssignableTo("java.lang.RuntimeException", parameterType)) { - return aCatch; - } - if (!includeJavaLangException && TypeUtils.isOfClassType(parameterType, "java.lang.Exception")) { - return aCatch; + + if (parameterType instanceof JavaType.MultiCatch) { + JavaType.MultiCatch mc = (JavaType.MultiCatch) parameterType; + String retainedTypes = mc.getThrowableTypes().stream() + .filter(it -> !unnecessaryTypes.contains(it)) + .map(TypeUtils::asFullyQualified) + .filter(Objects::nonNull) + .map(JavaType.FullyQualified::getClassName) + .collect(Collectors.joining(MULTI_CATCH_SEPARATOR)); + + String variableName = aCatch.getParameter().getTree().getVariables().get(0).getSimpleName(); + J.Try tempTry = JavaTemplate.builder(String.format(TRY_CATCH_TEMPLATE, retainedTypes, variableName)) + .contextSensitive() + .build() + .apply( + new Cursor(getCursor(), t), + t.getCoordinates().replace() + ); + + return aCatch.withParameter(tempTry.getCatches().get(0).getParameter()); + + } else if (unnecessaryTypes.contains(parameterType)) { + return null; } - if (!includeJavaLangThrowable && TypeUtils.isOfClassType(parameterType, "java.lang.Throwable")) { - return aCatch; + + return aCatch; + })); + } + + /** + * Retrieves a set of unique checked exception types declared within the catch blocks + * of a given {@link J.Try} statement. This method supports both single and multi-catch + * declarations. + * + * @param aTry The {@link J.Try} statement to analyze. + * @return A {@link Set} of {@link JavaType} instances representing the caught checked exceptions. + * An empty set is returned if no checked exceptions are found. + */ + private Set getUnnecessaryTypes(J.Try aTry, Collection thrownExceptions) { + Set caughtExceptions = new HashSet<>(); + + for (J.Try.Catch c : aTry.getCatches()) { + JavaType type = c.getParameter().getType(); + if (type == null) { + continue; } - for (JavaType e : thrownExceptions) { - if (TypeUtils.isAssignableTo(e, parameterType)) { - return aCatch; + + if (type instanceof JavaType.MultiCatch) { + for (JavaType throwable : ((JavaType.MultiCatch) type).getThrowableTypes()) { + if (isCheckedException(throwable) || isGenericTypeRemovableByOption(throwable)) { + caughtExceptions.add(throwable); + } + } + } else { // Single catch + if (isCheckedException(type) || isGenericTypeRemovableByOption(type)) { + caughtExceptions.add(c.getParameter().getType()); } } - maybeRemoveImport(TypeUtils.asFullyQualified(parameterType)); - return null; - })); + } + + caughtExceptions.removeAll(thrownExceptions); + return caughtExceptions; + } + + private boolean isGenericTypeRemovableByOption(JavaType type) { + if (includeJavaLangException && TypeUtils.isOfClassType(type, JAVA_LANG_EXCEPTION)) { + return true; + } + + return includeJavaLangThrowable && TypeUtils.isOfClassType(type, JAVA_LANG_THROWABLE); + } + + /** + * Determines if a given {@link JavaType} represents a checked exception. + * A checked exception is a subclass of {@code java.lang.Throwable} that is not + * a subclass of {@code java.lang.RuntimeException} or {@code java.lang.Error}. + * Source + * + * @param type The {@link JavaType} to evaluate. + * @return {@code true} if the type is a checked exception; {@code false} otherwise. + */ + private boolean isCheckedException(JavaType type) { + if (!(type instanceof JavaType.Class)) { + return false; + } + + JavaType.Class exceptionClass = (JavaType.Class) type; + + boolean isSubclassOfException = TypeUtils.isAssignableTo(JAVA_LANG_EXCEPTION, exceptionClass); + boolean isSubclassOfRuntimeException = TypeUtils.isAssignableTo(JAVA_LANG_RUNTIME_EXCEPTION, exceptionClass); + boolean isSubclassOfError = TypeUtils.isAssignableTo(JAVA_LANG_ERROR, exceptionClass); + boolean isExceptionItself = TypeUtils.isOfClassType(exceptionClass, JAVA_LANG_EXCEPTION); + boolean isThrowableItself = TypeUtils.isOfClassType(exceptionClass, JAVA_LANG_THROWABLE); + + if (!isSubclassOfException) { + return false; + } + + if (isSubclassOfRuntimeException) { + return false; + } + + if (isSubclassOfError) { + return false; + } + + if (isExceptionItself) { + return false; + } + + if (isThrowableItself) { + return false; + } + + return true; } }; } diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index 3395bd29d8..6bf3828085 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -34,7 +34,6 @@ public void defaults(RecipeSpec spec) { @Test void unwrapTry() { rewriteRun( - //language=java java( """ import java.io.IOException; @@ -95,6 +94,39 @@ public void method() { ); } + @Test + void removeFromMultiCatch() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + + public class AnExample { + public void method() { + try { + java.util.Base64.getDecoder().decode("abc".getBytes()); + } catch (IllegalArgumentException | IllegalStateException | IOException e) { + System.out.println("an exception!"); + } + } + } + """, + """ + public class AnExample { + public void method() { + try { + java.util.Base64.getDecoder().decode("abc".getBytes()); + } catch (IllegalArgumentException | IllegalStateException e) { + System.out.println("an exception!"); + } + } + } + """ + ) + ); + } + @Test void doNotRemoveRuntimeException() { rewriteRun( @@ -143,27 +175,27 @@ public void fred() throws IOException { @Test void doNotRemoveThrownExceptionFromConstructor() { rewriteRun( - //language=java - java( - """ - import java.io.IOException; - - public class AnExample { - public void method() { - try { - new Fred(); - } catch (IOException e) { - System.out.println("an exception!"); - } - } + //language=java + java( + """ + import java.io.IOException; - public static class Fred { - public Fred() throws IOException {} + public class AnExample { + public void method() { + try { + new Fred(); + } catch (IOException e) { + System.out.println("an exception!"); } + } + public static class Fred { + public Fred() throws IOException {} } - """ - ) + + } + """ + ) ); } @@ -187,23 +219,80 @@ void method() { ); } + @Test + void removeJavaLangException() { + rewriteRun( + spec -> spec.recipe(new UnnecessaryCatch(true, false)), + //language=java + java( + """ + class Scratch { + void method() { + try { + throw new RuntimeException(); + } catch (Exception e) { + System.out.println("an exception!"); + } + } + } + """, + """ + class Scratch { + void method() { + throw new RuntimeException(); + } + } + """ + ) + ); + } + @Test void doNotRemoveJavaLangThrowable() { rewriteRun( - //language=java - java( - """ - class Scratch { - void method() { - try { - throw new RuntimeException(); - } catch (Throwable e) { - System.out.println("an exception!"); - } + //language=java + java( + """ + class Scratch { + void method() { + try { + throw new RuntimeException(); + } catch (Throwable e) { + System.out.println("an exception!"); } } - """ - ) + } + """ + ) + ); + } + + @Test + void removeJavaLangThrowable() { + rewriteRun( + spec -> spec.recipe(new UnnecessaryCatch(false, true)), + //language=java + java( + """ + class Scratch { + void method() { + try { + throw new RuntimeException(); + } catch (Throwable e) { + System.out.println("an exception!"); + } + } + } + """, + """ + class Scratch { + void method() { + throw new RuntimeException(); + } + } + """ + ) ); } + } From 07c955393f1fc51bbd6727fb281e7e0c46cc2986 Mon Sep 17 00:00:00 2001 From: stefanod Date: Tue, 15 Jul 2025 10:29:01 +0200 Subject: [PATCH 06/19] Fixed case where entire multi-catch should be deleted --- .../staticanalysis/UnnecessaryCatch.java | 7 +++-- .../staticanalysis/UnnecessaryCatchTest.java | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index 066c1048c9..cd61e0ea61 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -28,7 +28,6 @@ import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.J.NewClass; import org.openrewrite.java.tree.JavaType; -import org.openrewrite.java.tree.TypeTree; import org.openrewrite.java.tree.TypeUtils; import java.util.ArrayList; @@ -135,7 +134,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integ } Set unnecessaryTypes = getUnnecessaryTypes(t, thrownExceptions); - if(unnecessaryTypes.isEmpty()) { + if (unnecessaryTypes.isEmpty()) { return t; } @@ -156,6 +155,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integ .map(JavaType.FullyQualified::getClassName) .collect(Collectors.joining(MULTI_CATCH_SEPARATOR)); + if (retainedTypes.isEmpty()) { + return null; + } + String variableName = aCatch.getParameter().getTree().getVariables().get(0).getSimpleName(); J.Try tempTry = JavaTemplate.builder(String.format(TRY_CATCH_TEMPLATE, retainedTypes, variableName)) .contextSensitive() diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index 6bf3828085..11e9ffba5e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -127,6 +127,36 @@ public void method() { ); } + @Test + void removeMultiCatch() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + import java.sql.SQLException; + + public class AnExample { + public void method() { + try { + java.util.Base64.getDecoder().decode("abc".getBytes()); + } catch (IOException | SQLException e) { + System.out.println("an exception!"); + } + } + } + """, + """ + public class AnExample { + public void method() { + java.util.Base64.getDecoder().decode("abc".getBytes()); + } + } + """ + ) + ); + } + @Test void doNotRemoveRuntimeException() { rewriteRun( From f02f258b1cc500c636f74a764f581590d26146fc Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 00:01:52 +0200 Subject: [PATCH 07/19] Apply formatter --- .../staticanalysis/UnnecessaryCatch.java | 13 ++-------- .../staticanalysis/UnnecessaryCatchTest.java | 24 +++++++++---------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index cd61e0ea61..6d1596faf7 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -17,11 +17,7 @@ import lombok.EqualsAndHashCode; import lombok.Value; -import org.openrewrite.Cursor; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Option; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; +import org.openrewrite.*; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaTemplate; @@ -30,12 +26,7 @@ import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypeUtils; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; -import java.util.Set; +import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index 11e9ffba5e..d06f14b8f0 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -133,19 +133,19 @@ void removeMultiCatch() { //language=java java( """ - import java.io.IOException; - import java.sql.SQLException; + import java.io.IOException; + import java.sql.SQLException; - public class AnExample { - public void method() { - try { - java.util.Base64.getDecoder().decode("abc".getBytes()); - } catch (IOException | SQLException e) { - System.out.println("an exception!"); - } - } - } - """, + public class AnExample { + public void method() { + try { + java.util.Base64.getDecoder().decode("abc".getBytes()); + } catch (IOException | SQLException e) { + System.out.println("an exception!"); + } + } + } + """, """ public class AnExample { public void method() { From b5948d02599a24707edeffbe1fab903df70cbcfd Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 00:04:33 +0200 Subject: [PATCH 08/19] Collapse conditionals in `isCheckedException` to return early --- .../staticanalysis/UnnecessaryCatch.java | 35 +++---------------- 1 file changed, 5 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index 6d1596faf7..0599240458 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -208,7 +208,6 @@ private boolean isGenericTypeRemovableByOption(JavaType type) { if (includeJavaLangException && TypeUtils.isOfClassType(type, JAVA_LANG_EXCEPTION)) { return true; } - return includeJavaLangThrowable && TypeUtils.isOfClassType(type, JAVA_LANG_THROWABLE); } @@ -225,36 +224,12 @@ private boolean isCheckedException(JavaType type) { if (!(type instanceof JavaType.Class)) { return false; } - JavaType.Class exceptionClass = (JavaType.Class) type; - - boolean isSubclassOfException = TypeUtils.isAssignableTo(JAVA_LANG_EXCEPTION, exceptionClass); - boolean isSubclassOfRuntimeException = TypeUtils.isAssignableTo(JAVA_LANG_RUNTIME_EXCEPTION, exceptionClass); - boolean isSubclassOfError = TypeUtils.isAssignableTo(JAVA_LANG_ERROR, exceptionClass); - boolean isExceptionItself = TypeUtils.isOfClassType(exceptionClass, JAVA_LANG_EXCEPTION); - boolean isThrowableItself = TypeUtils.isOfClassType(exceptionClass, JAVA_LANG_THROWABLE); - - if (!isSubclassOfException) { - return false; - } - - if (isSubclassOfRuntimeException) { - return false; - } - - if (isSubclassOfError) { - return false; - } - - if (isExceptionItself) { - return false; - } - - if (isThrowableItself) { - return false; - } - - return true; + return TypeUtils.isAssignableTo(JAVA_LANG_EXCEPTION, exceptionClass) && + !TypeUtils.isAssignableTo(JAVA_LANG_RUNTIME_EXCEPTION, exceptionClass) && + !TypeUtils.isAssignableTo(JAVA_LANG_ERROR, exceptionClass) && + !TypeUtils.isOfClassType(exceptionClass, JAVA_LANG_EXCEPTION) && + !TypeUtils.isOfClassType(exceptionClass, JAVA_LANG_THROWABLE); } }; } From fafb5c79ce0f247bea0d1b7d6f2a5b7a99b5160f Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 00:37:14 +0200 Subject: [PATCH 09/19] Remove need for JavaTemplate --- .../staticanalysis/UnnecessaryCatch.java | 60 ++++++++----------- .../staticanalysis/UnnecessaryCatchTest.java | 50 ++++++++-------- 2 files changed, 51 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index 0599240458..c17bad3d88 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -17,18 +17,17 @@ import lombok.EqualsAndHashCode; import lombok.Value; -import org.openrewrite.*; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Option; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; import org.openrewrite.java.JavaIsoVisitor; -import org.openrewrite.java.JavaTemplate; -import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.*; import org.openrewrite.java.tree.J.NewClass; -import org.openrewrite.java.tree.JavaType; -import org.openrewrite.java.tree.TypeUtils; import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; @EqualsAndHashCode(callSuper = false) @Value @@ -68,8 +67,6 @@ public TreeVisitor getVisitor() { private static final String JAVA_LANG_ERROR = "java.lang.Error"; private static final String JAVA_LANG_RUNTIME_EXCEPTION = "java.lang.RuntimeException"; private static final String JAVA_LANG_THROWABLE = "java.lang.Throwable"; - private static final String TRY_CATCH_TEMPLATE = "try {} catch (%s %s) {}"; - private static final String MULTI_CATCH_SEPARATOR = "|"; @Override public J.Block visitBlock(J.Block block, ExecutionContext ctx) { @@ -135,36 +132,31 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integ //For any checked exceptions being caught, if the exception is not thrown, remove the catch block. return t.withCatches(ListUtils.map(t.getCatches(), (i, aCatch) -> { - JavaType parameterType = aCatch.getParameter().getType(); - - if (parameterType instanceof JavaType.MultiCatch) { - JavaType.MultiCatch mc = (JavaType.MultiCatch) parameterType; - String retainedTypes = mc.getThrowableTypes().stream() - .filter(it -> !unnecessaryTypes.contains(it)) - .map(TypeUtils::asFullyQualified) - .filter(Objects::nonNull) - .map(JavaType.FullyQualified::getClassName) - .collect(Collectors.joining(MULTI_CATCH_SEPARATOR)); - - if (retainedTypes.isEmpty()) { + J.ControlParentheses parameter = aCatch.getParameter(); + TypeTree typeExpression = aCatch.getParameter().getTree().getTypeExpression(); + if (typeExpression instanceof J.MultiCatch) { + J.MultiCatch multiCatch = (J.MultiCatch) typeExpression; + List alternatives = ListUtils.map(multiCatch.getAlternatives(), typeTree -> { + if (typeTree instanceof TypeTree) { + JavaType type = ((TypeTree) typeTree).getType(); + if (unnecessaryTypes.contains(type)) { + return null; // Remove this type from the multi-catch + } + } + return typeTree; + }); + if (alternatives.isEmpty()) { return null; } - - String variableName = aCatch.getParameter().getTree().getVariables().get(0).getSimpleName(); - J.Try tempTry = JavaTemplate.builder(String.format(TRY_CATCH_TEMPLATE, retainedTypes, variableName)) - .contextSensitive() - .build() - .apply( - new Cursor(getCursor(), t), - t.getCoordinates().replace() - ); - - return aCatch.withParameter(tempTry.getCatches().get(0).getParameter()); - - } else if (unnecessaryTypes.contains(parameterType)) { + J.MultiCatch.Padding padding = multiCatch.withAlternatives(alternatives).getPadding(); + J.MultiCatch rightTrimmed = padding.withAlternatives( + ListUtils.mapLast(padding.getAlternatives(), last -> last.withAfter(Space.EMPTY))); + return aCatch.withParameter(aCatch.getParameter().withTree( + aCatch.getParameter().getTree().withTypeExpression(rightTrimmed))); + } + if (unnecessaryTypes.contains(parameter.getType())) { return null; } - return aCatch; })); } diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index d06f14b8f0..364358b383 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -38,8 +38,8 @@ void unwrapTry() { """ import java.io.IOException; - public class AnExample { - public void method() { + class AnExample { + void method() { try { java.util.Base64.getDecoder().decode("abc".getBytes()); } catch (IOException e) { @@ -49,8 +49,8 @@ public void method() { } """, """ - public class AnExample { - public void method() { + class AnExample { + void method() { java.util.Base64.getDecoder().decode("abc".getBytes()); } } @@ -67,8 +67,8 @@ void removeCatch() { """ import java.io.IOException; - public class AnExample { - public void method() { + class AnExample { + void method() { try { java.util.Base64.getDecoder().decode("abc".getBytes()); } catch (IOException e1) { @@ -80,8 +80,8 @@ public void method() { } """, """ - public class AnExample { - public void method() { + class AnExample { + void method() { try { java.util.Base64.getDecoder().decode("abc".getBytes()); } catch (IllegalStateException e2) { @@ -102,8 +102,8 @@ void removeFromMultiCatch() { """ import java.io.IOException; - public class AnExample { - public void method() { + class AnExample { + void method() { try { java.util.Base64.getDecoder().decode("abc".getBytes()); } catch (IllegalArgumentException | IllegalStateException | IOException e) { @@ -113,8 +113,8 @@ public void method() { } """, """ - public class AnExample { - public void method() { + class AnExample { + void method() { try { java.util.Base64.getDecoder().decode("abc".getBytes()); } catch (IllegalArgumentException | IllegalStateException e) { @@ -136,8 +136,8 @@ void removeMultiCatch() { import java.io.IOException; import java.sql.SQLException; - public class AnExample { - public void method() { + class AnExample { + void method() { try { java.util.Base64.getDecoder().decode("abc".getBytes()); } catch (IOException | SQLException e) { @@ -147,8 +147,8 @@ public void method() { } """, """ - public class AnExample { - public void method() { + class AnExample { + void method() { java.util.Base64.getDecoder().decode("abc".getBytes()); } } @@ -163,8 +163,8 @@ void doNotRemoveRuntimeException() { //language=java java( """ - public class AnExample { - public void method() { + class AnExample { + void method() { try { java.util.Base64.getDecoder().decode("abc".getBytes()); } catch (IllegalStateException e) { @@ -185,8 +185,8 @@ void doNotRemoveThrownException() { """ import java.io.IOException; - public class AnExample { - public void method() { + class AnExample { + void method() { try { fred(); } catch (IOException e) { @@ -194,7 +194,7 @@ public void method() { } } - public void fred() throws IOException { + void fred() throws IOException { } } """ @@ -210,8 +210,8 @@ void doNotRemoveThrownExceptionFromConstructor() { """ import java.io.IOException; - public class AnExample { - public void method() { + class AnExample { + void method() { try { new Fred(); } catch (IOException e) { @@ -219,8 +219,8 @@ public void method() { } } - public static class Fred { - public Fred() throws IOException {} + static class Fred { + Fred() throws IOException {} } } From 157a41fef64f0eab9e6d93c5b062c8c2d5e1e8ff Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 00:43:19 +0200 Subject: [PATCH 10/19] Reduce further --- .../openrewrite/staticanalysis/UnnecessaryCatch.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index c17bad3d88..0ae549e54e 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -136,15 +136,9 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integ TypeTree typeExpression = aCatch.getParameter().getTree().getTypeExpression(); if (typeExpression instanceof J.MultiCatch) { J.MultiCatch multiCatch = (J.MultiCatch) typeExpression; - List alternatives = ListUtils.map(multiCatch.getAlternatives(), typeTree -> { - if (typeTree instanceof TypeTree) { - JavaType type = ((TypeTree) typeTree).getType(); - if (unnecessaryTypes.contains(type)) { - return null; // Remove this type from the multi-catch - } - } - return typeTree; - }); + List alternatives = ListUtils.map(multiCatch.getAlternatives(), typeTree -> + typeTree != null && unnecessaryTypes.contains(typeTree.getType()) ? null : typeTree + ); if (alternatives.isEmpty()) { return null; } From d906394df7275cabc9e03cf2ee9fbb489ed0ed08 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 00:55:10 +0200 Subject: [PATCH 11/19] Also left trim alternatives --- .../staticanalysis/UnnecessaryCatch.java | 8 ++--- .../staticanalysis/UnnecessaryCatchTest.java | 35 ++++++++++++++++++- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index 0ae549e54e..203d06ada8 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -142,11 +142,11 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integ if (alternatives.isEmpty()) { return null; } - J.MultiCatch.Padding padding = multiCatch.withAlternatives(alternatives).getPadding(); - J.MultiCatch rightTrimmed = padding.withAlternatives( - ListUtils.mapLast(padding.getAlternatives(), last -> last.withAfter(Space.EMPTY))); + List leftTrimmed = ListUtils.mapFirst(alternatives, first -> first.withPrefix(multiCatch.getAlternatives().get(0).getPrefix())); + J.MultiCatch.Padding padding = multiCatch.withAlternatives(leftTrimmed).getPadding(); + List> rightTrimmed = ListUtils.mapLast(padding.getAlternatives(), last -> last.withAfter(Space.EMPTY)); return aCatch.withParameter(aCatch.getParameter().withTree( - aCatch.getParameter().getTree().withTypeExpression(rightTrimmed))); + aCatch.getParameter().getTree().withTypeExpression(padding.withAlternatives(rightTrimmed)))); } if (unnecessaryTypes.contains(parameter.getType())) { return null; diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index 364358b383..b4a3359ef0 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -95,7 +95,40 @@ void method() { } @Test - void removeFromMultiCatch() { + void removeFirstFromMultiCatch() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + + class AnExample { + void method() { + try { + java.util.Base64.getDecoder().decode("abc".getBytes()); + } catch (IOException | IllegalArgumentException | IllegalStateException e) { + System.out.println("an exception!"); + } + } + } + """, + """ + class AnExample { + void method() { + try { + java.util.Base64.getDecoder().decode("abc".getBytes()); + } catch (IllegalArgumentException | IllegalStateException e) { + System.out.println("an exception!"); + } + } + } + """ + ) + ); + } + + @Test + void removeMiddleFromMultiCatch() { rewriteRun( //language=java java( From b902b40960c6f1472c07b4c75bbbfad74bef1532 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 01:05:05 +0200 Subject: [PATCH 12/19] Do not remove catch for exceptions thrown --- .../staticanalysis/UnnecessaryCatch.java | 12 ++++++++++++ .../staticanalysis/UnnecessaryCatchTest.java | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index 203d06ada8..335d7effc4 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -114,6 +114,18 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Integ } return super.visitMethodInvocation(method, integer); } + + @Override + public J.Throw visitThrow(J.Throw thrown, Integer integer) { + JavaType type = thrown.getException().getType(); + if (type == null) { + //Do not make any changes if there is missing type information. + missingTypeInformation.set(true); + } else { + thrownExceptions.add(type); + } + return super.visitThrow(thrown, integer); + } }.visit(t.getBody(), 0); //If there is any missing type information, it is not safe to make any transformations. diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index b4a3359ef0..b8fe1307dc 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -358,4 +358,23 @@ void method() { ); } + @Test + void doNotRemoveCatchOfThrownException() { + rewriteRun( + //language=java + java( + """ + class Scratch { + void method() { + try { + throw new ClassNotFoundException(); + } catch (ClassNotFoundException e) { + // Caught + } + } + } + """ + ) + ); + } } From 9fdd61c25cf00f05a1aa63b3a562d3171ce1a349 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 01:18:12 +0200 Subject: [PATCH 13/19] Avoid making changes for try with resources --- .../staticanalysis/UnnecessaryCatch.java | 5 +++++ .../staticanalysis/UnnecessaryCatchTest.java | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index 335d7effc4..a251b9c797 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -87,6 +87,11 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { public J.Try visitTry(J.Try tryable, ExecutionContext ctx) { J.Try t = super.visitTry(tryable, ctx); + if (t.getResources() != null) { + // Hard to determine if `close()` might throw any exceptions, so do not make any changes for now + return t; + } + List thrownExceptions = new ArrayList<>(); AtomicBoolean missingTypeInformation = new AtomicBoolean(false); //Collect any checked exceptions thrown from the try block. diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index b8fe1307dc..2718a43aaf 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -377,4 +377,26 @@ void method() { ) ); } + + @Test + void doNotRemoveCatchForCloseOnTryWithResources() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + import java.io.StringWriter; + + class Scratch { + void method() { + try (StringWriter sw = new StringWriter()) { + } catch (IOException e) { + // Caught on .close() + } + } + } + """ + ) + ); + } } From a271474074e4c5dac5fb6039bcfed6f35e16a5d7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 01:23:24 +0200 Subject: [PATCH 14/19] Show yet another problematic case --- .../staticanalysis/UnnecessaryCatchTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index 2718a43aaf..494ce92dfe 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -399,4 +399,27 @@ void method() { ) ); } + + @Test + void doNotRemoveCatchForConstructorThatThrows() { + rewriteRun( + //language=java + java( + """ + import java.util.jar.JarFile; + import java.util.zip.ZipException; + + class Scratch { + JarFile method(String name) { + try { + return new JarFile(name); // throws IOException + } catch (ZipException e) { // extends IOException + // Catches subset + } + } + } + """ + ) + ); + } } From 510f34776bea4c4d4eaf9a36c69119836bc297eb Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 02:00:17 +0200 Subject: [PATCH 15/19] Fix final test --- .../staticanalysis/UnnecessaryCatch.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index a251b9c797..2066fca79f 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -203,8 +203,24 @@ private Set getUnnecessaryTypes(J.Try aTry, Collection throw } } - caughtExceptions.removeAll(thrownExceptions); - return caughtExceptions; + // Filter out caught exceptions that are necessary + Set unnecessaryExceptions = new HashSet<>(caughtExceptions); + unnecessaryExceptions.removeAll(thrownExceptions); + + // Also filter out caught exceptions that are subtypes of thrown exceptions + // For example, if IOException is thrown, don't remove catch for ZipException + Set toKeep = new HashSet<>(); + for (JavaType caughtException : unnecessaryExceptions) { + for (JavaType thrownException : thrownExceptions) { + if (TypeUtils.isAssignableTo(thrownException, caughtException)) { + toKeep.add(caughtException); + break; + } + } + } + unnecessaryExceptions.removeAll(toKeep); + + return unnecessaryExceptions; } private boolean isGenericTypeRemovableByOption(JavaType type) { From c13655dc4f4f209f2d75493d9c81fad2224400b4 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 02:09:48 +0200 Subject: [PATCH 16/19] Add another failing test --- .../staticanalysis/UnnecessaryCatchTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index 494ce92dfe..2c1fdc2f10 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -422,4 +422,29 @@ JarFile method(String name) { ) ); } + + @Test + void doNotRemoveCatchForWiderCatch() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + import java.io.FileNotFoundException; + + class Scratch { + void foo() throws FileNotFoundException {} // extends IOException + + void method(String name) { + try { + foo(); + } catch (IOException e) { + // Catches FileNotFoundException + } + } + } + """ + ) + ); + } } From 7f3dc947f430b9b14b35a80974b52306a2cbceb6 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 15:18:06 +0200 Subject: [PATCH 17/19] Update src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java Co-authored-by: Stefano Dalla Palma --- .../org/openrewrite/staticanalysis/UnnecessaryCatch.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index 2066fca79f..b420c46fd0 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -211,8 +211,12 @@ private Set getUnnecessaryTypes(J.Try aTry, Collection throw // For example, if IOException is thrown, don't remove catch for ZipException Set toKeep = new HashSet<>(); for (JavaType caughtException : unnecessaryExceptions) { + if (isGenericTypeRemovableByOption(caughtException)) { + continue; + } for (JavaType thrownException : thrownExceptions) { - if (TypeUtils.isAssignableTo(thrownException, caughtException)) { + if (TypeUtils.isAssignableTo(thrownException, caughtException) + || TypeUtils.isAssignableTo(caughtException, thrownException)) { toKeep.add(caughtException); break; } From 08496d1cfc7d3027b0f4311bcf67d759109507d9 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 15:23:40 +0200 Subject: [PATCH 18/19] Update src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../java/org/openrewrite/staticanalysis/UnnecessaryCatch.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java index b420c46fd0..434522d2dc 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnnecessaryCatch.java @@ -215,8 +215,8 @@ private Set getUnnecessaryTypes(J.Try aTry, Collection throw continue; } for (JavaType thrownException : thrownExceptions) { - if (TypeUtils.isAssignableTo(thrownException, caughtException) - || TypeUtils.isAssignableTo(caughtException, thrownException)) { + if (TypeUtils.isAssignableTo(thrownException, caughtException) || + TypeUtils.isAssignableTo(caughtException, thrownException)) { toKeep.add(caughtException); break; } From 78c5e98f5ee022ab8963ea7ff511c76151e71f71 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 17 Jul 2025 21:00:09 +0200 Subject: [PATCH 19/19] Add another test case seen --- .../staticanalysis/UnnecessaryCatchTest.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java index 2c1fdc2f10..b7322a1a4f 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnnecessaryCatchTest.java @@ -447,4 +447,28 @@ void method(String name) { ) ); } + + @Test + void doNotRemoveCatchOnMethodChain() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + import java.net.InetAddress; + + class Scratch { + void method(String name) { + String host; + try { + host = InetAddress.getLocalHost().getCanonicalHostName(); + } catch (IOException ignore) { + host = "ignored"; + } + } + } + """ + ) + ); + } }