OrganizeImports: strictly honor targetDialect

docs/rules/OrganizeImports.md

@@ -1333,7 +1333,7 @@ object RemoveUnused {
 --------------
 
 Specifies the [wildcards and renames](https://docs.scala-lang.org/scala3/reference/changed-features/imports.html)
-syntax used when imports are coalesced or rewritten.
+syntax to use and drives stripping of optional curly braces.
 
 ### Value type
 
@@ -1342,16 +1342,18 @@ Enum: `Auto | Scala2 | Scala3`
 #### `Auto`
 
 Infer the dialect from compilation settings (Scala version or `-Xsource` when
-provided). This is safe only for sources that are not cross-compiled (see
-rationale below).
+provided). This is safe only for sources that are not cross-compiled and
+therefore it is NOT the default value (see rationale below).
 
 #### `Scala2`
 
-Use `_` as wildcard and `=>` for renames.
+Use `_` as wildcard and `=>` for renames. Curly braces are stripped for importers
+with a single regular or wildcard importee.
 
 #### `Scala3`
 
-Use `*` as wildcard and `as` for renames.
+Use `*` as wildcard and `as` for renames. Curly braces are stripped for importers
+with a single importee.
 
 ### Default value
 
@@ -1374,11 +1376,16 @@ OrganizeImports {
 Before:
 
 ```scala
+import scala.collection.immutable.{List => L}
+import scala.collection.mutable.{Map, Buffer => _, Seq => S, _}
 ```
 
 After:
 
 ```scala
+import scala.collection.immutable.{List => L}
+import scala.collection.mutable.Map
+import scala.collection.mutable.{Buffer => _, Seq => S, _}
 ```
 
 #### `Scala3`  
@@ -1392,10 +1399,17 @@ OrganizeImports {
 Before:
 
 ```scala
+import scala.collection.immutable.{List => L}
+import scala.collection.mutable.{Map, Buffer => _, Seq => S, _}
+import scala.concurrent.ExecutionContext.Implicits.{given scala.concurrent.ExecutionContext}
 ```
 
 After:
 
 ```scala
+import scala.collection.immutable.List as L
+import scala.collection.mutable.Map
+import scala.collection.mutable.{Buffer as _, Seq as S, *}
+import scala.concurrent.ExecutionContext.Implicits.given scala.concurrent.ExecutionContext
 ```
 

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

@@ -572,7 +572,12 @@ class OrganizeImports(
 
       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
@@ -714,9 +719,58 @@ class OrganizeImports(
   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
+        // Position found, implies that `importer` was directly parsed from the source code. Rewrite
+        // importees to ensure they follow the target dialect. For importers with a single importee,
+        // strip enclosing braces if they exist (or add/preserve them for Rename & Unimport on Scala 2).
+
+        val syntax = new StringBuilder(pos.text)
+        val Importer(_, importees) = importer
+
+        def patchSyntax(
+            t: Tree,
+            newSyntax: String,
+            requireBracesOnSingleImportee: Boolean
+        ) = {
+          val start = t.pos.start - pos.start
+          syntax.replace(start, t.pos.end - pos.start, newSyntax)
+
+          if (importees.length == 1) {
+            val end = t.pos.start - pos.start + newSyntax.length
+            (
+              syntax.take(start).lastIndexOf('{'),
+              syntax.indexOf('}', end),
+              requireBracesOnSingleImportee
+            ) match {
+              case (-1, -1, true) =>
+                // braces required but not detected
+                syntax.append('}')
+                syntax.insert(start, '{')
+              case (opening, closing, false)
+                  if opening != -1 && closing != -1 =>
+                // braces detected but not required
+                println(syntax.toString() + " " + end + " " + closing)
+                syntax.delete(end, closing + 1)
+                syntax.delete(opening, start)
+              case _ =>
+            }
+          }
+        }
+
+        val dialectAlwaysRequiresBraces = !targetDialect.allowAsForImportRename
+
+        // traverse & patch backwards to avoid shifting indices
+        importees.reverse.foreach {
+          case i @ Importee.Rename(_, _) =>
+            patchSyntax(i, i.copy().syntax, dialectAlwaysRequiresBraces)
+          case i @ Importee.Unimport(_) =>
+            patchSyntax(i, i.copy().syntax, dialectAlwaysRequiresBraces)
+          case i @ Importee.Wildcard() =>
+            patchSyntax(i, i.copy().syntax, false)
+          case i =>
+            patchSyntax(i, i.syntax, false)
+        }
+
+        syntax.toString
 
       case Position.None =>
         // Position not found, implies that `importer` is derived from certain existing import
@@ -738,11 +792,15 @@ class OrganizeImports(
 
   implicit private class ImporterExtension(importer: Importer) {
 
-    /** Checks whether the `Importer` is curly-braced when pretty-printed. */
+    /**
+     * Checks whether the `Importer` should be curly-braced when pretty-printed.
+     */
     def isCurlyBraced: Boolean = {
       val importees @ Importees(_, renames, unimports, _, _, _) =
         importer.importees
-      renames.nonEmpty || unimports.nonEmpty || importees.length > 1
+
+      importees.length > 1 ||
+      ((renames.length == 1 || unimports.length == 1) && !targetDialect.allowAsForImportRename)
     }
 
     /**
@@ -984,7 +1042,7 @@ object OrganizeImports {
    *   - Names, e.g., `Seq`, `Option`, etc.
    *   - Renames, e.g., `{Long => JLong}`, `Duration as D`, etc.
    *   - Unimports, e.g., `{Foo => _}` or `Foo as _`.
-   *   - Givens, e.g., `{given Foo}`.
+   *   - Givens, e.g., `given Foo`.
    *   - GivenAll, i.e., `given`.
    *   - Wildcard, i.e., `_` or `*`.
    */

scalafix-tests/input/src/main/scala-2-xsource3/test/organizeImports/TargetDialectAuto.scala

@@ -0,0 +1,13 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports {
+  removeUnused = false
+  targetDialect = Auto
+}
+ */
+package test.organizeImports
+
+import scala.collection.immutable.{List => L}
+import scala.collection.mutable._
+
+object TargetDialectAuto

scalafix-tests/input/src/main/scala/test/organizeImports/CurlyBracedSingleImportee.scala → scalafix-tests/input/src/main/scala-2/test/organizeImports/CurlyBracedSingleImportee.scala

@@ -1,10 +1,14 @@
 /*
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
+OrganizeImports.targetDialect = Scala2
  */
 package test.organizeImports
 
+import scala.collection.{ BitSet }
 import scala.collection.{Map}
+import scala.collection.{Seq => _}
 import scala.collection.{Set => ImmutableSet}
+import scala.util.{_}
 
 object CurlyBracedSingleImportee

scalafix-tests/input/src/main/scala-3/test/organizeImports/CoalesceImporteesGivensAndNames.scala

@@ -4,6 +4,7 @@ OrganizeImports {
   groupedImports = Keep
   coalesceToWildcardImportThreshold = 2
   removeUnused = false
+  targetDialect = Scala3
 }
  */
 package test.organizeImports

scalafix-tests/input/src/main/scala-3/test/organizeImports/CoalesceImporteesNoGivens.scala

@@ -4,6 +4,7 @@ OrganizeImports {
   groupedImports = Keep
   coalesceToWildcardImportThreshold = 2
   removeUnused = false
+  targetDialect = Scala3
 }
  */
 package test.organizeImports

scalafix-tests/input/src/main/scala-3/test/organizeImports/CoalesceImporteesNoGivensNoNames.scala

@@ -4,6 +4,7 @@ OrganizeImports {
   groupedImports = Keep
   coalesceToWildcardImportThreshold = 2
   removeUnused = false
+  targetDialect = Scala3
 }
  */
 package test.organizeImports

scalafix-tests/input/src/main/scala-3/test/organizeImports/CoalesceImporteesNoNames.scala

@@ -4,6 +4,7 @@ OrganizeImports {
   groupedImports = Keep
   coalesceToWildcardImportThreshold = 2
   removeUnused = false
+  targetDialect = Scala3
 }
  */
 package test.organizeImports

scalafix-tests/input/src/main/scala-3/test/organizeImports/CurlyBracedSingleImportee.scala

@@ -0,0 +1,17 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports.removeUnused = false
+OrganizeImports.targetDialect = Scala3
+ */
+package test.organizeImports
+
+import scala.collection.{ BitSet }
+import scala.collection.{Map}
+import scala.collection.{Seq as _}
+import scala.collection.{Set as ImmutableSet}
+import scala.util.{*}
+
+import GivenImports.{given GivenImports.Alpha}
+import GivenImports.{given}
+
+object CurlyBracedSingleImportee

scalafix-tests/input/src/main/scala-3/test/organizeImports/DeduplicateGivenImportees.scala

@@ -1,6 +1,7 @@
 /*
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
+OrganizeImports.targetDialect = Scala3
  */
 package test.organizeImports
 

scalafix-tests/input/src/main/scala-3/test/organizeImports/ExpandGiven.scala

@@ -1,12 +1,13 @@
 /*
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
+OrganizeImports.targetDialect = Scala3
  */
 package test.organizeImports
 
 import test.organizeImports.GivenImports.Beta
 import test.organizeImports.GivenImports.Alpha
-import test.organizeImports.GivenImports.{given Beta, given Alpha}
+import test.organizeImports.GivenImports.{given Beta, given test.organizeImports.GivenImports.Gamma, given Alpha}
 import scala.util.Either
 
 object ExpandGiven

scalafix-tests/input/src/main/scala-3/test/organizeImports/ExpandUnimportGiven.scala

@@ -1,6 +1,7 @@
 /*
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
+OrganizeImports.targetDialect = Scala3
  */
 package test.organizeImports
 

scalafix-tests/input/src/main/scala-3/test/organizeImports/GroupedGivenImportsMergeUnimports.scala

@@ -2,18 +2,19 @@
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
 OrganizeImports.groupedImports = Merge
+OrganizeImports.targetDialect = Scala3
  */
 package test.organizeImports
 
-import test.organizeImports.GivenImports._
-import test.organizeImports.GivenImports.{alpha => _, given}
-import test.organizeImports.GivenImports.{given Beta}
-import test.organizeImports.GivenImports.{gamma => _, given}
-import test.organizeImports.GivenImports.{given Zeta}
+import test.organizeImports.GivenImports.*
+import test.organizeImports.GivenImports.{alpha as _, given}
+import test.organizeImports.GivenImports.given Beta
+import test.organizeImports.GivenImports.{gamma as _, given}
+import test.organizeImports.GivenImports.given Zeta
 
-import test.organizeImports.GivenImports2.{alpha => _}
-import test.organizeImports.GivenImports2.{beta => _}
-import test.organizeImports.GivenImports2.{given Gamma}
-import test.organizeImports.GivenImports2.{given Zeta}
+import test.organizeImports.GivenImports2.alpha as _
+import test.organizeImports.GivenImports2.beta as _
+import test.organizeImports.GivenImports2.given Gamma
+import test.organizeImports.GivenImports2.given Zeta
 
 object GroupedGivenImportsMergeUnimports

scalafix-tests/input/src/main/scala-3/test/organizeImports/GroupedImportsAggressiveMergeGivenAll.scala

@@ -2,6 +2,7 @@
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
 OrganizeImports.groupedImports = AggressiveMerge
+OrganizeImports.targetDialect = Scala3
  */
 package test.organizeImports
 

scalafix-tests/input/src/main/scala-3/test/organizeImports/MergeGiven.scala

@@ -2,13 +2,15 @@
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
 OrganizeImports.groupedImports = Merge
+OrganizeImports.targetDialect = Scala3
  */
 package test.organizeImports
 
 import test.organizeImports.GivenImports.Beta
 import test.organizeImports.GivenImports.Alpha
-import test.organizeImports.GivenImports.{given Beta}
-import test.organizeImports.GivenImports.{given Alpha}
+import test.organizeImports.GivenImports.given Beta
+import test.organizeImports.GivenImports.given test.organizeImports.GivenImports.Gamma
+import test.organizeImports.GivenImports.given Alpha
 import scala.util.Either
 
 object MergeGiven

scalafix-tests/input/src/main/scala-3/test/organizeImports/MergeImportsFormatPreservingGiven.scala

@@ -0,0 +1,13 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports.removeUnused = false
+OrganizeImports.groupedImports = Merge
+OrganizeImports.targetDialect = Scala3
+ */
+
+package test.organizeImports
+
+import test.organizeImports.GivenImports.*
+import test.organizeImports.GivenImports.{ given Alpha, given Beta }
+
+object MergeImportsFormatPreservingGiven

scalafix-tests/input/src/main/scala-3/test/organizeImports/TargetDialectDowngrade.scala

@@ -0,0 +1,11 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports.removeUnused = false
+OrganizeImports.targetDialect = Scala2
+ */
+package test.organizeImports
+
+import scala.util.Either as E
+import scala.util.Try as _
+
+object TargetDialectDowngrade

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

@@ -2,14 +2,15 @@
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
 OrganizeImports.groupedImports = Merge
+OrganizeImports.targetDialect = Auto
  */
 package test.organizeImports
 
 import scala.collection.mutable.{
   ArrayBuffer,
   Map,
   Queue,
-  Set
+  Set => S
 }
 
 object AlreadyOrganized

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

@@ -4,6 +4,7 @@ OrganizeImports {
   groupedImports = Keep
   coalesceToWildcardImportThreshold = 3
   removeUnused = false
+  targetDialect = Auto
 }
  */
 package test.organizeImports

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

@@ -2,6 +2,7 @@
 rules = [OrganizeImports]
 OrganizeImports.removeUnused = false
 OrganizeImports.expandRelative = true
+OrganizeImports.targetDialect = Auto
  */
 package test.organizeImports