RemoveUnusedLabels: preserve comments and Kotlin-referenced labels

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<String> 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<AtomicBoolean>() {
+                    @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());
+                // 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()))));
             }
         };
     }

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.,,

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,118 @@ void foo() {
           )
         );
     }
+
+    @Test
+    void doNotChangeKotlinLabelUsedByLabeledReturn() {
+        rewriteRun(
+          //language=kotlin
+          kotlin(
+            """
+              class A {
+                  fun foo(items: List<Int>) {
+                      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 name-based check keeps both rather than scope them
+        rewriteRun(
+          //language=kotlin
+          kotlin(
+            """
+              class A {
+                  fun foo(items: List<Int>) {
+                      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<Int>) {
+                      items.forEach lit@{
+                          println(it)
+                      }
+                  }
+              }
+              """,
+            """
+              class A {
+                  fun foo(items: List<Int>) {
+                      items.forEach {
+                          println(it)
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
 }