Merge pull request #636 from scala/backport-lts-3.3-24055

Backport "Unused lint does not rewrite suppressed warnings" to 3.3 LTS

Author: tgodzik

compiler/src/dotty/tools/dotc/Driver.scala

@@ -92,11 +92,13 @@ class Driver {
           case "help" =>
           case reporterClassName =>
             try
-              Class.forName(reporterClassName).getDeclaredConstructor().newInstance() match
-              case userReporter: Reporter =>
-                ictx.setReporter(userReporter)
-              case badReporter => report.error:
-                em"Not a reporter: ${ctx.settings.Yreporter.value}"
+              val newCls =  Class.forName(reporterClassName).nn
+              val newInst = newCls.getDeclaredConstructor().nn.newInstance().nn 
+              newInst match
+                case userReporter: Reporter =>
+                  ictx.setReporter(userReporter)
+                case badReporter => report.error:
+                  em"Not a reporter: ${ctx.settings.Yreporter.value}"
             catch case e: ReflectiveOperationException => report.error:
               em"Could not create reporter ${ctx.settings.Yreporter.value}: ${e}"
   }

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -377,7 +377,6 @@ private sealed trait YSettings:
   val YdisableFlatCpCaching: Setting[Boolean] = BooleanSetting("-YdisableFlatCpCaching", "Do not cache flat classpath representation of classpath elements from jars across compiler instances.")
 
   val Yscala2Unpickler: Setting[String] = StringSetting("-Yscala2-unpickler", "", "Control where we may get Scala 2 symbols from. This is either \"always\", \"never\", or a classpath.", "always")
-  val YnoReporter: Setting[Boolean] = BooleanSetting("-Yno-reporter", "Diagnostics are silently consumed")
   val Yreporter: Setting[String] = StringSetting(name = "-Yreporter", helpArg = "<class>", descr = "Specify a dotty.tools.dotc.reporting.Reporter", default = "dotty.tools.dotc.reporting.ConsoleReporter")
 
   val YnoImports: Setting[Boolean] = BooleanSetting("-Yno-imports", "Compile without importing scala.*, java.lang.*, or Predef.")

compiler/src/dotty/tools/dotc/interactive/Completion.scala

@@ -576,8 +576,8 @@ object Completion:
               termRef.denot.asSingleDenotation.mapInfo(_ => tpe)
             }
         catch case NonFatal(ex) =>
-          logger.warning(
-            s"Exception when trying to apply extension method:\n ${ex.getMessage()}\n${ex.getStackTrace().mkString("\n")}"
+          logger.nn.warning(
+            s"Exception when trying to apply extension method:\n ${ex.getMessage()}\n${ex.getStackTrace().nn.mkString("\n")}"
           )
           None
 
@@ -684,8 +684,8 @@ object Completion:
       interactiv.println(i"implicit conversion targets considered: ${conversions.toList}%, %")
       conversions
     } catch case NonFatal(ex) =>
-      logger.warning(
-        s"Exception when searching for implicit conversions:\n ${ex.getMessage()}\n${ex.getStackTrace().mkString("\n")}"
+      logger.nn.warning(
+        s"Exception when searching for implicit conversions:\n ${ex.getMessage()}\n${ex.getStackTrace().nn.mkString("\n")}"
       )
       Set.empty
 

compiler/src/dotty/tools/dotc/reporting/Diagnostic.scala

@@ -42,10 +42,12 @@ object Diagnostic:
    */
   trait OriginWarning(val origin: String):
     self: Warning =>
+  object OriginWarning:
+    val NoOrigin = "..."
 
   /** Lints are likely to be filtered. Provide extra axes for filtering by `-Wconf`.
    */
-  class LintWarning(msg: Message, pos: SourcePosition, origin: String)
+  class LintWarning(msg: Message, pos: SourcePosition, origin: String = OriginWarning.NoOrigin)
   extends Warning(msg, pos), OriginWarning(origin)
 
   class Warning(

compiler/src/dotty/tools/dotc/reporting/Reporter.scala

@@ -9,6 +9,7 @@ import dotty.tools.dotc.core.Mode
 import dotty.tools.dotc.core.Symbols.{NoSymbol, Symbol}
 import dotty.tools.dotc.reporting.Diagnostic.*
 import dotty.tools.dotc.reporting.Message.*
+import dotty.tools.dotc.rewrites.Rewrites
 import dotty.tools.dotc.util.NoSourcePosition
 
 import java.io.{BufferedReader, PrintWriter}
@@ -174,6 +175,8 @@ abstract class Reporter extends interfaces.ReporterResult {
           handleRecursive("error reporting", dia.message, ex)
         dia match {
           case w: Warning =>
+            if w.isInstanceOf[LintWarning] then
+              w.msg.actions.foreach(Rewrites.applyAction)
             warnings = w :: warnings
             _warningCount += 1
           case e: Error   =>

compiler/src/dotty/tools/dotc/reporting/WConf.scala

@@ -34,7 +34,8 @@ enum MessageFilter:
       pattern.findFirstIn(path).nonEmpty
     case Origin(pattern) =>
       message match
-      case message: OriginWarning => pattern.findFirstIn(message.origin).nonEmpty
+      case message: OriginWarning if message.origin != OriginWarning.NoOrigin =>
+        pattern.findFirstIn(message.origin).nonEmpty
       case _ => false
     case None => false
 

compiler/src/dotty/tools/dotc/transform/CheckUnused.scala

@@ -16,8 +16,8 @@ import dotty.tools.dotc.core.StdNames.nme
 import dotty.tools.dotc.core.Symbols.{ClassSymbol, NoSymbol, Symbol, defn, isDeprecated, requiredClass, requiredModule}
 import dotty.tools.dotc.core.Types.*
 import dotty.tools.dotc.report
-import dotty.tools.dotc.reporting.{CodeAction, UnusedSymbol}
-import dotty.tools.dotc.rewrites.Rewrites
+import dotty.tools.dotc.reporting.{CodeAction, Diagnostic, UnusedSymbol}
+import dotty.tools.dotc.rewrites.Rewrites.ActionPatch
 import dotty.tools.dotc.transform.MegaPhase.MiniPhase
 import dotty.tools.dotc.typer.{ImportInfo, Typer}
 import dotty.tools.dotc.typer.Deriving.OriginalTypeClass
@@ -527,16 +527,15 @@ object CheckUnused:
 
   def reportUnused()(using Context): Unit = if !refInfos.isNullified then
     for (msg, pos, origin) <- warnings do
-      if origin.isEmpty then report.warning(msg, pos)
-      else report.warning(msg, pos, origin)
-      msg.actions.headOption.foreach(Rewrites.applyAction)
+      report.warning(msg, pos, origin)
 
   type MessageInfo = (UnusedSymbol, SrcPos, String) // string is origin or empty
 
   def warnings(using Context): Array[MessageInfo] =
-    val actionable = ctx.settings.rewrite.value.nonEmpty
+    val actionable: true = true //ctx.settings.rewrite.value.nonEmpty
     val warnings = ArrayBuilder.make[MessageInfo]
-    def warnAt(pos: SrcPos)(msg: UnusedSymbol, origin: String = ""): Unit = warnings.addOne((msg, pos, origin))
+    def warnAt(pos: SrcPos)(msg: UnusedSymbol, origin: String = Diagnostic.OriginWarning.NoOrigin): Unit =
+      warnings.addOne((msg, pos, origin))
     val infos = refInfos
 
     // non-local sym was target of assignment or has a sibling setter that was referenced
@@ -699,7 +698,6 @@ object CheckUnused:
 
     def checkImports() =
       import scala.jdk.CollectionConverters.given
-      import Rewrites.ActionPatch
       type ImpSel = (Import, ImportSelector)
       def isUsed(sel: ImportSelector): Boolean = infos.sels.containsKey(sel)
       def warnImport(warnable: ImpSel, actions: List[CodeAction] = Nil): Unit =

compiler/src/dotty/tools/dotc/util/Chars.scala

@@ -137,7 +137,7 @@ object Chars:
 
   def escapedChar(ch: Char): String =
     if requiresFormat(ch) then
-      val b = StringBuilder().append('\'')
+      val b = StringBuilder().append('\'').nn
       escapedChar(b, ch)
       b.append('\'').toString
     else

presentation-compiler/test/dotty/tools/pc/tests/DiagnosticProviderSuite.scala

@@ -32,10 +32,11 @@ class DiagnosticProviderSuite extends PcAssertions {
     additionalChecks: List[Diagnostic] => Unit = identity
   ): Unit =
     val diagnostics = pc
-      .didChange(TestVirtualFileParams(URI.create("file:/Diagnostic.scala"), text))
+      .didChange(TestVirtualFileParams(URI.create("file:/Diagnostic.scala").nn, text))
+      .nn
       .asScala
 
-    val actual = diagnostics.map(d => TestDiagnostic(d.getRange().getStart().getOffset(text), d.getRange().getEnd().getOffset(text), d.getMessage(), d.getSeverity()))
+    val actual = diagnostics.map(d => TestDiagnostic(d.getRange().nn.getStart().nn.getOffset(text), d.getRange().nn.getEnd().nn.getOffset(text), d.getMessage().nn, d.getSeverity().nn))
     assertEquals(expected, actual, s"Expected [${expected.mkString(", ")}] but got [${actual.mkString(", ")}]")
     additionalChecks(diagnostics.toList)
 
@@ -77,8 +78,8 @@ class DiagnosticProviderSuite extends PcAssertions {
       ),
       diags =>
         val action = diags.head.getData().asInstanceOf[java.util.List[CodeAction]].asScala.head
-        assertWithDiff("Remove repeated modifier: \"private\"", action.getTitle(), false)
-        assertEquals(1, action.getEdit().getChanges().size(), "There should be one change")
+        assertWithDiff("Remove repeated modifier: \"private\"", action.getTitle().nn, false)
+        assertEquals(1, action.getEdit().nn.getChanges().nn.size(), "There should be one change")
     )
 
 }

tests/rewrites/i24009.scala

@@ -0,0 +1,9 @@
+//> using options -Wunused:imports -rewrite -Wconf:name=UnusedSymbol&origin=p.C:s
+
+package p:
+  class C
+
+package q:
+  import p.C // nowarn and no rewrite
+
+  class D