From 6ff875527766795233a4761d7ba811222aa3a82c Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Thu, 23 Nov 2023 01:17:35 +0100 Subject: [PATCH] OrganizeImports: favor Scala 3 syntax on Scala 3 sources --- .../internal/rule/OrganizeImports.scala | 127 +++++++++++------- .../organizeImports/CoalesceImportees.scala | 0 .../DeduplicateImportees.scala | 0 .../GroupedImportsExplodeMixed.scala | 0 .../GroupedImportsMergeDedup.scala | 0 .../GroupedImportsMergeRenames.scala | 0 .../GroupedImportsMergeUnimports.scala | 0 .../GroupedImportsMergeWildcard.scala | 0 .../organizeImports/CoalesceImportees.scala | 8 ++ .../CoalesceImporteesGivensAndNames.scala | 2 +- .../CoalesceImporteesNoGivens.scala | 2 +- .../CoalesceImporteesNoNames.scala | 2 +- .../DeduplicateImportees.scala | 7 + .../GroupedGivenImportsMergeUnimports.scala | 4 +- .../GroupedImportsExplodeMixed.scala | 7 + .../GroupedImportsMergeDedup.scala | 5 + .../GroupedImportsMergeRenames.scala | 7 + .../GroupedImportsMergeUnimports.scala | 6 + .../GroupedImportsMergeWildcard.scala | 7 + 19 files changed, 130 insertions(+), 54 deletions(-) rename scalafix-tests/output/src/main/{scala => scala-2}/test/organizeImports/CoalesceImportees.scala (100%) rename scalafix-tests/output/src/main/{scala => scala-2}/test/organizeImports/DeduplicateImportees.scala (100%) rename scalafix-tests/output/src/main/{scala => scala-2}/test/organizeImports/GroupedImportsExplodeMixed.scala (100%) rename scalafix-tests/output/src/main/{scala => scala-2}/test/organizeImports/GroupedImportsMergeDedup.scala (100%) rename scalafix-tests/output/src/main/{scala => scala-2}/test/organizeImports/GroupedImportsMergeRenames.scala (100%) rename scalafix-tests/output/src/main/{scala => scala-2}/test/organizeImports/GroupedImportsMergeUnimports.scala (100%) rename scalafix-tests/output/src/main/{scala => scala-2}/test/organizeImports/GroupedImportsMergeWildcard.scala (100%) create mode 100644 scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImportees.scala create mode 100644 scalafix-tests/output/src/main/scala-3/test/organizeImports/DeduplicateImportees.scala create mode 100644 scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsExplodeMixed.scala create mode 100644 scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeDedup.scala create mode 100644 scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeRenames.scala create mode 100644 scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeUnimports.scala create mode 100644 scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeWildcard.scala diff --git a/scalafix-rules/src/main/scala/scalafix/internal/rule/OrganizeImports.scala b/scalafix-rules/src/main/scala/scalafix/internal/rule/OrganizeImports.scala index 9abbb30158..bfd5fc62b7 100644 --- a/scalafix-rules/src/main/scala/scalafix/internal/rule/OrganizeImports.scala +++ b/scalafix-rules/src/main/scala/scalafix/internal/rule/OrganizeImports.scala @@ -28,6 +28,7 @@ import metaconfig.ConfEncoder import metaconfig.ConfOps import metaconfig.Configured import metaconfig.internal.ConfGet +import scalafix.internal.config.ScalaVersion import scalafix.internal.rule.ImportMatcher.* import scalafix.internal.rule.ImportMatcher.--- import scalafix.internal.rule.ImportMatcher.parse @@ -42,9 +43,12 @@ import scalafix.v1.Symbol import scalafix.v1.SymbolInformation import scalafix.v1.XtensionSeqPatch import scalafix.v1.XtensionTreeScalafix +import scala.meta.Dialect -class OrganizeImports(config: OrganizeImportsConfig) - extends SemanticRule("OrganizeImports") { +class OrganizeImports( + config: OrganizeImportsConfig, + implicit val dialect: Dialect +) extends SemanticRule("OrganizeImports") { import OrganizeImports._ import ImportMatcher._ @@ -58,7 +62,7 @@ class OrganizeImports(config: OrganizeImportsConfig) private val diagnostics: ArrayBuffer[Diagnostic] = ArrayBuffer.empty[Diagnostic] - def this() = this(OrganizeImportsConfig()) + def this() = this(OrganizeImportsConfig(), Dialect.current) override def isLinter: Boolean = true @@ -336,7 +340,10 @@ class OrganizeImports(config: OrganizeImportsConfig) // erases the source position information so that the pretty-printer would actually // pretty-print an `Importer` into a single line. case ImportsOrder.Ascii => - importeesSorted sortBy (i => importerSyntax(i.copy())) + importeesSorted sortBy {i => + println(importerSyntax(i.copy())) + importerSyntax(i.copy()) + } case ImportsOrder.SymbolsFirst => sortImportersSymbolsFirst(importeesSorted) case ImportsOrder.Keep => @@ -564,7 +571,8 @@ class OrganizeImports(config: OrganizeImportsConfig) importer match { case Importer(_, Importee.Wildcard() :: Nil) => - syntax.patch(syntax.lastIndexOfSlice("._"), ".\u0001", 2) + val wildcardSyntax = Importee.Wildcard().syntax + syntax.patch(syntax.lastIndexOfSlice(s".$wildcardSyntax"), ".\u0001", 2) case _ if importer.isCurlyBraced => syntax @@ -685,6 +693,49 @@ class OrganizeImports(config: OrganizeImportsConfig) Patch.addLeft(token, indented mkString "\n") } + + + private def prettyPrintImportGroup(group: Seq[Importer]): String = { + group + .map(i => "import " + importerSyntax(i)) + .mkString("\n") + } + /** + * HACK: The Scalafix pretty-printer decides to add spaces after open and + * before close braces in imports with multiple importees, i.e., `import a.{ + * b, c }` instead of `import a.{b, c}`. On the other hand, renames are + * pretty-printed without the extra spaces, e.g., `import a.{b => c}`. This + * behavior is not customizable and makes ordering imports by ASCII order + * complicated. + * + * This function removes the unwanted spaces as a workaround. In cases where + * users do want the inserted spaces, Scalafmt should be used after running + * the `OrganizeImports` rule. + */ + private def importerSyntax(importer: Importer): String = + importer.pos match { + case pos: Position.Range => + // Position found, implies that `importer` was directly parsed from the source code. Returns + // the original parsed text to preserve the original source level formatting. + pos.text + + case Position.None => + // Position not found, implies that `importer` is derived from certain existing import + // statement(s). Pretty-prints it. + val syntax = importer.syntax + + // NOTE: We need to check whether the input importer is curly braced first and then replace + // the first "{ " and the last " }" if any. Naive string replacement is insufficient, e.g., + // a quoted-identifier like "`{ d }`" may cause broken output. + (importer.isCurlyBraced, syntax lastIndexOfSlice " }") match { + case (_, -1) => + syntax + case (true, index) => + syntax.patch(index, "}", 2).replaceFirst("\\{ ", "{") + case _ => + syntax + } + } } object OrganizeImports { @@ -717,8 +768,28 @@ object OrganizeImports { } } + val maybeDialect = ScalaVersion.from(scalaVersion).map { scalaVersion => + def extractSuffixForScalacOption(prefix: String) = { + scalacOptions + .filter(_.startsWith(prefix)) + .lastOption + .map(_.stripPrefix(prefix)) + } + + // We only lookup the Scala 2 option (Scala 3 is `-source`), as the latest Scala 3 + // dialect is used no matter what the actual minor version is anyway, and as of now, + // the pretty printer is just more permissive with the latest dialect. + val sourceScalaVersion = + extractSuffixForScalacOption("-Xsource:") + .flatMap(ScalaVersion.from(_).toOption) + + scalaVersion.dialect(sourceScalaVersion) + } + if (!conf.removeUnused || hasWarnUnused) - Configured.ok(new OrganizeImports(conf)) + Configured.ok( + new OrganizeImports(conf, maybeDialect.getOrElse(Dialect.current)) + ) else if (hasCompilerSupport) Configured.error( "The Scala compiler option \"-Ywarn-unused\" is required to use OrganizeImports with" @@ -776,48 +847,6 @@ object OrganizeImports { } } - private def prettyPrintImportGroup(group: Seq[Importer]): String = - group - .map(i => "import " + importerSyntax(i)) - .mkString("\n") - - /** - * HACK: The Scalafix pretty-printer decides to add spaces after open and - * before close braces in imports with multiple importees, i.e., `import a.{ - * b, c }` instead of `import a.{b, c}`. On the other hand, renames are - * pretty-printed without the extra spaces, e.g., `import a.{b => c}`. This - * behavior is not customizable and makes ordering imports by ASCII order - * complicated. - * - * This function removes the unwanted spaces as a workaround. In cases where - * users do want the inserted spaces, Scalafmt should be used after running - * the `OrganizeImports` rule. - */ - private def importerSyntax(importer: Importer): String = - importer.pos match { - case pos: Position.Range => - // Position found, implies that `importer` was directly parsed from the source code. Returns - // the original parsed text to preserve the original source level formatting. - pos.text - - case Position.None => - // Position not found, implies that `importer` is derived from certain existing import - // statement(s). Pretty-prints it. - val syntax = importer.syntax - - // NOTE: We need to check whether the input importer is curly braced first and then replace - // the first "{ " and the last " }" if any. Naive string replacement is insufficient, e.g., - // a quoted-identifier like "`{ d }`" may cause broken output. - (importer.isCurlyBraced, syntax lastIndexOfSlice " }") match { - case (_, -1) => - syntax - case (true, index) => - syntax.patch(index, "}", 2).replaceFirst("\\{ ", "{") - case _ => - syntax - } - } - @tailrec private def topQualifierOf(term: Term): Term.Name = term match { case Term.Select(qualifier, _) => topQualifierOf(qualifier) @@ -912,7 +941,7 @@ object OrganizeImports { * - Unimports, e.g., `{Foo => _}`. * - Givens, e.g., `{given Foo}`. * - GivenAll, i.e., `given`. - * - Wildcard, i.e., `_`. + * - Wildcard, i.e., `_` or `*`. */ object Importees { def unapply(importees: Seq[Importee]): Option[ diff --git a/scalafix-tests/output/src/main/scala/test/organizeImports/CoalesceImportees.scala b/scalafix-tests/output/src/main/scala-2/test/organizeImports/CoalesceImportees.scala similarity index 100% rename from scalafix-tests/output/src/main/scala/test/organizeImports/CoalesceImportees.scala rename to scalafix-tests/output/src/main/scala-2/test/organizeImports/CoalesceImportees.scala diff --git a/scalafix-tests/output/src/main/scala/test/organizeImports/DeduplicateImportees.scala b/scalafix-tests/output/src/main/scala-2/test/organizeImports/DeduplicateImportees.scala similarity index 100% rename from scalafix-tests/output/src/main/scala/test/organizeImports/DeduplicateImportees.scala rename to scalafix-tests/output/src/main/scala-2/test/organizeImports/DeduplicateImportees.scala diff --git a/scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsExplodeMixed.scala b/scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsExplodeMixed.scala similarity index 100% rename from scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsExplodeMixed.scala rename to scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsExplodeMixed.scala diff --git a/scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsMergeDedup.scala b/scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsMergeDedup.scala similarity index 100% rename from scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsMergeDedup.scala rename to scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsMergeDedup.scala diff --git a/scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsMergeRenames.scala b/scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsMergeRenames.scala similarity index 100% rename from scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsMergeRenames.scala rename to scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsMergeRenames.scala diff --git a/scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsMergeUnimports.scala b/scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsMergeUnimports.scala similarity index 100% rename from scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsMergeUnimports.scala rename to scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsMergeUnimports.scala diff --git a/scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsMergeWildcard.scala b/scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsMergeWildcard.scala similarity index 100% rename from scalafix-tests/output/src/main/scala/test/organizeImports/GroupedImportsMergeWildcard.scala rename to scalafix-tests/output/src/main/scala-2/test/organizeImports/GroupedImportsMergeWildcard.scala diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImportees.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImportees.scala new file mode 100644 index 0000000000..51fb1f21b1 --- /dev/null +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImportees.scala @@ -0,0 +1,8 @@ +package test.organizeImports + +import scala.collection.immutable.{Map, Seq, Vector} +import scala.collection.mutable.* +import scala.concurrent.{Channel as Ch, *} +import scala.util.{Random as _, *} + +object CoalesceImportees diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesGivensAndNames.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesGivensAndNames.scala index e9530cea6c..c79180f488 100644 --- a/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesGivensAndNames.scala +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesGivensAndNames.scala @@ -1,6 +1,6 @@ package test.organizeImports import test.organizeImports.Givens._ -import test.organizeImports.Givens.{B => B1, C => _, _, given} +import test.organizeImports.Givens.{B as B1, C as _, *, given} object CoalesceImporteesGivensAndNames diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesNoGivens.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesNoGivens.scala index b5bbceec91..851e4a8d90 100644 --- a/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesNoGivens.scala +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesNoGivens.scala @@ -1,5 +1,5 @@ package test.organizeImports -import test.organizeImports.Givens.{C => C1, _} +import test.organizeImports.Givens.{C as C1, *} object CoalesceImporteesNoGivens diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesNoNames.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesNoNames.scala index c6a2d29400..f63f64440c 100644 --- a/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesNoNames.scala +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/CoalesceImporteesNoNames.scala @@ -1,6 +1,6 @@ package test.organizeImports import test.organizeImports.Givens._ -import test.organizeImports.Givens.{A => A1, given} +import test.organizeImports.Givens.{A as A1, given} object CoalesceImporteesNoNames diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/DeduplicateImportees.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/DeduplicateImportees.scala new file mode 100644 index 0000000000..e860e44d69 --- /dev/null +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/DeduplicateImportees.scala @@ -0,0 +1,7 @@ +package test.organizeImports + +import scala.collection.immutable.Vector +import scala.collection.immutable.{Map => Dict} +import scala.collection.immutable.{Set as _, *} + +object DeduplicateImportees diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedGivenImportsMergeUnimports.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedGivenImportsMergeUnimports.scala index 4a1a27b267..152026ece9 100644 --- a/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedGivenImportsMergeUnimports.scala +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedGivenImportsMergeUnimports.scala @@ -1,8 +1,8 @@ package test.organizeImports import test.organizeImports.GivenImports._ -import test.organizeImports.GivenImports.{gamma => _, given Beta, given Zeta, given} -import test.organizeImports.GivenImports2.{alpha => _, beta => _} +import test.organizeImports.GivenImports.{gamma as _, given Beta, given Zeta, given} +import test.organizeImports.GivenImports2.{alpha as _, beta as _} import test.organizeImports.GivenImports2.{given Gamma, given Zeta} object GroupedGivenImportsMergeUnimports diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsExplodeMixed.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsExplodeMixed.scala new file mode 100644 index 0000000000..757f398350 --- /dev/null +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsExplodeMixed.scala @@ -0,0 +1,7 @@ +package test.organizeImports + +import scala.collection.immutable._ +import scala.collection.mutable.Map +import scala.collection.mutable.{Buffer as _, Seq as S, *} + +object GroupedImportsExplodeMixed diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeDedup.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeDedup.scala new file mode 100644 index 0000000000..ce51ff68f0 --- /dev/null +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeDedup.scala @@ -0,0 +1,5 @@ +package test.organizeImports + +import test.organizeImports.MergeImports.Dedup.{a, b as b1, c as _} + +object GroupedImportsMergeDedup diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeRenames.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeRenames.scala new file mode 100644 index 0000000000..d50ab9199a --- /dev/null +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeRenames.scala @@ -0,0 +1,7 @@ +package test.organizeImports + +import test.organizeImports.MergeImports.Rename1.{a as A, b as B, c, d} +import test.organizeImports.MergeImports.Rename2.{a as A, b as B, c} +import test.organizeImports.MergeImports.Rename2.{a, b} + +object GroupedImportsMergeRenames diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeUnimports.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeUnimports.scala new file mode 100644 index 0000000000..27cb850b09 --- /dev/null +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeUnimports.scala @@ -0,0 +1,6 @@ +package test.organizeImports + +import test.organizeImports.MergeImports.Unimport1.{b as B, c as _, d, *} +import test.organizeImports.MergeImports.Unimport2.{a as _, b as _, c as C, d} + +object GroupedImportsMergeUnimports diff --git a/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeWildcard.scala b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeWildcard.scala new file mode 100644 index 0000000000..03acf518c1 --- /dev/null +++ b/scalafix-tests/output/src/main/scala-3/test/organizeImports/GroupedImportsMergeWildcard.scala @@ -0,0 +1,7 @@ +package test.organizeImports + +import test.organizeImports.MergeImports.Wildcard1.{b => B} +import test.organizeImports.MergeImports.Wildcard1.{d, *} +import test.organizeImports.MergeImports.Wildcard2.{a, b, *} + +object GroupedImportsMergeWildcard