diff --git a/src/main/java/org/openrewrite/staticanalysis/OnlyCatchDeclaredExceptions.java b/src/main/java/org/openrewrite/staticanalysis/OnlyCatchDeclaredExceptions.java new file mode 100644 index 0000000000..512ae8f2eb --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/OnlyCatchDeclaredExceptions.java @@ -0,0 +1,138 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.jspecify.annotations.Nullable; +import org.openrewrite.*; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.JavaType.FullyQualified; +import org.openrewrite.java.tree.TypeUtils; + +import java.util.*; + +import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toList; + +public class OnlyCatchDeclaredExceptions extends Recipe { + + private static final String JAVA_LANG_EXCEPTION = "java.lang.Exception"; + + @Override + public String getDisplayName() { + return "Replace `catch(Exception)` with specific declared exceptions thrown in the try block"; + } + + @Override + public String getDescription() { + return "Replaces `catch(Exception e)` blocks with a multi-catch block " + + "(`catch (SpecificException1 | SpecificException2 e)`) containing only the exceptions declared " + + "thrown by method or constructor invocations within the `try` block that are not already caught " + + "by more specific `catch` clauses."; + } + + @Override + public Set getTags() { + return new HashSet<>(Arrays.asList("CWE-396", "RSPEC-S2221")); + } + + @Override + public TreeVisitor getVisitor() { + JavaIsoVisitor visitor = new JavaIsoVisitor() { + @Override + public J.Try visitTry(J.Try aTry, ExecutionContext ctx) { + J.Try t = super.visitTry(aTry, ctx); + return t.withCatches(ListUtils.map(t.getCatches(), c -> { + if (isGenericCatch(c)) { + // Find declared thrown exceptions that are not already specifically caught + Set declaredThrown = getDeclaredThrownExceptions(t); + declaredThrown.removeAll(getCaughtExceptions(t)); + if (!declaredThrown.isEmpty()) { + return multiCatchWithDeclaredExceptions(c, declaredThrown); + } + } + return c; + })); + } + + private boolean isGenericCatch(J.Try.Catch aCatch) { + FullyQualified fq = TypeUtils.asFullyQualified(aCatch.getParameter().getType()); + if (fq != null) { + String fqn = fq.getFullyQualifiedName(); + return TypeUtils.fullyQualifiedNamesAreEqual(JAVA_LANG_EXCEPTION, fqn); + } + return false; + } + + private Set getCaughtExceptions(J.Try aTry) { + Set caughtExceptions = new HashSet<>(); + for (J.Try.Catch c : aTry.getCatches()) { + JavaType type = c.getParameter().getType(); + if (type instanceof JavaType.MultiCatch) { + caughtExceptions.addAll(((JavaType.MultiCatch) type).getThrowableTypes()); + } else if (type != null) { + caughtExceptions.add(type); + } + } + return caughtExceptions; + } + + private Set getDeclaredThrownExceptions(J.Try aTry) { + return new JavaIsoVisitor>() { + @Override + public @Nullable JavaType visitType(@Nullable JavaType javaType, Set javaTypes) { + if (javaType instanceof JavaType.Method) { + javaTypes.addAll(((JavaType.Method) javaType).getThrownExceptions()); + } + return super.visitType(javaType, javaTypes); + } + }.reduce(aTry.getBody(), new HashSet<>()); + } + + private J.Try.Catch multiCatchWithDeclaredExceptions(J.Try.Catch aCatch, Set thrownExceptions) { + List fqs = thrownExceptions.stream() + .map(TypeUtils::asFullyQualified) + .filter(Objects::nonNull) + .sorted(Comparator.comparing(FullyQualified::getClassName)) + .collect(toList()); + String throwableTypes = fqs + .stream() + .map(FullyQualified::getClassName) + .collect(joining("|")); + String[] imports = fqs.stream().map(FullyQualified::getFullyQualifiedName).toArray(String[]::new); + for (String s : imports) { + maybeAddImport(s, false); + } + + J.Try surroundingTry = getCursor().firstEnclosing(J.Try.class); + assert surroundingTry != null; + + // Preserve the existing variable name from the original generic catch block + String variableName = aCatch.getParameter().getTree().getVariables().get(0).getSimpleName(); + J.Try generatedTry = JavaTemplate.builder(String.format("try {} catch (%s %s) {}", throwableTypes, variableName)) + .imports(imports) + .build() + .apply(new Cursor(getCursor(), surroundingTry), surroundingTry.getCoordinates().replace()); + return aCatch.withParameter(generatedTry.getCatches().get(0).getParameter()); + } + }; + return Preconditions.check(new UsesType<>(JAVA_LANG_EXCEPTION, false), visitor); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/OnlyCatchDeclaredExceptionsTest.java b/src/test/java/org/openrewrite/staticanalysis/OnlyCatchDeclaredExceptionsTest.java new file mode 100644 index 0000000000..71d764322c --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/OnlyCatchDeclaredExceptionsTest.java @@ -0,0 +1,456 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class OnlyCatchDeclaredExceptionsTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new OnlyCatchDeclaredExceptions()) + .parser(JavaParser.fromJavaVersion().dependsOn( + //language=java + """ + class MockThrowingClass { + + MockThrowingClass() { + } + + MockThrowingClass(boolean throwSQLException) throws java.sql.SQLException { + if (throwSQLException) { + throw new java.sql.SQLException("Test Constructor SQL Exception"); + } + } + + void throwsIOException() throws java.io.IOException { + throw new java.io.IOException("Test IO Exception"); + } + + void throwsSQLException() throws java.sql.SQLException { + throw new java.sql.SQLException("Test SQL Exception"); + } + + void throwsBothIOAndSQL() throws java.io.IOException, java.sql.SQLException { + throw new java.io.IOException("Test IO or SQL Exception"); + } + + void throwsNothing() { + // This method throws no checked exceptions + } + + void throwsDeclaredRuntimeException() throws RuntimeException { + throw new RuntimeException("Test Runtime Exception"); + } + + void throwsUndeclaredRuntimeException() { + throw new RuntimeException("Test Runtime Exception"); + } + } + """ + ) + ); + } + + @DocumentExample + @Test + void shouldReplaceExceptionWithSingleCheckedException() { + rewriteRun( + java( + """ + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsIOException(); + } catch (Exception e) { + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """, + """ + import java.io.IOException; + + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsIOException(); + } catch (IOException e) { + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldReplaceExceptionWithMultipleCheckedExceptions() { + rewriteRun( + java( + """ + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsIOException(); + new MockThrowingClass().throwsSQLException(); + } catch (Exception e) { + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """, + """ + import java.io.IOException; + import java.sql.SQLException; + + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsIOException(); + new MockThrowingClass().throwsSQLException(); + } catch (IOException | SQLException e) { + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldReplaceExceptionWithMultipleCheckedExceptionsFromSingleCall() { + rewriteRun( + java( + """ + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsBothIOAndSQL(); + } catch (Exception e) { + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """, + """ + import java.io.IOException; + import java.sql.SQLException; + + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsBothIOAndSQL(); + } catch (IOException | SQLException e) { + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldKeepSpecificCatchAndRemoveGenericWhenCovered() { + rewriteRun( + java( + """ + import java.io.IOException; + import java.sql.SQLException; + + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsBothIOAndSQL(); + } catch (IOException e) { + System.out.println("Caught IO: " + e.getMessage()); + } catch (Exception e) { + System.out.println("Caught generic: " + e.getMessage()); + } + } + } + """, + """ + import java.io.IOException; + import java.sql.SQLException; + + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsBothIOAndSQL(); + } catch (IOException e) { + System.out.println("Caught IO: " + e.getMessage()); + } catch (SQLException e) { + System.out.println("Caught generic: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldNotModifyWhenOnlySpecificCatchesExist() { + rewriteRun( + java( + """ + import java.io.IOException; + + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsIOException(); + } catch (IOException e) { + System.out.println("Caught IO: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldNotModifyWhenNoCheckedExceptionIsThrown() { + rewriteRun( + java( + """ + class MyService { + void doSomething() { + try { + new MockThrowingClass(); // Nothing thrown + } catch (Exception e) { + System.out.println("Caught something unusual: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldHandleConstructorThrowingException() { + rewriteRun( + java( + """ + class MyService { + void createObject() { + try { + new MockThrowingClass(true); // Throws SQLException + } catch (Exception e) { + System.out.println("Caught constructor exception: " + e.getMessage()); + } + } + } + """, + """ + import java.sql.SQLException; + + class MyService { + void createObject() { + try { + new MockThrowingClass(true); // Throws SQLException + } catch (SQLException e) { + System.out.println("Caught constructor exception: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldHandleConstructorThrowingSpecificExceptionAndAnotherGenericCatch() { + rewriteRun( + java( + """ + import java.io.IOException; + + class MyService { + void createAndHandle() { + try { + new MockThrowingClass(true); // Throws SQLException + new MockThrowingClass().throwsIOException(); + } catch (Exception e) { + System.out.println("Caught constructor exception: " + e.getMessage()); + } + } + } + """, + """ + import java.io.IOException; + import java.sql.SQLException; + + class MyService { + void createAndHandle() { + try { + new MockThrowingClass(true); // Throws SQLException + new MockThrowingClass().throwsIOException(); + } catch (IOException | SQLException e) { + System.out.println("Caught constructor exception: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldHandleConstructorThrowingExceptionWithExistingSpecificCatch() { + rewriteRun( + java( + """ + import java.io.IOException; + import java.sql.SQLException; + + class MyService { + void createAndHandle() { + try { + new MockThrowingClass(true); // Throws SQLException + new MockThrowingClass().throwsIOException(); + } catch (SQLException e) { + System.out.println("Caught sql exception: " + e.getMessage()); + } catch (Exception e) { + System.out.println("Caught generic: " + e.getMessage()); + } + } + } + """, + """ + import java.io.IOException; + import java.sql.SQLException; + + class MyService { + void createAndHandle() { + try { + new MockThrowingClass(true); // Throws SQLException + new MockThrowingClass().throwsIOException(); + } catch (SQLException e) { + System.out.println("Caught sql exception: " + e.getMessage()); + } catch (IOException e) { + System.out.println("Caught generic: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void shouldHandleExistingMultiCatch() { + rewriteRun( + java( + """ + import java.io.IOException; + + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsBothIOAndSQL(); + } catch (IOException | NullPointerException e) { + System.out.println("Multi exception: " + e.getMessage()); + } catch (Exception e) { + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """, + """ + import java.io.IOException; + import java.sql.SQLException; + + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsBothIOAndSQL(); + } catch (IOException | NullPointerException e) { + System.out.println("Multi exception: " + e.getMessage()); + } catch (SQLException e) { + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void declaredRuntimeException() { + rewriteRun( + java( + """ + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsDeclaredRuntimeException(); + } catch (Exception e) { + // This is a generic catch block that should be replaced + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """, + """ + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsDeclaredRuntimeException(); + } catch (RuntimeException e) { + // This is a generic catch block that should be replaced + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """ + ) + ); + } + + @Test + void undeclaredRuntimeException() { + rewriteRun( + java( + """ + class MyService { + void doSomething() { + try { + new MockThrowingClass().throwsUndeclaredRuntimeException(); + } catch (Exception e) { + // This is a generic catch block that should be replaced + System.out.println("Caught exception: " + e.getMessage()); + } + } + } + """ + ) + ); + } +}