OrganizeImports: re-emit already-organized wrapped imports verbatim

A wrapped import the rule leaves untouched is currently collapsed onto one
line by the pretty-printer, fighting scalafmt over the same lines. Re-emit
such an import from its own source text instead, so its layout (indentation,
alignment, trailing comma) is the formatter's business, not this rule's.

The signal for "untouched" is position presence, the same invariant the
leaf-name emission already relies on: a node the pipeline rewrote is a
`copy()` and carries no position, so an `Importer` that still has its parsed
`Position.Range` was not restructured -- same qualifier, same importees, same
order. `isUnchangedFromSource` = has a range position, spans multiple lines
(single-line imports are still normalized, e.g. `import a.{ B }` ->
`import a.B`), and needs no dialect migration (`=>`/`as`, `_`/`*`). No
structural comparison, no `originalPrototype()` navigation.

The reconstruction branches are untouched: anything the rule rewrites --
reordered, merged, coalesced, exploded, expanded -- still goes through the
pretty-printer as before. A coalesced multi-line import, for instance,
re-flows to one line (see CoalesceMultiLineTrailingComma) and the formatter
re-wraps it.

Fixtures adapted from #2507.

Author: bjaglin

scalafix-rules/src/main/scala/scalafix/internal/rule/OrganizeImports.scala

