diff --git a/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java b/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java index 45e6e674a8..4ae1966075 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java @@ -24,10 +24,7 @@ import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import java.util.*; import java.util.function.Predicate; import java.util.regex.Pattern; @@ -73,11 +70,11 @@ public AddBreak(J.Case scope) { public J.Case visitCase(J.Case case_, P p) { J.Case c = super.visitCase(case_, p); if (scope.isScope(c) && - c.getStatements().stream().noneMatch(J.Break.class::isInstance) && - c.getStatements().stream() - .reduce((s1, s2) -> s2) - .map(s -> !(s instanceof J.Block)) - .orElse(true)) { + c.getStatements().stream().noneMatch(J.Break.class::isInstance) && + c.getStatements().stream() + .reduce((s1, s2) -> s2) + .map(s -> !(s instanceof J.Block)) + .orElse(true)) { List statements = new ArrayList<>(c.getStatements()); J.Break breakToAdd = autoFormat( new J.Break(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null), @@ -93,11 +90,11 @@ public J.Case visitCase(J.Case case_, P p) { public J.Block visitBlock(J.Block block, P p) { J.Block b = super.visitBlock(block, p); if (getCursor().isScopeInPath(scope) && - b.getStatements().stream().noneMatch(J.Break.class::isInstance) && - b.getStatements().stream() - .reduce((s1, s2) -> s2) - .map(s -> !(s instanceof J.Block)) - .orElse(true)) { + b.getStatements().stream().noneMatch(J.Break.class::isInstance) && + b.getStatements().stream() + .reduce((s1, s2) -> s2) + .map(s -> !(s instanceof J.Block)) + .orElse(true)) { List statements = b.getStatements(); J.Break breakToAdd = autoFormat( new J.Break(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null), @@ -132,25 +129,53 @@ private static Set find(J.Switch enclosingSwitch, J.Case scope) { private static class FindLastLineBreaksOrFallsThroughCommentsVisitor extends JavaIsoVisitor> { private static final Predicate HAS_RELIEF_PATTERN_COMMENT = comment -> comment instanceof TextComment && - RELIEF_PATTERN.matcher(((TextComment) comment).getText()).find(); + RELIEF_PATTERN.matcher(((TextComment) comment).getText()).find(); private final J.Case scope; public FindLastLineBreaksOrFallsThroughCommentsVisitor(J.Case scope) { this.scope = scope; } - private static boolean lastLineBreaksOrFallsThrough(List trees) { + private static boolean lastLineBreaksOrFallsThrough(List trees) { return trees.stream() .reduce((s1, s2) -> s2) // last statement - .map(s -> s instanceof J.Return || - s instanceof J.Break || - s instanceof J.Continue || - s instanceof J.Throw || - s instanceof J.Switch || // https://github.com/openrewrite/rewrite-static-analysis/issues/173 - ((J) s).getComments().stream().anyMatch(HAS_RELIEF_PATTERN_COMMENT) + .map(s -> breaks(s) || // https://github.com/openrewrite/rewrite-static-analysis/issues/173 + s.getComments().stream().anyMatch(HAS_RELIEF_PATTERN_COMMENT) || + s instanceof J.Block && ((J.Block) s).getEnd().getComments().stream().anyMatch(HAS_RELIEF_PATTERN_COMMENT) ).orElse(false); } + private static boolean breaks(Statement s) { + if (s instanceof J.Block) { + List statements = ((J.Block) s).getStatements(); + return !statements.isEmpty() && breaks(statements.get(statements.size() - 1)); + } else if (s instanceof J.If) { + J.If iff = (J.If) s; + return iff.getElsePart() != null && breaks(iff.getThenPart()) && breaks(iff.getThenPart()); + } else if (s instanceof J.Label) { + return breaks(((J.Label) s).getStatement()); + } else if (s instanceof J.Try) { + J.Try try_ = (J.Try) s; + if (try_.getFinally() != null && breaks(try_.getFinally())) { + return true; + } + if (!breaks(try_.getBody())) { + return false; + } + for (J.Try.Catch c : try_.getCatches()) { + if (!breaks(c.getBody())) { + return false; + } + } + return true; + } + return s instanceof J.Return || + s instanceof J.Break || + s instanceof J.Continue || + s instanceof J.Throw || + s instanceof J.Switch; + } + @Override public J.Switch visitSwitch(J.Switch switch_, Set ctx) { J.Switch s = super.visitSwitch(switch_, ctx); @@ -185,24 +210,12 @@ public J.Switch visitSwitch(J.Switch switch_, Set ctx) { @Override public J.Case visitCase(J.Case case_, Set ctx) { - J.Case c = super.visitCase(case_, ctx); - if (c == scope) { - if (c.getStatements().isEmpty() || lastLineBreaksOrFallsThrough(c.getStatements())) { - ctx.add(c); - } - } - return c; - } - - @Override - public J.Block visitBlock(J.Block block, Set ctx) { - J.Block b = super.visitBlock(block, ctx); - if (getCursor().isScopeInPath(scope)) { - if (lastLineBreaksOrFallsThrough(b.getStatements()) || b.getEnd().getComments().stream().anyMatch(HAS_RELIEF_PATTERN_COMMENT)) { - ctx.add(b); + if (case_ == scope) { + if (case_.getStatements().isEmpty() || lastLineBreaksOrFallsThrough(case_.getStatements())) { + ctx.add(case_); } } - return b; + return case_; } } diff --git a/src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java b/src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java index 2ab6754593..1797ead16d 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java @@ -269,6 +269,46 @@ public void oneCase(int i) { ); } + @Test + void abortOnAbruptCompletion() { + rewriteRun( + //language=java + java( + """ + public class A { + public void noCase(int i) { + for (;;) { + switch (i) { + case 0: + if (true) + return; + else + break; + case 1: + if (true) + return; + else { + { + continue; + } + } + case 1: + try { + return; + } catch (Exception e) { + break; + } + default: + System.out.println("default"); + } + } + } + } + """ + ) + ); + } + @Test void addBreaksFallthroughCasesComprehensive() { rewriteRun(