nowarn cleanups, fixes, tests

- undo ansi escapes before matching the nowarn msg=regex
- first tests for nowarn in parser, typer, refchecks warnings
- handle summarized conditional warnings in one place

Author: lrytz

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

@@ -21,15 +21,7 @@ object report:
     ctx.reporter.report(new Info(msg, pos.sourcePos))
 
   private def issueWarning(warning: Warning)(using Context): Unit =
-    if (!ctx.settings.silentWarnings.value)
-      if (ctx.settings.XfatalWarnings.value)
-        warning match {
-          case warning: ConditionalWarning if !warning.enablingOption.value =>
-            ctx.reporter.report(warning) // conditional warnings that are not enabled are not fatal
-          case _ =>
-            ctx.reporter.report(warning.toError)
-        }
-      else ctx.reporter.report(warning)
+    ctx.reporter.report(warning)
 
   def deprecationWarning(msg: Message, pos: SrcPos = NoSourcePosition)(using Context): Unit =
     issueWarning(new DeprecationWarning(msg, pos.sourcePos))

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

@@ -21,21 +21,16 @@ class ConsoleReporter(
 
   /** Prints the message with the given position indication. */
   def doReport(dia: Diagnostic)(using Context): Unit = {
-    val didPrint = dia match {
+    dia match
       case dia: Error =>
         printMessage(messageAndPos(dia.msg, dia.pos, diagnosticLevel(dia)))
         if (ctx.settings.Xprompt.value) Reporter.displayPrompt(reader, writer)
-        true
-      case dia: ConditionalWarning if !dia.enablingOption.value =>
-        false
       case dia =>
         printMessage(messageAndPos(dia.msg, dia.pos, diagnosticLevel(dia)))
-        true
-    }
 
-    if (didPrint && shouldExplain(dia))
+    if shouldExplain(dia) then
       printMessage(explanation(dia.msg))
-    else if (didPrint && dia.msg.canExplain)
+    else if dia.msg.canExplain then
       printMessage("\nlonger explanation available when compiling with `-explain`")
   }
 

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

@@ -37,6 +37,7 @@ object Diagnostic:
   ) extends Diagnostic(msg, pos, WARNING) {
     def toError: Error = new Error(msg, pos)
     def toInfo: Info = new Info(msg, pos)
+    def isSummarizedConditional(using Context): Boolean = false
   }
 
   class Info(
@@ -49,6 +50,7 @@ object Diagnostic:
     pos: SourcePosition
   ) extends Warning(msg, pos) {
     def enablingOption(using Context): Setting[Boolean]
+    override def isSummarizedConditional(using Context): Boolean = !enablingOption.value
   }
 
   class FeatureWarning(

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

@@ -21,11 +21,7 @@ object Reporter {
   /** Convert a SimpleReporter into a real Reporter */
   def fromSimpleReporter(simple: interfaces.SimpleReporter): Reporter =
     new Reporter with UniqueMessagePositions with HideNonSensicalMessages {
-      override def doReport(dia: Diagnostic)(using Context): Unit = dia match {
-        case dia: ConditionalWarning if !dia.enablingOption.value =>
-        case _ =>
-          simple.report(dia)
-      }
+      override def doReport(dia: Diagnostic)(using Context): Unit = simple.report(dia)
     }
 
   /** A reporter that ignores reports, and doesn't record errors */
@@ -143,47 +139,48 @@ abstract class Reporter extends interfaces.ReporterResult {
   def issueIfNotSuppressed(dia: Diagnostic)(using Context): Unit =
     def go() =
       import Action._
-      val toReport = dia match {
-        case w: Warning => WConf.parsed.action(dia) match {
-          case Silent  => None
-          case Info    => Some(w.toInfo)
-          case Warning => Some(w)
-          case Error   => Some(w.toError)
-        }
+
+      val toReport = dia match
+        case w: Warning =>
+          if ctx.settings.silentWarnings.value then None
+          else if ctx.settings.XfatalWarnings.value && !w.isSummarizedConditional then Some(w.toError)
+          else WConf.parsed.action(dia) match
+            case Silent  => None
+            case Info    => Some(w.toInfo)
+            case Warning => Some(w)
+            case Error   => Some(w.toError)
         case _ => Some(dia)
+
+      toReport foreach {
+        case cw: ConditionalWarning if cw.isSummarizedConditional =>
+          val key = cw.enablingOption.name
+          unreportedWarnings =
+            unreportedWarnings.updated(key, unreportedWarnings.getOrElse(key, 0) + 1)
+        case d if !isHidden(d) =>
+          withMode(Mode.Printing)(doReport(d))
+          d match {
+            case _: Warning => _warningCount += 1
+            case e: Error =>
+              errors = e :: errors
+              _errorCount += 1
+            case _: Info => // nothing to do here
+            // match error if d is something else
+          }
+        case _ => // hidden
       }
-      for (d <- toReport) {
-        withMode(Mode.Printing)(doReport(d))
-        d match
-          case cw: ConditionalWarning if !cw.enablingOption.value =>
-            val key = cw.enablingOption.name
-            unreportedWarnings =
-              unreportedWarnings.updated(key, unreportedWarnings.getOrElse(key, 0) + 1)
-          case _: Warning => _warningCount += 1
-          case e: Error =>
-            errors = e :: errors
-            _errorCount += 1
-          case _: Info => // nothing to do here
-        // match error if d is something else
-      }
+    end go
 
-    dia match {
+    dia match
       case w: Warning if ctx.run != null =>
         val sup = ctx.run.suppressions
         if sup.suppressionsComplete(w.pos.source) then
           if !sup.isSuppressed(w) then go()
         else
           sup.addSuspendedMessage(w)
       case _ => go()
-    }
+  end issueIfNotSuppressed
 
-  def report(dia: Diagnostic)(using Context): Unit =
-    val isSummarized = dia match
-      case cw: ConditionalWarning => !cw.enablingOption.value
-      case _ => false
-    // avoid isHidden test for summarized warnings so that message is not forced
-    if isSummarized || !isHidden(dia) then
-      issueIfNotSuppressed(dia)
+  def report(dia: Diagnostic)(using Context): Unit = issueIfNotSuppressed(dia)
 
   def incomplete(dia: Diagnostic)(using Context): Unit =
     incompleteHandler(dia, ctx)

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

@@ -15,9 +15,12 @@ enum MessageFilter:
     case Any => true
     case Deprecated => message.isInstanceOf[Diagnostic.DeprecationWarning]
     case Feature => message.isInstanceOf[Diagnostic.FeatureWarning]
-    case MessagePattern(pattern) => pattern.findFirstIn(message.msg.rawMessage).nonEmpty
+    case MessagePattern(pattern) =>
+      val noHighlight = message.msg.rawMessage.replaceAll("\\e\\[[\\d;]*[^\\d;]","")
+      pattern.findFirstIn(noHighlight).nonEmpty
+    case None => false
   }
-  case Any, Deprecated, Feature
+  case Any, Deprecated, Feature, None
   case MessagePattern(pattern: Regex)
 
 enum Action:

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

@@ -2106,7 +2106,7 @@ class Typer extends Namer
     for (annot <- mdef.mods.annotations)
       val annot1 = typedAnnotation(annot)(using annotCtx)
       checkAnnotApplicable(annot1, sym)
-      if (Annotations.annotClass(annot1) == defn.NowarnAnnot)
+      if Annotations.annotClass(annot1) == defn.NowarnAnnot then
         registerNowarn(annot1, mdef)
   }
 
@@ -2118,21 +2118,21 @@ class Typer extends Namer
     def argPos = annot.argument(0).getOrElse(tree).sourcePos
     val filters =
       if annot.arguments.isEmpty then List(MessageFilter.Any)
-      else annot.argumentConstantString(0) match {
-        case None => annot.argument(0) match {
-          case Some(t: Select) if t.name.toString == "$lessinit$greater$default$1" => List(MessageFilter.Any)
+      else annot.argumentConstantString(0) match
+        case None => annot.argument(0) match
+          case Some(t: Select) if t.name.is(DefaultGetterName) => List(MessageFilter.Any)
           case _ =>
             report.warning(s"filter needs to be a compile-time constant string", argPos)
             Nil
-        }
         case Some(s) =>
           if s.isEmpty then Nil
           else
             val (ms, fs) = s.split('&').map(WConf.parseFilter).toList.partitionMap(identity)
-            if (ms.nonEmpty)
+            if ms.nonEmpty then
               report.warning(s"Invalid message filter\n${ms.mkString("\n")}", argPos)
-            fs
-      }
+              List(MessageFilter.None)
+            else
+              fs
     val range = mdef.sourcePos
     ctx.run.suppressions.addSuppression(Suppression(tree.sourcePos, filters, range.start, range.end))
 

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

@@ -128,6 +128,7 @@ class CompilationTests {
       compileFilesInDir("tests/neg-no-kind-polymorphism", defaultOptions and "-Yno-kind-polymorphism"),
       compileFilesInDir("tests/neg-custom-args/deprecation", defaultOptions.and("-Xfatal-warnings", "-deprecation")),
       compileFilesInDir("tests/neg-custom-args/fatal-warnings", defaultOptions.and("-Xfatal-warnings")),
+      compileFilesInDir("tests/neg-custom-args/nowarn", defaultOptions.and("-deprecation", "-Werror")),
       compileFilesInDir("tests/neg-custom-args/erased", defaultOptions.and("-language:experimental.erasedDefinitions")),
       compileFilesInDir("tests/neg-custom-args/allow-double-bindings", allowDoubleBindings),
       compileFilesInDir("tests/neg-custom-args/allow-deep-subtypes", allowDeepSubtypes),

tests/neg-custom-args/nowarn/nowarn-parser-typer.check

@@ -0,0 +1,18 @@
+-- [E000] Syntax Error: tests/neg-custom-args/nowarn/nowarn-parser-typer.scala:8:10 ------------------------------------
+8 |def t1a = try 1 // error, parser warning
+  |          ^^^^^
+  |          A try without catch or finally is equivalent to putting
+  |          its body in a block; no exceptions are handled.
+
+longer explanation available when compiling with `-explain`
+-- [E129] Potential Issue Error: tests/neg-custom-args/nowarn/nowarn-parser-typer.scala:16:11 --------------------------
+16 |def t3 = { 1; 2 } // error, the invalid nowarn doesn't silence this warning
+   |           ^
+   |           A pure expression does nothing in statement position; you may be omitting necessary parentheses
+
+longer explanation available when compiling with `-explain`
+-- Error: tests/neg-custom-args/nowarn/nowarn-parser-typer.scala:15:8 --------------------------------------------------
+15 |@nowarn("wat?") // error, invalid filter
+   |        ^^^^^^
+   |        Invalid message filter
+   |        unknown filter: wat?

tests/neg-custom-args/nowarn/nowarn-parser-typer.scala

@@ -0,0 +1,16 @@
+import annotation.nowarn
+
+// parser warnigns are buffered until the end of typer (where `@nowarn` annotations are handled).
+
+// separate test file, because with `-Werror` this test only runs to the end of type checking.
+// later warnings (eg deprecations in refchecks) are not issued.
+
+def t1a = try 1 // error, parser warning
+@nowarn("msg=try without catch") def t1b = try 1
+
+@deprecated def f = 0
+
+def t2 = f // not reported because refchecks doesn't run
+
+@nowarn("wat?") // error, invalid filter
+def t3 = { 1; 2 } // error, the invalid nowarn doesn't silence this warning

tests/neg-custom-args/nowarn/nowarn-refchecks.check

@@ -0,0 +1,12 @@
+-- Error: tests/neg-custom-args/nowarn/nowarn-refchecks.scala:5:9 ------------------------------------------------------
+5 |def t1 = f // error
+  |         ^
+  |         method f is deprecated
+-- Error: tests/neg-custom-args/nowarn/nowarn-refchecks.scala:9:29 -----------------------------------------------------
+9 |@nowarn("msg=fish") def t4 = f // error
+  |                             ^
+  |                             method f is deprecated
+-- Error: tests/neg-custom-args/nowarn/nowarn-refchecks.scala:15:9 -----------------------------------------------------
+15 |def t9 = f: @nowarn("msg=fish") // error
+   |         ^
+   |         method f is deprecated

tests/neg/nowarn.scala renamed to tests/neg-custom-args/nowarn/nowarn-refchecks.scala

@@ -2,21 +2,16 @@ import annotation.nowarn
 
 @deprecated def f = 1
 
-def t1 = f // warn
+def t1 = f // error
 
 @nowarn("cat=deprecation") def t2 = f
 @nowarn("msg=deprecated") def t3 = f
-@nowarn("msg=fish") def t4 = f // warn
+@nowarn("msg=fish") def t4 = f // error
 @nowarn("") def t5 = f
 @nowarn def t6 = f
 
 def t7 = f: @nowarn("cat=deprecation")
 def t8 = f: @nowarn("msg=deprecated")
-def t9 = f: @nowarn("msg=fish") // warn
+def t9 = f: @nowarn("msg=fish") // error
 def t10 = f: @nowarn("")
 def t11 = f: @nowarn
-
-def t12 = List(List(1): _*) // warn -source:future-migration
-@nowarn("msg=vararg splices") def t13 = List(List(1): _*)
-
-class K { override def blup = 1 } // error