Fix `UnnecessaryCatch` Recipe Multi-Catch Handling #638

stefanodallapalma · 2025-07-15T09:15:49Z

Fix UnnecessaryCatch Recipe Multi-Catch Handling

Problem

The UnnecessaryCatch recipe incorrectly removed entire multi-catch blocks. This resulted in broken code when necessary exception handling was lost.

Example of the bug:

    @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!");
                      }
                  }
              }
              """
          )
        );
    }

Original implementation returns:

diff --git a/AnExample.java b/AnExample.java
index 957cc0c..a391407 100644
--- a/AnExample.java
+++ b/AnExample.java
@@ -1,9 +1,7 @@ 
+import java.io.IOException;
+
 public class AnExample {
     public void method() {
-        try {
-            java.util.Base64.getDecoder().decode("abc".getBytes());
-        } catch (IllegalArgumentException | IllegalStateException e) {
-            System.out.println("an exception!");
-        }
+        java.util.Base64.getDecoder().decode("abc".getBytes());
     }
 }

Solution

The recipe now filters caught exception types in multi-catch blocks. When only some exceptions are unnecessary, it rewrites the catch clause to include only the necessary types. If all types become unnecessary, the entire catch block is removed.

Before:

import java.io.IOException;

public class AnExample {
    public void method() {
        try {
            java.util.Base64.getDecoder().decode("abc".getBytes()); // Can throw IllegalArgumentException
        } catch (IllegalArgumentException | IllegalStateException | IOException e) { // IOException is not thrown here
            System.out.println("an exception!");
        }
    }
}

After:

public class AnExample {
    public void method() {
        try {
            java.util.Base64.getDecoder().decode("abc".getBytes());
        } catch (IllegalArgumentException | IllegalStateException e) { // IOException is correctly removed
            System.out.println("an exception!");
        }
    }
}

Implementation Details

Key Changes

Precise Multi-Catch Manipulation: Instead of deleting entire multi-catch blocks, the recipe now filters exception types individually
New isCheckedException Logic: for enhanced detection of checked exceptions per JLS definition
New getUnnecessaryTypes Method: Combines checked exception status, thrown exceptions analysis, and configuration options

Review Focus Areas

isCheckedException logic: Verify accurate handling of checked exceptions
getUnnecessaryTypes filtering: Confirm correct combination of checked exception status, thrown exceptions set, and configuration options
Multi-catch rewriting: Ensure proper AST manipulation when partially removing exception types

Limitations

Static analysis only: Relies on method signatures; doesn't perform data flow analysis
Checked exceptions focus: Primary benefit is for checked exceptions declared in method signatures

Future Considerations

exceptionList parameter: Consider adding an optional exceptionList parameter. If provided, the recipe would only target exceptions within this specified list for removal. This could enhance its utility in composite recipes, allowing for more surgical cleanup (e.g., removing only IOException catches during an API migration) without affecting unrelated catch blocks. If this idea is well-received, I am happy to implement it in a future PR.
Utility method: What do you think of implementing a JavaTypeUtils.isCheckedException() utility method for reusability across recipes?

Checklist

Added unit tests for positive and negative cases
Applied recipe conventions and best practices
Used IntelliJ IDEA auto-formatter

@timtebeek

* 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 <[email protected]>

) This reverts commit ec88484.

Fixes openrewrite#561

Use this link to re-run the recipe: https://app.moderne.io/recipes/org.openrewrite.java.recipes.ExamplesExtractor?organizationId=ODQ2MGExMTUtNDg0My00N2EwLTgzMGMtNGE1NGExMTBmZDkw Co-authored-by: Moderne <[email protected]>

github-actions