Skip to content

Commit

Permalink
Scalafix rule to remove instance imports when upgrading to 2.2.0 (#3566)
Browse files Browse the repository at this point in the history
* Bump scalafix

* Move the v1.0.0 tests to a subdirectory

* WIP scalafix rule to remove instance imports

* Catch the low-hanging fruit

* Support for removing "import cats.instances.all._"

The import is only removed if there is no possibility it is being used
to import Future instances.

* Support for removing "import cats.implicits._"

* Update the scalafix readme

* Update test command in .travis.yml

* Put sbt-scalafix back how it was

Upgrading it broke one of the v1.0.0 rules and I can't be bothered
investigating

* No need to list all the positives, just check the negatives

* Add -Ypartial-unification

The v1.0.0 expected output files won't compile without this.

* Remove special handling of Future instances

* Rewrite "import cats.implicits._" to "import cats.syntax.all._"

* Bump cats-core dependency in scalafix build.sbt

This is so we get Future instances in implicit scope

* Bump cats dependency to 2.2.0-RC4

* Add a test for rewriting of global imports

i.e. imports at the top of the file

* Work around a known Scalafix issue

* Improve the check for use of extension methods

* Handle implicit conversions

Do not remove an import of cats.implicits._ if it is being used to
import an implicit conversion

* Reorganise readme
  • Loading branch information
cb372 authored Sep 2, 2020
1 parent c2719aa commit 1719f64
Show file tree
Hide file tree
Showing 40 changed files with 430 additions and 30 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
- stage: test
name: Scalafix tests
env: TEST="scalafix"
script: cd scalafix && sbt tests/test
script: cd scalafix && sbt test

- &bincompat
stage: test
Expand Down
34 changes: 24 additions & 10 deletions scalafix/README.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,37 @@
# Scalafix rules for cats

## Try this!
## How to use

[Install the Scalafix sbt plugin](https://scalacenter.github.io/scalafix/docs/users/installation)
1. [Install the Scalafix sbt plugin](https://scalacenter.github.io/scalafix/docs/users/installation)

To run all rules that apply to version `1.0.0-RC1` run
2. Run the rules appropriate to your Cats version (see below)

## Migration to Cats v2.2.0

First configure the SemanticDB compiler plugin to enable synthetics:

```
ThisBuild / scalacOptions += "-P:semanticdb:synthetics:on"
```

Then run Scalafix:

```sh
sbt scalafix github:typelevel/cats/v1.0.0?sha=v1.0.0-RC1
sbt scalafix github:typelevel/cats/Cats_v2_2_0
```

to run all rules that apply to the current `1.0.0-SNAPSHOT` run
### Available rules

- Type class instances are now available in implicit scope, so there is a rule to
remove imports that are no longer needed

## Migration to Cats v1.0.0

```sh
sbt scalafix github:typelevel/cats/v1.0.0
sbt scalafix github:typelevel/cats/Cats_v1_0_0
```

## Available rules
### Available rules

- [x] All Unapply enabled methods, e.g. sequenceU, traverseU, etc. are removed. Unapply enabled syntax ops are also removed. Please use the partial unification SI-2712 fix instead. The easiest way might be this sbt-plugin.

Expand All @@ -40,7 +55,7 @@ sbt scalafix github:typelevel/cats/v1.0.0

- [x] Split is removed, and the method split is moved to Arrow. Note that only under CommutativeArrow does it guarantee the non-interference between the effects. see #1567

# WIP
### WIP

- [ ] cats no longer publishes the all-inclusive bundle package "org.typelevel" % "cats", use cats-core, cats-free, or cats-law accordingly instead. If you need cats.free, use "org.typelevel" % "cats-free", if you need cats-laws use "org.typelevel" % "cats-laws", if neither, use "org.typelevel" % "cats-core".

Expand All @@ -53,10 +68,9 @@ sbt scalafix github:typelevel/cats/v1.0.0
- [ ] iteratorFoldM was removed from Foldable due to #1716


## To test scala fix
## To test the Scalafix rules

```bash
sbt coreJVM/publishLocal freeJVM/publishLocal
cd scalafix
sbt test
```
74 changes: 55 additions & 19 deletions scalafix/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,68 @@ lazy val rules = project.settings(
libraryDependencies += "ch.epfl.scala" %% "scalafix-core" % V.scalafixVersion
)

lazy val input = project.settings(
libraryDependencies ++= Seq(
"org.typelevel" %% "cats" % "0.9.0"
),
scalacOptions += "-language:higherKinds"
)
lazy val v1_0_0_input = project.in(file("v1_0_0/input"))
.settings(
libraryDependencies ++= Seq(
"org.typelevel" %% "cats" % "0.9.0"
),
scalacOptions += "-language:higherKinds"
)

lazy val output = project.settings(
libraryDependencies ++= Seq(
"org.typelevel" %% "cats-core" % "1.0.0",
"org.typelevel" %% "cats-free" % "1.0.0"
),
scalacOptions += "-language:higherKinds"
)
lazy val v1_0_0_output = project.in(file("v1_0_0/output"))
.settings(
libraryDependencies ++= Seq(
"org.typelevel" %% "cats-core" % "1.0.0",
"org.typelevel" %% "cats-free" % "1.0.0"
),
scalacOptions ++= Seq(
"-language:higherKinds",
"-Ypartial-unification"
)
)

lazy val v1_0_0_tests = project.in(file("v1_0_0/tests"))
.settings(
libraryDependencies += "ch.epfl.scala" % "scalafix-testkit" % V.scalafixVersion % Test cross CrossVersion.full,
compile.in(Compile) :=
compile.in(Compile).dependsOn(compile.in(v1_0_0_input, Compile)).value,
scalafixTestkitOutputSourceDirectories :=
sourceDirectories.in(v1_0_0_output, Compile).value,
scalafixTestkitInputSourceDirectories :=
sourceDirectories.in(v1_0_0_input, Compile).value,
scalafixTestkitInputClasspath :=
fullClasspath.in(v1_0_0_input, Compile).value
)
.dependsOn(v1_0_0_input, rules)
.enablePlugins(ScalafixTestkitPlugin)

lazy val v2_2_0_input = project.in(file("v2_2_0/input"))
.settings(
libraryDependencies ++= Seq(
"org.typelevel" %% "cats-core" % "2.1.0"
),
scalacOptions ++= Seq("-language:higherKinds", "-P:semanticdb:synthetics:on")
)

lazy val v2_2_0_output = project.in(file("v2_2_0/output"))
.settings(
libraryDependencies ++= Seq(
"org.typelevel" %% "cats-core" % "2.2.0-RC4"
),
scalacOptions += "-language:higherKinds"
)

lazy val tests = project
lazy val v2_2_0_tests = project.in(file("v2_2_0/tests"))
.settings(
libraryDependencies += "ch.epfl.scala" % "scalafix-testkit" % V.scalafixVersion % Test cross CrossVersion.full,
compile.in(Compile) :=
compile.in(Compile).dependsOn(compile.in(input, Compile)).value,
compile.in(Compile).dependsOn(compile.in(v2_2_0_input, Compile)).value,
scalafixTestkitOutputSourceDirectories :=
sourceDirectories.in(output, Compile).value,
sourceDirectories.in(v2_2_0_output, Compile).value,
scalafixTestkitInputSourceDirectories :=
sourceDirectories.in(input, Compile).value,
sourceDirectories.in(v2_2_0_input, Compile).value,
scalafixTestkitInputClasspath :=
fullClasspath.in(input, Compile).value
fullClasspath.in(v2_2_0_input, Compile).value
)
.dependsOn(input, rules)
.dependsOn(v2_2_0_input, rules)
.enablePlugins(ScalafixTestkitPlugin)
89 changes: 89 additions & 0 deletions scalafix/rules/src/main/scala/fix/Cats_v2_2_0.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package fix
package v2_2_0

import scalafix.v0._
import scalafix.syntax._
import scala.meta._
import scala.meta.contrib._
import scala.meta.Term.Apply

// ref: https://github.com/typelevel/cats/issues/3563
case class RemoveInstanceImports(index: SemanticdbIndex)
extends SemanticRule(index, "RemoveInstanceImports") {

override def fix(ctx: RuleCtx): Patch = ctx.tree.collect {
// e.g. "import cats.instances.int._" or "import cats.instances.all._"
case i @ Import(Importer(Select(Select(Name("cats"), Name("instances")), x), _) :: _) =>
removeImportLine(ctx)(i)

// "import cats.implicits._"
case i @ Import(Importer(Select(Name("cats"), Name("implicits")), _) :: _) =>
val boundary = findLexicalBoundary(i)

// Find all synthetics between the import statement and the end of the lexical boundary
val lexicalStart = i.pos.end
val lexicalEnd = boundary.pos.end
try {
val relevantSynthetics =
ctx.index.synthetics.filter(x => x.position.start >= lexicalStart && x.position.end <= lexicalEnd)

val usesImplicitConversion = relevantSynthetics.exists(containsImplicitConversion)
val usesSyntax = relevantSynthetics.exists(containsCatsSyntax)

if (usesImplicitConversion) {
// the import is used to enable an implicit conversion,
// so we have to keep it
Patch.empty
} else if (usesSyntax) {
// the import is used to enable an extension method,
// so replace it with "import cats.syntax.all._"
ctx.replaceTree(i, "import cats.syntax.all._")
} else {
// the import is only used to import instances,
// so it's safe to remove
removeImportLine(ctx)(i)
}
} catch {
case e: scalafix.v1.MissingSymbolException =>
// see https://github.com/typelevel/cats/pull/3566#issuecomment-684007028
// and https://github.com/scalacenter/scalafix/issues/1123
println(s"Skipping rewrite of 'import cats.implicits._' in file ${ctx.input.label} because we ran into a Scalafix bug. $e")
e.printStackTrace()
Patch.empty
}
}.asPatch

private def removeImportLine(ctx: RuleCtx)(i: Import): Patch =
ctx.removeTokens(i.tokens) + removeWhitespaceAndNewlineBefore(ctx)(i.tokens.start)

private def containsImplicitConversion(synthetic: Synthetic) =
synthetic.names.exists(x => isCatsKernelConversion(x.symbol))

private def isCatsKernelConversion(symbol: Symbol) =
symbol.syntax.contains("cats/kernel") && symbol.syntax.contains("Conversion")

private def containsCatsSyntax(synthetic: Synthetic) =
synthetic.names.exists(x => isCatsSyntax(x.symbol))

private def isCatsSyntax(symbol: Symbol) =
symbol.syntax.contains("cats") && (symbol.syntax.contains("syntax") || symbol.syntax.contains("Ops"))

private def findLexicalBoundary(t: Tree): Tree = {
t.parent match {
case Some(b: Term.Block) => b
case Some(t: Template) => t
case Some(parent) => findLexicalBoundary(parent)
case None => t
}
}

private def removeWhitespaceAndNewlineBefore(ctx: RuleCtx)(index: Int): Patch = {
val whitespaceAndNewlines = ctx.tokens.take(index).takeRightWhile(t =>
t.is[Token.Space] ||
t.is[Token.Tab] ||
t.is[Token.LF]
)
ctx.removeTokens(whitespaceAndNewlines)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
rule = "scala:fix.v2_2_0.RemoveInstanceImports"
*/
package fix
package to2_2_0

import cats.Semigroup
import scala.concurrent.Future

object RemoveInstanceImportsTests {
{
import cats.instances.option._
import cats.instances.int._
Semigroup[Option[Int]].combine(Some(1), Some(2))
}

{
import cats.instances.all._
Semigroup[Option[Int]].combine(Some(1), Some(2))
}

{
import cats.implicits._
Semigroup[Option[Int]].combine(Some(1), Some(2))
}

{
import cats.instances.option._
import cats.instances.int._
import cats.syntax.semigroup._
Option(1) |+| Option(2)
}

{
import cats.implicits._
1.some |+| 2.some
}

{
import cats.instances.future._
import cats.instances.int._
import scala.concurrent.ExecutionContext.Implicits.global
Semigroup[Future[Int]]
}

{
import cats.instances.all._
import scala.concurrent.ExecutionContext.Implicits.global
Semigroup[Future[Int]]
}

{
import cats.implicits._
import scala.concurrent.ExecutionContext.Implicits.global
Semigroup[Future[Int]]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
rule = "scala:fix.v2_2_0.RemoveInstanceImports"
*/
package fix
package to2_2_0

import cats.implicits._

object RemoveInstanceImportsTests2 {
val x = "hello".some
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
rule = "scala:fix.v2_2_0.RemoveInstanceImports"
*/
package fix
package to2_2_0

import cats._
import cats.implicits._

object RemoveInstanceImportsTests3 {

def doSomethingMonadic[F[_]: Monad](x: F[Int]): F[String] =
for {
a <- x
b <- Monad[F].pure("hi")
c <- Monad[F].pure("hey")
} yield a.toString + b + c

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
rule = "scala:fix.v2_2_0.RemoveInstanceImports"
*/
package fix
package to2_2_0

import cats.implicits._
import cats.Order

sealed trait Resolver extends Product with Serializable {
import Resolver._

val path: String = {
val url = this match {
case MavenRepository(_, location, _) => location
case IvyRepository(_, pattern, _) => pattern.takeWhile(!Set('[', '(')(_))
}
url.replace(":", "")
}
}

object Resolver {
final case class Credentials(user: String, pass: String)

final case class MavenRepository(name: String, location: String, credentials: Option[Credentials])
extends Resolver
final case class IvyRepository(name: String, pattern: String, credentials: Option[Credentials])
extends Resolver

val mavenCentral: MavenRepository =
MavenRepository("public", "https://repo1.maven.org/maven2/", None)

implicit val resolverOrder: Order[Resolver] =
Order.by {
case MavenRepository(name, location, _) => (1, name, location)
case IvyRepository(name, pattern, _) => (2, name, pattern)
}
}

final case class Scope[A](value: A, resolvers: List[Resolver])

object Scope {

def combineByResolvers[A: Order](scopes: List[Scope[List[A]]]): List[Scope[List[A]]] =
scopes.groupByNel(_.resolvers).toList.map {
case (resolvers, group) => Scope(group.reduceMap(_.value).distinct.sorted, resolvers)
}

implicit def scopeOrder[A: Order]: Order[Scope[A]] =
Order.by((scope: Scope[A]) => (scope.value, scope.resolvers))
}
Loading

0 comments on commit 1719f64

Please sign in to comment.