Add suppression if nowarn differs (#23652)

Fixes #23651 

The [previous fix](#22383) for the
same `@nowarn` attached to multiple elements should have compared the
`annotPos` to identify duplicates (instead of the target range).

This commit defers detecting "bad" or duplicate suppressions (which
originate with the same annotation) to report time, after the
suppression is "unused"; there are few nowarns per file and fewer that
are unused.

While checking for a suppression, mark matching unused suppressions as
"superseded", so that if they remain unused, the warning can add an
"audit" that the nowarn matched a diagnostic (but was superseded by some
other nowarn).

~This commit goes further and checks for duplicates (including whether
the filters look the same).~

~If it finds a duplicate where the `annotPos` differs, warn about the
user-written annotation.~

~Filters match each other if they are the same type and, if they have a
pattern, the string representations of the patterns are equal.~

Author: som-snytt

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

@@ -92,14 +92,21 @@ extends ImplicitRunInfo, ConstraintRunInfo, cc.CaptureRunInfo {
       mySuspendedMessages.getOrElseUpdate(warning.pos.source, mutable.LinkedHashSet.empty) += warning
 
     def nowarnAction(dia: Diagnostic): Action.Warning.type | Action.Verbose.type | Action.Silent.type =
-      mySuppressions.getOrElse(dia.pos.source, Nil).find(_.matches(dia)) match {
-        case Some(s) =>
+      mySuppressions.get(dia.pos.source) match
+      case Some(suppressions) =>
+        val matching = suppressions.iterator.filter(_.matches(dia))
+        if matching.hasNext then
+          val s = matching.next()
+          for other <- matching do
+            if !other.used then
+              other.markSuperseded() // superseded unless marked used later
           s.markUsed()
-          if (s.verbose) Action.Verbose
+          if s.verbose then Action.Verbose
           else Action.Silent
-        case _ =>
+        else
           Action.Warning
-      }
+      case none =>
+        Action.Warning
 
     def registerNowarn(annotPos: SourcePosition, range: Span)(conf: String, pos: SrcPos)(using Context): Unit =
       var verbose = false
@@ -118,12 +125,10 @@ extends ImplicitRunInfo, ConstraintRunInfo, cc.CaptureRunInfo {
           .merge
       addSuppression:
         Suppression(annotPos, filters, range.start, range.end, verbose)
-          .tap: sup =>
-            if filters == List(MessageFilter.None) then sup.markUsed() // invalid suppressions, don't report as unused
 
     def addSuppression(sup: Suppression): Unit =
       val suppressions = mySuppressions.getOrElseUpdate(sup.annotPos.source, ListBuffer.empty)
-      if sup.start != sup.end && suppressions.forall(x => x.start != sup.start || x.end != sup.end) then
+      if sup.start != sup.end then
         suppressions += sup
 
     def reportSuspendedMessages(source: SourceFile)(using Context): Unit = {
@@ -134,18 +139,25 @@ extends ImplicitRunInfo, ConstraintRunInfo, cc.CaptureRunInfo {
       mySuspendedMessages.remove(source).foreach(_.foreach(ctx.reporter.issueIfNotSuppressed))
     }
 
-    def runFinished(hasErrors: Boolean): Unit =
+    def runFinished()(using Context): Unit =
+      val hasErrors = ctx.reporter.hasErrors
       // report suspended messages (in case the run finished before typer)
       mySuspendedMessages.keysIterator.toList.foreach(reportSuspendedMessages)
       // report unused nowarns only if all all phases are done
       if !hasErrors && ctx.settings.WunusedHas.nowarn then
         for
           source <- mySuppressions.keysIterator.toList
           sups   <- mySuppressions.remove(source)
-          sup    <- sups.reverse
-          if !sup.used
         do
-          report.warning("@nowarn annotation does not suppress any warnings", sup.annotPos)
+          val suppressions = sups.reverse.toList
+          for sup <- suppressions do
+            if !sup.used
+            && !suppressions.exists(s => s.ne(sup) && s.used && s.annotPos == sup.annotPos) // duplicate
+            && sup.filters != List(MessageFilter.None) // invalid suppression, don't report as unused
+            then
+              val more = if sup.superseded then " but matches a diagnostic" else ""
+              report.warning("@nowarn annotation does not suppress any warnings"+more, sup.annotPos)
+  end suppressions
 
   /** The compilation units currently being compiled, this may return different
    *  results over time.
@@ -411,7 +423,7 @@ extends ImplicitRunInfo, ConstraintRunInfo, cc.CaptureRunInfo {
     ctx.reporter.finalizeReporting()
     if (!ctx.reporter.hasErrors)
       Rewrites.writeBack()
-    suppressions.runFinished(hasErrors = ctx.reporter.hasErrors)
+    suppressions.runFinished()
     while (finalizeActions.nonEmpty && canProgress()) {
       val action = finalizeActions.remove(0)
       action()

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

@@ -10,6 +10,7 @@ import dotty.tools.dotc.interfaces.SourceFile
 import dotty.tools.dotc.reporting.MessageFilter.SourcePattern
 
 import java.util.regex.PatternSyntaxException
+import scala.PartialFunction.cond
 import scala.annotation.internal.sharable
 import scala.util.matching.Regex
 
@@ -136,13 +137,20 @@ object WConf:
       if (parseErrorss.nonEmpty) Left(parseErrorss.flatten)
       else Right(WConf(configs))
 
-class Suppression(val annotPos: SourcePosition, filters: List[MessageFilter], val start: Int, val end: Int, val verbose: Boolean):
-  private var _used = false
-  def used: Boolean = _used
+class Suppression(val annotPos: SourcePosition, val filters: List[MessageFilter], val start: Int, val end: Int, val verbose: Boolean):
+  inline def unusedState = 0
+  inline def usedState = 1
+  inline def supersededState = 2
+  private var _used = unusedState
+  def used: Boolean = _used == usedState
+  def superseded: Boolean = _used == supersededState
   def markUsed(): Unit =
-    _used = true
+    _used = usedState
+  def markSuperseded(): Unit =
+    _used = supersededState
   def matches(dia: Diagnostic): Boolean =
     val pos = dia.pos
     pos.exists && start <= pos.start && pos.end <= end && filters.forall(_.matches(dia))
 
   override def toString = s"Suppress in ${annotPos.source} $start..$end [${filters.mkString(", ")}]"
+end Suppression

tests/neg/nowarn.check

@@ -0,0 +1,27 @@
+//> using options -deprecation -Wunused:nowarn
+
+import scala.annotation.nowarn
+
+@deprecated
+class A
+
+@deprecated
+class B
+
+@nowarn("msg=trait C is deprecated") // warn
+  // @nowarn annotation does not suppress any warnings
+@nowarn("msg=class A is deprecated")
+@nowarn("cat=deprecation&msg=class A is deprecated") // warn
+  // @nowarn annotation does not suppress any warnings but matches a diagnostic
+@nowarn("cat=deprecation&msg=class B is deprecated")
+trait C1:
+  def a: A
+  def b: B
+
+@nowarn("cat=deprecation&msg=class B is deprecated")
+@nowarn("cat=deprecation&msg=class B is deprecated") // warn
+  // @nowarn annotation does not suppress any warnings but matches a diagnostic
+@nowarn("cat=deprecation&msg=class A is deprecated")
+trait C2:
+  def a: A
+  def b: B