Global takes legacy reporter

Let legacy Reporter handle adaptations.

Reporter knows how to adapt new API and enforce maxerrs.
The onus of supporting legacy reporter is on legacy reporter.

Keep new reporters simple enough.

Old reporters remain legacy reporters.
Some tests expect to downcast to the reporter
they assigned to `global.reporter`. Also avoid
delegating to `NoReporter`.

Author: som-snytt

src/compiler/scala/tools/nsc/Global.scala

@@ -14,8 +14,9 @@ import java.nio.charset.{Charset, CharsetDecoder, IllegalCharsetNameException, U
 import scala.collection.{immutable, mutable}
 import io.{AbstractFile, Path, SourceReader}
 import util.{ClassPath, returning}
+import reporters.{Reporter => LegacyReporter}
 import scala.reflect.ClassTag
-import scala.reflect.internal.Reporter
+import scala.reflect.internal.{Reporter => InternalReporter}
 import scala.reflect.internal.util.{BatchSourceFile, FreshNameCreator, NoSourceFile, ScalaClassLoader, ScriptSourceFile, SourceFile, StatisticsStatics}
 import scala.reflect.internal.pickling.PickleBuffer
 import symtab.{Flags, SymbolTable, SymbolTrackers}
@@ -34,7 +35,7 @@ import scala.tools.nsc.ast.{TreeGen => AstTreeGen}
 import scala.tools.nsc.classpath._
 import scala.tools.nsc.profile.Profiler
 
-class Global(var currentSettings: Settings, reporter0: Reporter)
+class Global(var currentSettings: Settings, reporter0: LegacyReporter)
     extends SymbolTable
     with CompilationUnits
     with Plugins
@@ -76,29 +77,21 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
 
   override def settings = currentSettings
 
-  private[this] var currentReporter: Reporter = { reporter = reporter0 ; currentReporter }
+  private[this] var currentReporter: LegacyReporter = { reporter = reporter0 ; currentReporter }
 
-  def reporter: Reporter = currentReporter
-  def reporter_=(newReporter: Reporter): Unit =
-    currentReporter = newReporter match {
-      case _: reporters.ConsoleReporter | _: reporters.DefaultReporter | _: reporters.LimitingReporter => newReporter
-      case _ if settings.maxerrs.isSetByUser && settings.maxerrs.value < settings.maxerrs.default =>
-        new reporters.LimitingReporter(settings, newReporter)
-      case _ => newReporter
-    }
-  def reporter_=(oldReporter: reporters.Reporter): Unit = reporter_=(oldReporter.asInstanceOf[Reporter])
+  def reporter: LegacyReporter = currentReporter
+  // enforce maxerrs if necessary
+  def reporter_=(newReporter: LegacyReporter): Unit = currentReporter = LegacyReporter.limitedReporter(settings, newReporter)
 
   /** Switch to turn on detailed type logs */
   var printTypings = settings.Ytyperdebug.value
 
-  def this(reporter: Reporter) =
+  def this(reporter: LegacyReporter) =
     this(new Settings(err => reporter.error(null, err)), reporter)
 
   def this(settings: Settings) =
     this(settings, Global.reporter(settings))
 
-  def this(settings: Settings, reporter: reporters.Reporter) = this(settings, reporter.asInstanceOf[Reporter])
-
   def picklerPhase: Phase = if (currentRun.isDefined) currentRun.picklerPhase else NoPhase
 
   def erasurePhase: Phase = if (currentRun.isDefined) currentRun.erasurePhase else NoPhase
@@ -261,10 +254,7 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
     // TODO: this is all very broken (only works for scaladoc comments, not regular ones)
     //       --> add hooks to parser and refactor Interactive global to handle comments directly
     //       in any case don't use reporter for parser hooks
-    reporter match {
-      case hooker: reporters.Reporter => hooker.comment(pos, comment)
-      case _ => ()
-    }
+    reporter.comment(pos, comment)
   }
 
   /** Register new context; called for every created context
@@ -344,7 +334,7 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
     }
 
     def loadReader(name: String): Option[SourceReader] = {
-      def ccon = Class.forName(name).getConstructor(classOf[CharsetDecoder], classOf[Reporter])
+      def ccon = Class.forName(name).getConstructor(classOf[CharsetDecoder], classOf[InternalReporter])
 
       try Some(ccon.newInstance(charset.newDecoder(), reporter).asInstanceOf[SourceReader])
       catch { case ex: Throwable =>
@@ -401,10 +391,7 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
 
     /** Is current phase cancelled on this unit? */
     def cancelled(unit: CompilationUnit) = {
-      val isCanceled = reporter match {
-        case cancelable: reporters.Reporter => cancelable.cancelled
-        case _ => false
-      }
+      val isCanceled = reporter.cancelled
       // run the typer only if in `createJavadoc` mode
       val maxJavaPhase = if (createJavadoc) currentRun.typerPhase.id else currentRun.namerPhase.id
       isCanceled || unit.isJava && this.id > maxJavaPhase
@@ -1276,10 +1263,7 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
     }
 
     // for sbt
