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.

Sign up for GitHub

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

Jump to bottom

Simplifify api for import patches #48

Merged
merged 2 commits into from
Jan 28, 2017
Merged

Simplifify api for import patches #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22522f1
Make project build in intellij
olafurpg Jan 28, 2017
fd40924
Simplify import patch api.
olafurpg Jan 28, 2017
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
28 changes: 19 additions & 9 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ lazy val `scalafix-root` = project
.aggregate(
`scalafix-nsc`,
`scalafix-tests`,
`scalafix-testutils`,
core,
// cli, // superseded by sbt plugin
readme,
Expand Down Expand Up @@ -158,11 +159,10 @@ 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",
"com.googlecode.java-diff-utils" % "diffutils" % "1.3.0" % "test",
"org.scalatest" %% "scalatest" % Build.testV % Test,
"com.lihaoyi" %% "ammonite-ops" % Build.ammoniteV % Test,
"org.scala-lang" % "scala-compiler" % scalaVersion.value,
"org.scalameta" %% "scalameta" % Build.metaV % "provided",
"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,
Expand Down Expand Up @@ -198,12 +198,12 @@ lazy val `scalafix-nsc` = project
},
exposePaths("scalafixNsc", Test)
)
.dependsOn(core)
.dependsOn(core, `scalafix-testutils` % Test)

lazy val cli = project
.settings(allSettings)
.settings(packSettings)
.settings(
allSettings,
packSettings,
moduleName := "scalafix-cli",
packJvmOpts := Map(
"scalafix" -> jvmOptions,
Expand All @@ -220,7 +220,7 @@ lazy val cli = project
"com.martiansoftware" % "nailgun-server" % "0.9.1"
)
)
.dependsOn(core % "compile->compile;test->test")
.dependsOn(core, `scalafix-testutils` % Test)

lazy val publishedArtifacts = Seq(
publishLocal in `scalafix-nsc`,
Expand Down Expand Up @@ -249,6 +249,16 @@ lazy val `scalafix-sbt` = project
)
.enablePlugins(BuildInfoPlugin)

lazy val `scalafix-testutils` = project.settings(
allSettings,
scalaVersion := "2.11.8",
crossScalaVersions := crossVersions,
libraryDependencies ++= Seq(
"com.googlecode.java-diff-utils" % "diffutils" % "1.3.0",
"org.scalatest" %% "scalatest" % Build.testV
)
)

