Skip to content

Commit

Permalink
Implement new patch infrastructure.
Browse files Browse the repository at this point in the history
This commit is too big, I'm sorry. Main changes:

- Patch(from, to, replace) replaced with TokenPatch(token, newToken)
  with helpful combinators such as Add{Left,Right}
- New high-level TreePatch datatype with can be converted to TokenPatch.
- Implemented two tree patches, AddGlobalImport and RemoveGlobalImport.
- Implemented OrganizeImport, which is a prerequisite to use import tree
  patches.
- OrganizeImports
  - orders imports by configurable groups
  - removes unused imports using scalac -Ywarn-unused-import
    infrastructure. The implementation is requires hijacking a few
    private fields in g.analyzer using evil reflection hackery.
  - option to expand relative imports
  - handles renames
  - configurable "always used" to force keeping an import such as
    acyclic.file

Known bugs:

- introduces false import on non-existing cats.data.UUID in circe
- makes an import unused by expanding relative imports to
  fully-qualified imports, requiring you to run scalafix
  twice to fully remove unused imports (e.g. in cats with
  -Ywarn-unused-import)
- crashes on scala.meta parser bug (in akka), not scalafix problem
  really.

Expand relative

Test that patch is unchanged
  • Loading branch information
olafurpg committed Jan 27, 2017
1 parent 66a0c82 commit 91561ea
Show file tree
Hide file tree
Showing 48 changed files with 5,305 additions and 320 deletions.
1 change: 1 addition & 0 deletions .scalafix.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
imports.optimize = true
1 change: 1 addition & 0 deletions bin/scalacfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
scalac -Ywarn-unused-import -Yrangepos -Xplugin:/Users/ollie/.ivy2/local/ch.epfl.scala/scalafix-nsc_2.11/0.2.0-SNAPSHOT/jars/scalafix-nsc_2.11.jar $1
22 changes: 14 additions & 8 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ lazy val commonSettings = Seq(
triggeredMessage in ThisBuild := Watched.clearWhenTriggered,
scalacOptions := compilerOptions,
scalacOptions in (Compile, console) := compilerOptions :+ "-Yrepl-class-based",
test in assembly := {},
testOptions in Test += Tests.Argument("-oD")
)

Expand Down Expand Up @@ -146,10 +147,7 @@ lazy val core = project
"com.typesafe" % "config" % "1.3.1",
"com.lihaoyi" %% "sourcecode" % "0.1.3",
"org.scalameta" %% "scalameta" % Build.metaV,
"org.scala-lang" % "scala-reflect" % scalaVersion.value,
// Test dependencies
"org.scalatest" %% "scalatest" % Build.testV % "test",
"com.googlecode.java-diff-utils" % "diffutils" % "1.3.0" % "test"
"org.scala-lang" % "scala-reflect" % scalaVersion.value
)
)
.enablePlugins(BuildInfoPlugin)
Expand All @@ -160,9 +158,17 @@ lazy val `scalafix-nsc` = project
scalaVersion := "2.11.8",
crossScalaVersions := crossVersions,
libraryDependencies ++= Seq(
"org.scala-lang" % "scala-compiler" % scalaVersion.value,
"org.scalameta" %% "scalameta" % Build.metaV % "provided",
"org.scalatest" %% "scalatest" % Build.testV % Test
"org.scala-lang" % "scala-compiler" % scalaVersion.value,
"org.scalameta" %% "scalameta" % Build.metaV % "provided",
"com.googlecode.java-diff-utils" % "diffutils" % "1.3.0" % "test",
"org.scalatest" %% "scalatest" % Build.testV % Test,
"com.lihaoyi" %% "ammonite-ops" % Build.ammoniteV % Test,
// integration property tests
"org.typelevel" %% "catalysts-platform" % "0.0.5" % Test,
"com.typesafe.slick" %% "slick" % "3.2.0-M2" % Test,
"io.circe" %% "circe-core" % "0.6.0" % Test,
"org.typelevel" %% "cats-core" % "0.7.2" % Test,
"org.scalacheck" %% "scalacheck" % "1.13.4" % Test
),
// sbt does not fetch transitive dependencies of compiler plugins.
// to overcome this issue, all transitive dependencies are included
Expand Down Expand Up @@ -256,7 +262,7 @@ lazy val `scalafix-tests` = project
)
.value,
libraryDependencies ++= Seq(
"com.lihaoyi" %% "ammonite-ops" % "0.8.0",
"com.lihaoyi" %% "ammonite-ops" % Build.ammoniteV,
"org.scalatest" %% "scalatest" % Build.testV % "test"
)
)
Expand Down
27 changes: 27 additions & 0 deletions core/src/main/scala/scalafix/FilterMatcher.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package scalafix

