From 4c30d52c30497783932617372bad08631d2671cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 27 May 2017 16:25:26 +0200 Subject: [PATCH] Implement RemoveUnusedImports rewrite. This rewrite does not group/sort/expand rewrites, only remove unused imports. --- bin/new-rewrite.sh | 7 ++ build.sbt | 5 -- .../scala/scalafix/cli/ScalafixOptions.scala | 3 + .../scala/scalafix/patch/ImportPatchOps.scala | 73 +++++++++++++---- .../src/main/scala/scalafix/patch/Patch.scala | 7 +- .../rewrite/RemoveUnusedImports.scala | 28 +++++++ .../scala/scalafix/rewrite/RewriteCtx.scala | 7 ++ .../scalafix/rewrite/ScalafixRewrites.scala | 2 + .../main/scala/scalafix/syntax/package.scala | 13 +--- .../scala/scalafix/util/MatchingParens.scala | 78 +++++++++++++++++++ .../scala/scalafix/util/TokenClasses.scala | 11 ++- .../main/scala/scalafix/util/TokenList.scala | 5 +- .../main/scala/scalafix/testkit/Command.scala | 6 +- .../scalafix/testkit/DiffAssertions.scala | 3 +- .../main/scala/test/RemoveUnusedImports.scala | 20 +++++ .../scala/scalafix/tests/ProjectTests.scala | 14 +++- .../main/scala/test/RemoveUnusedImports.scala | 14 ++++ 17 files changed, 251 insertions(+), 45 deletions(-) create mode 100755 bin/new-rewrite.sh create mode 100644 scalafix-core/src/main/scala/scalafix/rewrite/RemoveUnusedImports.scala create mode 100644 scalafix-core/src/main/scala/scalafix/util/MatchingParens.scala create mode 100644 scalafix-tests/input/src/main/scala/test/RemoveUnusedImports.scala create mode 100644 scalafix-tests/output/src/main/scala/test/RemoveUnusedImports.scala diff --git a/bin/new-rewrite.sh b/bin/new-rewrite.sh new file mode 100755 index 000000000..4d6e45c97 --- /dev/null +++ b/bin/new-rewrite.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash +set -eux +rewrite=$1 + +touch scalafix-core/src/main/scala/scalafix/rewrite/${rewrite}.scala +touch scalafix-tests/input/src/main/scala/test/${rewrite}.scala +touch scalafix-tests/output/src/main/scala/test/${rewrite}.scala diff --git a/build.sbt b/build.sbt index 96aa2635b..03ef7e61b 100644 --- a/build.sbt +++ b/build.sbt @@ -170,11 +170,6 @@ lazy val cli = project testkit % Test ) -lazy val publishedArtifacts = Seq( - publishLocal in core, - publishLocal in cli -) - lazy val `scalafix-sbt` = project .configs(IntegrationTest) .settings( diff --git a/scalafix-cli/src/main/scala/scalafix/cli/ScalafixOptions.scala b/scalafix-cli/src/main/scala/scalafix/cli/ScalafixOptions.scala index 775c87a71..e4f667a25 100644 --- a/scalafix-cli/src/main/scala/scalafix/cli/ScalafixOptions.scala +++ b/scalafix-cli/src/main/scala/scalafix/cli/ScalafixOptions.scala @@ -7,6 +7,7 @@ import java.io.PrintStream import scala.meta._ import scala.meta.io.AbsolutePath import scalafix.rewrite.ProcedureSyntax +import scalafix.rewrite.ScalafixRewrites import caseapp._ import metaconfig.Conf import metaconfig.ConfError @@ -74,6 +75,8 @@ case class ScalafixOptions( | scala:full.Name OR | https://gist.com/.../Rewrite.scala""".stripMargin ) + @HelpMessage( + s"Space separated list of rewrites to run. Available options include ${ScalafixRewrites.semanticNames}") rewrites: List[String] = Nil, @HelpMessage( "Files to fix. Runs on all *.scala files if given a directory") diff --git a/scalafix-core/src/main/scala/scalafix/patch/ImportPatchOps.scala b/scalafix-core/src/main/scala/scalafix/patch/ImportPatchOps.scala index 96047c361..674e1ed9f 100644 --- a/scalafix-core/src/main/scala/scalafix/patch/ImportPatchOps.scala +++ b/scalafix-core/src/main/scala/scalafix/patch/ImportPatchOps.scala @@ -2,14 +2,14 @@ package scalafix.patch import scala.collection.immutable.Seq import scala.collection.mutable -import scalafix.syntax._ import scala.meta._ import scala.meta.tokens.Token.Comment import scala.meta.tokens.Token.KwImport -import scalafix.Failure import scalafix.config.FilterMatcher import scalafix.patch.TreePatch.ImportPatch import scalafix.rewrite.RewriteCtx +import scalafix.syntax._ +import scalafix.util.Newline import scalafix.util.Whitespace import org.scalameta.logger @@ -21,7 +21,7 @@ object ImportPatchOps { ctx: RewriteCtx, importPatches: Seq[ImportPatch])( implicit mirror: Mirror): Iterable[Patch] = { - val allImports = getGlobalImports(ctx.tree) + val allImports = ctx.tree.collect { case i: Import => i } val allImporters = allImports.flatMap(_.importers) val allImportees = allImporters.flatMap(_.importees) val allImporteeSymbols = allImportees.flatMap { importee => @@ -48,24 +48,71 @@ object ImportPatchOps { ctx.addLeft(editToken, s"import $importer\n") } val isRemovedImporter = - allImporters.filter(_.importees.forall(isRemovedImportee)).toSet + allImporters.toIterator + .filter(_.importees.forall(isRemovedImportee)) + .toSet + val curlyBraceRemoves = allImporters.map { importer => + val keptImportees = importer.importees.filterNot(isRemovedImportee) + keptImportees match { + case (Importee.Wildcard() | Importee.Name(_)) +: Nil => + ctx + .toks(importer) + .collectFirst { + case open @ Token.LeftBrace() => + ctx.matching + .close(open) + .map(close => + ctx.removeToken(open) + + ctx.removeToken(close)) + .asPatch + } + .asPatch + case _ => Patch.empty + } + } val isRemovedImport = allImports.filter(_.importers.forall(isRemovedImporter)) def remove(toRemove: Tree) = { val tokens = ctx.toks(toRemove) def removeFirstComma(lst: Iterable[Token]) = - lst.find(!_.is[Whitespace]) match { - case Some(tok @ Token.Comma()) => TokenPatch.Remove(tok) - case _ => Patch.empty - } - val trailingComma = - removeFirstComma(ctx.tokenList.from(tokens.last)) + lst + .takeWhile { + case Token.Space() => true + case Token.Comma() => true + case _ => false + } + .map(ctx.removeToken(_)) val leadingComma = - removeFirstComma(ctx.tokenList.to(tokens.head).reverse) - trailingComma + leadingComma + PatchOps.removeTokens(tokens) + removeFirstComma(ctx.tokenList.to(tokens.head)) + val hadLeadingComma = leadingComma.exists { + case TokenPatch.Add(_: Token.Comma, _, _, keepTok @ false) => true + case _ => false + } + val trailingComma = + if (hadLeadingComma) List(Patch.empty) + else removeFirstComma(ctx.tokenList.from(tokens.last)) + PatchOps.removeTokens(tokens) ++ trailingComma ++ leadingComma + } + + val leadingNewlines = isRemovedImport.map { i => + var newline = false + ctx.tokenList + .to(ctx.toks(i).head) + .takeWhile(x => + !newline && { + x.is[Token.Space] || { + val isNewline = x.is[Newline] + if (isNewline) newline = true + isNewline + } + }) + .map(tok => ctx.removeToken(tok)) + .asPatch } - extraPatches ++ + leadingNewlines ++ + curlyBraceRemoves ++ + extraPatches ++ (isRemovedImportee ++ isRemovedImporter ++ isRemovedImport).map(remove) diff --git a/scalafix-core/src/main/scala/scalafix/patch/Patch.scala b/scalafix-core/src/main/scala/scalafix/patch/Patch.scala index 7b1a870de..d13134b0a 100644 --- a/scalafix-core/src/main/scala/scalafix/patch/Patch.scala +++ b/scalafix-core/src/main/scala/scalafix/patch/Patch.scala @@ -15,6 +15,7 @@ import scalafix.patch.TokenPatch.Remove import scalafix.patch.TreePatch.ImportPatch import scalafix.patch.TreePatch.RenamePatch import scalafix.patch.TreePatch.Replace +import scalafix.util.TokenOps import difflib.DiffUtils import org.scalameta.logger @@ -42,7 +43,7 @@ sealed abstract class Patch { else if (isEmpty) other else if (other.isEmpty) this else Concat(this, other) - def ++(other: Seq[Patch]): Patch = other.foldLeft(this)(_ + _) + def ++(other: Iterable[Patch]): Patch = other.foldLeft(this)(_ + _) def isEmpty: Boolean = this == EmptyPatch def nonEmpty: Boolean = !isEmpty } @@ -146,10 +147,10 @@ object Patch { private def syntaxApply(ctx: RewriteCtx, patches: Iterable[TokenPatch]): String = { val patchMap = patches - .groupBy(_.tok.hash) + .groupBy(x => TokenOps.hash(x.tok)) .mapValues(_.reduce(merge).newTok) ctx.tokens.toIterator - .map(tok => patchMap.getOrElse(tok.hash, tok.syntax)) + .map(tok => patchMap.getOrElse(TokenOps.hash(tok), tok.syntax)) .mkString } diff --git a/scalafix-core/src/main/scala/scalafix/rewrite/RemoveUnusedImports.scala b/scalafix-core/src/main/scala/scalafix/rewrite/RemoveUnusedImports.scala new file mode 100644 index 000000000..114169335 --- /dev/null +++ b/scalafix-core/src/main/scala/scalafix/rewrite/RemoveUnusedImports.scala @@ -0,0 +1,28 @@ +package scalafix +package rewrite + +import scala.meta._ +import scalafix.syntax._ +import org.scalameta.logger + +case class RemoveUnusedImports(mirror: Mirror) + extends SemanticRewrite(mirror) { + private val unusedImports = mirror.database.messages.toIterator.collect { + case Message(pos, _, "Unused import") => + pos + }.toSet + private def isUnused(importee: Importee) = importee match { + // NOTE: workaround for https://github.com/scalameta/scalameta/issues/899 + case Importee.Wildcard() => + val lookupPos = + importee.parents + .collectFirst { case x: Import => x.pos } + .getOrElse(importee.pos) + unusedImports.contains(lookupPos) + case _ => unusedImports.contains(importee.pos) + } + override def rewrite(ctx: RewriteCtx): Patch = + ctx.tree.collect { + case i: Importee if isUnused(i) => ctx.removeImportee(i) + }.asPatch +} diff --git a/scalafix-core/src/main/scala/scalafix/rewrite/RewriteCtx.scala b/scalafix-core/src/main/scala/scalafix/rewrite/RewriteCtx.scala index 480ec7143..deb57c73e 100644 --- a/scalafix-core/src/main/scala/scalafix/rewrite/RewriteCtx.scala +++ b/scalafix-core/src/main/scala/scalafix/rewrite/RewriteCtx.scala @@ -8,13 +8,20 @@ import scala.meta.tokens.Tokens import scalafix.syntax._ import scalafix.config.ScalafixConfig import scalafix.config.ScalafixReporter +import scalafix.util.MatchingParens import scalafix.util.TokenList +import org.scalameta.logger /** Bundle of useful things when implementing [[Rewrite]]. */ case class RewriteCtx(tree: Tree, config: ScalafixConfig) { + def syntax = + s"""${tree.input.syntax} + |${logger.revealWhitespace(tree.syntax.take(100))}""".stripMargin + override def toString: String = syntax def toks(t: Tree): Tokens = t.tokens(config.dialect) implicit lazy val tokens: Tokens = tree.tokens(config.dialect) lazy val tokenList: TokenList = new TokenList(tokens) + lazy val matching: MatchingParens = MatchingParens(tokens) lazy val comments: AssociatedComments = AssociatedComments(tokens) val reporter: ScalafixReporter = config.reporter } diff --git a/scalafix-core/src/main/scala/scalafix/rewrite/ScalafixRewrites.scala b/scalafix-core/src/main/scala/scalafix/rewrite/ScalafixRewrites.scala index b9a48f181..dc7d02dd3 100644 --- a/scalafix-core/src/main/scala/scalafix/rewrite/ScalafixRewrites.scala +++ b/scalafix-core/src/main/scala/scalafix/rewrite/ScalafixRewrites.scala @@ -12,6 +12,7 @@ object ScalafixRewrites { ) def semantic(mirror: Mirror): List[Rewrite] = List( ExplicitReturnTypes(mirror), + RemoveUnusedImports(mirror), Xor2Either(mirror), NoAutoTupling(mirror) ) @@ -25,4 +26,5 @@ object ScalafixRewrites { syntax.map(x => x.name -> x).toMap val emptyDatabase = Database(Nil) lazy val semanticNames: List[String] = semantic(emptyDatabase).map(_.name) + def allNames: List[String] = syntaxName2rewrite.keys.toList ++ semanticNames } diff --git a/scalafix-core/src/main/scala/scalafix/syntax/package.scala b/scalafix-core/src/main/scala/scalafix/syntax/package.scala index 958a90d5e..5f42c5489 100644 --- a/scalafix-core/src/main/scala/scalafix/syntax/package.scala +++ b/scalafix-core/src/main/scala/scalafix/syntax/package.scala @@ -19,6 +19,7 @@ import scala.meta.internal.prettyprinters.TreeSyntax import scala.meta.internal.prettyprinters.TreeToString import scala.meta.internal.scalafix.ScalafixScalametaHacks import scalafix.patch.TokenPatch +import scalafix.util.TreeOps import scalafix.util.Whitespace package object syntax { @@ -87,17 +88,6 @@ package object syntax { } } - implicit class XtensionToken(token: Token) { - // copy pasted from: - // https://github.com/scalameta/scalafmt/blob/ba319e0aaee6d4e1b30098ad57940c67910c33e9/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TokenOps.scala - private[scalafix] def hash: Long = { - val longHash: Long = - (token.productPrefix.hashCode.toLong << (62 - 8)) | - (token.start.toLong << (62 - (8 + 28))) | token.end - longHash - } - } - implicit class XtensionSymbol(symbol: Symbol) { def underlyingSymbols: Seq[Symbol] = symbol match { case Symbol.Multi(symbols) => symbols @@ -146,6 +136,7 @@ package object syntax { def trimSugar: String = str.trim.replaceAllLiterally(".this", "") } implicit class XtensionTreeScalafix(tree: Tree) { + def parents: Stream[Tree] = TreeOps.parents(tree) def input: Input = tree.tokens.head.input def treeSyntax: String = ScalafixScalametaHacks.resetOrigin(tree).syntax } diff --git a/scalafix-core/src/main/scala/scalafix/util/MatchingParens.scala b/scalafix-core/src/main/scala/scalafix/util/MatchingParens.scala new file mode 100644 index 000000000..ec08201b0 --- /dev/null +++ b/scalafix-core/src/main/scala/scalafix/util/MatchingParens.scala @@ -0,0 +1,78 @@ +package scalafix.util + +import scala.meta._ +import scalafix.util.TokenOps._ +import tokens.Token._ + +sealed abstract class MatchingParens(map: Map[TokenHash, Token]) { + private def lookup(token: Token) = map.get(hash(token)) + def close(open: Token.LeftParen): Option[Token.RightParen] = + lookup(open).collect { case x: Token.RightParen => x } + def close(open: Token.LeftBracket): Option[Token.RightBracket] = + lookup(open).collect { case x: Token.RightBracket => x } + def close(open: Token.LeftBrace): Option[Token.RightBrace] = + lookup(open).collect { case x: Token.RightBrace => x } + def open(close: Token.RightParen): Option[Token.LeftParen] = + lookup(close).collect { case x: Token.LeftParen => x } + def open(close: Token.RightBracket): Option[Token.LeftBracket] = + lookup(close).collect { case x: Token.LeftBracket => x } + def open(close: Token.RightBrace): Option[Token.LeftBrace] = + lookup(close).collect { case x: Token.LeftBrace => x } +} + +object MatchingParens { + private def assertValidParens(open: Token, close: Token): Unit = { + (open, close) match { + case (Interpolation.Start(), Interpolation.End()) => + case (LeftBrace(), RightBrace()) => + case (LeftBracket(), RightBracket()) => + case (LeftParen(), RightParen()) => + case (o, c) => + throw new IllegalArgumentException(s"Mismatching parens ($o, $c)") + } + } + + /** + * Finds matching parens [({})]. + * + * Contains lookup keys in both directions, opening [({ and closing })]. + */ + private def getMatchingParentheses(tokens: Tokens): Map[TokenHash, Token] = { + val ret = Map.newBuilder[TokenHash, Token] + var stack = List.empty[Token] + tokens.foreach { + case open @ (LeftBrace() | LeftBracket() | LeftParen() | + Interpolation.Start()) => + stack = open :: stack + case close @ (RightBrace() | RightBracket() | RightParen() | + Interpolation.End()) => + val open = stack.head + assertValidParens(open, close) + ret += hash(open) -> close + ret += hash(close) -> open + stack = stack.tail + case _ => + } + val result = ret.result() + result + } + def apply(tokens: Tokens): MatchingParens = + new MatchingParens(getMatchingParentheses(tokens)) {} +} + +object TreeOps { + def parents(tree: Tree): Stream[Tree] = + tree #:: (tree.parent match { + case Some(x) => parents(x) + case _ => Stream.empty + }) +} +object TokenOps { + type TokenHash = Long + def hash(token: Token): TokenHash = { + val longHash: Long = + (token.productPrefix.hashCode.toLong << (62 - 8)) | + (token.start.toLong << (62 - (8 + 28))) | token.end + longHash + } +} diff --git a/scalafix-core/src/main/scala/scalafix/util/TokenClasses.scala b/scalafix-core/src/main/scala/scalafix/util/TokenClasses.scala index ee73ec4d9..95b123a51 100644 --- a/scalafix-core/src/main/scala/scalafix/util/TokenClasses.scala +++ b/scalafix-core/src/main/scala/scalafix/util/TokenClasses.scala @@ -8,8 +8,7 @@ import scala.meta.tokens.Token._ trait Whitespace object Whitespace { def unapply(token: Token): Boolean = { - token.is[Space] || token.is[Tab] || token.is[CR] || token.is[LF] || - token.is[FF] + token.is[Space] || token.is[Tab] || token.is[Newline] || token.is[FF] } } @@ -20,3 +19,11 @@ object Trivia { token.is[Whitespace] || token.is[Comment] } } + +@classifier +trait Newline +object Newline { + def unapply(token: Token): Boolean = { + token.is[LF] || token.is[CR] + } +} diff --git a/scalafix-core/src/main/scala/scalafix/util/TokenList.scala b/scalafix-core/src/main/scala/scalafix/util/TokenList.scala index cbec5dbf6..ceae553d3 100644 --- a/scalafix-core/src/main/scala/scalafix/util/TokenList.scala +++ b/scalafix-core/src/main/scala/scalafix/util/TokenList.scala @@ -8,9 +8,9 @@ import scala.meta.tokens.Tokens /** Helper to traverse tokens as a doubly linked list. */ class TokenList(tokens: Tokens) { def from(token: Token): SeqView[Token, IndexedSeq[Token]] = - tokens.view(tok2idx(token), tokens.length) + tokens.view(tok2idx(token), tokens.length).drop(1) def to(token: Token): SeqView[Token, IndexedSeq[Token]] = - tokens.view(0, tok2idx(token)) + tokens.view(0, tok2idx(token)).drop(1).reverse private[this] val tok2idx = { val map = Map.newBuilder[Token, Int] var i = 0 @@ -60,3 +60,4 @@ class TokenList(tokens: Tokens) { } } + diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/Command.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/Command.scala index 8f3996748..3b85a8401 100644 --- a/scalafix-testkit/src/main/scala/scalafix/testkit/Command.scala +++ b/scalafix-testkit/src/main/scala/scalafix/testkit/Command.scala @@ -7,12 +7,10 @@ case class Command(cmd: String) { } object Command { + val enableWarnUnusedImports = + Command("set scalacOptions in ThisBuild += \"-Ywarn-unused-import\" ") val clean = Command("clean") - val enableScalafix = - Command("set scalafixEnabled in Global := true") - val disableScalafix = - Command("set scalafixEnabled in Global := false") val testCompile = Command("test:compile") val scalafixTask = diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/DiffAssertions.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/DiffAssertions.scala index ace51cf3a..fce287a28 100644 --- a/scalafix-testkit/src/main/scala/scalafix/testkit/DiffAssertions.scala +++ b/scalafix-testkit/src/main/scala/scalafix/testkit/DiffAssertions.scala @@ -58,7 +58,8 @@ trait DiffAssertions extends FunSuiteLike { def compareContents(original: Seq[String], revised: Seq[String]): String = { import collection.JavaConverters._ - val diff = difflib.DiffUtils.diff(original.asJava, revised.asJava) + def trim(lines: Seq[String]) = lines.map(_.trim).asJava + val diff = difflib.DiffUtils.diff(trim(original), trim(revised)) if (diff.getDeltas.isEmpty) "" else difflib.DiffUtils diff --git a/scalafix-tests/input/src/main/scala/test/RemoveUnusedImports.scala b/scalafix-tests/input/src/main/scala/test/RemoveUnusedImports.scala new file mode 100644 index 000000000..b2c2ba5c6 --- /dev/null +++ b/scalafix-tests/input/src/main/scala/test/RemoveUnusedImports.scala @@ -0,0 +1,20 @@ +/* +rewrite = RemoveUnusedImports +*/ +package test + +import scala.collection.mutable +import scala.util.control.{NonFatal, Breaks} +import scala.concurrent.{Await, Future} +import scala.util.{Properties, DynamicVariable, Try} +import scala.util.{Success => Successful, Random} +import scala.sys.process._ + +object RemoveUnusedImports { + import Future._ + val NonFatal(a) = new Exception + Future.successful(1) + Properties.ScalaCompilerVersion + Try(1) + Successful(1) +} \ No newline at end of file diff --git a/scalafix-tests/integration/src/it/scala/scalafix/tests/ProjectTests.scala b/scalafix-tests/integration/src/it/scala/scalafix/tests/ProjectTests.scala index 9ea603bfa..629dc1a90 100644 --- a/scalafix-tests/integration/src/it/scala/scalafix/tests/ProjectTests.scala +++ b/scalafix-tests/integration/src/it/scala/scalafix/tests/ProjectTests.scala @@ -3,6 +3,7 @@ package scalafix.tests import scala.meta.semantic.Database import scalafix.rewrite.ExplicitReturnTypes import scalafix.rewrite.ProcedureSyntax +import scalafix.rewrite.RemoveUnusedImports import scalafix.rewrite.ScalafixRewrites class Slick @@ -10,11 +11,16 @@ class Slick ItTest( name = "slick", repo = "https://github.com/slick/slick.git", - rewrites = - Seq(ProcedureSyntax.name, - ExplicitReturnTypes(ScalafixRewrites.emptyDatabase).name), + rewrites = Seq( + ProcedureSyntax.name, + ExplicitReturnTypes(ScalafixRewrites.emptyDatabase).name, + RemoveUnusedImports(ScalafixRewrites.emptyDatabase).name + ), hash = "bd3c24be419ff2791c123067668c81e7de858915", - addCoursier = false + addCoursier = false, + commands = + Command.enableWarnUnusedImports +: + Command.default ), skip = false ) diff --git a/scalafix-tests/output/src/main/scala/test/RemoveUnusedImports.scala b/scalafix-tests/output/src/main/scala/test/RemoveUnusedImports.scala new file mode 100644 index 000000000..877a8d17d --- /dev/null +++ b/scalafix-tests/output/src/main/scala/test/RemoveUnusedImports.scala @@ -0,0 +1,14 @@ +package test + +import scala.util.control.NonFatal +import scala.concurrent.Future +import scala.util.{Properties, Try} +import scala.util.{Success => Successful} + +object RemoveUnusedImports { + val NonFatal(a) = new Exception + Future.successful(1) + Properties.ScalaCompilerVersion + Try(1) + Successful(1) +} \ No newline at end of file