From 20dca50f7f5fc3f24dd572dbecf67fb402fdaa46 Mon Sep 17 00:00:00 2001 From: Arman Bilge Date: Mon, 20 Jun 2022 04:29:45 +0000 Subject: [PATCH 1/4] Add Cats RemoveInstancesImport rule Co-authored-by: Chris Birchall --- build.sbt | 5 + .../fix/RemoveInstanceImportsTests1.scala | 56 +++++++++ .../fix/RemoveInstanceImportsTests2.scala | 10 ++ .../fix/RemoveInstanceImportsTests3.scala | 18 +++ .../fix/RemoveInstanceImportsTests4.scala | 50 ++++++++ .../fix/RemoveInstanceImportsTests1.scala | 43 +++++++ .../fix/RemoveInstanceImportsTests2.scala | 7 ++ .../fix/RemoveInstanceImportsTests3.scala | 15 +++ .../fix/RemoveInstanceImportsTests4.scala | 47 ++++++++ .../META-INF/services/scalafix.v0.Rule | 1 + .../fix/CatsRemoveInstanceImports.scala | 110 ++++++++++++++++++ 11 files changed, 362 insertions(+) create mode 100644 modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests1.scala create mode 100644 modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests2.scala create mode 100644 modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests3.scala create mode 100644 modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests4.scala create mode 100644 modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests1.scala create mode 100644 modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests2.scala create mode 100644 modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests3.scala create mode 100644 modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests4.scala create mode 100644 modules/cats/rules/src/main/resources/META-INF/services/scalafix.v0.Rule create mode 100644 modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala diff --git a/build.sbt b/build.sbt index 358280d..ce10bda 100644 --- a/build.sbt +++ b/build.sbt @@ -37,6 +37,11 @@ lazy val cats = scalafixProject("cats") "org.typelevel" %% "cats-core" % CatsVersion ) ) + .outputSettings( + libraryDependencies ++= Seq( + "org.typelevel" %% "cats-core" % CatsVersion + ) + ) // typelevel/cats-effect Scalafix rules lazy val catsEffect = scalafixProject("cats-effect") diff --git a/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests1.scala b/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests1.scala new file mode 100644 index 0000000..3076057 --- /dev/null +++ b/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests1.scala @@ -0,0 +1,56 @@ +/* +rule = TypelevelCatsRemoveInstanceImports + */ +package fix + +import cats.Semigroup +import scala.concurrent.Future + +object RemoveInstanceImportsTests1 { + { + 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]] + } +} diff --git a/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests2.scala b/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests2.scala new file mode 100644 index 0000000..387eb2b --- /dev/null +++ b/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests2.scala @@ -0,0 +1,10 @@ +/* +rule = TypelevelCatsRemoveInstanceImports + */ +package fix + +import cats.implicits._ + +object RemoveInstanceImportsTests2 { + val x = "hello".some +} diff --git a/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests3.scala b/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests3.scala new file mode 100644 index 0000000..311b990 --- /dev/null +++ b/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests3.scala @@ -0,0 +1,18 @@ +/* +rule = TypelevelCatsRemoveInstanceImports + */ +package fix + +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 + +} diff --git a/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests4.scala b/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests4.scala new file mode 100644 index 0000000..92a2943 --- /dev/null +++ b/modules/cats/input/src/main/scala/fix/RemoveInstanceImportsTests4.scala @@ -0,0 +1,50 @@ +/* +rule = TypelevelCatsRemoveInstanceImports + */ +package fix + +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)) +} diff --git a/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests1.scala b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests1.scala new file mode 100644 index 0000000..4093473 --- /dev/null +++ b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests1.scala @@ -0,0 +1,43 @@ +package fix + +import cats.Semigroup +import scala.concurrent.Future + +object RemoveInstanceImportsTests { + { + Semigroup[Option[Int]].combine(Some(1), Some(2)) + } + + { + Semigroup[Option[Int]].combine(Some(1), Some(2)) + } + + { + Semigroup[Option[Int]].combine(Some(1), Some(2)) + } + + { + import cats.syntax.semigroup._ + Option(1) |+| Option(2) + } + + { + import cats.syntax.all._ + 1.some |+| 2.some + } + + { + import scala.concurrent.ExecutionContext.Implicits.global + Semigroup[Future[Int]] + } + + { + import scala.concurrent.ExecutionContext.Implicits.global + Semigroup[Future[Int]] + } + + { + import scala.concurrent.ExecutionContext.Implicits.global + Semigroup[Future[Int]] + } +} diff --git a/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests2.scala b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests2.scala new file mode 100644 index 0000000..58c62b8 --- /dev/null +++ b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests2.scala @@ -0,0 +1,7 @@ +package fix + +import cats.syntax.all._ + +object RemoveInstanceImportsTests2 { + val x = "hello".some +} diff --git a/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests3.scala b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests3.scala new file mode 100644 index 0000000..c765dd9 --- /dev/null +++ b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests3.scala @@ -0,0 +1,15 @@ +package fix + +import cats._ +import cats.syntax.all._ + +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 + +} diff --git a/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests4.scala b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests4.scala new file mode 100644 index 0000000..9155573 --- /dev/null +++ b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests4.scala @@ -0,0 +1,47 @@ +package fix + +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)) +} diff --git a/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v0.Rule b/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v0.Rule new file mode 100644 index 0000000..4718c6f --- /dev/null +++ b/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v0.Rule @@ -0,0 +1 @@ +org.typelevel.fix.CatsRemoveInstanceImports diff --git a/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala b/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala new file mode 100644 index 0000000..b798a26 --- /dev/null +++ b/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala @@ -0,0 +1,110 @@ +/* + * Copyright 2022 Typelevel + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.typelevel.fix + +import scalafix.v0._ +import scalafix.syntax._ +import scala.meta._ +import scala.meta.contrib._ + +// ref: https://github.com/typelevel/cats/issues/3563 +case class CatsRemoveInstanceImports(index: SemanticdbIndex) + extends SemanticRule(index, "TypelevelCatsRemoveInstanceImports") { + + 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")), _), _) :: _) => + 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) + } + +} From 82b45b04687955542df78b9ebeeae4d0f2470f8b Mon Sep 17 00:00:00 2001 From: David Gregory Date: Mon, 18 Jul 2022 15:04:46 +0100 Subject: [PATCH 2/4] WIP port CatsRemoveInstanceImports to scalafix v1 API --- build.sbt | 1 + .../META-INF/services/scalafix.v0.Rule | 1 - .../META-INF/services/scalafix.v1.Rule | 1 + .../fix/CatsRemoveInstanceImports.scala | 90 +++++++++++-------- 4 files changed, 55 insertions(+), 38 deletions(-) delete mode 100644 modules/cats/rules/src/main/resources/META-INF/services/scalafix.v0.Rule diff --git a/build.sbt b/build.sbt index f789beb..9f692c6 100644 --- a/build.sbt +++ b/build.sbt @@ -34,6 +34,7 @@ lazy val `typelevel-scalafix-rules` = project // typelevel/cats Scalafix rules lazy val cats = scalafixProject("cats") .inputSettings( + semanticdbOptions += "-P:semanticdb:synthetics:on", libraryDependencies ++= Seq( "org.typelevel" %% "cats-core" % CatsVersion ) diff --git a/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v0.Rule b/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v0.Rule deleted file mode 100644 index 4718c6f..0000000 --- a/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v0.Rule +++ /dev/null @@ -1 +0,0 @@ -org.typelevel.fix.CatsRemoveInstanceImports diff --git a/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index d1b5ca0..7a45b3b 100644 --- a/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/modules/cats/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -1,3 +1,4 @@ org.typelevel.fix.MapSequence org.typelevel.fix.UnusedShowInterpolator org.typelevel.fix.As +org.typelevel.fix.CatsRemoveInstanceImports \ No newline at end of file diff --git a/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala b/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala index b798a26..c2799b3 100644 --- a/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala +++ b/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala @@ -16,35 +16,34 @@ package org.typelevel.fix -import scalafix.v0._ -import scalafix.syntax._ +import scalafix.v1._ import scala.meta._ -import scala.meta.contrib._ // ref: https://github.com/typelevel/cats/issues/3563 -case class CatsRemoveInstanceImports(index: SemanticdbIndex) - extends SemanticRule(index, "TypelevelCatsRemoveInstanceImports") { +class CatsRemoveInstanceImports extends SemanticRule("TypelevelCatsRemoveInstanceImports") { - override def fix(ctx: RuleCtx): Patch = ctx.tree.collect { + override def fix(implicit doc: SemanticDocument): Patch = doc.tree.collect { // e.g. "import cats.instances.int._" or "import cats.instances.all._" - case i @ Import(Importer(Select(Select(Name("cats"), Name("instances")), _), _) :: _) => - removeImportLine(ctx)(i) + case i @ Import( + Importer(Term.Select(Term.Select(Name("cats"), Name("instances")), _), _) :: _ + ) => + removeImportLine(doc)(i) // "import cats.implicits._" - case i @ Import(Importer(Select(Name("cats"), Name("implicits")), _) :: _) => - val boundary = findLexicalBoundary(i) + case i @ Import(Importer(Term.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 + // // 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 relevantSynthetics = + // doc.synthetics.filter(x => + // x.symbol.flatMap(_.info).map(_.) + // ) - val usesImplicitConversion = relevantSynthetics.exists(containsImplicitConversion) - val usesSyntax = relevantSynthetics.exists(containsCatsSyntax) + val usesImplicitConversion = doc.synthetics.exists(containsImplicitConversion) + val usesSyntax = doc.synthetics.exists(containsCatsSyntax) if (usesImplicitConversion) { // the import is used to enable an implicit conversion, @@ -53,39 +52,56 @@ case class CatsRemoveInstanceImports(index: SemanticdbIndex) } 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._") + Patch.replaceTree(i, "import cats.syntax.all._") } else { // the import is only used to import instances, // so it's safe to remove - removeImportLine(ctx)(i) + removeImportLine(doc)(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" - ) + doc.input match { + case Input.File(path, _) => + println( + s"Skipping rewrite of 'import cats.implicits._' in file ${path.syntax} because we ran into a Scalafix bug. $e" + ) + case _ => + println( + s"Skipping rewrite of 'import cats.implicits._' 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 removeImportLine(doc: SemanticDocument)(i: Import): Patch = + Patch.removeTokens(i.tokens) + removeWhitespaceAndNewlineBefore(doc)(i.tokens.start) - private def containsImplicitConversion(synthetic: Synthetic) = - synthetic.names.exists(x => isCatsKernelConversion(x.symbol)) + private def containsImplicitConversion(synthetic: SemanticTree)(implicit doc: SemanticDocument) = + synthetic match { + case ApplyTree(fn, _) => + fn.symbol.map(sym => isCatsKernelConversion(sym)).getOrElse(false) + case _ => + false + } 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)) + symbol.value.contains("cats/kernel") && symbol.value.contains("Conversion") + + private def containsCatsSyntax(synthetic: SemanticTree)(implicit doc: SemanticDocument) = + synthetic match { + case ApplyTree(fn, _) => + fn.symbol.map(isCatsSyntax).getOrElse(false) + case _ => + false + } private def isCatsSyntax(symbol: Symbol) = - symbol.syntax - .contains("cats") && (symbol.syntax.contains("syntax") || symbol.syntax.contains("Ops")) + symbol.value + .contains("cats") && (symbol.value.contains("syntax") || symbol.value.contains("Ops")) private def findLexicalBoundary(t: Tree): Tree = { t.parent match { @@ -96,15 +112,15 @@ case class CatsRemoveInstanceImports(index: SemanticdbIndex) } } - private def removeWhitespaceAndNewlineBefore(ctx: RuleCtx)(index: Int): Patch = { - val whitespaceAndNewlines = ctx.tokens + private def removeWhitespaceAndNewlineBefore(doc: SemanticDocument)(index: Int): Patch = { + val whitespaceAndNewlines = doc.tokens .take(index) .takeRightWhile(t => t.is[Token.Space] || t.is[Token.Tab] || t.is[Token.LF] ) - ctx.removeTokens(whitespaceAndNewlines) + Patch.removeTokens(whitespaceAndNewlines) } } From c17c2c0be59b48ceaa7ab6d6d284d478d4e6bcd0 Mon Sep 17 00:00:00 2001 From: Antonio Gelameris Date: Fri, 5 Jul 2024 13:23:42 +0200 Subject: [PATCH 3/4] Ported CatsRemoveInstanceImports to v1 --- .../fix/RemoveInstanceImportsTests1.scala | 2 +- .../fix/CatsRemoveInstanceImports.scala | 80 +++++++------------ 2 files changed, 29 insertions(+), 53 deletions(-) diff --git a/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests1.scala b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests1.scala index 4093473..cafa078 100644 --- a/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests1.scala +++ b/modules/cats/output/src/main/scala/fix/RemoveInstanceImportsTests1.scala @@ -3,7 +3,7 @@ package fix import cats.Semigroup import scala.concurrent.Future -object RemoveInstanceImportsTests { +object RemoveInstanceImportsTests1 { { Semigroup[Option[Int]].combine(Some(1), Some(2)) } diff --git a/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala b/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala index c2799b3..38865aa 100644 --- a/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala +++ b/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala @@ -19,7 +19,6 @@ package org.typelevel.fix import scalafix.v1._ import scala.meta._ -// ref: https://github.com/typelevel/cats/issues/3563 class CatsRemoveInstanceImports extends SemanticRule("TypelevelCatsRemoveInstanceImports") { override def fix(implicit doc: SemanticDocument): Patch = doc.tree.collect { @@ -31,19 +30,13 @@ class CatsRemoveInstanceImports extends SemanticRule("TypelevelCatsRemoveInstanc // "import cats.implicits._" case i @ Import(Importer(Term.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 = - // doc.synthetics.filter(x => - // x.symbol.flatMap(_.info).map(_.) - // ) - val usesImplicitConversion = doc.synthetics.exists(containsImplicitConversion) - val usesSyntax = doc.synthetics.exists(containsCatsSyntax) + def treeContainsFunctionCallMatching(f: Symbol => Boolean): Boolean = + i.parent.fold(false)(treeContainsFunctionApplicationSymbolMatches(f)) + + def usesImplicitConversion = treeContainsFunctionCallMatching(catsKernelConversion) + def usesSyntax = treeContainsFunctionCallMatching(catsSyntax) if (usesImplicitConversion) { // the import is used to enable an implicit conversion, @@ -77,50 +70,33 @@ class CatsRemoveInstanceImports extends SemanticRule("TypelevelCatsRemoveInstanc } }.asPatch - private def removeImportLine(doc: SemanticDocument)(i: Import): Patch = - Patch.removeTokens(i.tokens) + removeWhitespaceAndNewlineBefore(doc)(i.tokens.start) - - private def containsImplicitConversion(synthetic: SemanticTree)(implicit doc: SemanticDocument) = - synthetic match { - case ApplyTree(fn, _) => - fn.symbol.map(sym => isCatsKernelConversion(sym)).getOrElse(false) - case _ => - false + // Recursively searches for a specific SemanticTree in the terms that this tree is comprised of + private def treeContainsFunctionApplicationSymbolMatches( + f: Symbol => Boolean + )(t: Tree)(implicit doc: SemanticDocument): Boolean = + t match { + case t: Term => + t.synthetics.exists { + case ApplyTree(fn, _) => fn.symbol.fold(false)(f) + case _ => false + } || t.children.exists(treeContainsFunctionApplicationSymbolMatches(f)) + case t => t.children.exists(treeContainsFunctionApplicationSymbolMatches(f)) } - private def isCatsKernelConversion(symbol: Symbol) = - symbol.value.contains("cats/kernel") && symbol.value.contains("Conversion") + private def catsKernelConversion(symbol: Symbol): Boolean = + symbol.value.contains("cats/kernel") // && symbol.value.contains("Conversion") - private def containsCatsSyntax(synthetic: SemanticTree)(implicit doc: SemanticDocument) = - synthetic match { - case ApplyTree(fn, _) => - fn.symbol.map(isCatsSyntax).getOrElse(false) - case _ => - false - } - - private def isCatsSyntax(symbol: Symbol) = - symbol.value - .contains("cats") && (symbol.value.contains("syntax") || symbol.value.contains("Ops")) + private def catsSyntax(symbol: Symbol): Boolean = symbol.value + .contains("cats") && (symbol.value.contains("syntax") || symbol.value.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 removeImportLine(doc: SemanticDocument)(i: Import): Patch = + Patch.removeTokens(i.tokens) + removeWhitespaceAndNewlineBefore(doc)(i.tokens.start) - private def removeWhitespaceAndNewlineBefore(doc: SemanticDocument)(index: Int): Patch = { - val whitespaceAndNewlines = doc.tokens - .take(index) - .takeRightWhile(t => - t.is[Token.Space] || - t.is[Token.Tab] || - t.is[Token.LF] - ) - Patch.removeTokens(whitespaceAndNewlines) - } + private def removeWhitespaceAndNewlineBefore(doc: SemanticDocument)(index: Int): Patch = + Patch.removeTokens( + doc.tokens + .take(index) + .takeRightWhile(t => t.is[Token.Space] || t.is[Token.Tab] || t.is[Token.LF]) + ) } From 4902d601ffb3936717eb2d20e9bb3fd69c5cd69c Mon Sep 17 00:00:00 2001 From: Antonio Gelameris Date: Fri, 5 Jul 2024 14:33:24 +0200 Subject: [PATCH 4/4] Slight optimisation --- .../fix/CatsRemoveInstanceImports.scala | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala b/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala index 38865aa..c80ab73 100644 --- a/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala +++ b/modules/cats/rules/src/main/scala/org/typelevel/fix/CatsRemoveInstanceImports.scala @@ -23,20 +23,19 @@ class CatsRemoveInstanceImports extends SemanticRule("TypelevelCatsRemoveInstanc override def fix(implicit doc: SemanticDocument): Patch = doc.tree.collect { // e.g. "import cats.instances.int._" or "import cats.instances.all._" - case i @ Import( - Importer(Term.Select(Term.Select(Name("cats"), Name("instances")), _), _) :: _ - ) => - removeImportLine(doc)(i) + case CatsInstancesImport(i) => removeImportLine(doc)(i) // "import cats.implicits._" - case i @ Import(Importer(Term.Select(Name("cats"), Name("implicits")), _) :: _) => + case CatsImplicitsImport(i) => try { + // These are all the sibling trees of the import + val siblings: List[Tree] = i.parent.fold(List.empty[Tree])(_.children).filterNot(_ == i) - def treeContainsFunctionCallMatching(f: Symbol => Boolean): Boolean = - i.parent.fold(false)(treeContainsFunctionApplicationSymbolMatches(f)) + def usesImplicitConversion: Boolean = + siblings.exists(treeContainsFunctionApplicationSymbolMatching(catsKernelConversion)) - def usesImplicitConversion = treeContainsFunctionCallMatching(catsKernelConversion) - def usesSyntax = treeContainsFunctionCallMatching(catsSyntax) + def usesSyntax: Boolean = + siblings.exists(treeContainsFunctionApplicationSymbolMatching(catsSyntax)) if (usesImplicitConversion) { // the import is used to enable an implicit conversion, @@ -70,8 +69,9 @@ class CatsRemoveInstanceImports extends SemanticRule("TypelevelCatsRemoveInstanc } }.asPatch - // Recursively searches for a specific SemanticTree in the terms that this tree is comprised of - private def treeContainsFunctionApplicationSymbolMatches( + // Recursively searches for a function symbol matching the predicate + // in all the function applications among all the children of the Tree t + private def treeContainsFunctionApplicationSymbolMatching( f: Symbol => Boolean )(t: Tree)(implicit doc: SemanticDocument): Boolean = t match { @@ -79,8 +79,8 @@ class CatsRemoveInstanceImports extends SemanticRule("TypelevelCatsRemoveInstanc t.synthetics.exists { case ApplyTree(fn, _) => fn.symbol.fold(false)(f) case _ => false - } || t.children.exists(treeContainsFunctionApplicationSymbolMatches(f)) - case t => t.children.exists(treeContainsFunctionApplicationSymbolMatches(f)) + } || t.children.exists(treeContainsFunctionApplicationSymbolMatching(f)) + case t => t.children.exists(treeContainsFunctionApplicationSymbolMatching(f)) } private def catsKernelConversion(symbol: Symbol): Boolean = @@ -98,5 +98,22 @@ class CatsRemoveInstanceImports extends SemanticRule("TypelevelCatsRemoveInstanc .take(index) .takeRightWhile(t => t.is[Token.Space] || t.is[Token.Tab] || t.is[Token.LF]) ) +} +object CatsInstancesImport { + def unapply(t: Tree): Option[Import] = t match { + case i @ Import( + Importer(Term.Select(Term.Select(Name("cats"), Name("instances")), _), _) :: _ + ) => + Some(i) + case _ => None + } +} + +object CatsImplicitsImport { + def unapply(t: Tree): Option[Import] = t match { + case i @ Import(Importer(Term.Select(Name("cats"), Name("implicits")), _) :: _) => + Some(i) + case _ => None + } }