import scala.util.matching.Regex
import scalafix.util.AbsoluteFile

case class FilterMatcher(include: Regex, exclude: Regex) {
def matches(file: AbsoluteFile): Boolean = matches(file.path)
def matches(input: String): Boolean =
include.findFirstIn(input).isDefined &&
exclude.findFirstIn(input).isEmpty
}

object FilterMatcher {
val matchEverything = new FilterMatcher(".*".r, mkRegexp(Nil))

def mkRegexp(filters: Seq[String]): Regex =
filters match {
case Nil => "$a".r // will never match anything
case head :: Nil => head.r
case _ => filters.mkString("(", "|", ")").r
}

def apply(includes: Seq[String], excludes: Seq[String]): FilterMatcher =
new FilterMatcher(mkRegexp(includes), mkRegexp(excludes))
def apply(include: String): FilterMatcher =
new FilterMatcher(mkRegexp(Seq(include)), mkRegexp(Nil))
}
17 changes: 12 additions & 5 deletions core/src/main/scala/scalafix/Scalafix.scala
Original file line number Diff line number Diff line change
@@ -1,23 +1,30 @@
package scalafix

import scala.meta.parsers.Parsed
import scala.collection.immutable.Seq
import scala.meta._
import scala.meta.inputs.Input
import scala.meta.parsers.Parsed
import scalafix.rewrite.RewriteCtx
import scalafix.rewrite.SemanticApi
import scalafix.util.AssociatedComments
import scalafix.util.Patch
import scalafix.util.TokenList
import scalafix.util.logger

object Scalafix {
def fix(code: Input,
config: ScalafixConfig,
semanticApi: Option[SemanticApi]): Fixed = {
config.parser.apply(code, config.dialect) match {
case Parsed.Success(ast) =>
val ctx = RewriteCtx(config, new TokenList(ast.tokens), semanticApi)
val patches: Seq[Patch] = config.rewrites.flatMap(_.rewrite(ast, ctx))
Fixed.Success(Patch.apply(ast.tokens, patches))
val tokens = ast.tokens
implicit val ctx = RewriteCtx(
config,
new TokenList(ast.tokens),
AssociatedComments(tokens),
semanticApi
)
val patches = config.rewrites.flatMap(_.rewrite(ast, ctx))
Fixed.Success(Patch.apply(ast, patches))
case Parsed.Error(pos, msg, e) =>
Fixed.Failed(Failure.ParseError(pos, msg, e))
}
Expand Down
56 changes: 48 additions & 8 deletions core/src/main/scala/scalafix/ScalafixConfig.scala
Original file line number Diff line number Diff line change
@@ -1,20 +1,40 @@
package scalafix

import scala.meta.Dialect
import scala.meta.Tree
import scala.collection.immutable.Seq
import scala.meta._
import scala.meta.dialects.Scala211
import scala.meta.parsers.Parse
import scala.util.control.NonFatal
import scalafix.rewrite.Rewrite
import scalafix.syntax._

import java.io.File

import com.typesafe.config.Config
import com.typesafe.config.ConfigFactory

case class ImportsConfig(
expandRelative: Boolean = true,
spaceAroundCurlyBrace: Boolean = false,
organize: Boolean = true,
removeUnused: Boolean = true,
alwaysUsed: Seq[Ref] = Seq(),
groups: Seq[FilterMatcher] = Seq(
FilterMatcher("scala.language.*"),
FilterMatcher("(scala|scala\\..*)$"),
FilterMatcher("(java|java\\..*)$"),
FilterMatcher(".*")
),
groupByPrefix: Boolean = false
)
object ImportsConfig {
def default: ImportsConfig = ImportsConfig()
}
case class ScalafixConfig(
rewrites: Seq[Rewrite] = Rewrite.defaultRewrites,
parser: Parse[_ <: Tree] = Parse.parseSource,
imports: ImportsConfig = ImportsConfig(),
fatalWarning: Boolean = true,
dialect: Dialect = Scala211
)

Expand All @@ -25,6 +45,7 @@ object ScalafixConfig {
catch {
case NonFatal(e) => Left(e.getMessage)
}
val default = ScalafixConfig()

def fromFile(file: File): Either[String, ScalafixConfig] =
saferThanTypesafe(() => ConfigFactory.parseFile(file))
Expand All @@ -34,15 +55,35 @@ object ScalafixConfig {

def fromConfig(config: Config): Either[String, ScalafixConfig] = {
import scala.collection.JavaConverters._
val base = ScalafixConfig(
fatalWarning = config.getBoolOrElse("fatalWarnings",
ScalafixConfig.default.fatalWarning),
imports = ImportsConfig(
expandRelative =
config.getBoolOrElse("imports.expandRelative",
ImportsConfig.default.expandRelative),
spaceAroundCurlyBrace =
config.getBoolOrElse("imports.spaceAroundCurlyBrace",
ImportsConfig.default.spaceAroundCurlyBrace),
organize = config.getBoolOrElse("imports.organize",
ImportsConfig.default.organize),
removeUnused =
config.getBoolOrElse("imports.removeUnused",
ImportsConfig.default.removeUnused),
groupByPrefix =
config.getBoolOrElse("imports.groupByPrefix",
ImportsConfig.default.groupByPrefix)
)
)
if (config.hasPath("rewrites"))
fromNames(config.getStringList("rewrites").asScala.toList)
fromNames(config.getStringList("rewrites").asScala.toList).right
.map(rewrites => base.copy(rewrites = rewrites))
else Right(ScalafixConfig())
}

def fromNames(names: List[String]): Either[String, ScalafixConfig] =
def fromNames(names: List[String]): Either[String, Seq[Rewrite]] =
names match {
case "all" :: Nil =>
Right(ScalafixConfig(rewrites = Rewrite.allRewrites))
case "all" :: Nil => Right(Rewrite.allRewrites)
case _ =>
val invalidNames =
names.filterNot(Rewrite.name2rewrite.contains)
Expand All @@ -51,8 +92,7 @@ object ScalafixConfig {
s"Invalid rewrite rule: ${invalidNames.mkString(",")}. " +
s"Valid rules are: ${Rewrite.name2rewrite.keys.mkString(",")}")
} else {
val rewrites = names.map(Rewrite.name2rewrite)
Right(ScalafixConfig(rewrites = rewrites))
Right(names.map(Rewrite.name2rewrite))
}
}
}
6 changes: 4 additions & 2 deletions core/src/main/scala/scalafix/rewrite/ExplicitImplicit.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package scalafix.rewrite
import scala.{meta => m}
import scalafix.util.Patch
import scalafix.util.Whitespace
import scala.collection.immutable.Seq
import scalafix.util.TokenPatch

case object ExplicitImplicit extends Rewrite {
// Don't explicitly annotate vals when the right-hand body is a single call
Expand All @@ -27,8 +29,8 @@ case object ExplicitImplicit extends Rewrite {
replace <- lhsTokens.reverseIterator.find(x =>
!x.is[Token.Equals] && !x.is[Whitespace])
typ <- semantic.typeSignature(defn)
} yield Patch(replace, replace, s"$replace: ${typ.syntax}")
}.toSeq
} yield TokenPatch.AddRight(replace, s": ${typ.syntax}")
}.to[Seq]
ast.collect {
case t @ m.Defn.Val(mods, _, None, body)
if mods.exists(_.syntax == "implicit") &&
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/scala/scalafix/rewrite/ProcedureSyntax.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import scala.meta.tokens.Token.RightBrace
import scala.meta.tokens.Token.RightParen
import scala.meta.tokens.Token.Space
import scalafix.util.Patch
import scala.collection.immutable.Seq
import scalafix.util.TokenPatch

case object ProcedureSyntax extends Rewrite {
override def rewrite(ast: Tree, ctx: RewriteCtx): Seq[Patch] = {
Expand All @@ -27,7 +29,7 @@ case object ProcedureSyntax extends Rewrite {
if (between.nonEmpty) " " + between
else ""
}
Patch(next(closingParen), bodyStart, s": Unit = {$comment")
TokenPatch.AddRight(closingParen, s": Unit =")
}
patches
}
Expand Down
7 changes: 6 additions & 1 deletion core/src/main/scala/scalafix/rewrite/Rewrite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package scalafix.rewrite

import scala.meta._
import scalafix.Failure.MissingSemanticApi
import scalafix.util.TreePatch
import scalafix.util.Patch
import scala.collection.immutable.Seq

abstract class Rewrite {
def getSemanticApi(ctx: RewriteCtx): SemanticApi = ctx.semantic.getOrElse {
Expand All @@ -16,7 +18,10 @@ object Rewrite {
t.map(x => x.source -> x.value).toMap
}

val syntaxRewrites: Seq[Rewrite] = Seq(ProcedureSyntax, VolatileLazyVal)
val syntaxRewrites: Seq[Rewrite] = Seq(
ProcedureSyntax,
VolatileLazyVal
)
val semanticRewrites: Seq[Rewrite] = Seq(ExplicitImplicit)
val allRewrites: Seq[Rewrite] = syntaxRewrites ++ semanticRewrites
val defaultRewrites: Seq[Rewrite] =
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/scala/scalafix/rewrite/RewriteCtx.scala
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package scalafix.rewrite
import scalafix.ScalafixConfig
import scalafix.util.AssociatedComments
import scalafix.util.TokenList

case class RewriteCtx(
config: ScalafixConfig,
tokenList: TokenList,
comments: AssociatedComments,
semantic: Option[SemanticApi]
)
12 changes: 10 additions & 2 deletions core/src/main/scala/scalafix/rewrite/SemanticApi.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package scalafix.rewrite

import scala.meta.Defn
import scala.meta.Type
import scala.meta._

/** A custom semantic api for scalafix rewrites.
*
Expand All @@ -18,4 +17,13 @@ trait SemanticApi {

/** Returns the type annotation for given val/def. */
def typeSignature(defn: Defn): Option[Type]

/** Returns the fully qualified name of this name, or none if unable to find it*/
def fqn(name: Ref): Option[Ref]

/** Returns all used refs in this compilation unit */
def usedFqns: Seq[Ref]

/** Returns true if importee is not used in this compilation unit, false otherwise */
def isUnusedImport(importee: Importee): Boolean
}
4 changes: 3 additions & 1 deletion core/src/main/scala/scalafix/rewrite/VolatileLazyVal.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package scalafix.rewrite

import scala.meta._
import scalafix.util.Patch
import scala.collection.immutable.Seq
import scalafix.util.TokenPatch

case object VolatileLazyVal extends Rewrite {
private object NonVolatileLazyVal {
Expand All @@ -17,7 +19,7 @@ case object VolatileLazyVal extends Rewrite {
override def rewrite(ast: Tree, ctx: RewriteCtx): Seq[Patch] = {
ast.collect {
case NonVolatileLazyVal(tok) =>
Patch(tok, tok, s"@volatile ${tok.syntax}")
TokenPatch.AddLeft(tok, s"@volatile ")
}
}
}
31 changes: 31 additions & 0 deletions core/src/main/scala/scalafix/syntax/package.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package scalafix

import scala.meta.Importee
import scala.meta.tokens.Token
import scalafix.util.CanonicalImport
import scalafix.util.ImportPatch
import scalafix.util.logger

import com.typesafe.config.Config

package object syntax {
implicit class XtensionImporter(i: CanonicalImport) {
def supersedes(patch: ImportPatch): Boolean =
i.ref.structure == patch.importer.ref.structure &&
(i.importee.is[Importee.Wildcard] ||
i.importee.structure == patch.importer.importee.structure)
}

implicit class XtensionToken(token: Token) {
def posTuple: (Int, Int) = token.start -> token.end
}

implicit class XtensionConfig(config: Config) {
def getBoolOrElse(key: String, els: Boolean): Boolean =
if (config.hasPath(key)) config.getBoolean(key)
else els
}
implicit class XtensionString(str: String) {
def reveal: String = logger.reveal(str)
}
}
31 changes: 31 additions & 0 deletions core/src/main/scala/scalafix/util/AbsoluteFile.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package scalafix.util

import java.io.File

/** Wrapper around java.io.File with an absolute path. */
sealed abstract case class AbsoluteFile(jfile: File) {
def path: String = jfile.getAbsolutePath
def /(other: String) = new AbsoluteFile(new File(jfile, other)) {}
}

object AbsoluteFile {
def fromFiles(files: Seq[File],
workingDir: AbsoluteFile): Seq[AbsoluteFile] = {
files.map(x => fromFile(x, workingDir))
}
private def makeAbsolute(workingDir: File)(file: File): File =
if (file.isAbsolute) file
else new File(workingDir, file.getPath)

// If file is already absolute, then workingDir is not used.
def fromFile(file: File, workingDir: AbsoluteFile): AbsoluteFile = {
new AbsoluteFile(makeAbsolute(workingDir.jfile)(file)) {}
}
def fromPath(path: String): Option[AbsoluteFile] = {
val file = new File(path)
if (file.isAbsolute) Some(new AbsoluteFile(file) {})
else None
}
def userDir = new AbsoluteFile(new File(System.getProperty("user.dir"))) {}
def homeDir = new AbsoluteFile(new File(System.getProperty("user.home"))) {}
}
Loading

0 comments on commit 91561ea

Please sign in to comment.