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

Implement RemoveUnusedImports rewrite. #166

Merged
merged 3 commits into from
May 27, 2017
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
7 changes: 7 additions & 0 deletions bin/new-rewrite.sh
Original file line number Diff line number Diff line change
@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️

6 changes: 1 addition & 5 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -248,6 +243,7 @@ lazy val testsInput = project
scalametaSourceroot := sourceDirectory.in(Compile).value,
scalametaSemanticdb := ScalametaSemanticdb.Fat,
scalacOptions ~= (_.filterNot(_ == "-Yno-adapted-args")),
scalacOptions += "-Ywarn-unused-import", // For RemoveUnusedImports
// TODO: Remove once scala-xml-quote is merged into scala-xml
resolvers += Resolver.bintrayRepo("allanrenucci", "maven"),
libraryDependencies ++= testsDeps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -74,6 +75,9 @@ 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: " +
s"${ScalafixRewrites.allNames.mkString(", ")}")
rewrites: List[String] = Nil,
@HelpMessage(
"Files to fix. Runs on all *.scala files if given a directory")
Expand Down
88 changes: 60 additions & 28 deletions scalafix-core/src/main/scala/scalafix/patch/ImportPatchOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 =>
Expand All @@ -48,42 +48,74 @@ 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
}

extraPatches ++
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
}

leadingNewlines ++
curlyBraceRemoves ++
extraPatches ++
(isRemovedImportee ++
isRemovedImporter ++
isRemovedImport).map(remove)
}

private def extractImports(stats: Seq[Stat]): Seq[Import] = {
stats
.takeWhile(_.is[Import])
.collect { case i: Import => i }
}

def getGlobalImports(ast: Tree): Seq[Import] =
ast match {
case Pkg(_, Seq(pkg: Pkg)) => getGlobalImports(pkg)
case Source(Seq(pkg: Pkg)) => getGlobalImports(pkg)
case Pkg(_, stats) => extractImports(stats)
case Source(stats) => extractImports(stats)
case _ => Nil
}

}
7 changes: 4 additions & 3 deletions scalafix-core/src/main/scala/scalafix/patch/Patch.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ object ScalafixRewrites {
)
def semantic(mirror: Mirror): List[Rewrite] = List(
ExplicitReturnTypes(mirror),
RemoveUnusedImports(mirror),
Xor2Either(mirror),
NoAutoTupling(mirror)
)
Expand All @@ -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
}
13 changes: 2 additions & 11 deletions scalafix-core/src/main/scala/scalafix/syntax/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
61 changes: 61 additions & 0 deletions scalafix-core/src/main/scala/scalafix/util/MatchingParens.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
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)) {}
}
11 changes: 9 additions & 2 deletions scalafix-core/src/main/scala/scalafix/util/TokenClasses.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
}

Expand All @@ -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]
}
}
4 changes: 2 additions & 2 deletions scalafix-core/src/main/scala/scalafix/util/TokenList.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading