Clean up rewrite unused imports

compiler/src/dotty/tools/dotc/core/Contexts.scala

@@ -25,7 +25,6 @@ import reporting._
 import io.{AbstractFile, NoAbstractFile, PlainFile, Path}
 import scala.io.Codec
 import collection.mutable
-import parsing.Parsers
 import printing._
 import config.{JavaPlatform, SJSPlatform, Platform, ScalaSettings, ScalaRelease}
 import classfile.ReusableDataReader
@@ -1024,7 +1023,6 @@ object Contexts:
   /** Collect information about the run for purposes of additional diagnostics.
    */
   class Usages:
-    import rewrites.Rewrites.patch
     private val selectors   = mutable.Map.empty[ImportInfo, Set[untpd.ImportSelector]].withDefaultValue(Set.empty)
     private val importInfos = mutable.Map.empty[CompilationUnit, List[(ImportInfo, Symbol)]].withDefaultValue(Nil)
 
@@ -1040,60 +1038,28 @@ object Contexts:
         selectors(info) += selector
 
     // unused import, owner, which selector
-    def unused(using Context): List[(ImportInfo, Symbol, untpd.ImportSelector)] =
-      var unusages = List.empty[(ImportInfo, Symbol, untpd.ImportSelector)]
+    def unused(using Context): List[(ImportInfo, Symbol, List[untpd.ImportSelector])] =
       if ctx.settings.WunusedHas.imports && !ctx.compilationUnit.isJava then
-        //if ctx.settings.Ydebug.value then
-        //  println(importInfos.get(ctx.compilationUnit).map(iss => iss.map((ii, s) => s"${ii.show} ($ii)")).getOrElse(Nil).mkString("Registered ImportInfos\n", "\n", ""))
-        //  println(selectors.toList.flatMap((k,v) => v.toList.map(sel => s"${k.show} -> $sel")).mkString("Used selectors\n", "\n", ""))
+        var unusages = List.empty[(ImportInfo, Symbol, List[untpd.ImportSelector])]
         def checkUsed(info: ImportInfo, owner: Symbol): Unit =
-          val used = selectors(info)
-          var needsPatch = false
+          val usedSelectors = selectors(info)
+          var unusedSelectors = List.empty[untpd.ImportSelector]
           def cull(toCheck: List[untpd.ImportSelector]): Unit =
             toCheck match
               case selector :: rest =>
                 cull(rest) // reverse
-                if !selector.isMask && !used(selector) then
-                  unusages ::= ((info, owner, selector))
-                  needsPatch = true
+                if !selector.isMask && !usedSelectors(selector) then
+                  unusedSelectors ::= selector
               case _ =>
           cull(info.selectors)
-          if needsPatch && ctx.settings.YrewriteImports.value then
-            val src = ctx.compilationUnit.source
-            val infoPos = info.qualifier.sourcePos
-            val lineSource = SourceFile.virtual(name = "import-line.scala", content = infoPos.lineContent)
-            val PackageDef(_, pieces) = Parsers.Parser(lineSource).parse(): @unchecked
-            // patch if there's just one import on the line, i.e., not import a.b, c.d
-            if pieces.length == 1 then
-              val retained = info.selectors.filter(sel => sel.isMask || used(sel))
-              val selectorSpan = info.selectors.map(_.span).reduce(_ union _)
-              val lineSpan = src.lineSpan(infoPos.start)
-              if retained.isEmpty then
-                patch(src, lineSpan, "")  // line deletion
-              else if retained.size == 1 && info.selectors.size > 1 then
-                var starting = info.selectors.head.span.start
-                while starting > lineSpan.start && src.content()(starting) != '{' do starting -= 1
-                var ending = info.selectors.last.span.end
-                while ending <= lineSpan.end && src.content()(ending) != '}' do ending += 1
-                if ending < lineSpan.end then ending += 1  // past the close brace
-                val widened = selectorSpan.withStart(starting).withEnd(ending)
-                patch(src, widened, toText(retained))  // try to remove braces
-              else
-                patch(src, selectorSpan, toText(retained))
+          if unusedSelectors.nonEmpty then unusages ::= (info, owner, unusedSelectors)
         end checkUsed
         importInfos.remove(ctx.compilationUnit).foreach(_.foreach(checkUsed))
-      unusages
+        unusages
+      else
+        Nil
     end unused
 
-    // just the selectors, no need to add braces
-    private def toText(retained: List[untpd.ImportSelector])(using Context): String =
-      def selected(sel: untpd.ImportSelector) =
-        if sel.isGiven then "given"
-        else if sel.isWildcard then "*"
-        else if sel.name == sel.rename then sel.name.show
-        else s"${sel.name.show} as ${sel.rename.show}"
-      retained.map(selected).mkString(", ")
-
     def clear()(using Context): Unit =
       importInfos.clear()
       selectors.clear()

compiler/src/dotty/tools/dotc/typer/TyperPhase.scala

@@ -2,6 +2,7 @@ package dotty.tools
 package dotc
 package typer
 
+import ast.untpd
 import core._
 import Phases._
 import Contexts._
@@ -56,7 +57,47 @@ class TyperPhase(addRootImports: Boolean = true) extends Phase {
   }
 
   def emitDiagnostics(using Context): Unit =
