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

run RemoveUnused #1922

Merged
merged 1 commit into from
Feb 4, 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
8 changes: 7 additions & 1 deletion .scalafix-scala2.conf
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
rules = [
ExplicitResultTypes,
OrganizeImports
OrganizeImports,
RemoveUnused
]

ExplicitResultTypes {
unsafeShortenNames = true
}

OrganizeImports {
groupedImports = Explode
expandRelative = true
Expand All @@ -17,3 +19,7 @@ OrganizeImports {
"*"
]
}

RemoveUnused {
imports = false
}
9 changes: 7 additions & 2 deletions .scalafix-scala3.conf
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
rules = [
OrganizeImports
OrganizeImports,
RemoveUnused
]

OrganizeImports {
groupedImports = Explode
expandRelative = true
removeUnused = false
removeUnused = true
groups = [
"re:javax?\\."
"scala."
"scala.meta."
"*"
]
}

RemoveUnused {
imports = false
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ object JGitDiff {
walk.dispose()
Right(treeParser)
} catch {
case missing: MissingObjectException =>
case _: MissingObjectException =>
unknown(id.getName)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package scalafix.internal.patch

import scala.annotation.tailrec
import scala.collection.compat._
Copy link
Collaborator Author

@bjaglin bjaglin Feb 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This false positive comes from the Scala3 run:

[warn] Failed to parse `-Wconf` configuration: origin=scala.collection.compat.*:s

Copy link
Collaborator Author

@bjaglin bjaglin Feb 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to either revisit #1797, revert it or stop running RemoveUnused on Scala 3 for projects that use scala.collection.compat.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up patching NoAutoTupling.scala & EscapeHatch.scala as they didn't make a critical usage of scala.collection.compat.

import scala.collection.immutable.TreeMap
import scala.collection.mutable
import scala.collection.mutable.ListBuffer
Expand Down Expand Up @@ -181,7 +180,7 @@ object EscapeHatch {
if (isEmpty) (true, None)
else {
val escapesUpToPos =
escapeTree.rangeTo(EscapeOffset(position)).valuesIterator.flatten
escapeTree.to(EscapeOffset(position)).valuesIterator.flatten
escapesUpToPos
.collectFirst {
case f @ EscapeFilter(_, _, _, Some(end))
Expand Down Expand Up @@ -328,7 +327,7 @@ object EscapeHatch {
if (disabling.isEmpty) (true, None)
else {
val disables =
disabling.rangeTo(EscapeOffset(position)).valuesIterator.flatten
disabling.to(EscapeOffset(position)).valuesIterator.flatten
loop(disables.toList, None)
}
}
Expand Down Expand Up @@ -485,7 +484,8 @@ object EscapeHatch {
}

val disable = TreeMap.newBuilder[EscapeOffset, List[EscapeFilter]]
val unused = ListBuffer.from(onOffTracker.allUnused)
val unused = ListBuffer.empty[Position]
unused ++= onOffTracker.allUnused

// for filters starting on the same offset, pick the one with the wider range
// and mark the others as unused
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ object ImportPatchOps {
.collect { case i: Import => i }
}

@tailrec private final def getLastTopLevelPkg(potPkg: Stat): Stat =
potPkg match {
case Pkg(_, head +: Nil) => getLastTopLevelPkg(head)
case Pkg(_, head +: _) => head
case _ => potPkg
}

@tailrec private final def getGlobalImports(ast: Tree): Seq[Import] =
ast match {
case Pkg(_, Seq(pkg: Pkg)) => getGlobalImports(pkg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ object PatchInternals {
)
case (_: Remove, add: Add) => add.copy(keepTok = false)
case (add: Add, _: Remove) => add.copy(keepTok = false)
case (rem: Remove, rem2: Remove) => rem
case (rem: Remove, _: Remove) => rem
case _ => throw Failure.TokenPatchMergeError(a, b)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ object ReplaceSymbolOps {
case (a @ Name(_), Symbol.Global(Symbol.None, SignatureName(b))) =>
ctx.replaceTree(a, b) -> Symbol.None
// ref is shorter
case (a @ Name(_), sym @ Symbol.Global(owner, SignatureName(b))) =>
case (a @ Name(_), sym @ Symbol.Global(_, SignatureName(b))) =>
if (isImport) {
val qual = SymbolOps.toTermRef(sym)
ctx.replaceTree(a, qual.syntax) -> Symbol.None
Expand Down Expand Up @@ -106,7 +106,7 @@ object ReplaceSymbolOps {
case Some(Importer(ref, `i` :: Nil)) =>
Patch.replaceTree(ref, SymbolOps.toTermRef(to.owner).syntax) +
Patch.replaceTree(name, to.signature.name)
case Some(Importer(ref, _)) =>
case Some(Importer(_, _)) =>
Patch.removeImportee(i) +
Patch.addGlobalImport(
Importer(
Expand All @@ -125,7 +125,7 @@ object ReplaceSymbolOps {
case Some(Identifier(parent, Symbol.Global(_, sig)))
if sig.name != parent.value =>
Patch.empty // do nothing because it was a renamed symbol
case Some(parent) =>
case Some(_) =>
val addImport =
if (n.isDefinition) Patch.empty
else ctx.addGlobalImport(to)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ object Pretty {
pretty(t.constant)
case t: MacroExpansionTree =>
`(` + Doc.text("`after-expansion` : ") + pretty(t.tpe) + `)`
case r: OriginalTree =>
case _: OriginalTree =>
`*`
case r: OriginalSubTree =>
Doc.text("orig") + `(` + Doc.text(r.tree.syntax) + `)`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ class PrettyType private (
toType(ret)
)
}
case s.ClassSignature(Some(tparams), parents, self, Some(decls)) =>
case s.ClassSignature(Some(tparams), parents, _, Some(decls)) =>
val declarations = decls.infos
val isCaseClass = info.is(p.CASE)
def objectDecls: List[Stat] =
Expand Down Expand Up @@ -568,7 +568,7 @@ class PrettyType private (
Term.This(Name.Anonymous()),
Type.Name(this.info(symbol).displayName)
)
case s.SuperType(prefix @ _, symbol) =>
case s.SuperType(_ @_, symbol) =>
// TODO: print prefix https://github.com/scalacenter/scalafix/issues/758
Type.Select(
Term.Super(Name.Anonymous(), Name.Anonymous()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ object SymbolOps {
}
def toImporter(symbol: Symbol): Option[Importer] = {
symbol match {
case Root(SignatureName(name)) =>
case Root(SignatureName(_)) =>
None
case Symbol.Global(qual, SignatureName(name)) =>
Some(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class LegacyCodePrinter(doc: SemanticDocument) {

private def pprint(signature: s.Signature): Unit = {
signature match {
case sig: s.ClassSignature =>
case _: s.ClassSignature =>
// Not supported
()

Expand All @@ -85,7 +85,7 @@ class LegacyCodePrinter(doc: SemanticDocument) {
}
pprint(sig.returnType)

case sig: s.TypeSignature =>
case _: s.TypeSignature =>
// Not supported
()

Expand Down
2 changes: 1 addition & 1 deletion scalafix-core/src/main/scala/scalafix/util/TreeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ object TreeOps {
}
}
val result = tree match {
case n: Name =>
case _: Name =>
tree.parent.flatMap(loop)
case _ => loop(tree)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import dotty.tools.repl.AbstractFileClassLoader
import metaconfig.ConfError
import metaconfig.Configured
import metaconfig.Input
import metaconfig.Position
class RuleCompiler(
classpath: String,
targetDirectory: Option[File] = None
Expand All @@ -43,8 +42,8 @@ class RuleCompiler(
val run: Run = compiler.newRun(using ctx)

val file: AbstractFile = input match {
case Input.File(path, _) => AbstractFile.getFile(input.path)
case Input.VirtualFile(path, _) =>
case Input.File(_, _) => AbstractFile.getFile(input.path)
case Input.VirtualFile(_, _) =>
VirtualFile(input.path, input.text.getBytes())
case _ => throw RuntimeException("Invalid Input file")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ object RuleInstrumentation {
}

(dialects.Scala213, code).parse[Source] match {
case parsers.Parsed.Error(pos, msg, details) =>
case parsers.Parsed.Error(pos, msg, _) =>
ConfError.parseError(pos.toMetaconfig, msg).notOk
case parsers.Parsed.Success(ast) =>
val result = List.newBuilder[String]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ class CompilerTypePrinter(g: ScalafixGlobal, config: ExplicitResultTypesConfig)(
context: Context,
tpe: Type
): Option[(Type, v1.Patch)] = tpe.finalResultType match {
case resultType @ RefinedType(parents, decls)
case RefinedType(_, decls)
if config.rewriteStructuralTypesToNamedSubclass &&
decls.filterNot(_.isOverridingSymbol).nonEmpty =>
val body: Option[m.Term] = defn match {
Expand All @@ -243,7 +243,7 @@ class CompilerTypePrinter(g: ScalafixGlobal, config: ExplicitResultTypesConfig)(
case _ => None
}
body match {
case Some(body @ m.Term.NewAnonymous(template))
case Some(body @ m.Term.NewAnonymous(_))
if body.tokens.head.syntax == "new" =>
val nameSyntax = gsym.nameSyntax
val suffixes = "" +: LazyList.from(1).map(_.toString())
Expand Down Expand Up @@ -303,7 +303,7 @@ class CompilerTypePrinter(g: ScalafixGlobal, config: ExplicitResultTypesConfig)(
case TypeRef(pre, sym, args)
if gsymbolReplacements.contains(semanticdbSymbol(sym)) =>
loop(TypeRef(pre, gsymbolReplacements(semanticdbSymbol(sym)), args))
case tp @ ThisType(sym)
case tp @ ThisType(_)
if tp.toString() == s"${gsym.owner.nameString}.this.type" =>
new PrettyType("this.type")
case ConstantType(Constant(c: Symbol)) if c.hasFlag(gf.JAVA_ENUM) =>
Expand Down Expand Up @@ -364,9 +364,9 @@ class CompilerTypePrinter(g: ScalafixGlobal, config: ExplicitResultTypesConfig)(
} else {
TypeRef(loop(pre), sym, args.map(loop))
}
case ExistentialType(head :: Nil, underlying) =>
case ExistentialType(head :: Nil, _) =>
head.info match {
case b @ TypeBounds(RefinedType(parents, _), hi)
case TypeBounds(RefinedType(parents, _), hi)
if parents.length > 1 =>
// Remove the lower bound large `Type[_ >: A with B with C <: D
// with Serializable]` so that it becomes only `Type[_ <: D]`.
Expand All @@ -376,7 +376,7 @@ class CompilerTypePrinter(g: ScalafixGlobal, config: ExplicitResultTypesConfig)(
case _ =>
}
tpe
case SingleType(ThisType(osym), sym)
case SingleType(ThisType(_), sym)
if sym.isKindaTheSameAs(sym) &&
gsymbolReplacements.contains(semanticdbSymbol(sym)) =>
loop(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ final class DisableSyntax(config: DisableSyntaxConfig)
t.pos
)
)
case t @ AbstractWithVals(vals) if config.noValInAbstract =>
case _ @AbstractWithVals(vals) if config.noValInAbstract =>
vals.map { v =>
Diagnostic(
"valInAbstract",
Expand All @@ -225,7 +225,7 @@ final class DisableSyntax(config: DisableSyntaxConfig)
t.pos
)
)
case t @ Defn.Def(mods, _, _, paramss, _, _)
case t @ Defn.Def(mods, _, _, _, _, _)
if mods.exists(_.is[Mod.Implicit]) &&
hasNonImplicitParam(t) &&
config.noImplicitConversion =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package scalafix.internal.rule

import scala.collection.compat._

import scala.meta._

import scalafix.v1._
Expand All @@ -21,7 +19,7 @@ class NoAutoTupling extends SemanticRule("NoAutoTupling") {

override def fix(implicit doc: SemanticDocument): Patch = {
val unitAdaptations: Set[Position] =
doc.diagnostics.iterator.collect {
doc.diagnostics.collect {
case message
if message.message.startsWith(
"Adaptation of argument list by inserting ()"
Expand All @@ -32,7 +30,7 @@ class NoAutoTupling extends SemanticRule("NoAutoTupling") {
}.toSet

val tupleAdaptations: Set[Position] =
doc.diagnostics.iterator.collect {
doc.diagnostics.collect {
case message
if message.message.startsWith(
"Adapting argument list by creating"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import scalafix.testkit._
object RuleSuite {
def main(args: Array[String]): Unit = {
if (Array("--save-expect").sameElements(args)) {
val suite = new AbstractSemanticRuleSuite(
new AbstractSemanticRuleSuite(
TestkitProperties.loadFromResources(),
isSaveExpect = true
) with AnyFunSuiteLike {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package scalafix.test

import scalafix.v1.SemanticRule
import scala.meta._

import scalafix.v1._

class ExplicitSynthetic() extends SemanticRule("ExplicitSynthetic") {
import scalafix.v1._
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that in Scala3, the SemanticRule reference above is found thanks to this, making the top-level import unused. To get a consistent behavior across Scala versions, I just hoisted them.

import scala.meta._

override def fix(implicit doc: SemanticDocument): Patch = {
val patches = doc.tree.collect {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// scalafix:off RemoveUnused
package test

import java.util.Map
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ trait BaseCliSuite extends AnyFunSuite with DiffAssertions {
files: String = removeImportsPath.toString()
): Unit = {
test(name, SkipWindows) {
val fileIsFixed = expectedExit.isOk
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused + no side effect -> line removed manually

val cwd = Files.createTempDirectory("scalafix")
val sourceDir =
inputSourceroot.toRelative(AbsolutePath(BuildInfo.baseDirectory)).toNIO
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class PrettyTypeFuzzSuite extends BasePrettyTypeSuite {
try {
checkPath(file)
} catch {
case scala.meta.internal.classpath.MissingSymbolException(e) =>
case scala.meta.internal.classpath.MissingSymbolException(_) =>
println(file)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class ScalafixImplSuite extends AnyFunSuite with DiffAssertions {
semicolon.toString,
excluded.toString
) ++ CompatSemanticdb.scalacOptions(src)
val compileSucceeded = scala.tools.nsc.Main.process(scalacOptions)
scala.tools.nsc.Main.process(scalacOptions)
val buf = List.newBuilder[ScalafixDiagnostic]
val callback = new ScalafixMainCallback {
override def reportDiagnostic(diagnostic: ScalafixDiagnostic): Unit = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ class ScalafixArgumentsSuite extends AnyFunSuite with DiffAssertions {
assert(maybeDiagnostic.isEmpty)
val content = FileIO.slurp(AbsolutePath(main), StandardCharsets.UTF_8)
assert(contentBeforeEvaluation == content)
val run = api
api
.withRules(
List(
removeUnsuedRule().name.toString(),
Expand Down