Unclutter Reporter. Move truncation to ReplReporter.

Refactor to reduce the Reporter interface. Working towards
minimal interfaces in scala.reflect.internal that can be consumed by sbt/IDE/....
The scala.tools.nsc package is entirely private to the compiler (in principle).

A `Reporter` should only be used to inform (info/warning/error). No state.
Ideally, we'd move to having only one reporter, whose lifetime is adjusted
appropriately (from per-run in general to per-context for type checking,
so errors can be buffered -- "silenced" -- during nested type checking calls).

Start the clean up by moving truncation to the REPL,
since it's not relevant for regular reporting. Perversely, we were checking
truncation all the time, even though it's only on during a repl run.
(Truncation is now always turned off in the repl under -verbose.)

Untangle error resetting on symbols from error reporting (reportAdditionalErrors).
This fixes a nice&subtle bug that caused feature warnings to be suppressed under
`-Xfatal-warnings`:

```
def reportCompileErrors() {
  if (!reporter.hasErrors && reporter.hasWarnings && settings.fatalWarnings)
    globalError("No warnings can be incurred under -Xfatal-warnings.")

  if (reporter.hasErrors) { ... }
  else {
    // will erroneously not get here if
    // `reporter.hasWarnings && settings.fatalWarnings`
    // since the `globalError` call above means `reporter.hasErrors`...
    allConditionalWarnings foreach (_.summarize())
    ...
  }
}
```

The second `if`'s condition depends on the `globalError` call in the first `if`...

7 files changed, 69 insertions, 58 deletions

diff --git a/src/compiler/scala/tools/nsc/CompilationUnits.scala b/src/compiler/scala/tools/nsc/CompilationUnits.scala
index c2caed70a0..924bb54ddb 100644
--- a/src/compiler/scala/tools/nsc/CompilationUnits.scala
+++ b/src/compiler/scala/tools/nsc/CompilationUnits.scala
@@ -146,7 +146,8 @@ trait CompilationUnits { global: Global =>
     def incompleteInputError(pos: Position, msg:String) =
       reporter.incompleteInputError(pos, msg)
 
-    def comment(pos: Position, msg: String) =
+    // used by the IDE -- TODO: don't use reporter to communicate comments from parser to IDE!
+    def comment(pos: Position, msg: String): Unit =
       reporter.comment(pos, msg)
 
     /** Is this about a .java source file? */
diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index cb785de4b3..836203e80d 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -291,8 +291,6 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
       log(s"!!!$pos_s $msg") // such warnings always at least logged
   }
 
-  def informComplete(msg: String): Unit    = reporter.withoutTruncating(inform(msg))
-
   def logError(msg: String, t: Throwable): Unit = ()
 
   override def shouldLogAtThisPhase = settings.log.isSetByUser && (
@@ -351,9 +349,9 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
   }
 
   if (settings.verbose || settings.Ylogcp) {
-    // Uses the "do not truncate" inform
-    informComplete("[search path for source files: " + classPath.sourcepaths.mkString(",") + "]")
-    informComplete("[search path for class files: " + classPath.asClasspathString + "]")
+    reporter.echo(
+      s"[search path for source files: ${classPath.sourcepaths.mkString(",")}]\n"+
+      s"[search path for class files: ${classPath.asClasspathString}")
   }
 
   // The current division between scala.reflect.* and scala.tools.nsc.* is pretty
@@ -1212,6 +1210,20 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
     def uncheckedWarnings: List[(Position, String)] = uncheckedWarnings0.warnings.toList // used in sbt
     def deprecationWarnings: List[(Position, String)] = deprecationWarnings0.warnings.toList // used in sbt
 
+    def reportAdditionalErrors(): Unit = {
+      if (!reporter.hasErrors) {
+        if (reporter.hasWarnings && settings.fatalWarnings)
+          globalError("No warnings can be incurred under -Xfatal-warnings.")
+
+        allConditionalWarnings foreach (_.summarize())
+
+        if (seenMacroExpansionsFallingBack)
+          warning("some macros could not be expanded and code fell back to overridden methods;"+
+                  "\nrecompiling with generated classfiles on the classpath might help.")
+        // todo: migrationWarnings
+      }
+    }
+
     var reportedFeature = Set[Symbol]()
 
     /** Has any macro expansion used a fallback during this run? */
@@ -1416,6 +1428,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
       refreshProgress()
     }
 
+    // for sbt
     def cancel() { reporter.cancelled = true }
 
     private def currentProgress   = (phasec * size) + unitc
@@ -1565,26 +1578,6 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
       }
     }
 
-    def reportCompileErrors() {
-      if (!reporter.hasErrors && reporter.hasWarnings && settings.fatalWarnings)
-        globalError("No warnings can be incurred under -Xfatal-warnings.")
-
-      if (reporter.hasErrors) {
-        for ((sym, file) <- symSource.iterator) {
-          sym.reset(new loaders.SourcefileLoader(file))
-          if (sym.isTerm)
-            sym.moduleClass reset loaders.moduleClassLoader
-        }
-      }
-      else {
-        allConditionalWarnings foreach (_.summarize())
-
-        if (seenMacroExpansionsFallingBack)
-          warning("some macros could not be expanded and code fell back to overridden methods;"+
-                  "\nrecompiling with generated classfiles on the classpath might help.")
-        // todo: migrationWarnings
-      }
-    }
 
     /** Caching member symbols that are def-s in Defintions because they might change from Run to Run. */
     val runDefinitions: definitions.RunDefinitions = new definitions.RunDefinitions
