Refactoring of message filtering

The previous pipeline forgot to filter out <nonsensical> tags in most situations

Author: odersky

compiler/src/dotty/tools/dotc/printing/Formatting.scala

@@ -6,7 +6,7 @@ import Texts._, Types._, Flags._, Symbols._, Contexts._
 import collection.mutable
 import Decorators._
 import scala.util.control.NonFatal
-import reporting.Diagnostic
+import reporting.Message
 import util.DiffUtil
 import Highlighting._
 
@@ -88,7 +88,7 @@ object Formatting {
   }
 
   private def wrapNonSensical(arg: Any, str: String)(implicit ctx: Context): String = {
-    import Diagnostic._
+    import Message._
     def isSensical(arg: Any): Boolean = arg match {
       case tpe: Type =>
         tpe.exists && !tpe.isErroneous

compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala

@@ -170,7 +170,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
             (" <: " ~ toText(bound) provided !bound.isAny)
         }.close
       case tp: ErrorType =>
-        s"<error ${tp.msg.msg}>"
+        s"<error ${tp.msg.rawMessage}>"
       case tp: WildcardType =>
         if (tp.optBounds.exists) "<?" ~ toTextRHS(tp.bounds) ~ ">" else "<?>"
       case NoType =>

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

@@ -23,19 +23,19 @@ class ConsoleReporter(
   def doReport(dia: Diagnostic)(implicit ctx: Context): Unit = {
     val didPrint = dia match {
       case dia: Error =>
-        printMessage(messageAndPos(dia.contained, dia.pos, diagnosticLevel(dia)))
+        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.contained, dia.pos, diagnosticLevel(dia)))
+        printMessage(messageAndPos(dia.msg, dia.pos, diagnosticLevel(dia)))
         true
     }
 
-    if (didPrint && ctx.shouldExplain(dia))
-      printMessage(explanation(dia.contained))
-    else if (didPrint && dia.contained.explanation.nonEmpty)
+    if (didPrint && shouldExplain(dia))
+      printMessage(explanation(dia.msg))
+    else if (didPrint && dia.msg.explanation.nonEmpty)
       printMessage("\nlonger explanation available when compiling with `-explain`")
   }
 

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

@@ -10,18 +10,9 @@ import interfaces.Diagnostic.{ERROR, INFO, WARNING}
 import java.util.Optional
 
 object Diagnostic:
-  val nonSensicalStartTag: String = "<nonsensical>"
-  val nonSensicalEndTag: String = "</nonsensical>"
 
-  implicit class MessageContext(val c: Context) extends AnyVal {
-    def shouldExplain(dia: Diagnostic): Boolean = {
-      implicit val ctx = c
-      dia.contained.explanation match {
-        case "" => false
-        case _ => ctx.settings.explain.value
-      }
-    }
-  }
+  def shouldExplain(dia: Diagnostic)(using ctx: Context): Boolean =
+    dia.msg.explanation.nonEmpty && ctx.settings.explain.value
 
   // `Diagnostics to be consumed by `Reporter` ---------------------- //
   class Error(
@@ -86,44 +77,14 @@ object Diagnostic:
   }
 
 class Diagnostic(
-  val contained: Message,
+  val msg: Message,
   val pos: SourcePosition,
   val level: Int
-) extends Exception with interfaces.Diagnostic {
-  import Diagnostic._
-  private var myMsg: String = null
-  private var myIsNonSensical: Boolean = false
-  private var myContained: Message = null
-
+) extends Exception with interfaces.Diagnostic:
   override def position: Optional[interfaces.SourcePosition] =
     if (pos.exists && pos.source.exists) Optional.of(pos) else Optional.empty()
+  override def message: String = msg.message
 
-  /** The message to report */
-  def message: String = {
-    if (myMsg == null) {
-      myMsg = contained.msg.replaceAll("\u001B\\[[;\\d]*m", "")
-      if (myMsg.contains(nonSensicalStartTag)) {
-        myIsNonSensical = true
-        // myMsg might be composed of several d"..." invocations -> nested
-        // nonsensical tags possible
-        myMsg =
-          myMsg
-          .replaceAllLiterally(nonSensicalStartTag, "")
-          .replaceAllLiterally(nonSensicalEndTag, "")
-      }
-    }
-    myMsg
-  }
-
-  /** A message is non-sensical if it contains references to <nonsensical>
-   *  tags.  Such tags are inserted by the error diagnostic framework if a
-   *  message contains references to internally generated error types. Normally
-   *  we want to suppress error messages referring to types like this because
-   *  they look weird and are normally follow-up errors to something that was
-   *  diagnosed before.
-   */
-  def isNonSensical: Boolean = { message; myIsNonSensical }
-
-  override def toString: String = s"$getClass at $pos: ${message}"
+  override def toString: String = s"$getClass at $pos: $message"
   override def getMessage(): String = message