@@ -779,57 +779,79 @@ class OrganizeImports(
 
         ps.foreach(_.begComment.foreach(appendBegComment))
         i.begComment.foreach(appendBegComment)
-        if (single != null) single.begComment.foreach(appendBegComment)
-        sb.append("import ").append(refSyntax(i.ref)).append('.')
-        if (single != null) {
-          val isCurly = single.isCurlyBraced
-          val useOuterSpace = isCurly && single
-            .originalPrototype()
-            .parent
-            .exists(_.hasSpaceInCurly)
-          if (isCurly) {
-            sb.append('{')
-            if (useOuterSpace) sb.append(' ')
-          }
-          sb.append(importeeSyntax(single))
-          if (isCurly) {
-            if (useOuterSpace) sb.append(' ')
-            sb.append('}')
+        if (i.isUnchangedFromSource) {
+          // An already-organized wrapped import: re-emit its source text as is,
+          // leaving its layout to the formatter. Comments within the source
+          // span are part of it; those attached around it are printed as usual.
+          def isWithin(pc: Tree.Comments): Boolean = pc.values.forall { c =>
+            c.pos.start >= i.pos.start && c.pos.end <= i.pos.end
           }
-          single.endComment.foreach(appendEndComment)
-        } else {
-          val lines = i.importees.iterator.map(_.pos.startLine).filter(_ >= 0)
-          val isMultiline = lines.hasNext && {
-            val line = lines.next()
-            lines.exists(_ != line)
-          }
-          val origImporters = i.importees
-            .flatMap(_.originalPrototype().parent)
-            .distinct
-          val useOuterSpace =
-            !isMultiline && origImporters.exists(_.hasSpaceInCurly)
-          // Preserve a trailing comma if the (single) source importer had one.
-          val trailingComma = isMultiline && (origImporters match {
-            case Seq(origImporter: Importer) => origImporter.hasTrailingComma
-            case _ => false
+          i.importees.foreach(_.begComment.foreach { pc =>
+            if (isWithin(pc)) commentsPrinted.add(pc) else appendBegComment(pc)
+          })
+          // Continuation lines are re-indented on insertion, like printed ones.
+          val srcIndent = " " * i.parent.fold(0)(_.pos.startColumn)
+          val srcLines = i.tokens.syntax.linesIterator
+          sb.append("import ").append(srcLines.next())
+          srcLines.foreach(l =>
+            sb.append('\n').append(l.stripPrefix(srcIndent))
+          )
+          i.importees.foreach(_.endComment.foreach { pc =>
+            if (isWithin(pc)) commentsPrinted.add(pc) else appendEndComment(pc)
           })
-          sb.append('{')
-          val sep = if (isMultiline) "\n  " else " "
-          if (isMultiline) sb.append(sep)
-          else if (useOuterSpace) sb.append(' ')
-          val sblen = sb.length
-          i.importees.foreach { i2 =>
-            if (sb.length > sblen) sb.append(',').append(sep)
-            val proto = i2.originalPrototype()
-            proto.begComment.foreach(appendBegComment)
-            sb.append(importeeSyntax(i2))
-            proto.endComment.foreach(appendEndComment)
+        } else {
+          if (single != null) single.begComment.foreach(appendBegComment)
+          sb.append("import ").append(refSyntax(i.ref)).append('.')
+          if (single != null) {
+            val isCurly = single.isCurlyBraced
+            val useOuterSpace = isCurly && single
+              .originalPrototype()
+              .parent
+              .exists(_.hasSpaceInCurly)
+            if (isCurly) {
+              sb.append('{')
+              if (useOuterSpace) sb.append(' ')
+            }
+            sb.append(importeeSyntax(single))
+            if (isCurly) {
+              if (useOuterSpace) sb.append(' ')
+              sb.append('}')
+            }
+            single.endComment.foreach(appendEndComment)
+          } else {
+            val lines = i.importees.iterator.map(_.pos.startLine).filter(_ >= 0)
+            val isMultiline = lines.hasNext && {
+              val line = lines.next()
+              lines.exists(_ != line)
+            }
+            val origImporters = i.importees
+              .flatMap(_.originalPrototype().parent)
+              .distinct
+            val useOuterSpace =
+              !isMultiline && origImporters.exists(_.hasSpaceInCurly)
+            // Preserve a trailing comma if the (single) source importer had one.
+            val trailingComma = isMultiline && (origImporters match {
+              case Seq(origImporter: Importer) => origImporter.hasTrailingComma
+              case _ => false
+            })
+            sb.append('{')
+            val sep = if (isMultiline) "\n  " else " "
+            if (isMultiline) sb.append(sep)
+            else if (useOuterSpace) sb.append(' ')
+            val sblen = sb.length
+            i.importees.foreach { i2 =>
+              if (sb.length > sblen) sb.append(',').append(sep)
+              val proto = i2.originalPrototype()
+              proto.begComment.foreach(appendBegComment)
+              sb.append(importeeSyntax(i2))
+              proto.endComment.foreach(appendEndComment)
+            }
+            if (isMultiline) {
+              if (trailingComma) sb.append(',')
+              sb.append('\n')
+            } else if (useOuterSpace) sb.append(' ')
+            sb.append('}')
           }
-          if (isMultiline) {
-            if (trailingComma) sb.append(',')
-            sb.append('\n')
-          } else if (useOuterSpace) sb.append(' ')
-          sb.append('}')
         }
         i.endComment.foreach(appendEndComment)
         ps.foreach(_.endComment.foreach(appendEndComment))
@@ -1158,16 +1180,17 @@ object OrganizeImports {
    * produce the same key regardless of how they were spelled in the source, so
    * ordering here deliberately follows the dialect-normalized spelling rather
    * than e.g. an author's backticks (see `refSyntax`/`importeeSyntax` for
-   * output, and https://github.com/scalacenter/scalafix/pull/2500 for why
-   * these two must not be conflated).
+   * output, and https://github.com/scalacenter/scalafix/pull/2500 for why these
+   * two must not be conflated).
    */
   private def treeSyntax(tree: Tree)(implicit dialect: Dialect): String =
     tree.reprint()
 
   /**
    * Emits a leaf `Name` as it was spelled in the source, so that backquotes
    * around identifiers the target dialect does not consider keywords are not
-   * dropped (soft keywords, e.g. `` `export` ``, https://github.com/scalacenter/scalafix/issues/2480).
+   * dropped (soft keywords, e.g. `` `export` ``,
+   * https://github.com/scalacenter/scalafix/issues/2480).
    *
    * NB: always reads the name's own position, never `originalPrototype()` --
    * the latter would resurrect pre-rewrite text for a node produced by
@@ -1180,9 +1203,9 @@ object OrganizeImports {
     }
 
   /**
-   * Emits an importer ref segment by segment: only leaf names can be taken
-   * from the source, so inter-segment trivia (`a . b`, a ref wrapped over
-   * several lines) can never leak into the output.
+   * Emits an importer ref segment by segment: only leaf names can be taken from
+   * the source, so inter-segment trivia (`a . b`, a ref wrapped over several
+   * lines) can never leak into the output.
    */
   private def refSyntax(ref: Term)(implicit dialect: Dialect): String =
     ref match {
@@ -1199,11 +1222,10 @@ object OrganizeImports {
     }
 
   /**
-   * Emits an importee: leaf names come from the source, every connective
-   * token (`=>` vs `as`, `given`) is produced for the target dialect, so a
-   * dialect migration keeps the source spelling of the names it moves.
-   * Separators match the pretty-printer's, so output is unchanged for
-   * unescaped names.
+   * Emits an importee: leaf names come from the source, every connective token
+   * (`=>` vs `as`, `given`) is produced for the target dialect, so a dialect
+   * migration keeps the source spelling of the names it moves. Separators match
+   * the pretty-printer's, so output is unchanged for unescaped names.
    */
   private def importeeSyntax(importee: Importee)(implicit
       dialect: Dialect
@@ -1287,6 +1309,40 @@ object OrganizeImports {
       tokens.getWideOpt(idx).exists(_.is[Token.Comma])
     }
 
+    def spansMultipleLines: Boolean =
+      importer.pos.startLine != importer.pos.endLine
+
+    /**
+     * Would pretty-printing under `dialect` rewrite `=>` to `as` or `_` to `*`?
+     */
+    def needsDialectRewrite(implicit dialect: Dialect): Boolean =
+      importer.importees.exists {
+        case i: Importee.Wildcard =>
+          i.tokens.exists(_.is[Token.Underscore]) ==
+            dialect.allowStarWildcardImport
+        case i @ (_: Importee.Rename | _: Importee.Unimport) =>
+          i.tokens.exists(_.is[Token.RightArrow]) ==
+            dialect.allowAsForImportRename
+        case _ => false
+      }
+
+    /**
+     * Checks whether this `Importer` can be re-emitted verbatim from its own
+     * source text, so its layout (indentation, alignment, trailing comma) is
+     * left to the formatter rather than re-flowed by this rule.
+     *
+     * The signal is position presence: a node the pipeline rewrote is a
+     * `copy()` and carries no position (same invariant `nameSyntax` relies on),
+     * so an `Importer` that still has its parsed `Position.Range` was not
+     * restructured -- same qualifier, same importees, same order. Only wrapped
+     * imports qualify: single-line ones are still normalized (`import a.{ B }`
+     * -> `import a.B`), and one needing a dialect migration is being rewritten.
+     */
+    def isUnchangedFromSource(implicit dialect: Dialect): Boolean =
+      importer.pos.isInstanceOf[Position.Range] &&
+        importer.spansMultipleLines &&
+        !importer.needsDialectRewrite
+
   }
 
   implicit private class TreeExtension(val tree: Tree) extends AnyVal {

scalafix-tests/input/src/main/scala/test/organizeImports/CoalesceMultiLineTrailingComma.scala

@@ -0,0 +1,24 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports {
+  groupedImports = Keep
+  removeUnused = false
+  coalesceToWildcardImportThreshold = 2
+}
+ */
+package test.organizeImports
+
+// Coalescing rewrites the importees, so the import no longer counts as
+// already-organized: it is pretty-printed (re-flowed to one line) rather than
+// re-emitted verbatim, and the formatter re-wraps it afterwards.
+import scala.collection.immutable.{
+  Map => M,
+  Set,
+  Seq,
+}
+
+object CoalesceMultiLineTrailingComma {
+  val m: M[Int, Int] = M.empty
+  val s: Set[Int] = Set.empty
+  val q: Seq[Int] = Seq.empty
+}

scalafix-tests/input/src/main/scala/test/organizeImports/SingleImporteeSplitInline.scala

@@ -0,0 +1,20 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports {
+  groupedImports = Explode
+  removeUnused = false
+}
+ */
+package test.organizeImports
+
+// Importees split out of a wrapped multi-importee import are printed inline:
+// the wrapping belonged to the source import, not to each importee.
+import scala.collection.immutable.{
+  Set,
+  Map => M,
+}
+
+object SingleImporteeSplitInline {
+  val m: M[Int, Int] = M.empty
+  val s: Set[Int] = Set.empty
+}

scalafix-tests/input/src/main/scala/test/organizeImports/SingleImporteeWrapped.scala

@@ -0,0 +1,17 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports {
+  groupedImports = Keep
+  removeUnused = false
+}
+ */
+package test.organizeImports
+
+// A wrapped single importee keeps its multi-line layout.
+import scala.collection.{
+  immutable => imm,
+}
+
+object SingleImporteeWrapped {
+  val m: imm.Map[Int, Int] = imm.Map.empty
+}

scalafix-tests/input/src/main/scala/test/organizeImports/SingleImporteeWrappedNoComma.scala

@@ -0,0 +1,18 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports {
+  groupedImports = Keep
+  removeUnused = false
+}
+ */
+package test.organizeImports
+
+// A wrapped single importee without a trailing comma keeps its layout, and
+// does not gain a comma.
+import scala.collection.{
+  immutable => imm
+}
+
+object SingleImporteeWrappedNoComma {
+  val m: imm.Map[Int, Int] = imm.Map.empty
+}

scalafix-tests/input/src/main/scala/test/organizeImports/nested/NestedPackageWrapped.scala

@@ -0,0 +1,23 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports {
+  groupedImports = Keep
+  removeUnused = false
+}
+ */
+package test.organizeImports.nested {
+  // Wrapped imports inside an indented block keep their relative indentation.
+  import scala.collection.immutable.{
+    Map => M,
+    Set,
+  }
+  import scala.collection.{
+    immutable => imm,
+  }
+
+  object NestedPackageWrapped {
+    val m: M[Int, Int] = M.empty
+    val s: Set[Int] = Set.empty
+    val i: imm.Map[Int, Int] = imm.Map.empty
+  }
+}

scalafix-tests/output/src/main/scala/test/organizeImports/CoalesceMultiLineTrailingComma.scala

@@ -0,0 +1,12 @@
+package test.organizeImports
+
+// Coalescing rewrites the importees, so the import no longer counts as
+// already-organized: it is pretty-printed (re-flowed to one line) rather than
+// re-emitted verbatim, and the formatter re-wraps it afterwards.
+import scala.collection.immutable.{Map => M, _}
+
+object CoalesceMultiLineTrailingComma {
+  val m: M[Int, Int] = M.empty
+  val s: Set[Int] = Set.empty
+  val q: Seq[Int] = Seq.empty
+}

scalafix-tests/output/src/main/scala/test/organizeImports/SingleImporteeSplitInline.scala

@@ -0,0 +1,11 @@
+package test.organizeImports
+
+// Importees split out of a wrapped multi-importee import are printed inline:
+// the wrapping belonged to the source import, not to each importee.
+import scala.collection.immutable.Set
+import scala.collection.immutable.{Map => M}
+
+object SingleImporteeSplitInline {
+  val m: M[Int, Int] = M.empty
+  val s: Set[Int] = Set.empty
+}

scalafix-tests/output/src/main/scala/test/organizeImports/SingleImporteeWrapped.scala

@@ -0,0 +1,10 @@
+package test.organizeImports
+
+// A wrapped single importee keeps its multi-line layout.
+import scala.collection.{
+  immutable => imm,
+}
+
+object SingleImporteeWrapped {
+  val m: imm.Map[Int, Int] = imm.Map.empty
+}

scalafix-tests/output/src/main/scala/test/organizeImports/SingleImporteeWrappedNoComma.scala

@@ -0,0 +1,11 @@
+package test.organizeImports
+
+// A wrapped single importee without a trailing comma keeps its layout, and
+// does not gain a comma.
+import scala.collection.{
+  immutable => imm
+}
+
+object SingleImporteeWrappedNoComma {
+  val m: imm.Map[Int, Int] = imm.Map.empty
+}