From 71adbc1b8a078075da07675cbff92a862ad5ce6d Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Sat, 2 May 2020 20:02:43 -0700 Subject: [PATCH] Allow coalescing grouped import selectors into a wildcard (#29) --- README.adoc | 86 ++++++++++++++++++- .../main/scala/fix/CoalesceImportees.scala | 14 +++ input/src/main/scala/fix/ExpandRelative.scala | 1 - .../main/scala/fix/CoalesceImportees.scala | 8 ++ .../src/main/scala/fix/OrganizeImports.scala | 48 +++++++---- .../scala/fix/OrganizeImportsConfig.scala | 7 +- 6 files changed, 141 insertions(+), 23 deletions(-) create mode 100644 input/src/main/scala/fix/CoalesceImportees.scala create mode 100644 output/src/main/scala/fix/CoalesceImportees.scala diff --git a/README.adoc b/README.adoc index 88aa440c3..4f60b9e5f 100644 --- a/README.adoc +++ b/README.adoc @@ -44,6 +44,86 @@ OrganizeImports { } ---- +=== `coalesceToWildcardImportThreshold` + +==== Description + +When the number of imported names exceeds a certain threshold, coalesce them into a wildcard import. Renames and unimports are left untouched. + +==== +WARNING: Coalescing grouped import selectors into a wildcard import may introduce [red]#_compilation errors_#! Having this feature in `OrganizeImports` is mostly for feature parity with the IntelliJ IDEA Scala import optimizer. + +For example, the following snippet compiles successfully: + +[source,scala] +---- +import scala.collection.immutable._ <1> +import scala.collection.mutable.{ArrayBuffer, Map, Set} <2> + +object Example { + val m: Map[Int, Int] = ??? <3> +} +---- +<1> The trait `scala.collection.immutable.Map` is imported via a wildcard. +<2> The trait `scala.collection.mutable.Map` is explicitly imported. +<3> The type `Map` here is not ambiguous because the mutable `Map` takes higher precedence than the immutable `Map`. + +However, if we coalesce the grouped importes in the second import statement into a wildcard, there will be a compilation error: +[source,scala] +---- +import scala.collection.immutable._ <1> +import scala.collection.mutable._ <1> + +object Example { + val m: Map[Int, Int] = ??? <2> +} +---- +<1> Both `scala.collection.immutable.Map` and `scala.collection.mutable.Map` are imported via a wildcard. +<2> Now the type `Map` here becomes ambiguous because both the mutable and immutable `Map` imported take the same precedence. +==== + +==== Value type + +Integer + +==== Default value + +`Int.MaxValue` + +Rationale:: Setting the default value to `Int.MaxValue` essentially disables this feature, since it can may correctness issues. + +==== Example + +Configuration: + +[source,scala] +---- +OrganizeImports { + groupedImports = Keep + coalesceToWildcardImportThreshold = 3 +} +---- + +Before: + +[source,scala] +---- +import scala.collection.immutable.{Seq, Map, Vector, Set} +import scala.collection.immutable.{Seq, Map, Vector} +import scala.collection.immutable.{Seq, Map, Vector => Vec, Set, Stream} +import scala.collection.immutable.{Seq, Map, Vector => _, Set, Stream} +---- + +After: + +[source,scala] +---- +import scala.collection.immutable._ +import scala.collection.immutable.{Map, Seq, Vector} +import scala.collection.immutable.{Vector => Vec, _} +import scala.collection.immutable.{Vector => _, _} +---- + [[expand-relative]] === `expandRelative` @@ -424,7 +504,7 @@ Do not sort import selectors. `Ascii` -==== Example +==== Examples `Ascii`:: + @@ -496,13 +576,13 @@ Enum: `Ascii | SymbolsFirst` Sort import statements by ASCII codes. `SymbolsFirst`:: -Put wildcard imports and grouped imports with braces first, otherwise same as `Ascii`. This is also the sorting order the IntelliJ IDEA Scala plugin picks. +Put wildcard imports and grouped imports with braces first, otherwise same as `Ascii`. This is also the sorting order the IntelliJ IDEA Scala import opitimizer picks. ==== Deafult value `Ascii` -==== Example +==== Examples `Ascii`:: + diff --git a/input/src/main/scala/fix/CoalesceImportees.scala b/input/src/main/scala/fix/CoalesceImportees.scala new file mode 100644 index 000000000..a6bf6a82c --- /dev/null +++ b/input/src/main/scala/fix/CoalesceImportees.scala @@ -0,0 +1,14 @@ +/* +rules = [OrganizeImports] +OrganizeImports.groupedImports = Keep +OrganizeImports.coalesceToWildcardImportThreshold = 3 + */ + +package fix + +import scala.collection.immutable.{Seq, Map, Vector} +import scala.collection.immutable.{Seq, Map, Vector, Set} +import scala.collection.immutable.{Seq, Map, Vector => Vec, Set, Stream} +import scala.collection.immutable.{Seq, Map, Vector => _, Set, Stream} + +object CoalesceImportees diff --git a/input/src/main/scala/fix/ExpandRelative.scala b/input/src/main/scala/fix/ExpandRelative.scala index 335f6f37e..3a6790314 100644 --- a/input/src/main/scala/fix/ExpandRelative.scala +++ b/input/src/main/scala/fix/ExpandRelative.scala @@ -2,7 +2,6 @@ rules = OrganizeImports OrganizeImports.expandRelative = true */ - package fix import scala.util diff --git a/output/src/main/scala/fix/CoalesceImportees.scala b/output/src/main/scala/fix/CoalesceImportees.scala new file mode 100644 index 000000000..087405552 --- /dev/null +++ b/output/src/main/scala/fix/CoalesceImportees.scala @@ -0,0 +1,8 @@ +package fix + +import scala.collection.immutable._ +import scala.collection.immutable.{Map, Seq, Vector} +import scala.collection.immutable.{Vector => Vec, _} +import scala.collection.immutable.{Vector => _, _} + +object CoalesceImportees diff --git a/rules/src/main/scala/fix/OrganizeImports.scala b/rules/src/main/scala/fix/OrganizeImports.scala index 76fb3b6e7..d35f31ccb 100644 --- a/rules/src/main/scala/fix/OrganizeImports.scala +++ b/rules/src/main/scala/fix/OrganizeImports.scala @@ -16,7 +16,13 @@ import scala.util.matching.Regex import metaconfig.Configured import scalafix.patch.Patch -import scalafix.v1._ +import scalafix.v1.Configuration +import scalafix.v1.Rule +import scalafix.v1.RuleName.stringToRuleName +import scalafix.v1.SemanticDocument +import scalafix.v1.SemanticRule +import scalafix.v1.Symbol +import scalafix.v1.XtensionTreeScalafix class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("OrganizeImports") { import OrganizeImports._ @@ -160,22 +166,6 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ else importer.copy(ref = toRef(importer.ref.symbol.normalized)) } - private def sortImportees(importer: Importer): Importer = { - import ImportSelectorsOrder._ - - // The Scala language spec allows an import expression to have at most one final wildcard, which - // can only appears in the last position. - val (wildcard, withoutWildcard) = importer.importees.partition(_.is[Importee.Wildcard]) - - val orderedImportees = config.importSelectorsOrder match { - case Ascii => withoutWildcard.sortBy(_.syntax) - case SymbolsFirst => sortImporteesSymbolsFirst(withoutWildcard) - case Keep => withoutWildcard - } - - importer.copy(importees = orderedImportees ++ wildcard) - } - private def organizeImporters(importers: Seq[Importer]): Seq[Importer] = { import GroupedImports._ import ImportsOrder._ @@ -186,7 +176,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ case Explode => explodeImportees(importers) case Keep => importers } - } map sortImportees + } map coalesceImportees map sortImportees config.importsOrder match { case Ascii => @@ -204,6 +194,28 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ } } + private def coalesceImportees(importer: Importer): Importer = { + val Importees(names, renames, unimports, _) = importer.importees + if (names.length <= config.coalesceToWildcardImportThreshold) importer + else importer.copy(importees = renames ++ unimports :+ Importee.Wildcard()) + } + + private def sortImportees(importer: Importer): Importer = { + import ImportSelectorsOrder._ + + // The Scala language spec allows an import expression to have at most one final wildcard, which + // can only appears in the last position. + val (wildcard, withoutWildcard) = importer.importees.partition(_.is[Importee.Wildcard]) + + val orderedImportees = config.importSelectorsOrder match { + case Ascii => withoutWildcard.sortBy(_.syntax) + case SymbolsFirst => sortImporteesSymbolsFirst(withoutWildcard) + case Keep => withoutWildcard + } + + importer.copy(importees = orderedImportees ++ wildcard) + } + // Returns the index of the group to which the given importer belongs. private def matchImportGroup(importer: Importer): Int = { val matchedGroups = importMatchers diff --git a/rules/src/main/scala/fix/OrganizeImportsConfig.scala b/rules/src/main/scala/fix/OrganizeImportsConfig.scala index d6f13502c..cb8238ab8 100644 --- a/rules/src/main/scala/fix/OrganizeImportsConfig.scala +++ b/rules/src/main/scala/fix/OrganizeImportsConfig.scala @@ -42,9 +42,14 @@ object GroupedImports { } final case class OrganizeImportsConfig( + coalesceToWildcardImportThreshold: Int = Int.MaxValue, expandRelative: Boolean = false, groupedImports: GroupedImports = GroupedImports.Explode, - groups: Seq[String] = Seq("re:javax?\\.", "scala.", "*"), + groups: Seq[String] = Seq( + "re:javax?\\.", + "scala.", + "*" + ), importSelectorsOrder: ImportSelectorsOrder = ImportSelectorsOrder.Ascii, importsOrder: ImportsOrder = ImportsOrder.Ascii, removeUnused: Boolean = true