Better message for invalid binary Scala versions

VirtusLab · Oct 6, 2021 · 14028df · 14028df
1 parent 18b1bf8
commit 14028df
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 54 deletions.
diff --git a/modules/build/src/main/scala/scala/build/Build.scala b/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}
@@ -44,6 +45,7 @@ object Build {
   final case class Successful(
     inputs: Inputs,
     options: BuildOptions,
+    scalaParams: ScalaParameters,
     sources: Sources,
     artifacts: Artifacts,
     project: Project,
@@ -126,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,
@@ -145,6 +147,7 @@ object Build {
         buildClient,
         bloopServer
       )
+      value(res)
     }
 
     val mainBuild = value(doBuild(options0))
@@ -354,9 +357,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)
@@ -436,7 +439,7 @@ object Build {
     buildClient.clear()
     buildClient.setGeneratedSources(generatedSources)
 
-    (classesDir0, artifacts, project, updatedBloopConfig)
+    (classesDir0, params, artifacts, project, updatedBloopConfig)
   }
 
   def buildOnce(
@@ -450,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,
@@ -493,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)

diff --git a/modules/build/src/main/scala/scala/build/CrossSources.scala b/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)

diff --git a/modules/build/src/main/scala/scala/build/bsp/BspImpl.scala b/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

diff --git a/modules/build/src/main/scala/scala/build/errors/InvalidBinaryScalaVersionError.scala b/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'")
diff --git a/modules/build/src/main/scala/scala/build/options/BuildOptions.scala b/modules/build/src/main/scala/scala/build/options/BuildOptions.scala
@@ -9,7 +9,8 @@ import java.nio.charset.StandardCharsets
 import java.nio.file.Path
 import java.security.MessageDigest
 
-import scala.build.errors.BuildException
+import scala.build.EitherCps.{either, value}
+import scala.build.errors.{BuildException, InvalidBinaryScalaVersionError}
 import scala.build.internal.Constants._
 import scala.build.internal.{Constants, OsLibc, Util}
 import scala.build.{Artifacts, Logger, Os}