@@ -1597,7 +1590,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
 
       def checkDeprecations() = {
         checkDeprecatedSettings(newCompilationUnit(""))
-        reportCompileErrors()
+        reportAdditionalErrors()
       }
 
       val units = sources map scripted map (new CompilationUnit(_))
@@ -1621,7 +1614,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
       checkDeprecatedSettings(unitbuf.head)
       globalPhase = fromPhase
 
-     while (globalPhase.hasNext && !reporter.hasErrors) {
+      while (globalPhase.hasNext && !reporter.hasErrors) {
         val startTime = currentTime
         phase = globalPhase
         globalPhase.run()
@@ -1667,6 +1660,8 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
         advancePhase()
       }
 
+      reportAdditionalErrors()
+
       if (traceSymbolActivity)
         units map (_.body) foreach (traceSymbols recordSymbolsInTree _)
 
@@ -1674,8 +1669,15 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
       if (settings.Yshow.isDefault)
         showMembers()
 
-      reportCompileErrors()
+      if (reporter.hasErrors) {
+        for ((sym, file) <- symSource.iterator) {
+          sym.reset(new loaders.SourcefileLoader(file))
+          if (sym.isTerm)
+            sym.moduleClass reset loaders.moduleClassLoader
+        }
+      }
       symSource.keys foreach (x => resetPackageClass(x.owner))
+
       informTime("total", startTime)
 
       // Clear any sets or maps created via perRunCaches.
diff --git a/src/compiler/scala/tools/nsc/reporters/ConsoleReporter.scala b/src/compiler/scala/tools/nsc/reporters/ConsoleReporter.scala
index 3f210a543c..0b218b711c 100644
--- a/src/compiler/scala/tools/nsc/reporters/ConsoleReporter.scala
+++ b/src/compiler/scala/tools/nsc/reporters/ConsoleReporter.scala
@@ -12,8 +12,7 @@ import scala.reflect.internal.util._
 import StringOps._
 
 /**
- * This class implements a Reporter that displays messages on a text
- * console.
+ * This class implements a Reporter that displays messages on a text console.
  */
 class ConsoleReporter(val settings: Settings, reader: BufferedReader, writer: PrintWriter) extends AbstractReporter {
   def this(settings: Settings) = this(settings, Console.in, new PrintWriter(Console.err, true))
diff --git a/src/compiler/scala/tools/nsc/reporters/Reporter.scala b/src/compiler/scala/tools/nsc/reporters/Reporter.scala
index 68362c066d..766a35960b 100644
--- a/src/compiler/scala/tools/nsc/reporters/Reporter.scala
+++ b/src/compiler/scala/tools/nsc/reporters/Reporter.scala
@@ -8,9 +8,10 @@ package reporters
 
 import scala.reflect.internal.util._
 
-/**
- * This interface provides methods to issue information, warning and
- * error messages.
+/** Report information, warnings and errors.
+ *
+ * This describes the stable interface for issuing information, warnings and errors.
+ * The only abstract method in this class must be info0.
  */
 abstract class Reporter {
   protected def info0(pos: Position, msg: String, severity: Severity, force: Boolean): Unit
@@ -29,19 +30,6 @@ abstract class Reporter {
     override def toString: String = "ERROR"
   }
 
-  /** Whether very long lines can be truncated.  This exists so important
-   *  debugging information (like printing the classpath) is not rendered
-   *  invisible due to the max message length.
-   */
-  private var _truncationOK: Boolean = true
-  def truncationOK = _truncationOK
-  def withoutTruncating[T](body: => T): T = {
-    val saved = _truncationOK
-    _truncationOK = false
-    try body
-    finally _truncationOK = saved
-  }
-
   private var incompleteHandler: (Position, String) => Unit = null
   def incompleteHandled = incompleteHandler != null
   def withIncompleteHandler[T](handler: (Position, String) => Unit)(thunk: => T) = {
@@ -51,6 +39,7 @@ abstract class Reporter {
     finally incompleteHandler = saved
   }
 
+  // used by sbt (via unit.cancel) to cancel a compile (see hasErrors)
   var cancelled   = false
   def hasErrors   = ERROR.count > 0 || cancelled
   def hasWarnings = WARNING.count > 0
@@ -58,21 +47,23 @@ abstract class Reporter {
   /** For sending a message which should not be labeled as a warning/error,
    *  but also shouldn't require -verbose to be visible.
    */
-  def echo(msg: String): Unit                                = info(NoPosition, msg, force = true)
-  def echo(pos: Position, msg: String): Unit                 = info(pos, msg, force = true)
+  def echo(msg: String): Unit                   = info(NoPosition, msg, force = true)
+  def echo(pos: Position, msg: String): Unit    = info(pos, msg, force = true)
 
-  /** Informational messages, suppressed unless -verbose or force=true. */
-  def info(pos: Position, msg: String, force: Boolean): Unit = info0(pos, msg, INFO, force)
+  /** Informational messages. If `!force`, they may be suppressed. */
+  final def info(pos: Position, msg: String, force: Boolean): Unit = info0(pos, msg, INFO, force)
 
   /** Warnings and errors. */
-  def warning(pos: Position, msg: String): Unit              = withoutTruncating(info0(pos, msg, WARNING, force = false))
-  def error(pos: Position, msg: String): Unit                = withoutTruncating(info0(pos, msg, ERROR, force = false))
+  def warning(pos: Position, msg: String): Unit = info0(pos, msg, WARNING, force = false)
+  def error(pos: Position, msg: String): Unit   = info0(pos, msg, ERROR, force = false)
   def incompleteInputError(pos: Position, msg: String): Unit = {
     if (incompleteHandled) incompleteHandler(pos, msg)
     else error(pos, msg)
   }
 
-  def comment(pos: Position, msg: String) { }
+  // overridden by sbt, IDE:
+  // `comment` is unrelated to reporting and should move out (IDE receives comments from ScaladocAnalyzer)
+  def comment(pos: Position, msg: String) {}
   def flush() { }
   def reset() {
     INFO.count        = 0
diff --git a/src/compiler/scala/tools/nsc/reporters/StoreReporter.scala b/src/compiler/scala/tools/nsc/reporters/StoreReporter.scala
index 04c5bdf824..24a61cb171 100644
--- a/src/compiler/scala/tools/nsc/reporters/StoreReporter.scala
+++ b/src/compiler/scala/tools/nsc/reporters/StoreReporter.scala
@@ -10,8 +10,7 @@ import scala.collection.mutable
 import scala.reflect.internal.util.Position
 
 /**
- * This class implements a Reporter that displays messages on a text
- * console.
+ * This class implements a Reporter that stores its reports in the set `infos`.
  */
 class StoreReporter extends Reporter {
   case class Info(pos: Position, msg: String, severity: Severity) {
diff --git a/src/repl/scala/tools/nsc/interpreter/IMain.scala b/src/repl/scala/tools/nsc/interpreter/IMain.scala
index 47d97dd4dd..2deeed2be3 100644
--- a/src/repl/scala/tools/nsc/interpreter/IMain.scala
+++ b/src/repl/scala/tools/nsc/interpreter/IMain.scala
@@ -110,7 +110,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
   lazy val reporter: ReplReporter = new ReplReporter(this)
 
   import formatting._
-  import reporter.{ printMessage, withoutTruncating }
+  import reporter.{ printMessage, printUntruncatedMessage }
 
   // This exists mostly because using the reporter too early leads to deadlock.
   private def echo(msg: String) { Console println msg }
@@ -609,7 +609,7 @@ class IMain(@BeanProperty val factory: ScriptEngineFactory, initialSettings: Set
       }
       else {
         // don't truncate stack traces
-        withoutTruncating(printMessage(result))
+        printUntruncatedMessage(result)
         IR.Error
       }
     }
diff --git a/src/repl/scala/tools/nsc/interpreter/ReplReporter.scala b/src/repl/scala/tools/nsc/interpreter/ReplReporter.scala
index b20166d070..88372334d6 100644
--- a/src/repl/scala/tools/nsc/interpreter/ReplReporter.scala
+++ b/src/repl/scala/tools/nsc/interpreter/ReplReporter.scala
@@ -9,11 +9,29 @@ package interpreter
 import reporters._
 import IMain._
 
+import scala.reflect.internal.util.Position
+
 /** Like ReplGlobal, a layer for ensuring extra functionality.
  */
 class ReplReporter(intp: IMain) extends ConsoleReporter(intp.settings, Console.in, new ReplStrippingWriter(intp)) {
   def printUntruncatedMessage(msg: String) = withoutTruncating(printMessage(msg))
 
+  /** Whether very long lines can be truncated.  This exists so important
+   *  debugging information (like printing the classpath) is not rendered
+   *  invisible due to the max message length.
+   */
+  private var _truncationOK: Boolean = !intp.settings.verbose
+  def truncationOK = _truncationOK
+  def withoutTruncating[T](body: => T): T = {
+    val saved = _truncationOK
+    _truncationOK = false
+    try body
+    finally _truncationOK = saved
+  }
+
+  override def warning(pos: Position, msg: String): Unit = withoutTruncating(super.warning(pos, msg))
+  override def error(pos: Position, msg: String): Unit   = withoutTruncating(super.error(pos, msg))
+
   override def printMessage(msg: String) {
     // Avoiding deadlock if the compiler starts logging before
     // the lazy val is complete.
@@ -31,4 +49,5 @@ class ReplReporter(intp: IMain) extends ConsoleReporter(intp.settings, Console.i
     if (intp.totalSilence) ()
     else super.displayPrompt()
   }
+
 }