Keep diagnostic info in a class that does not derive from Throwable

VirtusLab · Nov 25, 2021 · e1f5342 · e1f5342
1 parent 361f7a6
commit e1f5342
Show file tree

Hide file tree

Showing 9 changed files with 99 additions and 78 deletions.
diff --git a/modules/build/src/main/scala/scala/build/Build.scala b/modules/build/src/main/scala/scala/build/Build.scala
@@ -15,6 +15,7 @@ import scala.build.bloop.BloopServer
 import scala.build.blooprifle.{BloopRifleConfig, VersionUtil}
 import scala.build.errors._
 import scala.build.internal.{Constants, CustomCodeWrapper, MainClass, Util}
+import scala.build.options.validation.ValidationException
 import scala.build.options.{BuildOptions, ClassPathOptions, Platform, Scope}
 import scala.build.postprocessing._
 import scala.collection.mutable.ListBuffer
@@ -378,11 +379,9 @@ object Build {
     options: BuildOptions
   ): Either[BuildException, Unit] = {
     val (errors, otherDiagnostics) = options.validate.toSeq.partition(_.severity == Severity.ERROR)
-    otherDiagnostics.foreach { e =>
-      logger.log(e, e.severity)
-    }
+    logger.log(otherDiagnostics)
     if (errors.nonEmpty)
-      Left(CompositeBuildException(errors))
+      Left(CompositeBuildException(errors.map(new ValidationException(_))))
     else
       Right(())
   }

diff --git a/modules/build/src/main/scala/scala/build/Logger.scala b/modules/build/src/main/scala/scala/build/Logger.scala
@@ -3,7 +3,7 @@ package scala.build
 import java.io.{OutputStream, PrintStream}
 
 import scala.build.blooprifle.BloopRifleLogger