lazy val `scalafix-tests` = project
.settings(
allSettings,
Expand Down
7 changes: 1 addition & 6 deletions core/src/main/scala/scalafix/Scalafix.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@ object Scalafix {
config.parser.apply(code, config.dialect) match {
case Parsed.Success(ast) =>
val tokens = ast.tokens
implicit val ctx = RewriteCtx(
config,
new TokenList(ast.tokens),
AssociatedComments(tokens),
semanticApi
)
implicit val ctx = RewriteCtx.fromCode(ast, config, semanticApi)
val patches = config.rewrites.flatMap(_.rewrite(ast, ctx))
Fixed.Success(Patch.apply(ast, patches))
case Parsed.Error(pos, msg, e) =>
Expand Down
18 changes: 18 additions & 0 deletions core/src/main/scala/scalafix/rewrite/RewriteCtx.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,29 @@
package scalafix.rewrite
import scala.meta.Tree
import scala.meta.tokens.Tokens
import scalafix.ScalafixConfig
import scalafix.util.AssociatedComments
import scalafix.util.TokenList

case class RewriteCtx(
config: ScalafixConfig,
tokens: Tokens,
tokenList: TokenList,
comments: AssociatedComments,
semantic: Option[SemanticApi]
)

object RewriteCtx {
def fromCode(ast: Tree,
config: ScalafixConfig = ScalafixConfig(),
semanticApi: Option[SemanticApi] = None): RewriteCtx = {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if no SemanticApi is supplied? Is a new one created, or is functionality diminished in some way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. It depends on what rewrite is used, some rewrites like procedure syntax work 100% without the semantic api, organize imports works without it but can't handle relative imports in that case, explicit implicit will error.

I guess this could be better documented.

val tokens = ast.tokens(config.dialect)
RewriteCtx(
config,
tokens,
new TokenList(tokens),
AssociatedComments(tokens),
semanticApi
)
}
}
2 changes: 1 addition & 1 deletion core/src/main/scala/scalafix/syntax/package.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ package object syntax {
def supersedes(patch: ImportPatch): Boolean =
i.ref.structure == patch.importer.ref.structure &&
(i.importee.is[Importee.Wildcard] ||
i.importee.structure == patch.importer.importee.structure)
i.importee.structure == patch.importee.structure)
}

implicit class XtensionToken(token: Token) {
Expand Down
14 changes: 6 additions & 8 deletions core/src/main/scala/scalafix/util/OrganizeImports.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,14 @@ private[this] class OrganizeImports private (implicit ctx: RewriteCtx) {
.mkString("\n\n")
}

def getRemovePatches(oldImports: Seq[Import],
tokens: Tokens): Seq[TokenPatch.Remove] = {
def getRemovePatches(oldImports: Seq[Import]): Seq[TokenPatch.Remove] = {
val toRemove = for {
firstImport <- oldImports.headOption
first <- firstImport.tokens.headOption
lastImport <- oldImports.lastOption
last <- lastImport.tokens.lastOption
} yield {
tokens.toIterator
ctx.tokens.toIterator
.dropWhile(_.start < first.start)
.takeWhile { x =>
x.end <= last.end
Expand All @@ -186,9 +185,9 @@ private[this] class OrganizeImports private (implicit ctx: RewriteCtx) {
patch match {
case _: AddGlobalImport =>
if (is.exists(_.supersedes(patch))) is
else is :+ patch.importer
else is ++ getCanonicalImports(patch.toImport)
case remove: RemoveGlobalImport =>
is.filter(_.structure == remove.importer.structure)
is.filterNot(_.structure == remove.importer.structure)
}
patches.foldLeft(removeUnused(globalImports))(combine)
}
Expand All @@ -200,14 +199,13 @@ private[this] class OrganizeImports private (implicit ctx: RewriteCtx) {
val oldImports = getGlobalImports(code)
val globalImports = oldImports.flatMap(getCanonicalImports)
val cleanedUpImports = cleanUpImports(globalImports, patches)
val tokens = code.tokens
val tokenToEdit =
oldImports.headOption
.map(_.tokens.head)
.getOrElse(tokens.head)
.getOrElse(ctx.tokens.head)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know ctx.tokens is non-empty here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tree nodes that return empty tokens are incredibly few (if any) and I would say it was a truly exceptional case if those were passed in here. Top level trees always have a beginning of file token.

There's an interesting ongoing discussion in scalameta about validating trees for a given dialect. I would be interested to start a similar discussion about the possibility to use a non empty list type Where applicable. The challenge is try and encode as much as possible on the type level without burdening syntax too heavily. Quasi quotes help a lot here.

val toInsert = prettyPrint(cleanedUpImports)
TokenPatch.AddLeft(tokenToEdit, toInsert) +:
getRemovePatches(oldImports, tokens)
getRemovePatches(oldImports)
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/scala/scalafix/util/Patch.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ abstract class TokenPatch(val tok: Token, val newTok: String)
s"TokenPatch(${logger.reveal(tok.syntax)}, ${tok.structure}, $newTok)"
}

abstract class ImportPatch(val importer: CanonicalImport) extends TreePatch
abstract class ImportPatch(val importer: Importer) extends TreePatch {
def importee: Importee = importer.importees.head
def toImport: Import = Import(Seq(importer))
}
object TreePatch {
case class RemoveGlobalImport(override val importer: CanonicalImport)
case class RemoveGlobalImport(override val importer: Importer)
extends ImportPatch(importer)
case class AddGlobalImport(override val importer: CanonicalImport)
case class AddGlobalImport(override val importer: Importer)
extends ImportPatch(importer)
}

Expand Down
1 change: 1 addition & 0 deletions core/src/main/scala/scalafix/util/logger.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,4 @@ object logger {
s"$line\n=> $t\n$line"
}
}

2 changes: 0 additions & 2 deletions readme/Readme.scala → readme/src/main/scala/scalafix/Readme.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package scalafix

import scalatags.Text.TypedTag
import scalatags.Text.all._
import scalatex._
import scalatags.Text.all._

object Readme {
def note = b("Note. ")
Expand Down
23 changes: 23 additions & 0 deletions scalafix-nsc/src/test/scala/scalafix/ImportPatches.sc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Small worksheet to demonstrate usage of import patches
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ShaneDelmore I got this working on my laptop
screen shot 2017-01-28 at 21 30 54

import scala.collection.immutable.Seq
import scala.meta._
import scalafix.rewrite.RewriteCtx
import scalafix.util.Patch
import scalafix.util.TreePatch.AddGlobalImport
import scalafix.util.TreePatch.RemoveGlobalImport
import scala.meta._
import scalafix.util.logger
val code =
"""import scala.language.higherKinds
|import java.{util => ju}
|import scala.collection.mutable._
|
|object a
""".stripMargin.parse[Source].get
implicit val ctx: RewriteCtx = RewriteCtx.fromCode(code)
val add = AddGlobalImport(importer"cats.data.EitherT")
val addRedundant = AddGlobalImport(importer"scala.collection.mutable._")
val remove = RemoveGlobalImport(importer"scala.language.higherKinds")

val x = Patch.apply(code, Seq(add, addRedundant, remove))
logger.elem(x)
4 changes: 4 additions & 0 deletions scalafix-nsc/src/test/scala/scalafix/SemanticTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@ import scala.{meta => m}
import scalafix.nsc.ScalafixNscPlugin
import scalafix.rewrite.ExplicitImplicit
import scalafix.rewrite.Rewrite
import scalafix.rewrite.RewriteCtx
import scalafix.util.DiffAssertions
import scalafix.util.FileOps
import scalafix.util.Patch
import scalafix.util.TreePatch.AddGlobalImport
import scalafix.util.TreePatch.RemoveGlobalImport
import scalafix.util.logger

import org.scalatest.FunSuite
Expand Down
8 changes: 6 additions & 2 deletions .../scala/scalafix/util/DiffAssertions.scala → .../scala/scalafix/util/DiffAssertions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ trait DiffAssertions extends FunSuiteLike {
def assertEqual[A](a: A, b: A): Unit = {
assert(a === b)
}
def header[T](t: T): String = {
val line = s"=" * (t.toString.length + 3)
s"$line\n=> $t\n$line"
}

case class DiffFailure(title: String,
expected: String,
Expand All @@ -28,12 +32,12 @@ trait DiffAssertions extends FunSuiteLike {
val sb = new StringBuilder
if (obtained.length < 1000) {
sb.append(s"""
#${logger.header("Obtained")}
#${header("Obtained")}
#${trailingSpace(obtained)}
""".stripMargin('#'))
}
sb.append(s"""
#${logger.header("Diff")}
#${header("Diff")}
#${trailingSpace(compareContents(obtained, expected))}
""".stripMargin('#'))
sb.toString()
Expand Down