From 5863e8002ddbf08e4cee88b164a895f149695dca Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Tue, 15 Dec 2020 01:46:39 -0800 Subject: [PATCH 01/11] Add the blankLines configuration --- .../scala/fix/OrganizeImportsConfig.scala | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/rules/src/main/scala/fix/OrganizeImportsConfig.scala b/rules/src/main/scala/fix/OrganizeImportsConfig.scala index 86eaed3..4935d41 100644 --- a/rules/src/main/scala/fix/OrganizeImportsConfig.scala +++ b/rules/src/main/scala/fix/OrganizeImportsConfig.scala @@ -13,10 +13,9 @@ object ImportsOrder { case object SymbolsFirst extends ImportsOrder case object Keep extends ImportsOrder - implicit def reader: ConfDecoder[ImportsOrder] = - ReaderUtil.fromMap( - (List(Ascii, SymbolsFirst, Keep) map (v => v.toString -> v)).toMap - ) + implicit def reader: ConfDecoder[ImportsOrder] = ReaderUtil.fromMap( + (List(Ascii, SymbolsFirst, Keep) map (v => v.toString -> v)).toMap + ) } sealed trait ImportSelectorsOrder @@ -26,10 +25,9 @@ object ImportSelectorsOrder { case object SymbolsFirst extends ImportSelectorsOrder case object Keep extends ImportSelectorsOrder - implicit def reader: ConfDecoder[ImportSelectorsOrder] = - ReaderUtil.fromMap { - (List(Ascii, SymbolsFirst, Keep) map (v => v.toString -> v)).toMap - } + implicit def reader: ConfDecoder[ImportSelectorsOrder] = ReaderUtil.fromMap { + (List(Ascii, SymbolsFirst, Keep) map (v => v.toString -> v)).toMap + } } sealed trait GroupedImports @@ -40,13 +38,24 @@ object GroupedImports { case object Explode extends GroupedImports case object Keep extends GroupedImports - implicit def reader: ConfDecoder[GroupedImports] = - ReaderUtil.fromMap { - (List(AggressiveMerge, Merge, Explode, Keep) map (v => v.toString -> v)).toMap - } + implicit def reader: ConfDecoder[GroupedImports] = ReaderUtil.fromMap { + (List(AggressiveMerge, Merge, Explode, Keep) map (v => v.toString -> v)).toMap + } +} + +sealed trait BlankLines + +object BlankLines { + case object Auto extends BlankLines + case object Manual extends BlankLines + + implicit def reader: ConfDecoder[BlankLines] = ReaderUtil.fromMap { + (List(Auto, Manual) map (v => v.toString -> v)).toMap + } } final case class OrganizeImportsConfig( + blankLines: BlankLines = BlankLines.Auto, coalesceToWildcardImportThreshold: Int = Int.MaxValue, expandRelative: Boolean = false, groupExplicitlyImportedImplicitsSeparately: Boolean = false, From a5bc117a774e7fd5bd71136d991a503e2bae3ef0 Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Tue, 15 Dec 2020 01:55:15 -0800 Subject: [PATCH 02/11] Add the BlankLine ImportMatcher --- rules/src/main/scala/fix/ImportMatcher.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rules/src/main/scala/fix/ImportMatcher.scala b/rules/src/main/scala/fix/ImportMatcher.scala index bcc800c..31e582f 100644 --- a/rules/src/main/scala/fix/ImportMatcher.scala +++ b/rules/src/main/scala/fix/ImportMatcher.scala @@ -12,6 +12,7 @@ object ImportMatcher { def parse(pattern: String): ImportMatcher = pattern match { case p if p startsWith "re:" => ImportMatcher.RE(new Regex(p stripPrefix "re:")) + case "---" => ImportMatcher.BlankLine case "*" => ImportMatcher.Wildcard case p => ImportMatcher.PlainText(p) } @@ -30,4 +31,8 @@ object ImportMatcher { // import group matching process. def matches(importer: Importer): Int = 0 } + + case object BlankLine extends ImportMatcher { + override def matches(i: Importer): Int = 0 + } } From 5d84720f99be83ddc7de337b6a3c28e39120199a Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Tue, 15 Dec 2020 02:08:35 -0800 Subject: [PATCH 03/11] Create blank-line import matchers --- rules/src/main/scala/fix/OrganizeImports.scala | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/rules/src/main/scala/fix/OrganizeImports.scala b/rules/src/main/scala/fix/OrganizeImports.scala index e2c8375..5b331a1 100644 --- a/rules/src/main/scala/fix/OrganizeImports.scala +++ b/rules/src/main/scala/fix/OrganizeImports.scala @@ -32,9 +32,23 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ import OrganizeImports._ private val importMatchers = { - val matchers = config.groups map ImportMatcher.parse // The wildcard group should always exist. Append one at the end if omitted. - if (matchers contains ImportMatcher.Wildcard) matchers else matchers :+ ImportMatcher.Wildcard + val matchers = { + val parsed = config.groups map ImportMatcher.parse + if (parsed contains ImportMatcher.Wildcard) parsed else parsed :+ ImportMatcher.Wildcard + } + + // Inserts blank lines between adjacent import groups if necessary. + config.blankLines match { + case BlankLines.Manual => + matchers + + case BlankLines.Auto => + Seq.tabulate((matchers.length * 2 - 1) max 0) { + case i if i % 2 == 0 => matchers(i / 2) + case _ => ImportMatcher.BlankLine + } + } } private val wildcardGroupIndex: Int = importMatchers indexOf ImportMatcher.Wildcard From 6e2e18def420a31778a30f7338e7fb25569cc6bc Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Wed, 16 Dec 2020 01:58:01 -0800 Subject: [PATCH 04/11] A messy working prototype --- .../src/main/scala/fix/OrganizeImports.scala | 112 +++++++++++------- 1 file changed, 66 insertions(+), 46 deletions(-) diff --git a/rules/src/main/scala/fix/OrganizeImports.scala b/rules/src/main/scala/fix/OrganizeImports.scala index 5b331a1..1ebe6e5 100644 --- a/rules/src/main/scala/fix/OrganizeImports.scala +++ b/rules/src/main/scala/fix/OrganizeImports.scala @@ -38,17 +38,19 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ if (parsed contains ImportMatcher.Wildcard) parsed else parsed :+ ImportMatcher.Wildcard } - // Inserts blank lines between adjacent import groups if necessary. - config.blankLines match { - case BlankLines.Manual => - matchers - - case BlankLines.Auto => - Seq.tabulate((matchers.length * 2 - 1) max 0) { - case i if i % 2 == 0 => matchers(i / 2) - case _ => ImportMatcher.BlankLine - } - } + ( + config.blankLines match { + case BlankLines.Manual => + matchers + + // Inserts blank lines between adjacent import groups automatically. + case BlankLines.Auto => + Seq.tabulate((matchers.length * 2 - 1) max 0) { + case i if i % 2 == 0 => matchers(i / 2) + case _ => ImportMatcher.BlankLine + } + } + ) :+ ImportMatcher.BlankLine } private val wildcardGroupIndex: Int = importMatchers indexOf ImportMatcher.Wildcard @@ -118,7 +120,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ val (fullyQualifiedImporters, relativeImporters) = noImplicits partition isFullyQualified // Organizes all the fully-qualified global importers. - val fullyQualifiedGroups: Seq[Seq[Importer]] = { + val fullyQualifiedGroups: Seq[(Int, Seq[Importer])] = { val expanded = if (config.expandRelative) relativeImporters map expandRelative else Nil groupImporters(fullyQualifiedImporters ++ expanded) } @@ -131,13 +133,14 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ // require special handling. val orderPreservingGroup = { val relatives = if (config.expandRelative) Nil else relativeImporters - relatives ++ implicits sortBy (_.importees.head.pos.start) + val imports = (relatives ++ implicits).sortBy(_.importees.head.pos.start) + Option(imports).filter(_.nonEmpty) } // Builds a patch that inserts the organized imports. val insertionPatch = prependOrganizedImports( imports.head.tokens.head, - fullyQualifiedGroups :+ orderPreservingGroup filter (_.nonEmpty) + fullyQualifiedGroups ++ orderPreservingGroup.map(importMatchers.length -> _) ) // Builds a patch that removes all the tokens forming the original imports. @@ -286,15 +289,15 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ importer.copy(ref = replaceTopQualifier(importer.ref, fullyQualifiedTopQualifier)) } - private def groupImporters(importers: Seq[Importer]): Seq[Seq[Importer]] = + private def groupImporters(importers: Seq[Importer]): Seq[(Int, Seq[Importer])] = { importers - .groupBy(matchImportGroup) // Groups imports by importer prefix. + // Groups imports by importer prefix. + .groupBy(matchImportGroup) + .mapValues(deduplicateImportees) + .mapValues(organizeImportGroup) .toSeq - .sortBy { case (index, _) => index } // Sorts import groups by group index - .map { - // Organize imports within the same group. - case (_, importers) => organizeImportGroup(deduplicateImportees(importers)) - } + .sortBy { case (groupIndex, _) => groupIndex } + } private def deduplicateImportees(importers: Seq[Importer]): Seq[Importer] = { // Scalameta `Tree` nodes do not provide structural equality comparisons, here we pretty-print @@ -590,6 +593,48 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ index } } + + private def prependOrganizedImports( + token: Token, + importGroups: Seq[(Int, Seq[Importer])] + ): Patch = { + val withBlankLines = { + val prettyPrintedGroups: Seq[(Int, String)] = importGroups.map { case (index, imports) => + index -> prettyPrintImportGroup(imports) + } + + val blankLines: Seq[(Int, String)] = importMatchers.zipWithIndex + .filter { case (m, _) => m == ImportMatcher.BlankLine } + .map { case (_, index) => index -> "\n" } + + (prettyPrintedGroups ++ blankLines) + .sortBy { case (index, _) => index } + .map { case (_, prettyPrinted) => prettyPrinted } + } + + // Global imports within curly-braced packages must be indented accordingly, e.g.: + // + // package foo { + // package bar { + // import baz + // import qux + // } + // } + val indentedOutput: Iterator[String] = withBlankLines + .mkString("\n") + .trim() + .replaceAll("\n{2,}", "\n\n") + .linesIterator + .zipWithIndex + .map { + // The first line will be inserted at an already indented position. + case (line, 0) => line + case (line, _) if line.isEmpty => line + case (line, _) => " " * token.pos.startColumn + line + } + + Patch.addLeft(token, indentedOutput mkString "\n") + } } object OrganizeImports { @@ -737,31 +782,6 @@ object OrganizeImports { } } - private def prependOrganizedImports(token: Token, importGroups: Seq[Seq[Importer]]): Patch = { - // Global imports within curly-braced packages must be indented accordingly, e.g.: - // - // package foo { - // package bar { - // import baz - // import qux - // } - // } - val indentedOutput: Iterator[String] = - importGroups - .map(prettyPrintImportGroup) - .mkString("\n\n") - .linesIterator - .zipWithIndex - .map { - // The first line will be inserted at an already indented position. - case (line, 0) => line - case (line, _) if line.isEmpty => line - case (line, _) => " " * token.pos.startColumn + line - } - - Patch.addLeft(token, indentedOutput mkString "\n") - } - implicit private class SymbolExtension(symbol: Symbol) { /** From 7b922e9c980b220827b182788007ce49a5533956 Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Sat, 26 Dec 2020 01:14:07 -0800 Subject: [PATCH 05/11] Clean up the implementation --- .../src/main/scala/fix/OrganizeImports.scala | 107 +++++++++--------- 1 file changed, 51 insertions(+), 56 deletions(-) diff --git a/rules/src/main/scala/fix/OrganizeImports.scala b/rules/src/main/scala/fix/OrganizeImports.scala index 1ebe6e5..4f59f22 100644 --- a/rules/src/main/scala/fix/OrganizeImports.scala +++ b/rules/src/main/scala/fix/OrganizeImports.scala @@ -31,27 +31,7 @@ import scalafix.v1.XtensionTreeScalafix class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("OrganizeImports") { import OrganizeImports._ - private val importMatchers = { - // The wildcard group should always exist. Append one at the end if omitted. - val matchers = { - val parsed = config.groups map ImportMatcher.parse - if (parsed contains ImportMatcher.Wildcard) parsed else parsed :+ ImportMatcher.Wildcard - } - - ( - config.blankLines match { - case BlankLines.Manual => - matchers - - // Inserts blank lines between adjacent import groups automatically. - case BlankLines.Auto => - Seq.tabulate((matchers.length * 2 - 1) max 0) { - case i if i % 2 == 0 => matchers(i / 2) - case _ => ImportMatcher.BlankLine - } - } - ) :+ ImportMatcher.BlankLine - } + private val importMatchers = buildImportMatchers(config) private val wildcardGroupIndex: Int = importMatchers indexOf ImportMatcher.Wildcard @@ -120,7 +100,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ val (fullyQualifiedImporters, relativeImporters) = noImplicits partition isFullyQualified // Organizes all the fully-qualified global importers. - val fullyQualifiedGroups: Seq[(Int, Seq[Importer])] = { + val fullyQualifiedGroups: Seq[ImportGroup] = { val expanded = if (config.expandRelative) relativeImporters map expandRelative else Nil groupImporters(fullyQualifiedImporters ++ expanded) } @@ -138,9 +118,9 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ } // Builds a patch that inserts the organized imports. - val insertionPatch = prependOrganizedImports( + val insertionPatch = insertOrganizedImports( imports.head.tokens.head, - fullyQualifiedGroups ++ orderPreservingGroup.map(importMatchers.length -> _) + fullyQualifiedGroups ++ orderPreservingGroup.map(ImportGroup(importMatchers.length, _)) ) // Builds a patch that removes all the tokens forming the original imports. @@ -242,7 +222,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ // The top qualifier itself is `_root_`, e.g.: `import _root_.scala.util` || topQualifier.value == "_root_" - // Issue #64: Sometimes, the symbol of the top qualifier can be missing due to unknwon reasons + // Issue #64: Sometimes, the symbol of the top qualifier can be missing due to unknown reasons // (see https://github.com/liancheng/scalafix-organize-imports/issues/64). In this case, we // issue a warning and continue processing assuming that the top qualifier is fully-qualified. || topQualifierSymbol.isNone && { @@ -289,15 +269,15 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ importer.copy(ref = replaceTopQualifier(importer.ref, fullyQualifiedTopQualifier)) } - private def groupImporters(importers: Seq[Importer]): Seq[(Int, Seq[Importer])] = { + private def groupImporters(importers: Seq[Importer]): Seq[ImportGroup] = importers // Groups imports by importer prefix. .groupBy(matchImportGroup) .mapValues(deduplicateImportees) .mapValues(organizeImportGroup) + .map(ImportGroup.tupled) .toSeq - .sortBy { case (groupIndex, _) => groupIndex } - } + .sortBy(_.index) private def deduplicateImportees(importers: Seq[Importer]): Seq[Importer] = { // Scalameta `Tree` nodes do not provide structural equality comparisons, here we pretty-print @@ -523,7 +503,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ locally { val importerSyntaxMap = group.map { i => i.copy().syntax -> i }.toMap - newImporteeLists filter (_.nonEmpty) map { case importees => + newImporteeLists filter (_.nonEmpty) map { importees => val newImporter = Importer(ref, importees) importerSyntaxMap.getOrElse(newImporter.syntax, newImporter) } @@ -594,24 +574,28 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ } } - private def prependOrganizedImports( - token: Token, - importGroups: Seq[(Int, Seq[Importer])] - ): Patch = { - val withBlankLines = { - val prettyPrintedGroups: Seq[(Int, String)] = importGroups.map { case (index, imports) => - index -> prettyPrintImportGroup(imports) - } + private def insertOrganizedImports(token: Token, importGroups: Seq[ImportGroup]): Patch = { + val prettyPrintedGroups = importGroups.map { case ImportGroup(index, imports) => + index -> prettyPrintImportGroup(imports) + } - val blankLines: Seq[(Int, String)] = importMatchers.zipWithIndex - .filter { case (m, _) => m == ImportMatcher.BlankLine } - .map { case (_, index) => index -> "\n" } + val blankLines = { + val blankLineIndices = { + for ((ImportMatcher.BlankLine, index) <- importMatchers.zipWithIndex) yield index + }.toSet - (prettyPrintedGroups ++ blankLines) - .sortBy { case (index, _) => index } - .map { case (_, prettyPrinted) => prettyPrinted } + // Checks each pair of adjacent import groups. Inserts a blank line between them when needed. + importGroups map (_.index) sliding 2 filter (_.length == 2) flatMap { case Seq(lhs, rhs) => + val hasBlankLine = blankLineIndices exists (i => lhs < i && i < rhs) + if (hasBlankLine) Some((lhs + 1) -> "") else None + } } + val withBlankLines = (prettyPrintedGroups ++ blankLines) + .sortBy { case (index, _) => index } + .map { case (_, group) => group } + .mkString("\n") + // Global imports within curly-braced packages must be indented accordingly, e.g.: // // package foo { @@ -620,24 +604,35 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ // import qux // } // } - val indentedOutput: Iterator[String] = withBlankLines - .mkString("\n") - .trim() - .replaceAll("\n{2,}", "\n\n") - .linesIterator - .zipWithIndex - .map { - // The first line will be inserted at an already indented position. - case (line, 0) => line - case (line, _) if line.isEmpty => line - case (line, _) => " " * token.pos.startColumn + line - } + val indentedOutput = withBlankLines.linesIterator.zipWithIndex.map { + // The first line will be inserted at an already indented position. + case (line, 0) => line + case (line, _) if line.isEmpty => line + case (line, _) => " " * token.pos.startColumn + line + } Patch.addLeft(token, indentedOutput mkString "\n") } } object OrganizeImports { + private case class ImportGroup(index: Int, imports: Seq[Importer]) + + private def buildImportMatchers(config: OrganizeImportsConfig): Seq[ImportMatcher] = { + import ImportMatcher._ + + val withWildcard = { + val parsed = config.groups map parse + // The wildcard group should always exist. Appends one at the end if omitted. + if (parsed contains Wildcard) parsed else parsed :+ Wildcard + } + + config.blankLines match { + case BlankLines.Manual => withWildcard + case BlankLines.Auto => withWildcard.flatMap(_ :: BlankLine :: Nil) + } + } + private def positionOf(importee: Importee): Position = importee match { case Importee.Rename(from, _) => from.pos @@ -669,7 +664,7 @@ object OrganizeImports { /** * HACK: The Scalafix pretty-printer decides to add spaces after open and before close braces in * imports, i.e., `import a.{ b, c }` instead of `import a.{b, c}`. Unfortunately, this behavior - * cannot be overriden. This function removes the unwanted spaces as a workaround. In cases where + * cannot be overridden. 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. */ From ef21911775b6771cd5a9df4f2f217091eeea4b02 Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Sat, 26 Dec 2020 01:14:23 -0800 Subject: [PATCH 06/11] Add a simple test case --- .../src/main/scala/fix/BlankLinesManual.scala | 19 +++++++++++++++++++ .../src/main/scala/fix/BlankLinesManual.scala | 8 ++++++++ 2 files changed, 27 insertions(+) create mode 100644 input/src/main/scala/fix/BlankLinesManual.scala create mode 100644 output/src/main/scala/fix/BlankLinesManual.scala diff --git a/input/src/main/scala/fix/BlankLinesManual.scala b/input/src/main/scala/fix/BlankLinesManual.scala new file mode 100644 index 0000000..bcdd84f --- /dev/null +++ b/input/src/main/scala/fix/BlankLinesManual.scala @@ -0,0 +1,19 @@ +/* +rules = [OrganizeImports] +OrganizeImports { + blankLines = Manual + groups = [ + "re:javax?\\." + "scala." + "---" + "*" + ] +} + */ +package fix + +import scala.collection.mutable +import java.util.Arrays +import sun.misc.Unsafe + +object BlankLinesManual diff --git a/output/src/main/scala/fix/BlankLinesManual.scala b/output/src/main/scala/fix/BlankLinesManual.scala new file mode 100644 index 0000000..ee2a2d1 --- /dev/null +++ b/output/src/main/scala/fix/BlankLinesManual.scala @@ -0,0 +1,8 @@ +package fix + +import java.util.Arrays +import scala.collection.mutable + +import sun.misc.Unsafe + +object BlankLinesManual From 7c46e19b13ec7f4a2570ad370879fa3a7a8aedfa Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Mon, 28 Dec 2020 10:54:14 -0800 Subject: [PATCH 07/11] Minor refactoring --- rules/src/main/scala/fix/ImportMatcher.scala | 18 ++++---- .../src/main/scala/fix/OrganizeImports.scala | 46 ++++++++----------- 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/rules/src/main/scala/fix/ImportMatcher.scala b/rules/src/main/scala/fix/ImportMatcher.scala index 31e582f..61ba7e3 100644 --- a/rules/src/main/scala/fix/ImportMatcher.scala +++ b/rules/src/main/scala/fix/ImportMatcher.scala @@ -11,10 +11,10 @@ sealed trait ImportMatcher { object ImportMatcher { def parse(pattern: String): ImportMatcher = pattern match { - case p if p startsWith "re:" => ImportMatcher.RE(new Regex(p stripPrefix "re:")) - case "---" => ImportMatcher.BlankLine - case "*" => ImportMatcher.Wildcard - case p => ImportMatcher.PlainText(p) + case p if p startsWith "re:" => RE(new Regex(p stripPrefix "re:")) + case "---" => --- + case "*" => * + case p => PlainText(p) } case class RE(pattern: Regex) extends ImportMatcher { @@ -26,13 +26,15 @@ object ImportMatcher { override def matches(i: Importer): Int = if (i.syntax startsWith pattern) pattern.length else 0 } - case object Wildcard extends ImportMatcher { - // This matcher matches nothing. The wildcard group is always special-cased at the end of the - // import group matching process. + case object * extends ImportMatcher { + // The wildcard matcher matches nothing and is always special-cased at the end of the import + // group matching process. def matches(importer: Importer): Int = 0 } - case object BlankLine extends ImportMatcher { + case object --- extends ImportMatcher { + // Blank line matchers are a pseudo matchers matching nothing and always special-cased at the + // end of the import group matching process. override def matches(i: Importer): Int = 0 } } diff --git a/rules/src/main/scala/fix/OrganizeImports.scala b/rules/src/main/scala/fix/OrganizeImports.scala index 4f59f22..0230c72 100644 --- a/rules/src/main/scala/fix/OrganizeImports.scala +++ b/rules/src/main/scala/fix/OrganizeImports.scala @@ -5,6 +5,9 @@ import scala.collection.mutable import scala.collection.mutable.ArrayBuffer import scala.util.Try +import fix.ImportMatcher.* +import fix.ImportMatcher.--- +import fix.ImportMatcher.parse import metaconfig.Configured import scala.meta.Import import scala.meta.Importee @@ -30,10 +33,11 @@ import scalafix.v1.XtensionTreeScalafix class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("OrganizeImports") { import OrganizeImports._ + import ImportMatcher._ - private val importMatchers = buildImportMatchers(config) + private val matchers = buildImportMatchers(config) - private val wildcardGroupIndex: Int = importMatchers indexOf ImportMatcher.Wildcard + private val wildcardGroupIndex: Int = matchers indexOf * private val unusedImporteePositions: mutable.Set[Position] = mutable.Set.empty[Position] @@ -113,14 +117,13 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ // require special handling. val orderPreservingGroup = { val relatives = if (config.expandRelative) Nil else relativeImporters - val imports = (relatives ++ implicits).sortBy(_.importees.head.pos.start) - Option(imports).filter(_.nonEmpty) + Option(relatives ++ implicits sortBy (_.importees.head.pos.start)) filter (_.nonEmpty) } // Builds a patch that inserts the organized imports. val insertionPatch = insertOrganizedImports( imports.head.tokens.head, - fullyQualifiedGroups ++ orderPreservingGroup.map(ImportGroup(importMatchers.length, _)) + fullyQualifiedGroups ++ orderPreservingGroup.map(ImportGroup(matchers.length, _)) ) // Builds a patch that removes all the tokens forming the original imports. @@ -137,10 +140,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ private def removeUnused(imports: Seq[Import]): Patch = Patch.fromIterable { imports flatMap (_.importers) flatMap { case Importer(_, importees) => - val hasUsedWildcard = importees exists { - case i: Importee.Wildcard => !isUnused(i) - case _ => false - } + val hasUsedWildcard = importees exists { i => i.is[Importee.Wildcard] && !isUnused(i) } importees collect { case i @ Importee.Rename(_, to) if isUnused(i) && hasUsedWildcard => @@ -159,9 +159,8 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ private def removeUnused(importer: Importer): Option[Importer] = if (!config.removeUnused) Some(importer) else { - val hasUsedWildcard = importer.importees exists { - case i: Importee.Wildcard => !isUnused(i) - case _ => false + val hasUsedWildcard = importer.importees exists { i => + i.is[Importee.Wildcard] && !isUnused(i) } var rewritten = false @@ -194,8 +193,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ val (implicits, implicitPositions) = importers.flatMap { case importer @ Importer(_, importees) => importees - .filter(_.is[Importee.Name]) - .filter(name => name.symbol.infoNoThrow exists (_.isImplicit)) + .filter(i => i.is[Importee.Name] && i.symbol.infoNoThrow.exists(_.isImplicit)) .map(i => importer.copy(importees = i :: Nil) -> i.pos) }.unzip @@ -271,10 +269,8 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ private def groupImporters(importers: Seq[Importer]): Seq[ImportGroup] = importers - // Groups imports by importer prefix. - .groupBy(matchImportGroup) - .mapValues(deduplicateImportees) - .mapValues(organizeImportGroup) + .groupBy(matchImportGroup) // Groups imports by importer prefix. + .mapValues(deduplicateImportees _ andThen organizeImportGroup) .map(ImportGroup.tupled) .toSeq .sortBy(_.index) @@ -562,7 +558,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ // by an `ImportMatcher`. If multiple `ImporterMatcher`s match the given import, the one matches // the longest prefix wins. private def matchImportGroup(importer: Importer): Int = { - val matchedGroups = importMatchers + val matchedGroups = matchers .map(_ matches importer) .zipWithIndex .filter { case (length, _) => length > 0 } @@ -580,9 +576,9 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ } val blankLines = { - val blankLineIndices = { - for ((ImportMatcher.BlankLine, index) <- importMatchers.zipWithIndex) yield index - }.toSet + // Indices of all blank lines configured in `OrganizeImports.groups`, either automatically or + // manually. + val blankLineIndices = matchers.zipWithIndex.collect { case (`---`, index) => index }.toSet // Checks each pair of adjacent import groups. Inserts a blank line between them when needed. importGroups map (_.index) sliding 2 filter (_.length == 2) flatMap { case Seq(lhs, rhs) => @@ -619,17 +615,15 @@ object OrganizeImports { private case class ImportGroup(index: Int, imports: Seq[Importer]) private def buildImportMatchers(config: OrganizeImportsConfig): Seq[ImportMatcher] = { - import ImportMatcher._ - val withWildcard = { val parsed = config.groups map parse // The wildcard group should always exist. Appends one at the end if omitted. - if (parsed contains Wildcard) parsed else parsed :+ Wildcard + if (parsed contains *) parsed else parsed :+ * } config.blankLines match { case BlankLines.Manual => withWildcard - case BlankLines.Auto => withWildcard.flatMap(_ :: BlankLine :: Nil) + case BlankLines.Auto => withWildcard.flatMap(_ :: --- :: Nil) } } From 2f3f023971a3efed43f46cdc82db27096e94d1d3 Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Mon, 28 Dec 2020 10:54:59 -0800 Subject: [PATCH 08/11] Minor updates to existing tests --- input/src/main/scala/fix/ExpandRelative.scala | 2 +- input/src/main/scala/fix/ExpandRelativePackageObject.scala | 4 +--- output/src/main/scala/fix/ExpandRelative.scala | 2 +- output/src/main/scala/fix/ExpandRelativePackageObject.scala | 4 +--- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/input/src/main/scala/fix/ExpandRelative.scala b/input/src/main/scala/fix/ExpandRelative.scala index 697f51e..8fe5ebb 100644 --- a/input/src/main/scala/fix/ExpandRelative.scala +++ b/input/src/main/scala/fix/ExpandRelative.scala @@ -8,4 +8,4 @@ import scala.util import util.control import control.NonFatal -object ExpandRelativeImports +object ExpandRelative diff --git a/input/src/main/scala/fix/ExpandRelativePackageObject.scala b/input/src/main/scala/fix/ExpandRelativePackageObject.scala index 9f1451a..bb76292 100644 --- a/input/src/main/scala/fix/ExpandRelativePackageObject.scala +++ b/input/src/main/scala/fix/ExpandRelativePackageObject.scala @@ -6,6 +6,4 @@ package fix import PackageObject.a -object ExpandRelativePackageObject { - println(a) -} +object ExpandRelativePackageObject diff --git a/output/src/main/scala/fix/ExpandRelative.scala b/output/src/main/scala/fix/ExpandRelative.scala index d74e884..55059c5 100644 --- a/output/src/main/scala/fix/ExpandRelative.scala +++ b/output/src/main/scala/fix/ExpandRelative.scala @@ -4,4 +4,4 @@ import scala.util import scala.util.control import scala.util.control.NonFatal -object ExpandRelativeImports +object ExpandRelative diff --git a/output/src/main/scala/fix/ExpandRelativePackageObject.scala b/output/src/main/scala/fix/ExpandRelativePackageObject.scala index 46ad973..8d42e6a 100644 --- a/output/src/main/scala/fix/ExpandRelativePackageObject.scala +++ b/output/src/main/scala/fix/ExpandRelativePackageObject.scala @@ -2,6 +2,4 @@ package fix import fix.PackageObject.a -object ExpandRelativePackageObject { - println(a) -} +object ExpandRelativePackageObject From 8c397fe82556f38e582231bcbd87ce9b7e46c094 Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Mon, 28 Dec 2020 16:48:31 -0800 Subject: [PATCH 09/11] Update documentation --- README.adoc | 233 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 224 insertions(+), 9 deletions(-) diff --git a/README.adoc b/README.adoc index 27a4ebb..27ea0fe 100644 --- a/README.adoc +++ b/README.adoc @@ -60,7 +60,7 @@ def scalafixIvyDeps = Agg(ivy"com.github.liancheng::organize-imports:{latest-rel === Source formatting tools -The `OrganizeImports` rule plays nicely with source formatting tools like https://scalameta.org/scalafmt/[Scalafmt]. If an import statement is already organized according to the configuration, its original source level format would be preserved. For example, in an sbt project, if you run the following command sequence: +The `OrganizeImports` rule plays nicely with source-formatting tools like https://scalameta.org/scalafmt/[Scalafmt]. If an import statement is already organized according to the configuration, its original source level format would be preserved. For example, in an sbt project, if you run the following command sequence: [source] ---- @@ -91,6 +91,7 @@ By default, `OrganizeImports` already removes unused imports for you (see the << [source,hocon] ---- OrganizeImports { + blankLines = Auto coalesceToWildcardImportThreshold = 2147483647 # Int.MaxValue expandRelative = false groupExplicitlyImportedImplicitsSeparately = false @@ -102,6 +103,134 @@ OrganizeImports { } ---- +[[blank-lines]] +=== `blankLines` + +Configures whether blank lines between adjacent import groups are automatically or manually inserted. This option is used together with the <>. + +==== Value type + +Enum: `Auto | Manual` + +Auto:: A blank line is automatically inserted between adjacent import groups. All blank line markers (`---`) configured in the <> are ignored. + +Manual:: A blank line is inserted at all the positions where blank line markers appear in the <>. + +The following two configurations are equivalent: + +[source,hocon] +---- +OrganizeImports { + blankLines = Auto + groups = [ + "re:javax?\\." + "scala." + "*" + ] +} + +OrganizeImports { + blankLines = Manual + groups = [ + "re:javax?\\." + "---" + "scala." + "---" + "*" + ] +} +---- + +==== Default value + +`Auto` + +==== Examples + +`Auto`:: ++ +-- +Configuration: + +[source,hocon] +---- +OrganizeImports { + blankLines = Auto + groups = [ + "re:javax?\\." + "scala." + "*" + ] +} +---- + +Before: + +[source,scala] +---- +import scala.collection.JavaConverters._ +import java.time.Clock +import sun.misc.BASE64Encoder +import javax.annotation.Generated +import scala.concurrent.ExecutionContext +---- + +After: + +[source,scala] +---- +import java.time.Clock +import javax.annotation.Generated + +import scala.collection.JavaConverters._ +import scala.concurrent.ExecutionContext + +import sun.misc.BASE64Encoder +---- +-- + +`Manual`:: ++ +-- +Configuration: + +[source,hocon] +---- +OrganizeImports { + blankLines = Manual + groups = [ + "re:javax?\\." + "scala." + "---" + "*" + ] +} +---- + +Before: + +[source,scala] +---- +import scala.collection.JavaConverters._ +import java.time.Clock +import sun.misc.BASE64Encoder +import javax.annotation.Generated +import scala.concurrent.ExecutionContext +---- + +After: + +[source,scala] +---- +import java.time.Clock +import javax.annotation.Generated +import scala.collection.JavaConverters._ +import scala.concurrent.ExecutionContext + +import sun.misc.BASE64Encoder +---- +-- + [[coalesce]] === `coalesceToWildcardImportThreshold` @@ -124,7 +253,7 @@ object Example { ---- The type of `Example.m` above is not ambiguous because the mutable `Map` explicitly imported in the second import takes higher precedence than the immutable `Map` imported via wildcard in the first import. -However, if we coalesce the grouped importes in the second import statement into a wildcard, there will be a compilation error: +However, if we coalesce the grouped imports in the second import statement into a wildcard, there will be a compilation error: [source,scala] ---- import scala.collection.immutable._ @@ -147,7 +276,7 @@ Integer Rationale:: Setting the default value to `Int.MaxValue` essentially disables this feature, since it may cause correctness issues. -==== Example +==== Examples Configuration: @@ -217,7 +346,7 @@ Boolean `false` -==== Example +==== Examples Configuration: @@ -302,7 +431,7 @@ This behavior could be due to a Scala compiler bug since https://scala-lang.org/ Unfortunately, Scalafix is not able to surgically identify conflicting implicit values behind a wildcard import. In order to guarantee correctness in all cases, when the `groupExplicitlyImportedImplicitsSeparately` option is set to `true`, all explicitly imported implicit names are moved into the trailing order-preserving import group together with relative imports, if any (see the <> section for more details). -CAUTION: In general, order-sensitive imports are fragile, and can easily be broken by either human collaborators or tools (e.g., the IntelliJ IDEA Scala import optimizer does not handle this case correctly). They should be eliminated whenever possible. This option is mostly useful when you are dealing with a large trunk of legacy codebase and you want to minimize manual intervention and guarantee correctness in all cases. +CAUTION: In general, order-sensitive imports are fragile, and can easily be broken by either human collaborators or tools (e.g., the IntelliJ IDEA Scala import optimizer does not handle this case correctly). They should be eliminated whenever possible. This option is mostly useful when you are dealing with a large trunk of legacy codebase, and you want to minimize manual intervention and guarantee correctness in all cases. ==== Value type @@ -321,10 +450,10 @@ This option defaults to `false` due to the following reasons: + E.g., why my `scala.concurrent.ExecutionContext.Implicits.global` import is moved to a separate group even if I have a `scala.` group defined in the `groups` option? -. The concerned correctness issue is rarely seen in real life. When it really happens, it is usually a sign of bad coding style and you may want to tweak your imports to eliminate the root cause. +. The concerned correctness issue is rarely seen in real life. When it really happens, it is usually a sign of bad coding style, and you may want to tweak your imports to eliminate the root cause. -- -==== Example +==== Examples Configuration: @@ -462,7 +591,7 @@ object Example { } ---- -At a first glance, it seems feasible to simply drop the second import since `mutable._` already covers `mutble.Map`. However, similar to the example illustrated in the section about the <>, the type of `Example.m` above is `mutable.Map`, because the mutable `Map` explicitly imported in the second import takes higher precedence than the immutable `Map` imported via wildcard in the first import. If we merge the last two imports naively, we'll get: +At a first glance, it seems feasible to simply drop the second import since `mutable._` already covers `mutble.Map`. However, similar to the example illustrated in the section about the <>, the type of `Example.m` above is `mutable.Map`, because the mutable `Map` explicitly imported in the second import takes higher precedence than the immutable `Map` imported via wildcard in the first import. If we merge the last two imports naively, we'll get: [source,scala] ---- @@ -668,6 +797,50 @@ OrganizeImports.groups = ["re:javax?\\.", "scala."] ---- -- +[[blank-line-marker]] +A blank line marker:: ++ +-- +A blank line marker, `"---"`, defines a blank line between two adjacent import groups when <> is set to `Manual`. It is ignored when `blankLines` is `Auto`. Leading and trailing blank line markers are always ignored. Multiple consecutive blank line markers are treated as a single one. So the following three configurations are all equivalent: + +[source,hocon] +---- +OrganizeImports { + blankLines = Manual + groups = [ + "----" + "re:javax?\\." + "----" + "scala." + "----" + "----" + "*" + "----" + ] +} + +OrganizeImports { + blankLines = Manual + groups = [ + "re:javax?\\." + "---" + "scala." + "---" + "*" + ] +} + +OrganizeImports { + blankLines = Auto + groups = [ + "re:javax?\\." + "scala." + "*" + ] +} +---- +-- + ==== Default value [source,hocon] @@ -846,6 +1019,48 @@ import sun.misc.BASE64Encoder ---- -- +With manually configured blank lines:: ++ +-- +Configuration: + +[source,hocon] +---- +OrganizeImports { + blankLines = Manual + groups = [ + "*" + "---" + "re:javax?\\." + "scala." + ] +} +---- + +Before: + +[source,scala] +---- +import scala.collection.JavaConverters._ +import java.time.Clock +import sun.misc.BASE64Encoder +import javax.annotation.Generated +import scala.concurrent.ExecutionContext +---- + +After: + +[source,scala] +---- +import sun.misc.BASE64Encoder + +import java.time.Clock +import javax.annotation.Generated +import scala.collection.JavaConverters._ +import scala.concurrent.ExecutionContext +---- +-- + === `importSelectorsOrder` Specifies the order of grouped import selectors within a single import expression. @@ -1029,7 +1244,7 @@ Boolean `true` -==== Example +==== Examples Configuration: From 22d25a854d838b45b50187a730dd96d9038e9368 Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Mon, 28 Dec 2020 18:35:03 -0800 Subject: [PATCH 10/11] Minor refactoring --- rules/src/main/scala/fix/ImportMatcher.scala | 8 +++--- .../src/main/scala/fix/OrganizeImports.scala | 25 +++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/rules/src/main/scala/fix/ImportMatcher.scala b/rules/src/main/scala/fix/ImportMatcher.scala index 61ba7e3..5d3689e 100644 --- a/rules/src/main/scala/fix/ImportMatcher.scala +++ b/rules/src/main/scala/fix/ImportMatcher.scala @@ -27,14 +27,14 @@ object ImportMatcher { } case object * extends ImportMatcher { - // The wildcard matcher matches nothing and is always special-cased at the end of the import - // group matching process. + // The wildcard matcher matches nothing. It is special-cased at the end of the import group + // matching process. def matches(importer: Importer): Int = 0 } case object --- extends ImportMatcher { - // Blank line matchers are a pseudo matchers matching nothing and always special-cased at the - // end of the import group matching process. + // Blank line matchers are pseudo matchers matching nothing. They are special-cased at the end + // of the import group matching process. override def matches(i: Importer): Int = 0 } } diff --git a/rules/src/main/scala/fix/OrganizeImports.scala b/rules/src/main/scala/fix/OrganizeImports.scala index 0230c72..0e7080f 100644 --- a/rules/src/main/scala/fix/OrganizeImports.scala +++ b/rules/src/main/scala/fix/OrganizeImports.scala @@ -73,10 +73,9 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ } override def fix(implicit doc: SemanticDocument): Patch = { - unusedImporteePositions ++= - doc.diagnostics - .filter(_.message == "Unused import") - .map(_.position) + unusedImporteePositions ++= doc.diagnostics.collect { + case d if d.message == "Unused import" => d.position + } val (globalImports, localImports) = collectImports(doc.tree) @@ -192,9 +191,10 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ )(implicit doc: SemanticDocument): (Seq[Importer], Seq[Importer]) = { val (implicits, implicitPositions) = importers.flatMap { case importer @ Importer(_, importees) => - importees - .filter(i => i.is[Importee.Name] && i.symbol.infoNoThrow.exists(_.isImplicit)) - .map(i => importer.copy(importees = i :: Nil) -> i.pos) + importees collect { + case i: Importee.Name if i.symbol.infoNoThrow exists (_.isImplicit) => + importer.copy(importees = i :: Nil) -> i.pos + } }.unzip val noImplicits = importers.flatMap { @@ -373,8 +373,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ // either. If a name is renamed more than once, it only keeps one of the renames in the // result and may break compilation (unless other renames are not actually referenced). val renames = allImportees - .filter(_.is[Importee.Rename]) - .map { case rename: Importee.Rename => rename } + .collect { case rename: Importee.Rename => rename } .groupBy(_.name.value) .map { case (_, rename :: Nil) => rename @@ -580,7 +579,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ // manually. val blankLineIndices = matchers.zipWithIndex.collect { case (`---`, index) => index }.toSet - // Checks each pair of adjacent import groups. Inserts a blank line between them when needed. + // Checks each pair of adjacent import groups. Inserts a blank line between them if necessary. importGroups map (_.index) sliding 2 filter (_.length == 2) flatMap { case Seq(lhs, rhs) => val hasBlankLine = blankLineIndices exists (i => lhs < i && i < rhs) if (hasBlankLine) Some((lhs + 1) -> "") else None @@ -589,7 +588,7 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ val withBlankLines = (prettyPrintedGroups ++ blankLines) .sortBy { case (index, _) => index } - .map { case (_, group) => group } + .map { case (_, lines) => lines } .mkString("\n") // Global imports within curly-braced packages must be indented accordingly, e.g.: @@ -600,14 +599,14 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ // import qux // } // } - val indentedOutput = withBlankLines.linesIterator.zipWithIndex.map { + val indented = withBlankLines.linesIterator.zipWithIndex.map { // The first line will be inserted at an already indented position. case (line, 0) => line case (line, _) if line.isEmpty => line case (line, _) => " " * token.pos.startColumn + line } - Patch.addLeft(token, indentedOutput mkString "\n") + Patch.addLeft(token, indented mkString "\n") } } From a05593b41534f607a7cf951e3b756d40b6a5c584 Mon Sep 17 00:00:00 2001 From: Cheng Lian Date: Sun, 10 Jan 2021 21:57:56 -0800 Subject: [PATCH 11/11] Minor comment update --- rules/src/main/scala/fix/OrganizeImports.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rules/src/main/scala/fix/OrganizeImports.scala b/rules/src/main/scala/fix/OrganizeImports.scala index 0e7080f..bd920f3 100644 --- a/rules/src/main/scala/fix/OrganizeImports.scala +++ b/rules/src/main/scala/fix/OrganizeImports.scala @@ -795,9 +795,9 @@ object OrganizeImports { } /** - * Returns an `Importer` with all the `Importee`s selected from the input `Importer` that - * satisfy a predicate. If all the `Importee`s are selected, the input `Importer` instance is - * returned to preserve the original source level formatting. If none of the `Importee`s are + * Returns an `Importer` with all the `Importee`s that are selected from the input `Importer` + * and satisfy a predicate. If all the `Importee`s are selected, the input `Importer` instance + * is returned to preserve the original source level formatting. If none of the `Importee`s are * selected, returns a `None`. */ def filterImportees(f: Importee => Boolean): Option[Importer] = {