-import scala.build.errors.{BuildException, Severity}
+import scala.build.errors.{BuildException, Diagnostic}
 import scala.scalanative.{build => sn}
 
 trait Logger {
@@ -13,7 +13,8 @@ trait Logger {
   def log(s: => String, debug: => String): Unit
   def debug(s: => String): Unit
 
-  def log(ex: BuildException, severity: Severity = Severity.ERROR): Unit
+  def log(diagnostics: Seq[Diagnostic]): Unit
+  def log(ex: BuildException): Unit
   def exit(ex: BuildException): Nothing
 
   def coursierLogger: coursier.cache.CacheLogger
@@ -30,7 +31,8 @@ object Logger {
     def log(s: => String, debug: => String): Unit = ()
     def debug(s: => String): Unit                 = ()
 
-    def log(ex: BuildException, severity: Severity): Unit = ()
+    def log(diagnostics: Seq[Diagnostic]): Unit = ()
+    def log(ex: BuildException): Unit           = ()
     def exit(ex: BuildException): Nothing =
       throw new Exception(ex)
 

diff --git a/modules/build/src/main/scala/scala/build/errors/BuildException.scala b/modules/build/src/main/scala/scala/build/errors/BuildException.scala
@@ -2,6 +2,12 @@ package scala.build.errors
 
 import scala.build.Position
 
+case class Diagnostic(
+  val message: String,
+  val positions: Seq[Position] = Nil,
+  val severity: Severity
+)
+
 abstract class BuildException(
   val message: String,
   val positions: Seq[Position] = Nil,

diff --git a/modules/build/src/main/scala/scala/build/options/BuildOptions.scala b/modules/build/src/main/scala/scala/build/options/BuildOptions.scala
@@ -10,10 +10,10 @@ import java.nio.file.Path
 import java.security.MessageDigest
 
 import scala.build.EitherCps.{either, value}
-import scala.build.errors.{BuildException, InvalidBinaryScalaVersionError}
+import scala.build.errors.{BuildException, Diagnostic, InvalidBinaryScalaVersionError}
 import scala.build.internal.Constants._
 import scala.build.internal.{Constants, OsLibc, Util}
-import scala.build.options.validation.{BuildOptionsRule, ValidationException}
+import scala.build.options.validation.BuildOptionsRule
 import scala.build.{Artifacts, Logger, Os, Position, Positioned}
 import scala.util.Properties
 
@@ -360,7 +360,7 @@ final case class BuildOptions(
   def orElse(other: BuildOptions): BuildOptions =
     BuildOptions.monoid.orElse(this, other)
 
-  def validate: Seq[ValidationException] = BuildOptionsRule.validateAll(this)
+  def validate: Seq[Diagnostic] = BuildOptionsRule.validateAll(this)
 }
 
 object BuildOptions {

diff --git a/modules/build/src/main/scala/scala/build/options/ConfigMonoid.scala b/modules/build/src/main/scala/scala/build/options/ConfigMonoid.scala
@@ -1,7 +1,6 @@
 package scala.build.options
 
 import shapeless._
-import scala.build.Positioned
 
 trait ConfigMonoid[T] {
   def zero: T

diff --git a/modules/build/src/main/scala/scala/build/options/validation/BuildOptionsRule.scala b/modules/build/src/main/scala/scala/build/options/validation/BuildOptionsRule.scala
@@ -1,31 +1,25 @@
 package scala.build.options.validation
-
-import scala.build.Position
-import scala.build.errors.{BuildException, Severity}
+import scala.build.errors.{BuildException, Diagnostic, Severity}
 import scala.build.options.BuildOptions
 
 trait BuildOptionsRule {
-  def validate(options: BuildOptions): Seq[ValidationException]
+  def validate(options: BuildOptions): Seq[Diagnostic]
 }
 
 object BuildOptionsRule {
-  def validateAll(options: BuildOptions): Seq[ValidationException] =
+  def validateAll(options: BuildOptions): Seq[Diagnostic] =
     List(JvmOptionsForNonJvmBuild).flatMap(_.validate(options))
 }
 
 class ValidationException(
-  message: String,
-  positions: Seq[Position] = Nil,
-  severity0: Severity
-) extends BuildException(message, positions) {
-  def severity = this.severity0
-}
+  diagnostic: Diagnostic
+) extends BuildException(diagnostic.message, diagnostic.positions) {}
 
 object JvmOptionsForNonJvmBuild extends BuildOptionsRule {
-  def validate(options: BuildOptions): List[ValidationException] = {
+  def validate(options: BuildOptions): List[Diagnostic] = {
     val jvmOptions = options.javaOptions.javaOpts.find(p => p.value.nonEmpty)
     if (jvmOptions.nonEmpty && options.platform.value != scala.build.options.Platform.JVM)
-      List(new ValidationException(
+      List(Diagnostic(
         "Conflicting options. Jvm Options are valid only for jvm platform.",
         options.platform.positions ++ jvmOptions.get.positions,
         Severity.WARNING

diff --git a/modules/build/src/test/scala/scala/build/tests/TestLogger.scala b/modules/build/src/test/scala/scala/build/tests/TestLogger.scala
@@ -4,11 +4,19 @@ import coursier.cache.CacheLogger
 import coursier.cache.loggers.{FallbackRefreshDisplay, RefreshLogger}
 
 import scala.build.blooprifle.BloopRifleLogger
-import scala.build.errors.{BuildException, Severity}
+import scala.build.errors.BuildException
 import scala.build.Logger
 import scala.scalanative.{build => sn}
+import scala.build.errors.Diagnostic
 
 case class TestLogger(info: Boolean = true, debug: Boolean = false) extends Logger {
+
+  override def log(diagnostics: Seq[Diagnostic]): Unit = {
+    diagnostics.foreach { d =>
+      System.err.println(d.positions.map(_.render).mkString("/") ++ ": " ++ d.message)
+    }
+  }
+
   def message(message: => String): Unit =
     if (info)
       System.err.println(message)
@@ -24,7 +32,7 @@ case class TestLogger(info: Boolean = true, debug: Boolean = false) extends Logg
     if (debug)
       System.err.println(s)
 
-  def log(ex: BuildException, severity: Severity): Unit =
+  def log(ex: BuildException): Unit =
     System.err.println(ex.getMessage)
   def exit(ex: BuildException): Nothing =
     throw new Exception(ex)

diff --git a/modules/cli/src/main/scala/scala/cli/internal/CliLogger.scala b/modules/cli/src/main/scala/scala/cli/internal/CliLogger.scala
@@ -7,7 +7,7 @@ import coursier.cache.loggers.{FallbackRefreshDisplay, ProgressBarRefreshDisplay
 import java.io.PrintStream
 
 import scala.build.blooprifle.BloopRifleLogger
-import scala.build.errors.{BuildException, CompositeBuildException, Severity}
+import scala.build.errors.{BuildException, CompositeBuildException, Diagnostic, Severity}
 import scala.build.{ConsoleBloopBuildClient, Logger, Position}
 import scala.collection.mutable
 import scala.scalanative.{build => sn}
@@ -18,6 +18,18 @@ class CliLogger(
   out: PrintStream
 ) extends Logger { logger =>
 
+  override def log(diagnostics: Seq[Diagnostic]): Unit = {
+    val hashMap = new mutable.HashMap[os.Path, Seq[String]]
+    diagnostics.foreach { d =>
+      printDiagnostic(
+        d.positions,
+        d.severity,
+        d.message,
+        hashMap
+      )
+    }
+  }
+
   def message(message: => String) =
     if (verbosity >= 0)
       out.println(message)
@@ -33,72 +45,73 @@ class CliLogger(
     if (verbosity >= 2)
       out.println(message)
 
+  def printDiagnostic(
+    positions: Seq[Position],
+    severity: Severity,
+    message: String,
+    contentCache: mutable.Map[os.Path, Seq[String]]
+  ) = {
+    val filePositions = positions.collect {
+      case f: Position.File => f
+    }
+    val otherPositions = positions.filter {
+      case _: Position.File => false
+      case _                => true
+    }
+
+    for (f <- filePositions) {
+      val startPos = new b.Position(f.startPos._1, f.startPos._2)
+      val endPos   = new b.Position(f.endPos._1, f.endPos._2)
+      val range    = new b.Range(startPos, endPos)
+      val diag     = new b.Diagnostic(range, message)
+      diag.setSeverity(severity match {
+        case Severity.ERROR   => b.DiagnosticSeverity.ERROR
+        case Severity.WARNING => b.DiagnosticSeverity.WARNING
+      })
+
+      for (file <- f.path) {
+        val lines = contentCache.getOrElseUpdate(file, os.read(file).linesIterator.toVector)
+        if (f.startPos._1 < lines.length)
+          diag.setCode(lines(f.startPos._1))
+      }
+      ConsoleBloopBuildClient.printDiagnostic(
+        out,
+        f.path,
+        diag
+      )
+    }
+
+    if (otherPositions.nonEmpty) {
+      for (pos <- otherPositions)
+        out.println(pos.render())
+      out.print("  ")
+      out.println(message)
+    }
+  }
+
   private def printEx(
     ex: BuildException,
-    contentCache: mutable.Map[os.Path, Seq[String]],
-    severity: Severity
+    contentCache: mutable.Map[os.Path, Seq[String]]
   ): Unit =
     ex match {
       case c: CompositeBuildException =>
         // FIXME We might want to order things here… Or maybe just collect all b.Diagnostics
         // below, and order them before printing them.
         for (ex <- c.exceptions)
-          printEx(ex, contentCache, severity)
+          printEx(ex, contentCache)
       case _ =>
-        if (ex.positions.isEmpty)
-          out.println(ex.getMessage)
-        else {
-          val positions = ex.positions.distinct
-          val filePositions = positions.collect {
-            case f: Position.File => f
-          }
-          val otherPositions = positions.filter {
-            case _: Position.File => false
-            case _                => true
-          }
-
-          for (f <- filePositions) {
-            val startPos = new b.Position(f.startPos._1, f.startPos._2)
-            val endPos   = new b.Position(f.endPos._1, f.endPos._2)
-            val range    = new b.Range(startPos, endPos)
-            val diag     = new b.Diagnostic(range, ex.getMessage)
-            diag.setSeverity(severity match {
-              case Severity.ERROR   => b.DiagnosticSeverity.ERROR
-              case Severity.WARNING => b.DiagnosticSeverity.WARNING
-            })
-
-            for (file <- f.path) {
-              val lines = contentCache.getOrElseUpdate(file, os.read(file).linesIterator.toVector)
-              if (f.startPos._1 < lines.length)
-                diag.setCode(lines(f.startPos._1))
-            }
-            ConsoleBloopBuildClient.printDiagnostic(
-              out,
-              f.path,
-              diag
-            )
-          }
-
-          if (otherPositions.nonEmpty) {
-            for (pos <- otherPositions)
-              out.println(pos.render())
-            out.print("  ")
-            out.println(ex.getMessage)
-          }
-        }
+        printDiagnostic(ex.positions, Severity.ERROR, ex.getMessage(), contentCache)
     }
 
-  def log(ex: BuildException, severity: Severity): Unit =
+  def log(ex: BuildException): Unit =
     if (verbosity >= 0)
-      printEx(ex, new mutable.HashMap, severity)
-
-  def exit(ex: BuildException): Nothing = exit(ex, Severity.ERROR)
+      printEx(ex, new mutable.HashMap)
 
-  def exit(ex: BuildException, severity: Severity): Nothing =
+  def exit(ex: BuildException): Nothing =
     if (verbosity < 0)
       sys.exit(1)
     else if (verbosity == 0) {
-      printEx(ex, new mutable.HashMap, severity)
+      printEx(ex, new mutable.HashMap)
       sys.exit(1)
     }
     else

diff --git a/website/docs/reference/directives.md b/website/docs/reference/directives.md
@@ -31,7 +31,7 @@ Adds compiler plugins
 
 Manually add JAR(s) to the class path
 
-using jar _path_ 
+using jar _path_
 
 using jars _path1_ _path2_ …