From 1a0fa4494166fc3bc5ecd97ae8e3ee045d3c5c84 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 10 Aug 2026 10:49:15 +0200 Subject: [PATCH 1/3] RemoveUnusedLabels: preserve comments and Kotlin-referenced labels Removing a label replaced the labeled statement's prefix with the label's prefix, so any comment sitting between the label and its statement was silently dropped, which a recipe must never do to source comments. The comments of the label prefix, of the space after the colon, and of the statement's own prefix are now concatenated in source order, so comments before the label, after the colon, and before the statement all survive. The used-label check only looked at J.Break and J.Continue, so a Kotlin label referenced through `return@label` or `this@label` counted as unused and was deleted, leaving source that no longer compiles. The scan now also covers K.Return and K.This, guarded by ReflectionUtils.isClassAvailable in the manner of KotlinFileChecker so the recipe keeps working on classpaths without rewrite-kotlin, where an unguarded instanceof would raise NoClassDefFoundError. The Kotlin check matches on label name alone. When a nested lambda reuses an enclosing label's name, the enclosing label is now kept even though only the inner one is referenced; that is a deliberate trade against modelling Kotlin label scoping. Java behaviour is unchanged. The recipe description and its recipes.csv row are updated to match. --- .../staticanalysis/RemoveUnusedLabels.java | 30 +- .../resources/META-INF/rewrite/recipes.csv | 2 +- .../RemoveUnusedLabelsTest.java | 302 ++++++++++++++++++ 3 files changed, 331 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java index c88828a26..c15ed2c06 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java @@ -16,11 +16,15 @@ package org.openrewrite.staticanalysis; import lombok.Getter; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.internal.ReflectionUtils; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.tree.J; +import org.openrewrite.kotlin.tree.K; import java.time.Duration; import java.util.Set; @@ -31,9 +35,12 @@ @Getter public class RemoveUnusedLabels extends Recipe { + private static final boolean IS_KOTLIN_AVAILABLE = ReflectionUtils.isClassAvailable("org.openrewrite.kotlin.tree.K"); + final String displayName = "Remove unused labels"; - final String description = "Remove labels that are not referenced by any `break` or `continue` statement."; + final String description = "Remove labels that are not referenced by any `break` or `continue` statement " + + "or by a Kotlin labeled `return` or `this` expression."; final Set tags = singleton("RSPEC-S1065"); @@ -48,6 +55,23 @@ public J visitLabel(J.Label label, ExecutionContext ctx) { String labelName = l.getLabel().getSimpleName(); boolean used = new JavaVisitor() { + @Override + public @Nullable J preVisit(J tree, AtomicBoolean u) { + // Kotlin also references labels through `return@label` and `this@label` + if (IS_KOTLIN_AVAILABLE) { + J.Identifier kotlinLabel = null; + if (tree instanceof K.Return) { + kotlinLabel = ((K.Return) tree).getLabel(); + } else if (tree instanceof K.This) { + kotlinLabel = ((K.This) tree).getLabel(); + } + if (kotlinLabel != null && labelName.equals(kotlinLabel.getSimpleName())) { + u.set(true); + } + } + return tree; + } + @Override public J visitBreak(J.Break breakStatement, AtomicBoolean u) { if (breakStatement.getLabel() != null && @@ -70,7 +94,9 @@ public J visitContinue(J.Continue continueStatement, AtomicBoolean u) { if (used) { return l; } - return l.getStatement().withPrefix(l.getPrefix()); + // The label is removed, so any comments attached to it move onto the statement it labeled + return l.getStatement().withPrefix(l.getPrefix().withComments(ListUtils.concatAll(l.getPrefix().getComments(), + ListUtils.concatAll(l.getPadding().getLabel().getAfter().getComments(), l.getStatement().getComments())))); } }; } diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index bdfbd9e95..41b561962 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -122,7 +122,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveUnneededAssertion,Remove unneeded assertions,"Remove unneeded assertions like `assert true`, `assertTrue(true)`, or `assertFalse(false)`.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveUnneededBlock,Remove unneeded block,"Flatten blocks into inline statements when possible. Unnecessary nested blocks add indentation and scope boundaries that obscure the control flow, often indicating code that should be extracted into its own method.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveUnreachableMultiCatchAlternative,Remove unreachable `catch` alternatives shadowed by earlier `catch` clauses,"When an earlier `catch` clause already covers a type, any later `catch` (including a multi-catch alternative) for the same type or a subtype is unreachable and is a Java compile error. This commonly appears after type-substitution migrations (for example, renaming an exception so that two `catch` clauses end up overlapping). This recipe drops the unreachable alternatives from later multi-catches, collapses a multi-catch to a regular `catch` when only one alternative remains, and removes the entire `catch` clause when all of its declared types are already covered.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, -maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveUnusedLabels,Remove unused labels,Remove labels that are not referenced by any `break` or `continue` statement.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveUnusedLabels,Remove unused labels,Remove labels that are not referenced by any `break` or `continue` statement or by a Kotlin labeled `return` or `this` expression.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveUnusedLocalVariables,Remove unused local variables,"If a local variable is declared but not used, it is dead code and should be removed. Unused variables increase cognitive load for readers who must determine whether the variable matters, and they may signal incomplete implementations or missed refactoring.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,"[{""name"":""ignoreVariablesNamed"",""type"":""String[]"",""displayName"":""Ignore matching variable names"",""description"":""An array of variable identifier names for local variables to ignore, even if the local variable is unused."",""example"":""[unused, notUsed, IGNORE_ME]""},{""name"":""withType"",""type"":""String"",""displayName"":""Only remove variables of a given type"",""description"":""A fully qualified class name. Only unused local variables whose type matches this will be removed. If empty or not set, all unused local variables are considered for removal."",""example"":""java.lang.String""},{""name"":""withSideEffects"",""type"":""Boolean"",""displayName"":""Remove unused local variables with side effects in initializer"",""description"":""Whether to remove unused local variables despite side effects in the initializer. Default false.""}]", maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveUnusedPrivateFields,Remove unused private fields,"If a private field is declared but not used in the program, it can be considered dead code and should therefore be removed. Dead fields clutter the class, increase its memory footprint, and can mislead developers into thinking they are part of the class's behavior.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.RemoveUnusedPrivateMethods,Remove unused private methods,`private` methods that are never executed are dead code and should be removed. Keeping unreachable methods around adds maintenance burden and can give a false impression of the class's capabilities.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java index 83f95319d..9dc689885 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java @@ -21,6 +21,7 @@ import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.kotlin.Assertions.kotlin; @SuppressWarnings({"UnusedLabel", "unused"}) class RemoveUnusedLabelsTest implements RewriteTest { @@ -156,6 +157,192 @@ void foo() { ); } + @Test + void preserveBlockCommentAfterLabel() { + rewriteRun( + //language=java + java( + """ + class A { + void foo() { + label: /* why this loop exists */ + while (true) { + break; + } + } + } + """, + """ + class A { + void foo() { + /* why this loop exists */ + while (true) { + break; + } + } + } + """ + ) + ); + } + + @Test + void preserveCommentsAroundLabel() { + rewriteRun( + //language=java + java( + """ + class A { + void foo() { + // before the label + label: /* after the colon */ // end of line + for (int i = 0; i < 10; i++) { + System.out.println(i); + } + } + } + """, + """ + class A { + void foo() { + // before the label + /* after the colon */ // end of line + for (int i = 0; i < 10; i++) { + System.out.println(i); + } + } + } + """ + ) + ); + } + + @Test + void preserveCommentBeforeLabelColon() { + rewriteRun( + //language=java + java( + """ + class A { + void foo() { + label /* an odd place */ : while (true) { + break; + } + } + } + """, + """ + class A { + void foo() { + /* an odd place */ while (true) { + break; + } + } + } + """ + ) + ); + } + + @Test + void preserveCommentsOnEveryLabeledStatementShape() { + rewriteRun( + //language=java + java( + """ + class A { + void foo(int i) { + block: /* a block */ { + System.out.println("hello"); + } + loop: /* a do while */ do { + System.out.println("hello"); + } while (true); + choice: /* a switch */ switch (i) { + default: + break; + } + statement: /* an expression */ System.out.println("hello"); + } + } + """, + """ + class A { + void foo(int i) { + /* a block */ { + System.out.println("hello"); + } + /* a do while */ do { + System.out.println("hello"); + } while (true); + /* a switch */ switch (i) { + default: + break; + } + /* an expression */ System.out.println("hello"); + } + } + """ + ) + ); + } + + @Test + void removeUnusedNestedLabelsKeepingComments() { + rewriteRun( + //language=java + java( + """ + class A { + void foo() { + outer: /* outer loop */ + for (int i = 0; i < 10; i++) { + inner: /* inner loop */ + for (int j = 0; j < 10; j++) { + System.out.println(j); + } + } + } + } + """, + """ + class A { + void foo() { + /* outer loop */ + for (int i = 0; i < 10; i++) { + /* inner loop */ + for (int j = 0; j < 10; j++) { + System.out.println(j); + } + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeUsedLabelWithComment() { + rewriteRun( + //language=java + java( + """ + class A { + void foo() { + outer: /* why this loop exists */ + for (int i = 0; i < 10; i++) { + for (int j = 0; j < 10; j++) { + if (j == 5) continue outer; + } + } + } + } + """ + ) + ); + } + @Test void unusedLabelOnBlock() { rewriteRun( @@ -182,4 +369,119 @@ void foo() { ) ); } + + @Test + void doNotChangeKotlinLabelUsedByLabeledReturn() { + rewriteRun( + //language=kotlin + kotlin( + """ + class A { + fun foo(items: List) { + items.forEach lit@{ + if (it == 0) return@lit + println(it) + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeKotlinLabelUsedByQualifiedThis() { + rewriteRun( + //language=kotlin + kotlin( + """ + class A { + fun render(): String { + val f = outer@ fun StringBuilder.(): Unit { + this@outer.append("x") + } + return StringBuilder().apply(f).toString() + } + } + """ + ) + ); + } + + @Test + void doNotChangeKotlinLabelWhenNestedLambdaLabelHasSameName() { + // `return@lit` binds to the inner lambda label, so the outer loop label is technically + // unused; the name-based check conservatively keeps both rather than reason about scoping + rewriteRun( + //language=kotlin + kotlin( + """ + class A { + fun foo(items: List) { + lit@ for (i in items) { + items.forEach lit@{ + if (it == 0) return@lit + println(it) + } + } + } + } + """ + ) + ); + } + + @Test + void removeUnusedKotlinLabel() { + rewriteRun( + //language=kotlin + kotlin( + """ + class A { + fun foo() { + unused@ while (true) { + break + } + } + } + """, + """ + class A { + fun foo() { + while (true) { + break + } + } + } + """ + ) + ); + } + + @Test + void removeUnusedKotlinLabelOnLambda() { + rewriteRun( + //language=kotlin + kotlin( + """ + class A { + fun foo(items: List) { + items.forEach lit@{ + println(it) + } + } + } + """, + """ + class A { + fun foo(items: List) { + items.forEach { + println(it) + } + } + } + """ + ) + ); + } } From 6dcbb25f4d1e07893685deb539aad782340f0eef Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 11 Aug 2026 11:08:47 +0200 Subject: [PATCH 2/3] Align continuation indent with the surrounding sources --- .../org/openrewrite/staticanalysis/RemoveUnusedLabels.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java index c15ed2c06..ce55a08f6 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java @@ -40,7 +40,7 @@ public class RemoveUnusedLabels extends Recipe { final String displayName = "Remove unused labels"; final String description = "Remove labels that are not referenced by any `break` or `continue` statement " + - "or by a Kotlin labeled `return` or `this` expression."; + "or by a Kotlin labeled `return` or `this` expression."; final Set tags = singleton("RSPEC-S1065"); @@ -96,7 +96,7 @@ public J visitContinue(J.Continue continueStatement, AtomicBoolean u) { } // The label is removed, so any comments attached to it move onto the statement it labeled return l.getStatement().withPrefix(l.getPrefix().withComments(ListUtils.concatAll(l.getPrefix().getComments(), - ListUtils.concatAll(l.getPadding().getLabel().getAfter().getComments(), l.getStatement().getComments())))); + ListUtils.concatAll(l.getPadding().getLabel().getAfter().getComments(), l.getStatement().getComments())))); } }; } From e0647386c5f6d57597a568d259501aeefe459313 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Aug 2026 00:27:54 +0200 Subject: [PATCH 3/3] Trim commentary --- .../org/openrewrite/staticanalysis/RemoveUnusedLabels.java | 2 +- .../org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java index ce55a08f6..86f35a202 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLabels.java @@ -94,7 +94,7 @@ public J visitContinue(J.Continue continueStatement, AtomicBoolean u) { if (used) { return l; } - // The label is removed, so any comments attached to it move onto the statement it labeled + // Comments attached to the removed label move onto the statement it labeled return l.getStatement().withPrefix(l.getPrefix().withComments(ListUtils.concatAll(l.getPrefix().getComments(), ListUtils.concatAll(l.getPadding().getLabel().getAfter().getComments(), l.getStatement().getComments())))); } diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java index 9dc689885..6d439a87b 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLabelsTest.java @@ -410,8 +410,7 @@ fun render(): String { @Test void doNotChangeKotlinLabelWhenNestedLambdaLabelHasSameName() { - // `return@lit` binds to the inner lambda label, so the outer loop label is technically - // unused; the name-based check conservatively keeps both rather than reason about scoping + // `return@lit` binds to the inner lambda label, so the name-based check keeps both rather than scope them rewriteRun( //language=kotlin kotlin(