Complete better structure to diagnostic messages

15 files changed, 198 insertions, 150 deletions

diff --git a/src/dotty/tools/dotc/config/ScalaSettings.scala b/src/dotty/tools/dotc/config/ScalaSettings.scala
index a4daefcba..872cb0667 100644
--- a/src/dotty/tools/dotc/config/ScalaSettings.scala
+++ b/src/dotty/tools/dotc/config/ScalaSettings.scala
@@ -23,7 +23,7 @@ class ScalaSettings extends Settings.SettingGroup {
   val migration = BooleanSetting("-migration", "Emit warning and location for migration issues from Scala 2.")
   val encoding = StringSetting("-encoding", "encoding", "Specify character encoding used by source files.", Properties.sourceEncoding)
   val explaintypes = BooleanSetting("-explaintypes", "Explain type errors in more detail.")
-  val explainerrors = BooleanSetting("-explain", "Explain errors in more detail.")
+  val explain = BooleanSetting("-explain", "Explain errors in more detail.")
   val feature = BooleanSetting("-feature", "Emit warning and location for usages of features that should be imported explicitly.")
   val g = ChoiceSetting("-g", "level", "Set level of generated debugging info.", List("none", "source", "line", "vars", "notailcalls"), "vars")
   val help = BooleanSetting("-help", "Print a synopsis of standard options")
diff --git a/src/dotty/tools/dotc/parsing/JavaParsers.scala b/src/dotty/tools/dotc/parsing/JavaParsers.scala
index 90eb90877..ed7cf9e3f 100644
--- a/src/dotty/tools/dotc/parsing/JavaParsers.scala
+++ b/src/dotty/tools/dotc/parsing/JavaParsers.scala
@@ -30,7 +30,6 @@ import scala.reflect.internal.util.Collections._
 object JavaParsers {
 
   import ast.untpd._
-  import reporting.ErrorMessages.Syntax._
 
   class JavaParser(source: SourceFile)(implicit ctx: Context) extends ParserCommon(source) {
 
diff --git a/src/dotty/tools/dotc/parsing/MarkupParsers.scala b/src/dotty/tools/dotc/parsing/MarkupParsers.scala
index ea40eb568..f648b9e2c 100644
--- a/src/dotty/tools/dotc/parsing/MarkupParsers.scala
+++ b/src/dotty/tools/dotc/parsing/MarkupParsers.scala
@@ -32,7 +32,6 @@ import Utility._
 object MarkupParsers {
 
   import ast.untpd._
-  import reporting.ErrorMessages.Syntax._
 
   case object MissingEndTagControl extends ControlThrowable {
     override def getMessage = "start tag was here: "
diff --git a/src/dotty/tools/dotc/parsing/Parsers.scala b/src/dotty/tools/dotc/parsing/Parsers.scala
index 48422650c..1b451ced7 100644
--- a/src/dotty/tools/dotc/parsing/Parsers.scala
+++ b/src/dotty/tools/dotc/parsing/Parsers.scala
@@ -17,7 +17,6 @@ import ast.Trees._
 import Decorators._
 import StdNames._
 import util.Positions._
-import reporting.ErrorMessages._
 import Constants._
 import ScriptParsers._
 import Comments._
@@ -28,7 +27,9 @@ import rewrite.Rewrites.patch
 object Parsers {
 
   import ast.untpd._
-  import reporting.ErrorMessages.Syntax._
+  import reporting.diagnostic.MessageCreator
+  import MessageCreator._
+  import reporting.diagnostic.syntax._
 
   case class OpInfo(operand: Tree, operator: Name, offset: Offset)
 
@@ -99,7 +100,7 @@ object Parsers {
     /** Issue an error at given offset if beyond last error offset
       *  and update lastErrorOffset.
       */
-    def syntaxError(expl: ErrorMessage, offset: Int = in.offset): Unit =
+    def syntaxError(expl: MessageCreator, offset: Int = in.offset): Unit =
       if (offset > lastErrorOffset) {
         syntaxError(expl, Position(offset))
         lastErrorOffset = in.offset
@@ -108,7 +109,7 @@ object Parsers {
     /** Unconditionally issue an error at given position, without
       *  updating lastErrorOffset.
       */
-    def syntaxError(expl: ErrorMessage, pos: Position): Unit =
+    def syntaxError(expl: MessageCreator, pos: Position): Unit =
       ctx.explainError(expl, source atPos pos)
 
   }
@@ -215,7 +216,7 @@ object Parsers {
       }
     }
 
-    def warning(msg: ErrorMessage, offset: Int = in.offset) =
+    def warning(msg: MessageCreator, offset: Int = in.offset) =
       ctx.explainWarning(msg, source atPos Position(offset))
 
     def deprecationWarning(msg: String, offset: Int = in.offset) =
@@ -1016,7 +1017,7 @@ object Parsers {
 
           handler match {
             case Block(Nil, EmptyTree) =>
-              syntaxError(EmptyCatchBlock(body), handler.pos)
+              syntaxError(new EmptyCatchBlock(body), handler.pos)
             case _ =>
           }
 
diff --git a/src/dotty/tools/dotc/reporting/ConsoleReporter.scala b/src/dotty/tools/dotc/reporting/ConsoleReporter.scala
index 4d8897820..3a4f60aeb 100644
--- a/src/dotty/tools/dotc/reporting/ConsoleReporter.scala
+++ b/src/dotty/tools/dotc/reporting/ConsoleReporter.scala
@@ -9,7 +9,7 @@ import Reporter._
 import java.io.{ BufferedReader, IOException, PrintWriter }
 import scala.reflect.internal.util._
 import diagnostic.Message
-import diagnostic.basic._
+import diagnostic.messages._
 
 /**
  * This class implements a Reporter that displays messages on a text
diff --git a/src/dotty/tools/dotc/reporting/ErrorMessages.scala b/src/dotty/tools/dotc/reporting/ErrorMessages.scala
deleted file mode 100644
index 2b4c0db7e..000000000
--- a/src/dotty/tools/dotc/reporting/ErrorMessages.scala
+++ /dev/null
@@ -1,117 +0,0 @@
-package dotty.tools
-package dotc
-package reporting
-
-import dotc.core._
-import Contexts.Context, Decorators._, Symbols._
-import dotc.printing.SyntaxHighlighting._
-import util.{SourcePosition, NoSourcePosition}
-
-object ErrorMessages {
-  import dotc.ast.Trees._
-  import dotc.ast.untpd
-
-  implicit class ShouldExplainCtx(val c: Context) extends AnyVal {
-    def shouldExplain(expl: ErrorMessage): Boolean = {
-      implicit val ctx = c
-      expl match {
-        case _: NoExplanation => false
-        case expl if ctx.settings.explainerrors.value => true
-        case _ => false
-      }
-    }
-  }
-
-  trait ErrorMessage {
-    def kind: String
-    def msg: String
-    def explanation: String
-  }
-
-  case class NoExplanation(msg: String)(implicit val kind: String) extends ErrorMessage {
-    val explanation = ""
-  }
-
-  object Syntax {
-    implicit val kind: String = "Syntax"
-    implicit def stringToErrorMessage(s: String): ErrorMessage = NoExplanation(s)
-
-    abstract class EmptyCatchOrFinallyBlock(tryBody: untpd.Tree)(implicit ctx: Context, val kind: String) extends ErrorMessage {
-      val explanation = {
-        val tryString = tryBody match {
-          case Block(Nil, untpd.EmptyTree) => "{}"
-          case _ => tryBody.show
-        }
-
-        val code1 =
-          s"""|try $tryString catch {
-              |  case t: Throwable => ???
-              |}""".stripMargin
-
-        val code2 =
-          s"""|try $tryString finally {
-              |  // perform your cleanup here!
-              |}""".stripMargin
-
-        hl"""|Explanation:
-             |============
-             |A ${"try"} expression should be followed by some mechanism to handle any exceptions
-             |thrown. Typically a ${"catch"} expression follows the ${"try"} and pattern matches
-             |on any expected exceptions. For example:
-             |
-             |$code1
-             |
-             |It is also possible to follow a ${"try"} immediately by a ${"finally"} - letting the
-             |exception propagate - but still allowing for some clean up in ${"finally"}:
-             |
-             |$code2
-             """.stripMargin
-      }
-    }
-
-    case class EmptyCatchBlock(tryBody: untpd.Tree)(implicit ctx: Context, override val kind: String)
-    extends EmptyCatchOrFinallyBlock(tryBody) {
-      val msg =
-        hl"""The ${"catch"} block does not contain a valid expression, try adding a case like - `${"case e: Exception =>"}` to the block"""
-    }
-
-    case class EmptyCatchAndFinallyBlock(tryBody: untpd.Tree)(implicit ctx: Context, override val kind: String)
-    extends EmptyCatchOrFinallyBlock(tryBody) {
-      val msg =
-        hl"""A ${"try"} without ${"catch"} or ${"finally"} is equivalent to putting its body in a block; no exceptions are handled."""
-    }
-  }
-
-  object Type {
-    implicit val kind: String = "Type"
-    implicit def stringToErrorMessage(s: String): ErrorMessage = NoExplanation(s)
-
-    case class DuplicateBind(bind: untpd.Bind, tree: untpd.CaseDef)(implicit ctx: Context, val kind: String) extends ErrorMessage {
-      val msg =
-        em"duplicate pattern variable: `${bind.name}`"
-
-      val explanation = {
-        val pat = tree.pat.show
-        val guard = tree.guard match {
-          case untpd.EmptyTree => ""
-          case guard => s"if ${guard.show}"
-        }
-
-        val body = tree.body match {
-          case Block(Nil, untpd.EmptyTree) => ""
-          case body => s" ${body.show}"
-        }
-
-        val caseDef = s"case $pat$guard => $body"
-
-        hl"""|Explanation
-             |===========
-             |For each ${"case"} bound variable names  have to be unique. In:
-             |
-             |$caseDef
-             |
-             |`${bind.name}` is not unique. Rename one of the bound variables!""".stripMargin
-      }
-    }
-  }
-}
diff --git a/src/dotty/tools/dotc/reporting/Reporter.scala b/src/dotty/tools/dotc/reporting/Reporter.scala
index ccbae94bf..5c3dcccb7 100644
--- a/src/dotty/tools/dotc/reporting/Reporter.scala
+++ b/src/dotty/tools/dotc/reporting/Reporter.scala
@@ -10,9 +10,9 @@ import config.Printers
 import java.lang.System.currentTimeMillis
 import core.Mode
 import dotty.tools.dotc.core.Symbols.Symbol
-import diagnostic.Message
-import ErrorMessages._
-import diagnostic.basic._
+import diagnostic.messages._
+import diagnostic._
+import MessageCreator._
 
 object Reporter {
   /** Convert a SimpleReporter into a real Reporter */
@@ -75,10 +75,10 @@ trait Reporting { this: Context =>
   def warning(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
     reporter.report(new Warning(msg, pos))
 
-  def explainWarning(err: => ErrorMessage, pos: SourcePosition = NoSourcePosition): Unit = {
-    reporter.report(new Warning(err.msg, pos, s"${err.kind} warning"))
-    if (this.shouldExplain(err))
-      reporter.report(new Info(err.explanation, NoSourcePosition))
+  def explainWarning(msg: => MessageCreator, pos: SourcePosition = NoSourcePosition): Unit = {
+    reporter.report(msg.warning(pos))
+    if (this.shouldExplain(msg))
+      reporter.report(new Info(msg.explanation, NoSourcePosition))
   }
 
   def strictWarning(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
@@ -90,10 +90,10 @@ trait Reporting { this: Context =>
     reporter.report(new Error(msg, pos))
   }
 
-  def explainError(err: => ErrorMessage, pos: SourcePosition = NoSourcePosition): Unit = {
-    reporter.report(new Error(err.msg, pos, s"${err.kind} error"))
-    if (this.shouldExplain(err))
-      reporter.report(new Info(err.explanation, NoSourcePosition))
+  def explainError(msg: => MessageCreator, pos: SourcePosition = NoSourcePosition): Unit = {
+    reporter.report(msg.error(pos))
+    if (this.shouldExplain(msg))
+      reporter.report(new Info(msg.explanation, NoSourcePosition))
   }
 
   def errorOrMigrationWarning(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
diff --git a/src/dotty/tools/dotc/reporting/StoreReporter.scala b/src/dotty/tools/dotc/reporting/StoreReporter.scala
index 2aa2253a0..fa4fd991f 100644
--- a/src/dotty/tools/dotc/reporting/StoreReporter.scala
+++ b/src/dotty/tools/dotc/reporting/StoreReporter.scala
@@ -7,7 +7,7 @@ import collection.mutable
 import Reporter.{Error, Warning}
 import config.Printers.typr
 import diagnostic.Message
-import diagnostic.basic._
+import diagnostic.messages._
 
 /**
  * This class implements a Reporter that stores all messages
diff --git a/src/dotty/tools/dotc/reporting/ThrowingReporter.scala b/src/dotty/tools/dotc/reporting/ThrowingReporter.scala
index b08145654..f4e3b472d 100644
--- a/src/dotty/tools/dotc/reporting/ThrowingReporter.scala
+++ b/src/dotty/tools/dotc/reporting/ThrowingReporter.scala
@@ -5,7 +5,7 @@ package reporting
 import core.Contexts.Context
 import collection.mutable
 import diagnostic.Message
-import diagnostic.basic.Error
+import diagnostic.messages.Error
 import Reporter._
 
 /**
diff --git a/src/dotty/tools/dotc/reporting/diagnostic/Message.scala b/src/dotty/tools/dotc/reporting/diagnostic/Message.scala
index 90ebf11a2..d7cfa2e2b 100644
--- a/src/dotty/tools/dotc/reporting/diagnostic/Message.scala
+++ b/src/dotty/tools/dotc/reporting/diagnostic/Message.scala
@@ -4,6 +4,7 @@ package reporting
 package diagnostic
 
 import util.SourcePosition
+import core.Contexts.Context
 
 import java.util.Optional
 
@@ -55,9 +56,3 @@ class Message(
   override def toString = s"$getClass at $pos: $message"
   override def getMessage() = message
 }
-
-object NoExplanation {
-  def unapply(m: Message): Option[Message] =
-    if (m.explanation == "") Some(m)
-    else None
-}
diff --git a/src/dotty/tools/dotc/reporting/diagnostic/MessageCreator.scala b/src/dotty/tools/dotc/reporting/diagnostic/MessageCreator.scala
new file mode 100644
index 000000000..d74099f4f
--- /dev/null
+++ b/src/dotty/tools/dotc/reporting/diagnostic/MessageCreator.scala
@@ -0,0 +1,62 @@
+package dotty.tools
+package dotc
+package reporting
+package diagnostic
+
+import util.{SourcePosition, NoSourcePosition}
+import core.Contexts.Context
+
+object MessageCreator {
+  implicit class DiagnosticContext(val c: Context) extends AnyVal {
+    def shouldExplain(msg: MessageCreator): Boolean = {
+      implicit val ctx: Context = c
+      msg match {
+        case NoExplanation(_) => false
+        case _ => ctx.settings.explain.value
+      }
+    }
+  }
+
+  implicit def toNoExplanation(str: String) =
+    new NoExplanation(str)
+}
+
+trait MessageCreator {
+  import messages._
+
+  def msg: String
+  def kind: String
+  def explanation: String
+
+  def error(pos: SourcePosition) =
+    new Error(msg, pos, kind, explanation)
+
+  def warning(pos: SourcePosition) =
+    new Warning(msg, pos, kind, explanation)
+
+  def info(pos: SourcePosition) =
+    new Info(msg, pos, kind, explanation)
+
+  def featureWarnign(pos: SourcePosition) =
+    new FeatureWarning(msg, pos, kind, explanation)
+
+  def uncheckedWarning(pos: SourcePosition) =
+    new UncheckedWarning(msg, pos, kind, explanation)
+
+  def deprecationWarning(pos: SourcePosition) =
+    new DeprecationWarning(msg, pos, kind, explanation)
+
+  def migrationWarning(pos: SourcePosition) =
+    new MigrationWarning(msg, pos, kind, explanation)
+}
+
+class NoExplanation(val msg: String) extends MessageCreator {
+  val explanation = ""
+  val kind = ""
+}
+
+object NoExplanation {
+  def unapply(m: MessageCreator): Option[MessageCreator] =
+    if (m.explanation == "") Some(m)
+    else None
+}
diff --git a/src/dotty/tools/dotc/reporting/diagnostic/basic.scala b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
index 2da986d89..382e9a295 100644
--- a/src/dotty/tools/dotc/reporting/diagnostic/basic.scala
+++ b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
@@ -9,7 +9,7 @@ import util.{SourcePosition, NoSourcePosition}
 import config.Settings.Setting
 import interfaces.Diagnostic.{ERROR, WARNING, INFO}
 
-object basic {
+object messages {
 
   class Error(
     msgFn: => String,
diff --git a/src/dotty/tools/dotc/reporting/diagnostic/syntax.scala b/src/dotty/tools/dotc/reporting/diagnostic/syntax.scala
new file mode 100644
index 000000000..675cacbed
--- /dev/null
+++ b/src/dotty/tools/dotc/reporting/diagnostic/syntax.scala
@@ -0,0 +1,63 @@
+package dotty.tools
+package dotc
+package reporting
+package diagnostic
+
+import dotc.core._
+import Contexts.Context, Decorators._, Symbols._
+import dotc.printing.SyntaxHighlighting._
+import util.{SourcePosition, NoSourcePosition}
+
+object syntax {
+  import dotc.ast.Trees._
+  import dotc.ast.untpd
+
+  abstract class EmptyCatchOrFinallyBlock(tryBody: untpd.Tree)(implicit ctx: Context) extends MessageCreator {
+    val explanation = {
+      val tryString = tryBody match {
+        case Block(Nil, untpd.EmptyTree) => "{}"
+        case _ => tryBody.show
+      }
+
+      val code1 =
+        s"""|try $tryString catch {
+            |  case t: Throwable => ???
+            |}""".stripMargin
+
+      val code2 =
+        s"""|try $tryString finally {
+            |  // perform your cleanup here!
+            |}""".stripMargin
+
+      hl"""|Explanation:
+           |============
+           |A ${"try"} expression should be followed by some mechanism to handle any exceptions
+           |thrown. Typically a ${"catch"} expression follows the ${"try"} and pattern matches
+           |on any expected exceptions. For example:
+           |
+           |$code1
+           |
+           |It is also possible to follow a ${"try"} immediately by a ${"finally"} - letting the
+           |exception propagate - but still allowing for some clean up in ${"finally"}:
+           |
+           |$code2
+           """.stripMargin
+    }
+  }
+
+  class EmptyCatchBlock(tryBody: untpd.Tree)(implicit ctx: Context)
+  extends EmptyCatchOrFinallyBlock(tryBody) {
+    val kind = "Syntax"
+    val msg =
+      hl"""|The ${"catch"} block does not contain a valid expression, try
+           |adding a case like - `${"case e: Exception =>"}` to the block""".stripMargin
+  }
+
+  case class EmptyCatchAndFinallyBlock(tryBody: untpd.Tree)(implicit ctx: Context)
+  extends EmptyCatchOrFinallyBlock(tryBody) {
+    val kind = "Syntax"
+    val msg =
+      hl"""|A ${"try"} without ${"catch"} or ${"finally"} is equivalent to putting
+           |its body in a block; no exceptions are handled.""".stripMargin
+  }
+}
diff --git a/src/dotty/tools/dotc/reporting/diagnostic/tpe.scala b/src/dotty/tools/dotc/reporting/diagnostic/tpe.scala
new file mode 100644
index 000000000..4aa24c440
--- /dev/null
+++ b/src/dotty/tools/dotc/reporting/diagnostic/tpe.scala
@@ -0,0 +1,47 @@
+package dotty.tools
+package dotc
+package reporting
+package diagnostic
+
+import dotc.core._
+import Contexts.Context, Decorators._, Symbols._
+import dotc.printing.SyntaxHighlighting._
+import util.{SourcePosition, NoSourcePosition}
+
+object tpe {
+  import dotc.ast.Trees._
+  import dotc.ast.untpd
+
+  class DuplicateBind(
+    bind: untpd.Bind,
+    tree: untpd.CaseDef
+  )(implicit ctx: Context) extends MessageCreator {
+    val kind = "Naming"
+
+    val msg =
+      em"duplicate pattern variable: `${bind.name}`"
+
+    val explanation = {
+      val pat = tree.pat.show
+      val guard = tree.guard match {
+        case untpd.EmptyTree => ""
+        case guard => s"if ${guard.show}"
+      }
+
+      val body = tree.body match {
+        case Block(Nil, untpd.EmptyTree) => ""
+        case body => s" ${body.show}"
+      }
+
+      val caseDef = s"case $pat$guard => $body"
+
+      hl"""|Explanation
+           |===========
+           |For each ${"case"} bound variable names  have to be unique. In:
+           |
+           |$caseDef
+           |
+           |`${bind.name}` is not unique. Rename one of the bound variables!""".stripMargin
+    }
+  }
+}
diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala
index 6fddd928b..32d5e0d96 100644
--- a/src/dotty/tools/dotc/typer/Typer.scala
+++ b/src/dotty/tools/dotc/typer/Typer.scala
@@ -38,7 +38,6 @@ import NavigateAST._
 import transform.SymUtils._
 import language.implicitConversions
 import printing.SyntaxHighlighting._
-import reporting.ErrorMessages._
 
 object Typer {
 
@@ -66,7 +65,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
   import tpd.{cpy => _, _}
   import untpd.cpy
   import Dynamic.isDynamicMethod
-  import reporting.ErrorMessages.Type._
+  import reporting.diagnostic.tpe._
 
   /** A temporary data item valid for a single typed ident:
    *  The set of all root import symbols that have been
@@ -848,7 +847,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
         super.transform(trt.withType(elimWildcardSym(trt.tpe))) match {
           case b: Bind =>
             if (ctx.scope.lookup(b.name) == NoSymbol) ctx.enter(b.symbol)
-            else ctx.explainError(DuplicateBind(b, tree), b.pos)
+            else ctx.explainError(new DuplicateBind(b, tree), b.pos)
             b.symbol.info = elimWildcardSym(b.symbol.info)
             b
           case t => t