@@ -31,38 +32,41 @@ final case class BuildOptions(
   replOptions: ReplOptions = ReplOptions()
 ) {
 
-  lazy val projectParams: Seq[String] = {
+  lazy val projectParams: Either[BuildException, Seq[String]] = either {
     val platform =
       if (scalaJsOptions.enable) "Scala.JS"
       else if (scalaNativeOptions.enable) "Scala Native"
       else "JVM"
-    Seq(s"Scala ${scalaParams.scalaVersion}", platform)
+    Seq(s"Scala ${value(scalaParams).scalaVersion}", platform)
   }
 
   def addRunnerDependency: Option[Boolean] =
     internalDependencies.addRunnerDependencyOpt
       .orElse(if (scalaJsOptions.enable || scalaNativeOptions.enable) Some(false) else None)
 
-  private def scalaLibraryDependencies: Seq[AnyDependency] =
+  private def scalaLibraryDependencies: Either[BuildException, Seq[AnyDependency]] = either {
     if (scalaOptions.addScalaLibrary.getOrElse(true)) {
+      val scalaParams0 = value(scalaParams)
       val lib =
-        if (scalaParams.scalaVersion.startsWith("3."))
-          dep"org.scala-lang::scala3-library::${scalaParams.scalaVersion}"
+        if (scalaParams0.scalaVersion.startsWith("3."))
+          dep"org.scala-lang::scala3-library::${scalaParams0.scalaVersion}"
         else
-          dep"org.scala-lang:scala-library:${scalaParams.scalaVersion}"
+          dep"org.scala-lang:scala-library:${scalaParams0.scalaVersion}"
       Seq(lib)
     }
     else Nil
+  }
 
-  private def dependencies: Seq[AnyDependency] =
-    scalaJsOptions.jsDependencies(scalaParams.scalaVersion) ++
+  private def dependencies: Either[BuildException, Seq[AnyDependency]] = either {
+    scalaJsOptions.jsDependencies(value(scalaParams).scalaVersion) ++
       scalaNativeOptions.nativeDependencies ++
-      scalaLibraryDependencies ++
+      value(scalaLibraryDependencies) ++
       classPathOptions.extraDependencies
+  }
 
-  private def semanticDbPlugins: Seq[AnyDependency] = {
+  private def semanticDbPlugins: Either[BuildException, Seq[AnyDependency]] = either {
     val generateSemDbs = scalaOptions.generateSemanticDbs.getOrElse(false) &&
-      scalaParams.scalaVersion.startsWith("2.")
+      value(scalaParams).scalaVersion.startsWith("2.")
     if (generateSemDbs)
       Seq(
         dep"$semanticDbPluginOrganization:::$semanticDbPluginModuleName:$semanticDbPluginVersion"
@@ -71,11 +75,12 @@ final case class BuildOptions(
       Nil
   }
 
-  def compilerPlugins: Seq[AnyDependency] =
-    scalaJsOptions.compilerPlugins(scalaParams.scalaVersion) ++
+  def compilerPlugins: Either[BuildException, Seq[AnyDependency]] = either {
+    scalaJsOptions.compilerPlugins(value(scalaParams).scalaVersion) ++
       scalaNativeOptions.compilerPlugins ++
-      semanticDbPlugins ++
+      value(semanticDbPlugins) ++
       scalaOptions.compilerPlugins
+  }
 
   def allExtraJars: Seq[Path] =
     classPathOptions.extraClassPath.map(_.toNIO)
@@ -144,7 +149,7 @@ final case class BuildOptions(
   private def computeScalaVersions(
     scalaVersion: Option[String],
     scalaBinaryVersion: Option[String]
-  ): (String, String) = {
+  ): Either[BuildException, (String, String)] = either {
     import coursier.core.Version
     lazy val allVersions = {
       import coursier._
@@ -170,37 +175,38 @@ final case class BuildOptions(
       }
       modules.flatMap(moduleVersions).distinct
     }
-    val sv = scalaVersion match {
-      case None => Constants.defaultScalaVersion
+    val maybeSv = scalaVersion match {
+      case None => Right(Constants.defaultScalaVersion)
       case Some(sv0) =>
-        if (Util.isFullScalaVersion(sv0)) sv0
+        if (Util.isFullScalaVersion(sv0)) Right(sv0)
         else {
           val prefix           = if (sv0.endsWith(".")) sv0 else sv0 + "."
           val matchingVersions = allVersions.filter(_.startsWith(prefix))
           if (matchingVersions.isEmpty)
-            sys.error(s"Cannot find matching Scala version for '$sv0'")
+            Left(new InvalidBinaryScalaVersionError(sv0))
           else
-            matchingVersions.map(Version(_)).max.repr
+            Right(matchingVersions.map(Version(_)).max.repr)
         }
     }
+    val sv  = value(maybeSv)
     val sbv = scalaBinaryVersion.getOrElse(ScalaVersion.binary(sv))
     (sv, sbv)
   }
 
-  lazy val scalaParams: ScalaParameters = {
+  lazy val scalaParams: Either[BuildException, ScalaParameters] = either {
     val (scalaVersion, scalaBinaryVersion) =
-      computeScalaVersions(scalaOptions.scalaVersion, scalaOptions.scalaBinaryVersion)
+      value(computeScalaVersions(scalaOptions.scalaVersion, scalaOptions.scalaBinaryVersion))
     val maybePlatformSuffix =
       scalaJsOptions.platformSuffix
         .orElse(scalaNativeOptions.platformSuffix)
     ScalaParameters(scalaVersion, scalaBinaryVersion, maybePlatformSuffix)
   }
 
-  def artifacts(logger: Logger): Either[BuildException, Artifacts] =
-    Artifacts(
-      params = scalaParams,
-      compilerPlugins = compilerPlugins,
-      dependencies = dependencies,
+  def artifacts(logger: Logger): Either[BuildException, Artifacts] = either {
+    val maybeArtifacts = Artifacts(
+      params = value(scalaParams),
+      compilerPlugins = value(compilerPlugins),
+      dependencies = value(dependencies),
       extraClassPath = allExtraJars,
       extraCompileOnlyJars = allExtraCompileOnlyJars,
       extraSourceJars = allExtraSourceJars,
@@ -213,6 +219,8 @@ final case class BuildOptions(
       extraRepositories = finalRepositories,
       logger = logger
     )
+    value(maybeArtifacts)
+  }
 
   // FIXME We'll probably need more refined rules if we start to support extra Scala.JS or Scala Native specific types
   def packageTypeOpt: Option[PackageType] =

diff --git a/modules/build/src/test/scala/scala/build/tests/SourcesTests.scala b/modules/build/src/test/scala/scala/build/tests/SourcesTests.scala
@@ -38,7 +38,7 @@ class SourcesTests extends munit.FunSuite {
     testInputs.withInputs { (_, inputs) =>
       val crossSources =
         CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow
-      val sources = crossSources.sources(BuildOptions())
+      val sources = crossSources.sources(BuildOptions()).orThrow
 
       expect(sources.buildOptions.classPathOptions.extraDependencies == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -68,7 +68,7 @@ class SourcesTests extends munit.FunSuite {
     testInputs.withInputs { (_, inputs) =>
       val crossSources =
         CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow
-      val sources = crossSources.sources(BuildOptions())
+      val sources = crossSources.sources(BuildOptions()).orThrow
 
       expect(sources.buildOptions.classPathOptions.extraDependencies == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -96,7 +96,7 @@ class SourcesTests extends munit.FunSuite {
     testInputs.withInputs { (_, inputs) =>
       val crossSources =
         CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow
-      val sources = crossSources.sources(BuildOptions())
+      val sources = crossSources.sources(BuildOptions()).orThrow
 
       expect(sources.buildOptions.classPathOptions.extraDependencies == expectedDeps)
       expect(sources.paths.isEmpty)
@@ -154,7 +154,7 @@ class SourcesTests extends munit.FunSuite {
     testInputs.withInputs { (_, inputs) =>
       val crossSources =
         CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow
-      val sources = crossSources.sources(BuildOptions())
+      val sources = crossSources.sources(BuildOptions()).orThrow
 
       val parsedCodes: Seq[String] = sources.inMemory.map(_._3)
 
@@ -183,7 +183,7 @@ class SourcesTests extends munit.FunSuite {
     testInputs.withInputs { (_, inputs) =>
       val crossSources =
         CrossSources.forInputs(inputs, Sources.defaultPreprocessors(CustomCodeWrapper)).orThrow
-      val sources = crossSources.sources(BuildOptions())
+      val sources = crossSources.sources(BuildOptions()).orThrow
 
       expect(sources.buildOptions.classPathOptions.extraDependencies == expectedDeps)
       expect(sources.paths.isEmpty)

diff --git a/modules/cli/src/main/scala/scala/cli/commands/Export.scala b/modules/cli/src/main/scala/scala/cli/commands/Export.scala
@@ -30,7 +30,7 @@ object Export extends ScalaCommand[ExportOptions] {
         )
       )
     }
-    val sources = crossSources.sources(buildOptions)
+    val sources = value(crossSources.sources(buildOptions))
 
     if (verbosity >= 3)
       pprint.better.log(sources)

diff --git a/modules/cli/src/main/scala/scala/cli/commands/Metabrowse.scala b/modules/cli/src/main/scala/scala/cli/commands/Metabrowse.scala
@@ -57,7 +57,7 @@ object Metabrowse extends ScalaCommand[MetabrowseOptions] {
       .map(os.Path(_, os.pwd))
       .getOrElse {
         val (url, changing) =
-          options.metabrowseBinaryUrl(successfulBuild.options.scalaParams.scalaVersion)
+          options.metabrowseBinaryUrl(successfulBuild.scalaParams.scalaVersion)
         FetchExternalBinary.fetch(
           url,
           changing,
@@ -115,7 +115,7 @@ object Metabrowse extends ScalaCommand[MetabrowseOptions] {
     }
 
     def defaultDialect = {
-      val sv = successfulBuild.options.scalaParams.scalaVersion
+      val sv = successfulBuild.scalaParams.scalaVersion
       if (sv.startsWith("2.12.")) "Scala212"
       else if (sv.startsWith("2.13.")) "Scala213"
       else "Scala3"