-    def cancel(): Unit = reporter match {
-      case cancelable: reporters.Reporter => cancelable.cancelled = true
-      case _ => ()
-    }
+    def cancel(): Unit = reporter.cancelled = true
 
     private def currentProgress   = (phasec * size) + unitc
     private def totalProgress     = (phaseDescriptors.size - 1) * size // -1: drops terminal phase
@@ -1669,14 +1653,16 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
 }
 
 object Global {
-  def apply(settings: Settings, reporter: Reporter): Global = new Global(settings, reporter)
+  def apply(settings: Settings, reporter: LegacyReporter): Global = new Global(settings, reporter)
 
   def apply(settings: Settings): Global = new Global(settings, reporter(settings))
 
-  private def reporter(settings: Settings): Reporter = {
+  private def reporter(settings: Settings): LegacyReporter = {
     //val loader = ScalaClassLoader(getClass.getClassLoader)  // apply does not make delegate
     val loader = new ClassLoader(getClass.getClassLoader) with ScalaClassLoader
-    loader.create[Reporter](settings.reporter.value, settings.errorFn)(settings)
+    val res = loader.create[InternalReporter](settings.reporter.value, settings.errorFn)(settings)
+    if (res.isInstanceOf[LegacyReporter]) res.asInstanceOf[LegacyReporter]
+    else res: LegacyReporter  // adaptable
   }
   private object InitPhase extends Phase(null) {
     def name = "<init phase>"

src/compiler/scala/tools/nsc/reporters/DisplayReporter.scala

@@ -97,8 +97,7 @@ object DisplayReporter {
 class DefaultReporter(settings: Settings, writer: PrintWriter, echo: PrintWriter)
   extends PositionFilter(settings, DisplayReporter(settings, Console.in, writer, echo))
   with CountingReporter
-  with LimitFilter
-  with SummaryReporter {
+  with LimitFilter {
   // required constructor for -Xreporter
   def this(settings: Settings) = this(settings, new PrintWriter(Console.err, true), new PrintWriter(Console.out, true))
   def shortname_=(flag: Boolean): Unit = delegate.asInstanceOf[DisplayReporter].shortname = flag
@@ -124,11 +123,20 @@ trait FilteringReporter extends ForwardingReporter {
   override def error(pos: Position, msg: String)   = if (filter(pos, msg, ERROR)) delegate.error(pos, msg)
 }
 
+/** A `Reporter` that counts messages that are passed by the filter and echos a summary in `finish`. */
 trait CountingReporter extends FilteringReporter {
   abstract override protected def filter(pos: Position, msg: String, severity: Severity): Boolean =
     super.filter(pos, msg, severity) && { severity.count += 1 ; true }
+  /** Prints the number of warnings and errors if there are any. */
+  override def finish(): Unit = {
+    import reflect.internal.util.StringOps.{countElementsAsString => countAs}
+    if (hasWarnings) echo(s"${countAs(WARNING.count, WARNING.toString.toLowerCase)} found")
+    if (hasErrors)   echo(s"${countAs(ERROR.count, ERROR.toString.toLowerCase)} found")
+    super.finish()
+  }
 }
 
+/** Disable a message when super.filter has passed the message but max limit has been reached. */
 trait LimitFilter extends FilteringReporter {
   def maxerrs: Int
   def maxwarns: Int
@@ -146,22 +154,3 @@ trait LimitFilter extends FilteringReporter {
       case _       => true
     }}
 }
-
-/** A `Reporter` that echos a summary in `finish`.
- */
-trait SummaryReporter extends InternalReporter { _: CountingReporter =>
-  /** Prints the number of warnings and errors if there are any. */
-  override def finish(): Unit = {
-    import reflect.internal.util.StringOps.{countElementsAsString => countAs}
-    if (hasWarnings) echo(s"${countAs(WARNING.count, WARNING.toString.toLowerCase)} found")
-    if (hasErrors)   echo(s"${countAs(ERROR.count, ERROR.toString.toLowerCase)} found")
-    super.finish()
-  }
-}
-
-trait TracingReporter extends ForwardingReporter {
-  private def debug(pos: Position, msg: String, sev: Severity) = println(s"[$sev] $pos $msg")
-  override def echo(pos: Position, msg: String)    = { debug(pos, msg, INFO); delegate.echo(pos, msg) }
-  override def warning(pos: Position, msg: String) = { debug(pos, msg, WARNING); delegate.warning(pos, msg) }
-  override def error(pos: Position, msg: String)   = { debug(pos, msg, ERROR); delegate.error(pos, msg) }
-}

src/compiler/scala/tools/nsc/reporters/LimitingReporter.scala

@@ -4,6 +4,6 @@ import scala.reflect.internal.util.Position
 
 /** A reporter that ignores reports.
  */
-object NoReporter extends scala.reflect.internal.Reporter {
+object NoReporter extends Reporter {
   protected def info0(pos: Position, msg: String, severity: Severity, force: Boolean): Unit = ()
 }

src/compiler/scala/tools/nsc/reporters/NoReporter.scala

@@ -55,9 +55,3 @@ class PositionFilter(settings: Settings, protected val delegate: InternalReporte
       }
     }
 }
-
-/** This reporter filters info messages based on `-verbose`.
- */
-class VerboseFilter(settings: Settings, protected val delegate: InternalReporter) extends InternalReporter with FilteringReporter {
-  override protected def filter(pos: Position, msg: String, severity: Severity) = settings.verbose || severity != INFO
-}

src/compiler/scala/tools/nsc/reporters/PositionFilter.scala

@@ -6,7 +6,8 @@
 package scala.tools.nsc
 package reporters
 
-import scala.reflect.internal.util._
+import scala.reflect.internal.{ForwardingReporter, Reporter => InternalReporter}
+import scala.reflect.internal.util.Position
 
 /** Report information, warnings and errors.
  *
@@ -16,16 +17,11 @@ import scala.reflect.internal.util._
  *  TODO: Move external clients (sbt/ide) to reflect.internal.Reporter, and remove this class.
  */
 @deprecated("Use reflect.internal.Reporter", since="2.13.0")
-abstract class Reporter extends scala.reflect.internal.Reporter {
+abstract class Reporter extends InternalReporter {
   /** Informational messages. If `!force`, they may be suppressed. */
   @deprecated("Use echo, as internal.Reporter does not support unforced info", since="2.13.0")
   final def info(pos: Position, msg: String, force: Boolean): Unit = info0(pos, msg, INFO, force)
 
-  /* For sending a message which should not be labelled as a warning/error,
-   * but also shouldn't require -verbose to be visible.
-  def echo(msg: String): Unit = info(NoPosition, msg, force = true)
-   */
-
   // overridden by sbt, IDE -- should not be in the reporting interface
   // (IDE receives comments from ScaladocAnalyzer using this hook method)
   // TODO: IDE should override a hook method in the parser instead
@@ -42,3 +38,43 @@ abstract class Reporter extends scala.reflect.internal.Reporter {
     cancelled = false
   }
 }
+
+object Reporter {
+  /** Adapt a reporter to legacy reporter API. Handle `info` by forwarding to `echo`. */
+  class AdaptedReporter(val delegate: InternalReporter) extends Reporter with ForwardingReporter {
+    override protected def info0(pos: Position, msg: String, severity: Severity, force: Boolean): Unit = delegate.echo(pos, msg)
+  }
+  /** A marker trait for adapted reporters that respect maxerrs. */
+  trait LimitedReporter { _: Reporter => }
+  /** A legacy `Reporter` adapter that respects `-Xmaxerrs` and `-Xmaxwarns`.  */
+  class LimitingReporter(settings: Settings, protected val delegate: Reporter) extends Reporter with FilteringReporter with LimitedReporter {
+    override protected def filter(pos: Position, msg: String, severity: Severity) =
+      severity match {
+        case ERROR   => errorCount   < settings.maxerrs.value
+        case WARNING => warningCount < settings.maxwarns.value
+        case _       => true
+      }
+    // work around fractured API to support `reporters.Reporter.info`, which is not forwarded
+    override protected def info0(pos: Position, msg: String, severity: Severity, force: Boolean): Unit =
+      severity match {
+        case ERROR      => delegate.error(pos, msg)     // for symmetry, but error and warn are already forwarded
+        case WARNING    => delegate.warning(pos, msg)
+        case _ if force => delegate.echo(pos, msg)
+        case _          => delegate.info(pos, msg, force = false)
+      }
+  }
+  // mark reporters known to respect maxerrs
+  implicit def `adapt reporter to legacy API`(reporter: InternalReporter): Reporter =
+    reporter match {
+      case _: LimitFilter => new AdaptedReporter(reporter) with LimitedReporter
+      case _              => new AdaptedReporter(reporter)
+    }
+  // whitelist reporters known to respect maxerrs; otherwise, enforce user-specified reduced limit
+  def limitedReporter(settings: Settings, reporter: Reporter): Reporter =
+    reporter match {
+      case _: ConsoleReporter | _: LimitedReporter => reporter
+      case _ if settings.maxerrs.isSetByUser && settings.maxerrs.value < settings.maxerrs.default =>
+        new LimitingReporter(settings, reporter)
+      case _ => reporter
+    }
+}

src/compiler/scala/tools/nsc/reporters/Reporter.scala

@@ -10,7 +10,7 @@ import scala.collection.mutable
 import scala.reflect.internal.util.Position
 
 /** This class implements a Reporter that stores its reports in the set `infos`. */
-class StoreReporter extends scala.reflect.internal.Reporter {
+class StoreReporter extends Reporter {
   case class Info(pos: Position, msg: String, severity: Severity) {
     override def toString() = s"pos: $pos $msg $severity"
   }

src/compiler/scala/tools/nsc/reporters/StoreReporter.scala

@@ -136,7 +136,7 @@ trait ForwardingReporter extends Reporter {
     }
 
   /* Always throws `UnsupportedOperationException`. */
-  protected def info0(pos: Position, msg: String, severity: Severity, force: Boolean): Nothing =
+  protected def info0(pos: Position, msg: String, severity: Severity, force: Boolean): Unit =
     throw new UnsupportedOperationException(s"$msg ($pos)")
 
   override def echo(pos: Position, msg: String)    = delegate.echo(pos, msg)

src/reflect/scala/reflect/internal/Reporting.scala

@@ -33,7 +33,7 @@ class ConsoleReporterTest {
     try {
       test(pos)
       val buf = writerOut.toString
-      if (msg.isEmpty && severity.isEmpty) assertTrue(buf.isEmpty)
+      if (msg.isEmpty && severity.isEmpty) assertTrue(s"Expected no message output but saw: [$buf]", buf.isEmpty)
       else if (!pos.isDefined) assertEquals(severity + msg, buf.lines.next)
       else {
         val it = buf.lines
@@ -169,7 +169,7 @@ class ConsoleReporterTest {
       settings.maxerrs.value  = 1
       settings.maxwarns.value = 1
 
-      new LimitingReporter(settings, reporter)
+      new Reporter.LimitingReporter(settings, reporter)
     }
 
     // pass one message