Make Reporter mixins produce functions evaluated in leaf reporters

Leaf reporters will now be the ones forcing the evaluation of `Message`
in `MessageContainer`. The mixins will not force the evaluation of the
message, only produce a composed function that can possibly force the
message.

Author: felixmulder

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

@@ -24,23 +24,30 @@ class ConsoleReporter(
   def printMessage(msg: String): Unit = { writer.print(msg + "\n"); writer.flush() }
 
   /** Prints the message with the given position indication. */
-  def doReport(m: MessageContainer)(implicit ctx: Context): Unit = {
-    val didPrint = m match {
-      case m: Error =>
-        printMessage(messageAndPos(m.contained, m.pos, diagnosticLevel(m)))
-        if (ctx.settings.prompt.value) displayPrompt()
-        true
-      case m: ConditionalWarning if !m.enablingOption.value =>
-        false
-      case m =>
-        printMessage(messageAndPos(m.contained, m.pos, diagnosticLevel(m)))
-        true
+  def report(m: MessageContainer)(implicit ctx: Context): Boolean = {
+
+    def doReport(m: MessageContainer) = {
+      val didPrint = m match {
+        case m: Error =>
+          printMessage(messageAndPos(m.contained, m.pos, diagnosticLevel(m)))
+          if (ctx.settings.prompt.value) displayPrompt()
+          true
+        case m: ConditionalWarning if !m.enablingOption.value =>
+          false
+        case m =>
+          printMessage(messageAndPos(m.contained, m.pos, diagnosticLevel(m)))
+          true
+      }
+
+      if (didPrint && ctx.shouldExplain(m))
+        printMessage(explanation(m.contained))
+      else if (didPrint && m.contained.explanation.nonEmpty)
+        printMessage("\nlonger explanation available when compiling with `-explain`")
+
+      didPrint
     }
 
-    if (didPrint && ctx.shouldExplain(m))
-      printMessage(explanation(m.contained))
-    else if (didPrint && m.contained.explanation.nonEmpty)
-      printMessage("\nlonger explanation available when compiling with `-explain`")
+    reportable(ctx)(m).map(doReport).getOrElse(false)
   }
 
   /** Show prompt if `-Xprompt` is passed as a flag to the compiler */

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

@@ -6,16 +6,25 @@ import core.Contexts.Context
 import diagnostic.MessageContainer
 
 /**
- * This trait implements `isHidden` so that we avoid reporting non-sensical messages.
+ * This trait implements `isHidden` so that we avoid reporting non-sensical
+ * messages.
+ *
+ * @note mixin of this trait and applying `reportable` will force the contained
+ *       message.
  */
 trait HideNonSensicalMessages extends Reporter {
   /** Hides non-sensical messages, unless we haven't reported any error yet or
    *  `-Yshow-suppressed-errors` is set.
    */
-  override def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean =
-    super.isHidden(m) || {
-        m.isNonSensical &&
-        hasErrors && // if there are no errors yet, report even if diagnostic is non-sensical
-        !ctx.settings.YshowSuppressedErrors.value
+  override def reportable(implicit ctx: Context): MessageContainer => Option[MessageContainer] = { m =>
+    super.reportable(ctx)(m) flatMap { m =>
+        val notReportable =
+          m.isNonSensical &&
+          hasErrors && // if there are no errors yet, report even if diagnostic is non-sensical
+          !ctx.settings.YshowSuppressedErrors.value
+
+        if (notReportable) None
+        else Some(m)
     }
+  }
 }

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

@@ -18,10 +18,9 @@ object Reporter {
   /** Convert a SimpleReporter into a real Reporter */
   def fromSimpleReporter(simple: interfaces.SimpleReporter): Reporter =
     new Reporter with UniqueMessagePositions with HideNonSensicalMessages {
-      override def doReport(m: MessageContainer)(implicit ctx: Context): Unit = m match {
-        case m: ConditionalWarning if !m.enablingOption.value =>
-        case _ =>
-          simple.report(m)
+      def report(m: MessageContainer)(implicit ctx: Context): Boolean = m match {
+        case m: ConditionalWarning if !m.enablingOption.value => false
+        case _ => simple.report(m); true
       }
     }
 }
@@ -35,19 +34,19 @@ trait Reporting { this: Context =>
     if (this.settings.verbose.value) this.echo(msg, pos)
 
   def echo(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(new Info(msg, pos))
+    reporter.prepareReport(new Info(msg, pos))
 
   def deprecationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(new DeprecationWarning(msg, pos))
+    reporter.prepareReport(new DeprecationWarning(msg, pos))
 
   def migrationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(new MigrationWarning(msg, pos))
+    reporter.prepareReport(new MigrationWarning(msg, pos))
 
   def uncheckedWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(new UncheckedWarning(msg, pos))
+    reporter.prepareReport(new UncheckedWarning(msg, pos))
 
   def featureWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(new FeatureWarning(msg, pos))
+    reporter.prepareReport(new FeatureWarning(msg, pos))
 
   def featureWarning(feature: String, featureDescription: String, isScala2Feature: Boolean,
       featureUseSite: Symbol, required: Boolean, pos: SourcePosition): Unit = {
@@ -69,26 +68,26 @@ trait Reporting { this: Context =>
 
     val msg = s"$featureDescription $req be enabled\nby making the implicit value $fqname visible.$explain"
     if (required) error(msg, pos)
-    else reporter.report(new FeatureWarning(msg, pos))
+    else reporter.prepareReport(new FeatureWarning(msg, pos))
   }
 
   def warning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(new Warning(msg, pos))
+    reporter.prepareReport(new Warning(msg, pos))
 
   def strictWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
     if (this.settings.strict.value) error(msg, pos)
-    else reporter.report {
+    else reporter.prepareReport {
       new ExtendMessage(() => msg)(_ + "\n(This would be an error under strict mode)").warning(pos)
     }
 
   def error(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report(new Error(msg, pos))
+    reporter.prepareReport(new Error(msg, pos))
 
   def errorOrMigrationWarning(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
     if (ctx.scala2Mode) migrationWarning(msg, pos) else error(msg, pos)
 
   def restrictionError(msg: => Message, pos: SourcePosition = NoSourcePosition): Unit =
-    reporter.report {
+    reporter.prepareReport {
       new ExtendMessage(() => msg)(m => s"Implementation restriction: $m").error(pos)
     }
 
@@ -190,13 +189,58 @@ trait Reporting { this: Context =>
 }
 
 /**
- * This interface provides methods to issue information, warning and
- * error messages.
- */
+  * This interface provides methods to issue information, warning and
+  * error messages.
+  *
+  * To implement this interface one only needs to implement `report` which
+  * should do the actual reporting.
+  */
 abstract class Reporter extends interfaces.ReporterResult {
 
-  /** Report a diagnostic */
-  def doReport(d: MessageContainer)(implicit ctx: Context): Unit
+  /** Report should be defined in leaf node reporters where forcing of the
+    * message is allowed.
+    *
+    * @note Report should make a call to `reportable` which, when applied will
+    *       have forced the message depending on which traits have been mixed
+    *       in
+    * @return true if did report, else false
+    */
+  protected def report(m: MessageContainer)(implicit ctx: Context): Boolean
+
+  /** `reportable` will return a function which can inspect `MessageContainer`s
+    * to see if they should be reported or not
+    *
+    * @note implementing traits should compose functions by calling
+    *       `super.reportable(m)` so that all mixins get applied
+    * @return `MessageContainer => Some[MessageContainer]` iff should report
+    */
+  def reportable(implicit ctx: Context): MessageContainer => Option[MessageContainer] =
+    m => if (ctx.mode.is(Mode.Printing)) Some(m) else None
+
+  /** Called when for each generated `MessageContainer`, the implementing reporter
+    * will then decide whether to report the message or not
+    *
+    * @return `true` iff message was reported
+    */
+  def prepareReport(m: MessageContainer)(implicit ctx: Context): Boolean = {
+    val didReport = report(m)(ctx.addMode(Mode.Printing))
+
+    if (didReport) {
+      m match {
+        case m: ConditionalWarning if !m.enablingOption.value =>
+          unreportedWarnings(m.enablingOption.name) += 1
+        case m: Warning =>
+          warningCount += 1
+        case m: Error =>
+          errors = m :: errors
+          errorCount += 1
+        case m: Info =>
+          // nothing to do here, match error if other container
+      }
+    }
+
+    didReport
+  }
 
   /** Whether very long lines can be truncated.  This exists so important
    *  debugging information (like printing the classpath) is not rendered
@@ -240,20 +284,6 @@ abstract class Reporter extends interfaces.ReporterResult {
     override def default(key: String) = 0
   }
 
-  def report(d: => MessageContainer)(implicit ctx: Context): Unit =
-    if (!isHidden(d)) {
-      doReport(d)(ctx.addMode(Mode.Printing))
-      d match {
-        case d: ConditionalWarning if !d.enablingOption.value => unreportedWarnings(d.enablingOption.name) += 1
-        case d: Warning => warningCount += 1
-        case d: Error =>
-          errors = d :: errors
-          errorCount += 1
-        case d: Info => // nothing to do here
-        // match error if d is something else
-      }
-    }
-
   def incomplete(d: MessageContainer)(implicit ctx: Context): Unit =
     incompleteHandler(d)(ctx)
 
@@ -285,11 +315,8 @@ abstract class Reporter extends interfaces.ReporterResult {
     case _ => n + " " + elements + "s"
   }
 
-  /** Should this diagnostic not be reported at all? */
-  def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean = ctx.mode.is(Mode.Printing)
-
   /** Does this reporter contain not yet reported errors or warnings? */
-  def hasPending: Boolean = false
+  def hasPending(implicit ctx: Context): Boolean = false
 
   /** Issue all error messages in this reporter to next outer one, or make sure they are written. */
   def flush()(implicit ctx: Context): Unit = {}

src/dotty/tools/dotc/reporting/StoreReporter.scala

@@ -15,25 +15,27 @@ class StoreReporter(outer: Reporter) extends Reporter {
 
   private var infos: mutable.ListBuffer[MessageContainer] = null
 
-  def doReport(m: MessageContainer)(implicit ctx: Context): Unit = {
-    typr.println(s">>>> StoredError: ${m.message}") // !!! DEBUG
+  def report(m: MessageContainer)(implicit ctx: Context): Boolean = {
+    typr.println(s">>>> StoredError: ${m.message}")
     if (infos == null) infos = new mutable.ListBuffer
     infos += m
+    wasInteresting(m)
   }
 
-  override def hasPending: Boolean = infos != null && {
-    infos exists {
-      case _: Error => true
-      case _: Warning => true
+  private def wasInteresting(m: MessageContainer)(implicit ctx: Context) =
+    m match {
+      case m: ConditionalWarning if !m.enablingOption.value => true
+      case _: Warning | _: Error => true
       case _ => false
     }
-  }
 
-  override def flush()(implicit ctx: Context) =
-    if (infos != null) {
-      infos.foreach(ctx.reporter.report(_))
-      infos = null
-    }
+  override def hasPending(implicit ctx: Context): Boolean =
+    infos != null && infos.exists(wasInteresting)
+
+  override def flush()(implicit ctx: Context) = if (infos != null) {
+    infos.foreach(outer.prepareReport(_))
+    infos = null
+  }
 
   override def errorsReported = hasErrors || outer.errorsReported
 }

src/dotty/tools/dotc/reporting/ThrowingReporter.scala

@@ -3,6 +3,7 @@ package dotc
 package reporting
 
 import core.Contexts.Context
+import core.Mode
 import collection.mutable
 import diagnostic.MessageContainer
 import diagnostic.messages.Error
@@ -13,8 +14,8 @@ import Reporter._
  * info to the underlying reporter.
  */
 class ThrowingReporter(reportInfo: Reporter) extends Reporter {
-  def doReport(m: MessageContainer)(implicit ctx: Context): Unit = m match {
+  def report(m: MessageContainer)(implicit ctx: Context): Boolean = m match {
     case _: Error => throw m
-    case _ => reportInfo.doReport(m)
+    case _ => reportInfo.prepareReport(m)
   }
 }

src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala

@@ -16,17 +16,20 @@ trait UniqueMessagePositions extends Reporter {
   /** Logs a position and returns true if it was already logged.
    *  @note  Two positions are considered identical for logging if they have the same point.
    */
-  override def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean =
-    super.isHidden(m) || {
-      m.pos.exists && {
-        var shouldHide = false
-        for (pos <- m.pos.start to m.pos.end) {
-          positions get (ctx.source, pos) match {
-            case Some(level) if level >= m.level => shouldHide = true
-            case _ => positions((ctx.source, pos)) = m.level
-          }
-        }
-        shouldHide
-      }
-    }
+   override def reportable(implicit ctx: Context): MessageContainer => Option[MessageContainer] = { m =>
+     super.reportable(ctx)(m) flatMap { m =>
+       val hidden = m.pos.exists && {
+         var shouldHide = false
+         for (pos <- m.pos.start to m.pos.end) {
+           positions get (ctx.source, pos) match {
+             case Some(level) if level >= m.level => shouldHide = true
+             case _ => positions((ctx.source, pos)) = m.level
+           }
+         }
+         shouldHide
+       }
+       if (hidden) None
+       else Some(m)
+     }
+   }
 }

test/dotty/tools/dotc/reporting/TestMessageLaziness.scala

@@ -16,9 +16,7 @@ class TestMessageLaziness extends DottyTest {
 
   class NonchalantReporter(implicit ctx: Context) extends Reporter
   with UniqueMessagePositions with HideNonSensicalMessages {
-    def doReport(d: MessageContainer)(implicit ctx: Context) = ???
-
-    override def report(d: => MessageContainer)(implicit ctx: Context) = ()
+    override def report(m: MessageContainer)(implicit ctx: Context) = false
   }
 
   case class LazyError() extends Message(1000) {

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

@@ -34,7 +34,7 @@ with UniqueMessagePositions with HideNonSensicalMessages {
     printPos(pos)
   }
 
-  override def doReport(m: MessageContainer)(implicit ctx: Context): Unit = {
+  override def report(m: MessageContainer)(implicit ctx: Context) = {
     // Here we add extra information that we should know about the error message
     val extra = m.contained match {
       case pm: PatternMatchExhaustivity => s": ${pm.uncovered}"
@@ -44,9 +44,12 @@ with UniqueMessagePositions with HideNonSensicalMessages {
     m match {
       case m: Error =>
         printMessageAndPos(m.contained.kind + extra, m.pos)
+        true
       case w: Warning =>
         printMessageAndPos(w.contained.kind + extra, w.pos)
+        true
       case _ =>
+        false
     }
   }
 }

test/test/CompilerTest.scala

@@ -238,13 +238,13 @@ abstract class CompilerTest {
     val storeReporter = new Reporter with UniqueMessagePositions with HideNonSensicalMessages {
       private val consoleReporter = new ConsoleReporter()
       private val innerStoreReporter = new StoreReporter(consoleReporter)
-      def doReport(m: MessageContainer)(implicit ctx: Context): Unit = {
+      def report(m: MessageContainer)(implicit ctx: Context) = {
         if (m.level == ERROR) {
           innerStoreReporter.flush()
-          consoleReporter.doReport(m)
+          consoleReporter.report(m)
         }
-        else if (errorCount > 0) consoleReporter.doReport(m)
-        else innerStoreReporter.doReport(m)
+        else if (errorCount > 0) consoleReporter.report(m)
+        else innerStoreReporter.report(m)
       }
     }
     val reporter = processor.process(allArgs, storeReporter)