-}
+end Diagnostic

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

@@ -13,7 +13,7 @@ trait HideNonSensicalMessages extends Reporter {
    */
   override def isHidden(dia: Diagnostic)(implicit ctx: Context): Boolean =
     super.isHidden(dia) || {
-        dia.isNonSensical &&
+        dia.msg.isNonSensical &&
         hasErrors && // if there are no errors yet, report even if diagnostic is non-sensical
         !ctx.settings.YshowSuppressedErrors.value
     }

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

@@ -7,6 +7,9 @@ import util.SourcePosition
 import messages._
 
 object Message {
+  val nonSensicalStartTag: String = "<nonsensical>"
+  val nonSensicalEndTag: String = "</nonsensical>"
+
   /** This implicit conversion provides a fallback for error messages that have
     * not yet been ported to the new scheme. Comment out this `implicit def` to
     * see where old errors still exist
@@ -32,16 +35,18 @@ object Message {
   *                used to reference documentation online
   */
 abstract class Message(val errorId: ErrorMessageID) { self =>
+  import Message._
 
   /** The `msg` contains the diagnostic message e.g:
     *
     * > expected: String
     * > found:    Int
     *
     * This message will be placed underneath the position given by the enclosing
-    * `Diagnostic`
+    * `Diagnostic`. The message is given in raw form, with possible embedded
+    *  <nonsensical> tags.
     */
-  def msg: String
+  protected def msg: String
 
   /** The kind of the error message is something like "Syntax" or "Type
     * Mismatch"
@@ -54,6 +59,35 @@ abstract class Message(val errorId: ErrorMessageID) { self =>
     */
   def explanation: String
 
+  private var myMsg: String | Null = null
+  private var myIsNonSensical: Boolean = false
+
+  /** The message with potential embedded <nonsensical> tags */
+  def rawMessage = message
+
+  /** The message to report. <nonsensical> tags are filtered out */
+  def message: String =
+    if myMsg == null then
+      myMsg =
+        if msg.contains(nonSensicalStartTag) then
+          myIsNonSensical = true
+          // myMsg might be composed of several d"..." invocations -> nested
+          // nonsensical tags possible
+          msg
+            .replaceAllLiterally(nonSensicalStartTag, "")
+            .replaceAllLiterally(nonSensicalEndTag, "")
+        else msg
+    myMsg
+
+  /** A message is non-sensical if it contains references to <nonsensical>
+   *  tags.  Such tags are inserted by the error diagnostic framework if a
+   *  message contains references to internally generated error types. Normally
+   *  we want to suppress error messages referring to types like this because
+   *  they look weird and are normally follow-up errors to something that was
+   *  diagnosed before.
+   */
+   def isNonSensical: Boolean = { message; myIsNonSensical }
+
   /** The implicit `Context` in messages is a large thing that we don't want
     * persisted. This method gets around that by duplicating the message,
     * forcing its `msg` and `explanation` vals and dropping the implicit context

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

@@ -151,10 +151,10 @@ trait MessageRendering {
       val pos1 = pos.nonInlined
       val (srcBefore, srcAfter, offset) = sourceLines(pos1, diagnosticLevel)
       val marker = columnMarker(pos1, offset, diagnosticLevel)
-      val err = errorMsg(pos1, msg.msg, offset)
+      val err = errorMsg(pos1, msg.message, offset)
       sb.append((srcBefore ::: marker :: err :: outer(pos, " " * (offset - 1)) ::: srcAfter).mkString(EOL))
     }
-    else sb.append(msg.msg)
+    else sb.append(msg.message)
     sb.toString
   }
 

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

@@ -1066,8 +1066,8 @@ trait Applications extends Compatibility {
    */
   def saysNotFound(state: TyperState, memberName: Name)(using Context): Boolean =
     state.reporter.pendingMessages match
-      case msg :: Nil =>
-        msg.contained match
+      case dia :: Nil =>
+        dia.msg match
           case msg: NotFoundMsg => memberName.isEmpty || msg.name == memberName
           case _ => false
       case _ => false

compiler/src/dotty/tools/repl/ReplDriver.scala

@@ -381,7 +381,7 @@ class ReplDriver(settings: Array[String],
 
   /** Render messages using the `MessageRendering` trait */
   private def renderMessage(dia: Diagnostic): Context => String =
-    messageRenderer.messageAndPos(dia.contained, dia.pos, messageRenderer.diagnosticLevel(dia))(_)
+    messageRenderer.messageAndPos(dia.msg, dia.pos, messageRenderer.diagnosticLevel(dia))(_)
 
   /** Output errors to `out` */
   private def displayErrors(errs: Seq[Diagnostic])(implicit state: State): State = {

compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTest.scala

@@ -36,7 +36,7 @@ trait ErrorMessagesTest extends DottyTest {
     if (!runCtx.reporter.hasErrors) new EmptyReport
     else {
       val rep = runCtx.reporter.asInstanceOf[StoreReporter]
-      val msgs = rep.removeBufferedMessages(runCtx).map(_.contained).reverse
+      val msgs = rep.removeBufferedMessages(runCtx).map(_.msg).reverse
       new Report(msgs, runCtx)
     }
   }

compiler/test/dotty/tools/dotc/reporting/TestReporter.scala

@@ -54,7 +54,7 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M
 
   /** Prints the message with the given position indication. */
   def printMessageAndPos(dia: Diagnostic, extra: String)(implicit ctx: Context): Unit = {
-    val msg = messageAndPos(dia.contained, dia.pos, diagnosticLevel(dia))
+    val msg = messageAndPos(dia.msg, dia.pos, diagnosticLevel(dia))
     val extraInfo = inlineInfo(dia.pos)
 
     if (dia.level >= logLevel) {
@@ -69,7 +69,7 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M
   override def doReport(dia: Diagnostic)(implicit ctx: Context): Unit = {
 
     // Here we add extra information that we should know about the error message
-    val extra = dia.contained match {
+    val extra = dia.msg match {
       case pm: PatternMatchExhaustivity => s": ${pm.uncovered}"
       case _ => ""
     }
@@ -127,7 +127,7 @@ object TestReporter {
       /** Prints the message with the given position indication in a simplified manner */
       override def printMessageAndPos(dia: Diagnostic, extra: String)(implicit ctx: Context): Unit = {
         def report() = {
-          val msg = s"${dia.pos.line + 1}: " + dia.contained.kind + extra
+          val msg = s"${dia.pos.line + 1}: " + dia.msg.kind + extra
           val extraInfo = inlineInfo(dia.pos)
 
           writer.println(msg)

doc-tool/test/dotty/tools/dottydoc/DottyDocTest.scala

@@ -54,7 +54,7 @@ trait DottyDocTest extends MessageRendering {
             System.err.println("reporter had errors:")
             ctx.reporter.removeBufferedMessages.foreach { msg =>
               System.err.println {
-                messageAndPos(msg.contained, msg.pos, diagnosticLevel(msg))
+                messageAndPos(msg.msg, msg.pos, diagnosticLevel(msg))
               }
             }
           }

language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala

@@ -709,12 +709,13 @@ object DottyLanguageServer {
         }
       }
 
-      val message = dia.contained
+      val message = dia.msg
       if (displayMessage(message, dia.pos.source)) {
         val code = message.errorId.errorNumber.toString
         range(dia.pos).map(r =>
             new lsp4j.Diagnostic(
-              r, dia.message, severity(dia.level), /*source =*/ "", code))
+              r, dia.message.replaceAll("\u001B\\[[;\\d]*m", ""),
+              severity(dia.level), /*source =*/ "", code))
       } else {
         None
       }

sbt-bridge/src/xsbt/DelegatingReporter.java

@@ -57,9 +57,9 @@ public void printSummary(Context ctx) {
     delegate.printSummary();
   }
 
-  public void doReport(dotty.tools.dotc.reporting.Diagnostic cont, Context ctx) {
+  public void doReport(dotty.tools.dotc.reporting.Diagnostic dia, Context ctx) {
     Severity severity;
-    switch (cont.level()) {
+    switch (dia.level()) {
       case Diagnostic.ERROR:
         severity = Severity.Error;
         break;
@@ -70,12 +70,12 @@ public void doReport(dotty.tools.dotc.reporting.Diagnostic cont, Context ctx) {
         severity = Severity.Info;
         break;
       default:
-        throw new IllegalArgumentException("Bad diagnostic level: " + cont.level());
+        throw new IllegalArgumentException("Bad diagnostic level: " + dia.level());
     }
 
     Position position;
-    if (cont.pos().exists()) {
-      SourcePosition pos = cont.pos();
+    if (dia.pos().exists()) {
+      SourcePosition pos = dia.pos();
       SourceFile src = pos.source();
       position = new Position() {
         public Optional<java.io.File> sourceFile() {
@@ -123,10 +123,10 @@ public Optional<String> pointerSpace() {
       position = noPosition;
     }
 
-    Message message = cont.contained();
+    Message message = dia.msg();
     StringBuilder rendered = new StringBuilder();
-    rendered.append(messageAndPos(message, cont.pos(), diagnosticLevel(cont), ctx));
-    boolean shouldExplain = new dotty.tools.dotc.reporting.Diagnostic.MessageContext(ctx).shouldExplain(cont);
+    rendered.append(messageAndPos(message, dia.pos(), diagnosticLevel(dia), ctx));
+    boolean shouldExplain = dotty.tools.dotc.reporting.Diagnostic.shouldExplain(dia, ctx);
     if (shouldExplain && !message.explanation().isEmpty()) {
       rendered.append(explanation(message, ctx));
     }