Merge pull request #1123 from smarter/refactor/doReport

ConsoleReporter: handling of non-sensical messages is now reusable 

9 files changed, 91 insertions, 72 deletions

diff --git a/src/dotty/tools/dotc/reporting/ConsoleReporter.scala b/src/dotty/tools/dotc/reporting/ConsoleReporter.scala
index 8f1fbf797..61678dee1 100644
--- a/src/dotty/tools/dotc/reporting/ConsoleReporter.scala
+++ b/src/dotty/tools/dotc/reporting/ConsoleReporter.scala
@@ -16,7 +16,7 @@ import scala.reflect.internal.util._
 class ConsoleReporter(
     reader: BufferedReader = Console.in,
     writer: PrintWriter = new PrintWriter(Console.err, true))
-  extends Reporter with UniqueMessagePositions {
+  extends Reporter with UniqueMessagePositions with HideNonSensicalMessages {
 
   /** maximal number of error messages to be printed */
   protected def ErrorLimit = 100
@@ -40,21 +40,17 @@ class ConsoleReporter(
     }
   }
 
-  override def doReport(d: Diagnostic)(implicit ctx: Context): Boolean = {
-    val issue = !(d.isSuppressed && hasErrors)
-    if (issue) d match {
-      case d: Error =>
-        printMessageAndPos(s"error: ${d.msg}", d.pos)
-        if (ctx.settings.prompt.value) displayPrompt()
-      case d: ConditionalWarning if !d.enablingOption.value =>
-      case d: MigrationWarning =>
-        printMessageAndPos(s"migration warning: ${d.msg}", d.pos)
-      case d: Warning =>
-        printMessageAndPos(s"warning: ${d.msg}", d.pos)
-      case _ =>
-        printMessageAndPos(d.msg, d.pos)
-    }
-    issue
+  override def doReport(d: Diagnostic)(implicit ctx: Context): Unit = d match {
+    case d: Error =>
+      printMessageAndPos(s"error: ${d.msg}", d.pos)
+      if (ctx.settings.prompt.value) displayPrompt()
+    case d: ConditionalWarning if !d.enablingOption.value =>
+    case d: MigrationWarning =>
+      printMessageAndPos(s"migration warning: ${d.msg}", d.pos)
+    case d: Warning =>
+      printMessageAndPos(s"warning: ${d.msg}", d.pos)
+    case _ =>
+      printMessageAndPos(d.msg, d.pos)
   }
 
   def displayPrompt(): Unit = {
diff --git a/src/dotty/tools/dotc/reporting/Diagnostic.scala b/src/dotty/tools/dotc/reporting/Diagnostic.scala
new file mode 100644
index 000000000..c57be3d26
--- /dev/null
+++ b/src/dotty/tools/dotc/reporting/Diagnostic.scala
@@ -0,0 +1,47 @@
+package dotty.tools
+package dotc
+package reporting
+
+import util.SourcePosition
+
+object Diagnostic {
+
+  // Error levels
+  val ERROR = 2
+  val WARNING = 1
+  val INFO = 0
+
+  val nonSensicalStartTag = "<nonsensical>"
+  val nonSensicalEndTag = "</nonsensical>"
+}
+
+class Diagnostic(msgFn: => String, val pos: SourcePosition, val level: Int) extends Exception {
+  import Diagnostic._
+  private var myMsg: String = null
+  private var myIsNonSensical: Boolean = false
+
+  /** The message to report */
+  def msg: String = {
+    if (myMsg == null) {
+      myMsg = msgFn
+      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 = { msg; myIsNonSensical }
+
+  override def toString = s"$getClass at $pos: $msg"
+  override def getMessage() = msg
+}
diff --git a/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala b/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala
new file mode 100644
index 000000000..a325fe9f7
--- /dev/null
+++ b/src/dotty/tools/dotc/reporting/HideNonSensicalMessages.scala
@@ -0,0 +1,20 @@
+package dotty.tools
+package dotc
+package reporting
+
+import core.Contexts.Context
+
+/**
+ * This trait implements `isHidden` so that we avoid reporting non-sensical messages.
+ */
+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(d: Diagnostic)(implicit ctx: Context): Boolean =
+    super.isHidden(d) || {
+        d.isNonSensical &&
+        hasErrors && // if there are no errors yet, report even if diagnostic is non-sensical
+        !ctx.settings.YshowSuppressedErrors.value
+    }
+}
diff --git a/src/dotty/tools/dotc/reporting/Reporter.scala b/src/dotty/tools/dotc/reporting/Reporter.scala
index 272b1880f..f4eb551a1 100644
--- a/src/dotty/tools/dotc/reporting/Reporter.scala
+++ b/src/dotty/tools/dotc/reporting/Reporter.scala
@@ -10,46 +10,10 @@ import collection.mutable
 import config.Settings.Setting
 import config.Printers
 import java.lang.System.currentTimeMillis
-import typer.ErrorReporting.DiagnosticString
 import typer.Mode
+import Diagnostic.{ERROR, WARNING, INFO}
 
 object Reporter {
-
-  private val ERROR = 2
-  private val WARNING = 1
-  private val INFO = 0
-
-  class Diagnostic(msgFn: => String, val pos: SourcePosition, val level: Int) extends Exception {
-    import DiagnosticString._
-
-    private var myMsg: String = null
-    private var myIsNonSensical: Boolean = false
-
-    /** The message to report */
-    def msg: String = {
-      if (myMsg == null) {
-        myMsg = msgFn
-        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
-    }
-
-    /** Report in current reporter */
-    def report(implicit ctx: Context) = ctx.reporter.report(this)
-
-    def isNonSensical = { msg; myIsNonSensical }
-    def isSuppressed(implicit ctx: Context): Boolean = !ctx.settings.YshowSuppressedErrors.value && isNonSensical
-
-    override def toString = s"$getClass at $pos: $msg"
-    override def getMessage() = msg
-
-    def checkingStr: String = msgFn
-  }
-
   class Error(msgFn: => String, pos: SourcePosition) extends Diagnostic(msgFn, pos, ERROR)
   class Warning(msgFn: => String, pos: SourcePosition) extends Diagnostic(msgFn, pos, WARNING)
   class Info(msgFn: => String, pos: SourcePosition) extends Diagnostic(msgFn, pos, INFO)
@@ -195,10 +159,8 @@ trait Reporting { this: Context =>
  */
 abstract class Reporter {
 
-  /** Report a diagnostic, unless it is suppressed because it is nonsensical
-   *  @return a diagnostic was reported.
-   */
-  def doReport(d: Diagnostic)(implicit ctx: Context): Boolean
+  /** Report a diagnostic */
+  def doReport(d: Diagnostic)(implicit ctx: Context): Unit
 
  /** Whether very long lines can be truncated.  This exists so important
    *  debugging information (like printing the classpath) is not rendered
@@ -239,7 +201,8 @@ abstract class Reporter {
   }
 
   def report(d: Diagnostic)(implicit ctx: Context): Unit =
-    if (!isHidden(d) && doReport(d)(ctx.addMode(Mode.Printing)))
+    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
@@ -249,6 +212,7 @@ abstract class Reporter {
         case d: Info => // nothing to do here
         // match error if d is something else
       }
+    }
 
   def incomplete(d: Diagnostic)(implicit ctx: Context): Unit =
     incompleteHandler(d)(ctx)
diff --git a/src/dotty/tools/dotc/reporting/StoreReporter.scala b/src/dotty/tools/dotc/reporting/StoreReporter.scala
index 8209839eb..d19e8cf5c 100644
--- a/src/dotty/tools/dotc/reporting/StoreReporter.scala
+++ b/src/dotty/tools/dotc/reporting/StoreReporter.scala
@@ -4,7 +4,7 @@ package reporting
 
 import core.Contexts.Context
 import collection.mutable
-import Reporter.{Diagnostic, Error, Warning}
+import Reporter.{Error, Warning}
 import config.Printers._
 
 /**
@@ -14,11 +14,10 @@ class StoreReporter(outer: Reporter) extends Reporter {
 
   private var infos: mutable.ListBuffer[Diagnostic] = null
 
-  def doReport(d: Diagnostic)(implicit ctx: Context): Boolean = {
+  def doReport(d: Diagnostic)(implicit ctx: Context): Unit = {
     typr.println(s">>>> StoredError: ${d.msg}") // !!! DEBUG
     if (infos == null) infos = new mutable.ListBuffer
     infos += d
-    true
   }
 
   override def hasPending: Boolean = infos != null && {
diff --git a/src/dotty/tools/dotc/reporting/ThrowingReporter.scala b/src/dotty/tools/dotc/reporting/ThrowingReporter.scala
index 7c63383e9..026453036 100644
--- a/src/dotty/tools/dotc/reporting/ThrowingReporter.scala
+++ b/src/dotty/tools/dotc/reporting/ThrowingReporter.scala
@@ -11,7 +11,7 @@ import Reporter._
  * info to the underlying reporter.
  */
 class ThrowingReporter(reportInfo: Reporter) extends Reporter {
-  def doReport(d: Diagnostic)(implicit ctx: Context): Boolean = d match {
+  def doReport(d: Diagnostic)(implicit ctx: Context): Unit = d match {
     case _: Error => throw d
     case _ => reportInfo.doReport(d)
   }
diff --git a/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala b/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala
index c1f240a23..32554e6b6 100644
--- a/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala
+++ b/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala
@@ -4,7 +4,6 @@ package reporting
 
 import scala.collection.mutable
 import util.{SourcePosition, SourceFile}
-import Reporter.Diagnostic
 import core.Contexts.Context
 
 /**
diff --git a/src/dotty/tools/dotc/typer/ErrorReporting.scala b/src/dotty/tools/dotc/typer/ErrorReporting.scala
index b3089c99c..d6a87acf6 100644
--- a/src/dotty/tools/dotc/typer/ErrorReporting.scala
+++ b/src/dotty/tools/dotc/typer/ErrorReporting.scala
@@ -8,6 +8,7 @@ import Trees._
 import Types._, ProtoTypes._, Contexts._, Decorators._, Denotations._, Symbols._
 import Applications._, Implicits._, Flags._
 import util.Positions._
+import reporting.Diagnostic
 import printing.Showable
 import printing.Disambiguation.disambiguated
 
@@ -127,7 +128,6 @@ object ErrorReporting {
    *  message composition methods, this is crucial.
    */
   implicit class DiagnosticString(val sc: StringContext) extends AnyVal {
-    import DiagnosticString._
     def d(args: Any*)(implicit ctx: Context): String = {
       def isSensical(arg: Any): Boolean = arg match {
         case l: Seq[_] => l.forall(isSensical(_))
@@ -139,13 +139,8 @@ object ErrorReporting {
       }
 
       val s = new StringInterpolators(sc).i(args : _*)
-      if (args.forall(isSensical(_))) s else nonSensicalStartTag + s + nonSensicalEndTag
+      if (args.forall(isSensical(_))) s
+      else Diagnostic.nonSensicalStartTag + s + Diagnostic.nonSensicalEndTag
     }
   }
-
-  object DiagnosticString {
-    final val nonSensicalStartTag = "<nonsensical>"
-    final val nonSensicalEndTag = "</nonsensical>"
-  }
-
 }
diff --git a/src/dotty/tools/dotc/typer/Inferencing.scala b/src/dotty/tools/dotc/typer/Inferencing.scala
index 9717b5625..99e8cd150 100644
--- a/src/dotty/tools/dotc/typer/Inferencing.scala
+++ b/src/dotty/tools/dotc/typer/Inferencing.scala
@@ -15,7 +15,6 @@ import util.{Stats, SimpleMap}
 import util.common._
 import Decorators._
 import Uniques._
-import ErrorReporting.{errorType, DiagnosticString}
 import config.Printers._
 import annotation.tailrec
 import collection.mutable