-    ctx.usages.unused.foreach((importInfo, owner, selector) => report.warning(s"Unused import", pos = selector.srcPos))
+    ctx.usages.unused.foreach { (info, owner, selectors) =>
+      import rewrites.Rewrites.patch
+      import parsing.Parsers
+      import util.SourceFile
+      import ast.Trees.*
+      def reportSelectors() = selectors.foreach(selector => report.warning(s"Unused import", pos = selector.srcPos))
+      if ctx.settings.YrewriteImports.value then
+        val src = ctx.compilationUnit.source
+        val infoPos = info.qualifier.sourcePos
+        val lineSource = SourceFile.virtual(name = "import-line.scala", content = infoPos.lineContent)
+        val PackageDef(_, pieces) = Parsers.Parser(lineSource).parse(): @unchecked
+        // patch if there's just one import on the line, i.e., not import a.b, c.d
+        if pieces.length == 1 then
+          val retained = info.selectors.filterNot(selectors.contains)
+          val selectorSpan = info.selectors.map(_.span).reduce(_ union _)
+          val lineSpan = src.lineSpan(infoPos.start)
+          if retained.isEmpty then
+            patch(src, lineSpan, "")  // line deletion
+          else if retained.size == 1 && info.selectors.size > 1 then
+            var starting = info.selectors.head.span.start
+            while starting > lineSpan.start && src.content()(starting) != '{' do starting -= 1
+            var ending = info.selectors.last.span.end
+            while ending <= lineSpan.end && src.content()(ending) != '}' do ending += 1
+            if ending < lineSpan.end then ending += 1  // past the close brace
+            val widened = selectorSpan.withStart(starting).withEnd(ending)
+            patch(src, widened, toText(retained))  // try to remove braces
+          else
+            patch(src, selectorSpan, toText(retained))
+        else
+          reportSelectors()
+      else
+        reportSelectors()
+    }
+  // just the selectors, no need to add braces
+  private def toText(retained: List[untpd.ImportSelector])(using Context): String =
+    def selected(sel: untpd.ImportSelector) =
+      if sel.isGiven then "given"
+      else if sel.isWildcard then "*"
+      else if sel.name == sel.rename then sel.name.show
+      else s"${sel.name.show} as ${sel.rename.show}"
+    retained.map(selected).mkString(", ")
   def clearDiagnostics()(using Context): Unit =
     ctx.usages.clear()
 

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -79,6 +79,7 @@ class CompilationTests {
       compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")),
       compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")),
       compileFile("tests/rewrites/i12340.scala", unindentOptions.and("-rewrite")),
+      compileFile("tests/rewrites/unused-imports.scala", defaultOptions),
     ).checkRewrites()
   }
 

tests/rewrites/unused-imports.check

@@ -0,0 +1,60 @@
+// scalac: -Wunused:imports -Werror -feature -rewrite -Yrewrite-imports
+
+import language.implicitConversions
+import scala.concurrent.ExecutionContext.Implicits.*
+
+class C:
+  def c = 42
+  import scala.collection.mutable.{Seq as _, ListBuffer, Set as OK} //?error //?error
+
+  def buf = ListBuffer.empty[String]
+  def ok: OK[Int] = ???
+  def f   = concurrent.Future(42)  // implicit usage
+
+  import Thread.*
+  import State.{NEW, BLOCKED}
+
+  def state(t: Thread) =
+    t.getState match
+      case NEW =>
+        import State.RUNNABLE
+        t.getState match
+          case RUNNABLE => 0
+          case BLOCKED => 1
+          case _ => -1
+      case _ => -1
+
+  enum E:
+    case E0, E1
+
+  def e(x: E) =
+    x match
+      case E.E0 => "e0"
+      case E.E1 => "e1"
+
+  locally {
+    import Givens.{*, given}
+
+    def g(s: String)(using Ordering[C]) = ???
+    def ordered = g(greeting)
+  }
+  locally {
+    import Givens.{cOrdering, *}
+
+    def g(s: String)(using Ordering[C]) = ???
+    def ordered = g(greeting)
+  }
+
+object Givens:
+  given cOrdering: Ordering[C] with
+    override def compare(c0: C, c1: C) = 0
+  val greeting = "we love Givens"
+
+class A:
+  def f(b: B): Unit = b.f(this)
+
+object A:
+  implicit val a2b: Conversion[A, B] = _ => B()
+
+class B:
+  def f(b: B): Unit = ()

tests/rewrites/unused-imports.scala

@@ -0,0 +1,62 @@
+// scalac: -Wunused:imports -Werror -feature -rewrite -Yrewrite-imports
+
+import language.implicitConversions
+import language.postfixOps //?error
+import scala.concurrent.* //?error
+import scala.concurrent.ExecutionContext.Implicits.*
+
+class C:
+  def c = 42
+  import scala.collection.mutable.{HashMap as GoodMap, Seq as _, ListBuffer, Buffer, Set as OK} //?error //?error
+
+  def buf = ListBuffer.empty[String]
+  def ok: OK[Int] = ???
+  def f   = concurrent.Future(42)  // implicit usage
+
+  import Thread.*
+  import State.{NEW, BLOCKED}
+
+  def state(t: Thread) =
+    t.getState match
+      case NEW =>
+        import State.RUNNABLE
+        t.getState match
+          case RUNNABLE => 0
+          case BLOCKED => 1
+          case _ => -1
+      case _ => -1
+
+  enum E:
+    case E0, E1
+
+  def e(x: E) =
+    x match
+      case E.E0 => "e0"
+      case E.E1 => "e1"
+
+  locally {
+    import Givens.{*, given}
+
+    def g(s: String)(using Ordering[C]) = ???
+    def ordered = g(greeting)
+  }
+  locally {
+    import Givens.{cOrdering, *}
+
+    def g(s: String)(using Ordering[C]) = ???
+    def ordered = g(greeting)
+  }
+
+object Givens:
+  given cOrdering: Ordering[C] with
+    override def compare(c0: C, c1: C) = 0
+  val greeting = "we love Givens"
+
+class A:
+  def f(b: B): Unit = b.f(this)
+
+object A:
+  implicit val a2b: Conversion[A, B] = _ => B()
+
+class B:
+  def f(b: B): Unit = ()