diff --git a/src/main/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocks.java b/src/main/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocks.java index ff9decf822..9406b7d5ef 100644 --- a/src/main/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocks.java +++ b/src/main/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocks.java @@ -75,7 +75,7 @@ public J visitTry(J.Try tryable, ExecutionContext ctx) { if (!semanticallyEqualCatchesMap.isEmpty()) { // Collect the identifiers of caught exceptions that are subtypes or implementations of an exception that is caught later in a different catch. - Map>> parentChildClassRelationship = new HashMap<>(); + Map>> parentChildClassRelationship = new HashMap<>(); for (int i = 0; i < catches.size(); i++) { J.Try.Catch from = catches.get(i); for (int j = i + 1; j < catches.size(); j++) { @@ -86,10 +86,10 @@ public J visitTry(J.Try tryable, ExecutionContext ctx) { JavaType fromType = TypeUtils.asFullyQualified(fromException.getType()); JavaType toType = TypeUtils.asFullyQualified(toException.getType()); if (fromType != null && toType != null && TypeUtils.isAssignableTo(toType, fromType)) { - Map> subTypesMap = parentChildClassRelationship.computeIfAbsent(from, key -> new HashMap<>()); - Set childClassIdentifiers = subTypesMap.computeIfAbsent(to, key -> new HashSet<>()); - if (fromException instanceof J.Identifier) { - childClassIdentifiers.add((J.Identifier) fromException); + Map> subTypesMap = parentChildClassRelationship.computeIfAbsent(from, key -> new HashMap<>()); + Set childClassIdentifiers = subTypesMap.computeIfAbsent(to, key -> new HashSet<>()); + if (fromException instanceof J.Identifier || fromException instanceof J.FieldAccess) { + childClassIdentifiers.add(fromException); } } } @@ -171,11 +171,11 @@ static class RemoveCatches extends JavaVisitor { private static class CombineCatches extends JavaVisitor { private final J.Try.Catch scope; private final List equivalentCatches; - private final Map> childClassesToExclude; + private final Map> childClassesToExclude; CombineCatches(J.Try.Catch scope, List equivalentCatches, - Map> childClassesToExclude) { + Map> childClassesToExclude) { this.scope = scope; this.equivalentCatches = equivalentCatches; this.childClassesToExclude = childClassesToExclude; @@ -211,24 +211,24 @@ public J visitCatch(J.Try.Catch _catch, ExecutionContext ctx) { } private List> combineEquivalentCatches() { - Set removeIdentifiers = new HashSet<>(); + Set removeExceptions = new HashSet<>(); List> combinedCatches = new ArrayList<>(); for (J.Try.Catch equivalentCatch : equivalentCatches) { - Set childClasses = childClassesToExclude.get(equivalentCatch); + Set childClasses = childClassesToExclude.get(equivalentCatch); if (childClasses != null) { // Remove child classes that will be unnecessary since the parent exists in the new multi-catch. - removeIdentifiers.addAll(childClasses); + removeExceptions.addAll(childClasses); } TypeTree typeExpr = equivalentCatch.getParameter().getTree().getTypeExpression(); if (typeExpr instanceof J.MultiCatch) { for (JRightPadded alternative : ((J.MultiCatch) typeExpr).getPadding().getAlternatives()) { NameTree name = alternative.getElement(); + if (removeExceptions.contains(name)) { + continue; // Skip redundant subtype + } if (name instanceof J.Identifier) { - if (removeIdentifiers.contains((J.Identifier) name)) { - continue; // Skip redundant subtype - } combinedCatches.add(alternative.withElement(((J.Identifier) name).withPrefix(EMPTY))); } else if (name instanceof J.FieldAccess) { combinedCatches.add(alternative.withElement(((J.FieldAccess) name).withPrefix(EMPTY))); @@ -238,11 +238,14 @@ private List> combineEquivalentCatches() { } } else if (typeExpr instanceof J.Identifier) { J.Identifier identifier = (J.Identifier) typeExpr; - if (!removeIdentifiers.contains(identifier)) { + if (!removeExceptions.contains(identifier)) { combinedCatches.add(JRightPadded.build(identifier.withPrefix(EMPTY))); } } else if (typeExpr instanceof J.FieldAccess) { - combinedCatches.add(JRightPadded.build(((J.FieldAccess) typeExpr).withPrefix(EMPTY))); + J.FieldAccess fieldAccess = (J.FieldAccess) typeExpr; + if (!removeExceptions.contains(fieldAccess)) { + combinedCatches.add(JRightPadded.build(fieldAccess.withPrefix(EMPTY))); + } } } @@ -251,17 +254,21 @@ private List> combineEquivalentCatches() { List> alternatives = ((J.MultiCatch) scopeExpr).getPadding().getAlternatives(); for (int i = alternatives.size() - 1; i >= 0; i--) { NameTree name = alternatives.get(i).getElement(); - if (name instanceof J.Identifier && !removeIdentifiers.contains(name)) { - combinedCatches.add(0, alternatives.get(i).withElement(((J.Identifier) name).withPrefix(EMPTY))); - } else if (!(name instanceof J.Identifier)) { - combinedCatches.add(0, alternatives.get(i)); + if (!removeExceptions.contains(name)) { + if (name instanceof J.Identifier) { + combinedCatches.add(0, alternatives.get(i).withElement(((J.Identifier) name).withPrefix(EMPTY))); + } else { + combinedCatches.add(0, alternatives.get(i)); + } } } } else { - if (scopeExpr instanceof J.Identifier && !removeIdentifiers.contains(scopeExpr)) { - combinedCatches.add(0, JRightPadded.build(((J.Identifier) scopeExpr).withPrefix(EMPTY))); - } else if (scopeExpr instanceof J.FieldAccess) { - combinedCatches.add(0, JRightPadded.build(((J.FieldAccess) scopeExpr).withPrefix(EMPTY))); + if (!removeExceptions.contains(scopeExpr)) { + if (scopeExpr instanceof J.Identifier) { + combinedCatches.add(0, JRightPadded.build(((J.Identifier) scopeExpr).withPrefix(EMPTY))); + } else if (scopeExpr instanceof J.FieldAccess) { + combinedCatches.add(0, JRightPadded.build(((J.FieldAccess) scopeExpr).withPrefix(EMPTY))); + } } } diff --git a/src/test/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocksTest.java b/src/test/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocksTest.java index e747769c8c..cc0adb81c2 100644 --- a/src/test/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocksTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CombineSemanticallyEqualCatchBlocksTest.java @@ -528,4 +528,41 @@ void method() { ) ); } + + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/771") + @Test + void removeRedundantInnerExceptionWhenParentIsCaught() { + rewriteRun( + //language=java + java( + """ + class Outer { + public static class InnerException extends Exception {} + } + """ + ), + //language=java + java( + """ + class Test { + void method() { + try { + } catch (Outer.InnerException e) { + } catch (Exception e) { + } + } + } + """, + """ + class Test { + void method() { + try { + } catch (Exception e) { + } + } + } + """ + ) + ); + } }