From 80490f461d64be0c3b8be22b36447ef075e93a59 Mon Sep 17 00:00:00 2001 From: Niloyyy Date: Mon, 17 Aug 2026 05:12:35 +0600 Subject: [PATCH] fix: don't add break after a guarded early exit in FallThrough --- .../staticanalysis/FallThroughVisitor.java | 23 ++++ .../staticanalysis/FallThroughTest.java | 110 ++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java b/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java index ef4a61c8e..9b3a8a5d4 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/FallThroughVisitor.java @@ -156,11 +156,34 @@ private static boolean lastLineBreaksOrFallsThrough(List tr return trees.stream() .reduce((s1, s2) -> s2) // last statement .map(s -> breaks(s) || // https://github.com/openrewrite/rewrite-static-analysis/issues/173 + guardsWithEarlyExit(s) || // https://github.com/openrewrite/rewrite-static-analysis/issues/460 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); } + /** + * An {@code if} without an {@code else} whose then part completes abruptly guards the statements + * after it, so a fall-through past it is deliberate rather than an omission and adding a + * {@code break} would change behavior. Only blocks and labels are unwrapped; constructs such as + * {@code try} and {@code switch} are deliberately not, because they can complete normally even + * when every {@code if} they contain completes abruptly. + */ + private static boolean guardsWithEarlyExit(Statement s) { + if (s instanceof J.Block) { + List statements = ((J.Block) s).getStatements(); + return !statements.isEmpty() && guardsWithEarlyExit(statements.get(statements.size() - 1)); + } + if (s instanceof J.Label) { + return guardsWithEarlyExit(((J.Label) s).getStatement()); + } + if (s instanceof J.If) { + J.If iff = (J.If) s; + return iff.getElsePart() == null && breaks(iff.getThenPart()); + } + return false; + } + private static boolean breaks(Statement s) { if (s instanceof J.Block) { List statements = ((J.Block) s).getStatements(); diff --git a/src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java b/src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java index 9c8e6c713..575ae1d8e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FallThroughTest.java @@ -452,6 +452,116 @@ public void noCase(int i) { ); } + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/460") + @Test + void doNotAddBreakAfterGuardedEarlyExit() { + // Reduced from quarkus JavadocToAsciidocTransformer.appendEscapedAsciiDoc; the `]` case + // deliberately falls through to the shared escaping branch when not in inline macro mode. + rewriteRun( + //language=java + java( + """ + public class A { + public void escape(String text, boolean inlineMacroMode) { + StringBuilder sb = new StringBuilder(); + boolean escaping = false; + for (int i = 0; i < text.length(); i++) { + char ch = text.charAt(i); + switch (ch) { + case ']': + if (inlineMacroMode) { + sb.append("]"); + break; + } + case '#': + case '*': + if (!escaping) { + sb.append("++"); + escaping = true; + } + sb.append(ch); + break; + default: + sb.append(ch); + } + } + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/460") + @Test + void doNotAddBreakAfterGuardedEarlyExitInsideBlock() { + rewriteRun( + //language=java + java( + """ + public class A { + public void n(int i, boolean b) { + switch (i) { + case 0: { + if (b) { + System.out.println("zero"); + break; + } + } + case 1: + System.out.println("one"); + break; + } + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/460") + @Test + void addBreakWhenCodeFollowsGuardedEarlyExit() { + rewriteRun( + //language=java + java( + """ + public class A { + public void n(int i, boolean b) { + switch (i) { + case 0: + if (b) { + break; + } + System.out.println("zero"); + case 1: + System.out.println("one"); + break; + } + } + } + """, + """ + public class A { + public void n(int i, boolean b) { + switch (i) { + case 0: + if (b) { + break; + } + System.out.println("zero"); + break; + case 1: + System.out.println("one"); + break; + } + } + } + """ + ) + ); + } + @Test void addBreaksFallthroughCasesComprehensive() { rewriteRun(