Merge pull request #1978 from bjaglin/removeuunsed-block-rhs

RemoveUnused: guard Term.Block RHS in unused Defn

Author: bjaglin

scalafix-rules/src/main/scala/scalafix/internal/rule/RemoveUnused.scala

@@ -156,15 +156,15 @@ class RemoveUnused(config: RemoveUnusedConfig)
               Patch.removeImportee(i).atomic
           }.asPatch
         case i: Defn if isUnusedTerm(i.pos) =>
-          defnTokensToRemove(i).map(Patch.removeTokens).asPatch.atomic
+          defnPatch(i).asPatch.atomic
         case i @ Defn.Def(_, name, _, _, _, _) if isUnusedTerm(name.pos) =>
-          defnTokensToRemove(i).map(Patch.removeTokens).asPatch.atomic
+          defnPatch(i).asPatch.atomic
         case i @ Defn.Val(_, List(pat), _, _)
             if isUnusedTerm.exists(p => p.start == pat.pos.start) =>
-          defnTokensToRemove(i).map(Patch.removeTokens).asPatch.atomic
+          defnPatch(i).asPatch.atomic
         case i @ Defn.Var(_, List(pat), _, _)
             if isUnusedTerm.exists(p => p.start == pat.pos.start) =>
-          defnTokensToRemove(i).map(Patch.removeTokens).asPatch.atomic
+          defnPatch(i).asPatch.atomic
         case Term.Match(_, cases) =>
           patchPatVarsIn(cases)
         case Term.PartialFunction(cases) =>
@@ -197,11 +197,8 @@ class RemoveUnused(config: RemoveUnusedConfig)
   }
 
   // Given ("val x = 2", "2"), returns "val x = ".
-  private def leftTokens(t: Tree, right: Tree): Tokens = {
-    val startT = t.tokens.start
-    val startRight = right.tokens.start
-    t.tokens.take(startRight - startT)
-  }
+  private def leftTokens(t: Tree, right: Tree): Tokens =
+    t.tokens.dropRightWhile(_.start >= right.pos.start)
 
   private def defnTokensToRemove(defn: Defn): Option[Tokens] = defn match {
     case i @ Defn.Val(_, _, _, Lit(_)) => Some(i.tokens)
@@ -212,6 +209,35 @@ class RemoveUnused(config: RemoveUnusedConfig)
     case _ => None
   }
 
+  private def defnPatch(defn: Defn): Option[Patch] =
+    defnTokensToRemove(defn).map { tokens =>
+      val maybeRHSBlock = defn match {
+        case Defn.Val(_, _, _, x @ Term.Block(_)) => Some(x)
+        case Defn.Var(_, _, _, Some(x @ Term.Block(_))) => Some(x)
+        case _ => None
+      }
+
+      val maybeLocally = maybeRHSBlock.map { block =>
+        if (Some(block.pos.start) == block.stats.headOption.map(_.pos.start))
+          // only significant indentation blocks have their first stat
+          // aligned with the block itself (otherwise there is a heading "{")
+          "locally:"
+        else "locally"
+      }
+
+      maybeLocally match {
+        case Some(locally) =>
+          // Preserve comments between the LHS and the RHS, as well as
+          // newlines & whitespaces for significant indentation
+          val tokensNoTrailingTrivia = tokens.dropRightWhile(_.is[Trivia])
+
+          Patch.removeTokens(tokensNoTrailingTrivia) +
+            tokensNoTrailingTrivia.lastOption.map(Patch.addRight(_, locally))
+        case _ =>
+          Patch.removeTokens(tokens)
+      }
+    }
+
   private def posExclParens(tree: Tree): Position = {
     val leftTokenCount =
       tree.tokens.takeWhile(tk => tk.is[Token.LeftParen] || tk.is[Trivia]).size

scalafix-tests/input/src/main/scala-3/test/removeUnused/RemoveUnusedSignificantIndentation.scala

@@ -0,0 +1,26 @@
+/*
+rules = RemoveUnused
+*/
+package test.removeUnused
+
+object RemoveUnusedTermsSignificantIndentation:
+
+  private val a = // lost
+    println(5)
+
+  private var b1 =
+    println("foo")
+    1
+
+  private val b2 =
+    { println("foo") }
+    { 1 }
+
+  private
+  var
+  b3:
+  Integer
+  = // preserved
+    // preserved
+    println("foo")
+    1

scalafix-tests/input/src/main/scala/test/removeUnused/RemoveUnusedTerms.scala

@@ -7,7 +7,8 @@ object RemoveUnusedTerms {
 
   def foo = {
     val a = "unused"
-    val aa = println(5)
+    val aa = // lost
+      println(5)
     var b = 0
     var bb = println(0)
     println(1)
@@ -19,4 +20,20 @@ object RemoveUnusedTerms {
   val dd = 0
   def f(x: Int) = "unused"
   private def ff(x: Int) = "unused"
+
+  private val b1: Integer = { println("foo"); 1 }
+  private var b2: Integer = /* preserved */ {
+    println("foo")
+    1
+  }
+  private
+  var
+  b3:
+  Integer
+  = /* preserved */
+    // preserved
+    {
+      println("foo")
+      1
+    }
 }

scalafix-tests/output/src/main/scala-3/test/removeUnused/RemoveUnusedSignificantIndentation.scala

@@ -0,0 +1,18 @@
+package test.removeUnused
+
+object RemoveUnusedTermsSignificantIndentation:
+
+  println(5)
+
+  locally:
+    println("foo")
+    1
+
+  locally:
+    { println("foo") }
+    { 1 }
+
+  locally: // preserved
+    // preserved
+    println("foo")
+    1

scalafix-tests/output/src/main/scala/test/removeUnused/RemoveUnusedTerms.scala

@@ -16,4 +16,16 @@ object RemoveUnusedTerms {
   val dd = 0
   def f(x: Int) = "unused"
 
+
+  locally { println("foo"); 1 }
+  locally /* preserved */ {
+    println("foo")
+    1
+  }
+  locally /* preserved */
+    // preserved
+    {
+      println("foo")
+      1
+    }
 }