Merge pull request #191 from alexarchambault/better-errors

Better errors

modules/build/src/main/scala/scala/build/Artifacts.scala

@@ -10,11 +10,15 @@ import java.nio.file.Path
 
 import scala.build.EitherCps.{either, value}
 import scala.build.Ops._
-import scala.build.errors.{BuildException, CompositeBuildException}
+import scala.build.errors.{
+  BuildException,
+  CompositeBuildException,
+  FetchingDependenciesError,
+  RepositoryFormatError
+}
 import scala.build.internal.Constants
 import scala.build.internal.Constants._
 import scala.build.internal.Util.ScalaDependencyOps
-import scala.util.control.NonFatal
 
 final case class Artifacts(
   compilerDependencies: Seq[AnyDependency],
@@ -200,15 +204,16 @@ object Artifacts {
     params: ScalaParameters,
     logger: Logger,
     classifiersOpt: Option[Set[String]]
-  ): Either[BuildException, Fetch.Result] = {
+  ): Either[BuildException, Fetch.Result] = either {
     logger.debug {
       s"Fetching $dependencies" +
         (if (extraRepositories.isEmpty) "" else s", adding $extraRepositories")
     }
 
-    val extraRepositories0 = RepositoryParser.repositories(extraRepositories).either match {
-      case Left(errors) => sys.error(s"Error parsing repositories: ${errors.mkString(", ")}")
-      case Right(repos) => repos
+    val extraRepositories0 = value {
+      RepositoryParser.repositories(extraRepositories)
+        .either
+        .left.map(errors => new RepositoryFormatError(errors))
     }
 
     val cache = FileCache().withLogger(logger.coursierLogger)
@@ -225,10 +230,9 @@ object Artifacts {
         .addClassifiers(classifiers.toSeq.filter(_ != "_").map(coursier.Classifier(_)): _*)
     }
 
-    try Right(fetcher.runResult())
-    catch {
-      case NonFatal(e) =>
-        throw new Exception(e)
+    value {
+      fetcher.eitherResult()
+        .left.map(ex => new FetchingDependenciesError(ex))
     }
   }
 

modules/build/src/main/scala/scala/build/Build.scala

@@ -3,6 +3,7 @@ package scala.build
 import ch.epfl.scala.bsp4j
 import com.swoval.files.FileTreeViews.Observer
 import com.swoval.files.{PathWatcher, PathWatchers}
+import dependency.ScalaParameters
 
 import java.io.File
 import java.nio.file.{FileSystemException, Path}
@@ -11,7 +12,14 @@ import java.util.concurrent.{ScheduledExecutorService, ScheduledFuture}
 import scala.build.EitherCps.{either, value}
 import scala.build.Ops._
 import scala.build.blooprifle.BloopRifleConfig
-import scala.build.errors.{BuildException, CompositeBuildException}
+import scala.build.errors.{
+  BuildException,
+  CompositeBuildException,
+  JmhBuildFailedError,
+  MainClassError,
+  NoMainClassFoundError,
+  SeveralMainClassesFoundError
+}
 import scala.build.internal.{Constants, CustomCodeWrapper, MainClass, Util}
 import scala.build.options.BuildOptions
 import scala.build.postprocessing._
@@ -37,6 +45,7 @@ object Build {
   final case class Successful(
     inputs: Inputs,
     options: BuildOptions,
+    scalaParams: ScalaParameters,
     sources: Sources,
     artifacts: Artifacts,
     project: Project,
@@ -50,28 +59,28 @@ object Build {
       Seq(output.toNIO) ++ sources.resourceDirs.map(_.toNIO) ++ artifacts.classPath
     def foundMainClasses(): Seq[String] =
       MainClass.find(output)
-    def retainedMainClassOpt(warnIfSeveral: Boolean = false): Option[String] = {
+    def retainedMainClass: Either[MainClassError, String] = {
       lazy val foundMainClasses0 = foundMainClasses()
       val defaultMainClassOpt = sources.mainClass
         .filter(name => foundMainClasses0.contains(name))
-      def foundMainClassOpt =
+      def foundMainClass =
         if (foundMainClasses0.isEmpty) {
           val msg = "No main class found"
           System.err.println(msg)
-          sys.error(msg)
-        }
-        else if (foundMainClasses0.length == 1) foundMainClasses0.headOption
-        else {
-          if (warnIfSeveral) {
-            System.err.println("Found several main classes:")
-            for (name <- foundMainClasses0)
-              System.err.println(s"  $name")
-            System.err.println("Please specify which one to use with --main-class")
-          }
-          None
+          Left(new NoMainClassFoundError)
         }
+        else if (foundMainClasses0.length == 1) Right(foundMainClasses0.head)
+        else
+          Left(
+            new SeveralMainClassesFoundError(
+              ::(foundMainClasses0.head, foundMainClasses0.tail.toList)
+            )
+          )
 
-      defaultMainClassOpt.orElse(foundMainClassOpt)
+      defaultMainClassOpt match {
+        case Some(cls) => Right(cls)
+        case None      => foundMainClass
+      }
     }
   }
 
@@ -119,16 +128,16 @@ object Build {
       )
     }
 
-    val sources = crossSources.sources(options)
+    val sources = value(crossSources.sources(options))
 
     val options0 = options.orElse(sources.buildOptions)
     val inputs0  = updateInputs(inputs, options)
 
     val generatedSources = sources.generateSources(inputs0.generatedSrcRoot)
 
-    def doBuild(buildOptions: BuildOptions) = {
-      buildClient.setProjectParams(buildOptions.projectParams)
-      build(
+    def doBuild(buildOptions: BuildOptions) = either {
+      buildClient.setProjectParams(value(buildOptions.projectParams))
+      val res = build(
         inputs0,
         sources,
         inputs0.generatedSrcRoot,
@@ -138,6 +147,7 @@ object Build {
         buildClient,
         bloopServer
       )
+      value(res)
     }
 
     val mainBuild = value(doBuild(options0))
@@ -183,16 +193,18 @@ object Build {
       case successful: Successful =>
         if (options.jmhOptions.runJmh.getOrElse(false))
           value {
-            jmhBuild(
+            val res = jmhBuild(
               inputs,
               successful,
               logger,
               successful.options.javaCommand(),
               buildClient,
               bloopServer
             )
-          }.getOrElse {
-            sys.error("JMH build failed") // suppress stack trace?
+            res.flatMap {
+              case Some(b) => Right(b)
+              case None    => Left(new JmhBuildFailedError)
+            }
           }
         else
           build0
@@ -346,9 +358,9 @@ object Build {
     options: BuildOptions,
     logger: Logger,
     buildClient: BloopBuildClient
-  ): Either[BuildException, (os.Path, Artifacts, Project, Boolean)] = either {
+  ): Either[BuildException, (os.Path, ScalaParameters, Artifacts, Project, Boolean)] = either {
 
-    val params     = options.scalaParams
+    val params     = value(options.scalaParams)
     val allSources = sources.paths.map(_._1) ++ generatedSources.map(_.generated)
 
     val classesDir0 = classesDir(inputs.workspace, inputs.projectName)
@@ -427,7 +439,7 @@ object Build {
     buildClient.clear()
     buildClient.setGeneratedSources(generatedSources)
 
-    (classesDir0, artifacts, project, updatedBloopConfig)
+    (classesDir0, params, artifacts, project, updatedBloopConfig)
   }
 
   def buildOnce(
@@ -441,7 +453,7 @@ object Build {
     bloopServer: bloop.BloopServer
   ): Either[BuildException, Build] = either {
 
-    val (classesDir0, artifacts, project, updatedBloopConfig) = value {
+    val (classesDir0, scalaParams, artifacts, project, updatedBloopConfig) = value {
       prepareBuild(
         inputs,
         sources,
@@ -484,7 +496,16 @@ object Build {
         updateTasty = project.scalaCompiler.scalaVersion.startsWith("3.")
       )
 
-      Successful(inputs, options, sources, artifacts, project, classesDir0, buildClient.diagnostics)
+      Successful(
+        inputs,
+        options,
+        scalaParams,
+        sources,
+        artifacts,
+        project,
+        classesDir0,
+        buildClient.diagnostics
+      )
     }
     else
       Failed(inputs, options, sources, artifacts, project, buildClient.diagnostics)

modules/build/src/main/scala/scala/build/CrossSources.scala

@@ -14,14 +14,14 @@ final case class CrossSources(
   buildOptions: Seq[HasBuildRequirements[BuildOptions]]
 ) {
 
-  def sources(baseOptions: BuildOptions): Sources = {
+  def sources(baseOptions: BuildOptions): Either[BuildException, Sources] = either {
 
     val sharedOptions = buildOptions
       .filter(_.requirements.isEmpty)
       .map(_.value)
       .foldLeft(baseOptions)(_ orElse _)
 
-    val retainedScalaVersion = sharedOptions.scalaParams.scalaVersion
+    val retainedScalaVersion = value(sharedOptions.scalaParams).scalaVersion
 
     val buildOptionsWithScalaVersion = buildOptions
       .flatMap(_.withScalaVersion(retainedScalaVersion).toSeq)

modules/build/src/main/scala/scala/build/bsp/BspImpl.scala

@@ -55,7 +55,7 @@ final class BspImpl(
     if (verbosity >= 3)
       pprint.better.log(crossSources)
 
-    val sources = crossSources.sources(buildOptions)
+    val sources = value(crossSources.sources(buildOptions))
 
     if (verbosity >= 3)
       pprint.better.log(sources)
@@ -67,7 +67,7 @@ final class BspImpl(
     actualLocalServer.setExtraDependencySources(buildOptions.classPathOptions.extraSourceJars)
     actualLocalServer.setGeneratedSources(generatedSources)
 
-    val (classesDir0, artifacts, project, buildChanged) = value {
+    val (classesDir0, scalaParams, artifacts, project, buildChanged) = value {
       Build.prepareBuild(
         inputs,
         sources,
@@ -78,15 +78,24 @@ final class BspImpl(
       )
     }
 
-    (sources, options0, classesDir0, artifacts, project, generatedSources, buildChanged)
+    (
+      sources,
+      options0,
+      classesDir0,
+      scalaParams,
+      artifacts,
+      project,
+      generatedSources,
+      buildChanged
+    )
   }
 
   private def buildE(
     actualLocalServer: BspServer,
     bloopServer: BloopServer,
     notifyChanges: Boolean
   ): Either[BuildException, Unit] = either {
-    val (sources, buildOptions, _, _, _, generatedSources, buildChanged) =
+    val (sources, buildOptions, _, _, _, _, generatedSources, buildChanged) =
       value(prepareBuild(actualLocalServer))
     if (notifyChanges && buildChanged)
       notifyBuildChange(actualLocalServer)
@@ -120,7 +129,7 @@ final class BspImpl(
   ): CompletableFuture[b.CompileResult] = {
     val preBuild = CompletableFuture.supplyAsync(
       () => {
-        val (_, _, classesDir0, artifacts, project, generatedSources, buildChanged) =
+        val (_, _, classesDir0, _, artifacts, project, generatedSources, buildChanged) =
           prepareBuild(actualLocalServer).orThrow
         if (buildChanged)
           notifyBuildChange(actualLocalServer)
@@ -241,7 +250,7 @@ final class BspImpl(
     val remoteClient = launcher.getRemoteProxy
     actualLocalClient.forwardToOpt = Some(remoteClient)
 
-    prepareBuild(actualLocalServer)
+    prepareBuild(actualLocalServer) // FIXME We're discarding the error here
 
     logger.log {
       val hasConsole = System.console() != null

modules/build/src/main/scala/scala/build/errors/FetchingDependenciesError.scala

@@ -0,0 +1,6 @@
+package scala.build.errors
+
+import coursier.error.CoursierError
+
+final class FetchingDependenciesError(underlying: CoursierError)
+    extends BuildException(s"Error fetching dependencies: ${underlying.getMessage}", underlying)

modules/build/src/main/scala/scala/build/errors/FileNotFoundException.scala

@@ -0,0 +1,4 @@
+package scala.build.errors
+
+final class FileNotFoundException(val path: os.Path)
+    extends BuildException(s"File not found: $path")

modules/build/src/main/scala/scala/build/errors/InvalidBinaryScalaVersionError.scala

@@ -0,0 +1,5 @@
+package scala.build.errors
+
+final class InvalidBinaryScalaVersionError(
+  val binaryVersion: String
+) extends BuildException(s"Cannot find matching Scala version for '$binaryVersion'")

modules/build/src/main/scala/scala/build/errors/JmhBuildFailedError.scala

@@ -0,0 +1,3 @@
+package scala.build.errors
+
+final class JmhBuildFailedError extends BuildException("JMH build failed")

modules/build/src/main/scala/scala/build/errors/MainClassError.scala

@@ -0,0 +1,3 @@
+package scala.build.errors
+
+abstract class MainClassError(message: String) extends BuildException(message)

modules/build/src/main/scala/scala/build/errors/NoFrameworkFoundByBridgeError.scala

@@ -0,0 +1,4 @@
+package scala.build.errors
+
+final class NoFrameworkFoundByBridgeError
+    extends TestError("No framework found by Scala.JS test bridge")

modules/build/src/main/scala/scala/build/errors/NoMainClassFoundError.scala

@@ -0,0 +1,3 @@
+package scala.build.errors
+
+final class NoMainClassFoundError extends MainClassError("No main class found")

modules/build/src/main/scala/scala/build/errors/NoTestFrameworkFoundError.scala

@@ -0,0 +1,3 @@
+package scala.build.errors
+
+final class NoTestFrameworkFoundError extends TestError("No test framework found")

modules/build/src/main/scala/scala/build/errors/SeveralMainClassesFoundError.scala

@@ -0,0 +1,4 @@
+package scala.build.errors
+
+final class SeveralMainClassesFoundError(mainClasses: ::[String])
+    extends MainClassError(s"Found several main classes: ${mainClasses.mkString(", ")}")

modules/build/src/main/scala/scala/build/errors/TestError.scala

@@ -0,0 +1,3 @@
+package scala.build.errors
+
+abstract class TestError(message: String) extends BuildException(message)

modules/build/src/main/scala/scala/build/errors/TooManyFrameworksFoundByBridgeError.scala

@@ -0,0 +1,4 @@
+package scala.build.errors
+
+final class TooManyFrameworksFoundByBridgeError
+    extends TestError("Too many frameworks found by Scala.JS test bridge")