Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RemoveUnused: guard Term.Block RHS in unused Defn #1978

Merged
merged 2 commits into from
Apr 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}