Merge pull request #210 from alexarchambault/using-directives-errors

Report errors for using directives in console and via BSP

Author: alexarchambault

build.sc

@@ -367,8 +367,6 @@ trait CliIntegrationBase extends SbtModule with ScalaCliPublishModule with HasTe
            |  def scala3   = "${Scala.scala3}"
            |  def defaultScala = "${Scala.defaultUser}"
            |  def bloopVersion = "${Deps.bloopConfig.dep.version}"
-           |  def oldBloopVersion = "${TestDeps.oldBloopConfig.dep.version}"
-           |  def newBloopVersion = "${TestDeps.newBloopConfig.dep.version}"
            |  def pprintVersion = "${TestDeps.pprint.dep.version}"
            |  def munitVersion = "${TestDeps.munit.dep.version}"
            |  def dockerTestImage = "${Docker.testImage}"
@@ -504,6 +502,7 @@ class BloopRifle(val crossScalaVersion: String) extends CrossSbtModule with Scal
          |/** Build-time constants. Generated by mill. */
          |object Constants {
          |  def bloopVersion = "${Deps.bloopConfig.dep.version}"
+         |  def bloopScalaVersion = "${Scala.scala212}"
          |  def bspVersion = "${Deps.bsp4j.dep.version}"
          |}
          |""".stripMargin

modules/bloop-rifle/src/main/scala/scala/build/blooprifle/BloopRifle.scala

@@ -221,22 +221,20 @@ object BloopRifle {
   def shutdownBloopIfVersionIncompatible(
     config: BloopRifleConfig,
     logger: BloopRifleLogger,
-    workdir: Path,
+    workDir: Path,
     scheduler: ScheduledExecutorService
   ): Boolean = {
-    val currentBloopVersion = getCurrentBloopVersion(config, logger, workdir, scheduler)
-    val bloopExitNeeded = config.acceptBloopVersion.exists { f =>
-      currentBloopVersion.map(f).getOrElse(true)
+    val currentBloopVersion = getCurrentBloopVersion(config, logger, workDir, scheduler)
+    val isOk = config.acceptBloopVersion.forall { f =>
+      currentBloopVersion.forall(f(_))
     }
-    if (bloopExitNeeded) {
-      logger.debug(
-        s"Shutting down unsupported bloop v$currentBloopVersion."
-      )
-      val retCode = exit(config, workdir, logger)
-      logger.debug(s"Bloop exit return code: $retCode")
+    if (isOk)
+      logger.debug("No need to restart Bloop")
+    else {
+      logger.debug(s"Shutting down unsupported Bloop $currentBloopVersion.")
+      val retCode = exit(config, workDir, logger)
+      logger.debug(s"Bloop exit code: $retCode")
     }
-    else
-      logger.debug("No need to reset bloop")
-    bloopExitNeeded
+    !isOk
   }
 }

modules/bloop-rifle/src/main/scala/scala/build/blooprifle/BloopRifleConfig.scala

@@ -69,6 +69,8 @@ object BloopRifleConfig {
     "ch.epfl.scala:bloop-frontend_2.12"
   def hardCodedDefaultVersion: String =
     Constants.bloopVersion
+  def hardCodedDefaultScalaVersion: String =
+    Constants.bloopScalaVersion
 
   lazy val defaultModule: String = {
     val fromEnv   = Option(System.getenv("BLOOP_MODULE")).map(_.trim).filter(_.nonEmpty)
@@ -84,6 +86,13 @@ object BloopRifleConfig {
       .orElse(fromProps)
       .getOrElse(hardCodedDefaultVersion)
   }
+  lazy val defaultScalaVersion: String = {
+    val fromEnv   = Option(System.getenv("BLOOP_SCALA_VERSION")).filter(_.nonEmpty)
+    def fromProps = sys.props.get("bloop.scala-version").filter(_.nonEmpty)
+    fromEnv
+      .orElse(fromProps)
+      .getOrElse(hardCodedDefaultScalaVersion)
+  }
 
   def default(
     bloopClassPath: () => Either[Throwable, Seq[File]]

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

@@ -61,8 +61,8 @@ object Artifacts {
 
   def apply(
     params: ScalaParameters,
-    compilerPlugins: Seq[AnyDependency],
-    dependencies: Seq[AnyDependency],
+    compilerPlugins: Seq[Positioned[AnyDependency]],
+    dependencies: Seq[Positioned[AnyDependency]],
     extraClassPath: Seq[Path],
     extraCompileOnlyJars: Seq[Path],
     extraSourceJars: Seq[Path],
@@ -119,19 +119,20 @@ object Artifacts {
 
     val allExtraRepositories = maybeSnapshotRepo ++ extraRepositories
 
-    val updatedDependencies = dependencies ++
-      jvmRunnerDependencies ++
-      jvmTestRunnerDependencies ++
-      jsTestBridgeDependencies ++
-      jmhDependencies
+    val updatedDependencies =
+      dependencies ++
+        jvmRunnerDependencies.map(Positioned.none(_)) ++
+        jvmTestRunnerDependencies.map(Positioned.none(_)) ++
+        jsTestBridgeDependencies.map(Positioned.none(_)) ++
+        jmhDependencies.map(Positioned.none(_))
 
     val compilerArtifacts = value {
-      artifacts(compilerDependencies, allExtraRepositories, params, logger)
+      artifacts(Positioned.none(compilerDependencies), allExtraRepositories, params, logger)
     }
 
     val fetchRes = value {
       fetch(
-        updatedDependencies,
+        Positioned.sequence(updatedDependencies),
         allExtraRepositories,
         params,
         logger,
@@ -143,7 +144,7 @@ object Artifacts {
       if (addStubs)
         value {
           artifacts(
-            Seq(dep"$stubsOrganization:$stubsModuleName:$stubsVersion"),
+            Positioned.none(Seq(dep"$stubsOrganization:$stubsModuleName:$stubsVersion")),
             allExtraRepositories,
             params,
             logger
@@ -154,10 +155,11 @@ object Artifacts {
 
     val compilerPlugins0 = value {
       compilerPlugins
-        .map { dep =>
-          val dep0 = dep.copy(userParams = dep.userParams + ("intransitive" -> None))
-          artifacts(Seq(dep0), allExtraRepositories, params, logger)
-            .map(_.map { case (url, path) => (dep0, url, path) })
+        .map { posDep =>
+          val posDep0 =
+            posDep.map(dep => dep.copy(userParams = dep.userParams + ("intransitive" -> None)))
+          artifacts(posDep0.map(Seq(_)), allExtraRepositories, params, logger)
+            .map(_.map { case (url, path) => (posDep0.value, url, path) })
         }
         .sequence
         .left.map(CompositeBuildException(_))
@@ -168,7 +170,7 @@ object Artifacts {
       compilerDependencies,
       compilerArtifacts,
       compilerPlugins0,
-      updatedDependencies,
+      updatedDependencies.map(_.value),
       fetchRes.fullDetailedArtifacts.collect { case (d, p, a, Some(f)) => (d, p, a, f.toPath) },
       extraClassPath ++ extraStubsJars,
       extraCompileOnlyJars,
@@ -178,7 +180,7 @@ object Artifacts {
   }
 
   private[build] def artifacts(
-    dependencies: Seq[AnyDependency],
+    dependencies: Positioned[Seq[AnyDependency]],
     extraRepositories: Seq[String],
     params: ScalaParameters,
     logger: Logger,
@@ -199,14 +201,14 @@ object Artifacts {
   }
 
   private[build] def fetch(
-    dependencies: Seq[AnyDependency],
+    dependencies: Positioned[Seq[AnyDependency]],
     extraRepositories: Seq[String],
     params: ScalaParameters,
     logger: Logger,
     classifiersOpt: Option[Set[String]]
   ): Either[BuildException, Fetch.Result] = either {
     logger.debug {
-      s"Fetching $dependencies" +
+      s"Fetching ${dependencies.value}" +
         (if (extraRepositories.isEmpty) "" else s", adding $extraRepositories")
     }
 
@@ -222,7 +224,7 @@ object Artifacts {
     var fetcher = coursier.Fetch()
       .withCache(cache)
       .addRepositories(extraRepositories0: _*)
-      .addDependencies(dependencies.map(_.toCs(params)): _*)
+      .addDependencies(dependencies.value.map(_.toCs(params)): _*)
     for (classifiers <- classifiersOpt) {
       if (classifiers("_"))
         fetcher = fetcher.withMainArtifacts()
@@ -232,7 +234,7 @@ object Artifacts {
 
     value {
       fetcher.eitherResult()
-        .left.map(ex => new FetchingDependenciesError(ex))
+        .left.map(ex => new FetchingDependenciesError(ex, dependencies.positions))
     }
   }
 

modules/build/src/main/scala/scala/build/Bloop.scala

@@ -11,7 +11,6 @@ import scala.build.blooprifle.BloopRifleConfig
 import scala.build.errors.{BuildException, ModuleFormatError}
 import scala.concurrent.duration.FiniteDuration
 import scala.jdk.CollectionConverters._
-import scala.util.Properties
 
 object Bloop {
 
@@ -49,18 +48,24 @@ object Bloop {
     logger: Logger
   ): Either[BuildException, Seq[File]] =
     either {
-      value(Artifacts.artifacts(Seq(dep), Nil, params, logger))
+      value(Artifacts.artifacts(Positioned.none(Seq(dep)), Nil, params, logger))
         .map(_._2.toFile)
     }
 
-  def bloopClassPath(logger: Logger): Either[BuildException, Seq[File]] = either {
+  def bloopClassPath(logger: Logger): Either[BuildException, Seq[File]] =
+    bloopClassPath(logger, BloopRifleConfig.defaultVersion)
+
+  def bloopClassPath(
+    logger: Logger,
+    bloopVersion: String
+  ): Either[BuildException, Seq[File]] = either {
     val moduleStr = BloopRifleConfig.defaultModule
     val mod = value {
       ModuleParser.parse(moduleStr)
         .left.map(err => new ModuleFormatError(moduleStr, err, Some("Bloop")))
     }
-    val dep    = DependencyLike(mod, BloopRifleConfig.defaultVersion)
-    val sv     = Properties.versionNumberString
+    val dep    = DependencyLike(mod, bloopVersion)
+    val sv     = BloopRifleConfig.defaultScalaVersion
     val sbv    = ScalaVersion.binary(sv)
     val params = ScalaParameters(sv, sbv)
     value(bloopClassPath(dep, params, logger))

modules/build/src/main/scala/scala/build/ConsoleBloopBuildClient.scala

@@ -67,42 +67,6 @@ class ConsoleBloopBuildClient(
         (originalPath, updatedDiag)
     }
 
-  private def printDiagnostic(path: Either[String, os.Path], diag: bsp4j.Diagnostic): Unit = {
-    val isWarningOrError = diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR ||
-      diag.getSeverity == bsp4j.DiagnosticSeverity.WARNING
-    if (isWarningOrError) {
-      val red    = Console.RED
-      val yellow = Console.YELLOW
-      val reset  = Console.RESET
-      val prefix =
-        if (diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR) s"[${red}error$reset] "
-        else s"[${yellow}warn$reset] "
-
-      val line  = (diag.getRange.getStart.getLine + 1).toString + ":"
-      val col   = (diag.getRange.getStart.getCharacter + 1).toString + ":"
-      val msgIt = diag.getMessage.linesIterator
-
-      val path0 = path match {
-        case Left(source) => source
-        case Right(p) if p.startsWith(Os.pwd) =>
-          "." + File.separator + p.relativeTo(Os.pwd).toString
-        case Right(p) => p.toString
-      }
-      out.println(s"$prefix$path0:$line$col" + (if (msgIt.hasNext) " " + msgIt.next() else ""))
-      for (line <- msgIt)
-        out.println(prefix + line)
-      for (code <- Option(diag.getCode))
-        code.linesIterator.map(prefix + _).foreach(out.println(_))
-      val canPrintUnderline = diag.getRange.getStart.getLine == diag.getRange.getEnd.getLine &&
-        diag.getRange.getStart.getCharacter != null &&
-        diag.getRange.getEnd.getCharacter != null
-      if (canPrintUnderline)
-        out.println(
-          prefix + " " * diag.getRange.getStart.getCharacter + "^" * (diag.getRange.getEnd.getCharacter - diag.getRange.getStart.getCharacter + 1)
-        )
-    }
-  }
-
   override def onBuildPublishDiagnostics(params: bsp4j.PublishDiagnosticsParams): Unit = {
     logger.debug("Received onBuildPublishDiagnostics from bloop: " + params)
     for (diag <- params.getDiagnostics.asScala) {
@@ -121,7 +85,7 @@ class ConsoleBloopBuildClient(
         .getOrElse((Right(path), diag))
       if (keepDiagnostics)
         diagnostics0 += updatedPath -> updatedDiag
-      printDiagnostic(updatedPath, updatedDiag)
+      ConsoleBloopBuildClient.printDiagnostic(out, updatedPath, updatedDiag)
     }
   }
 
@@ -181,3 +145,63 @@ class ConsoleBloopBuildClient(
     printedStart = false
   }
 }
+
+object ConsoleBloopBuildClient {
+
+  def printDiagnostic(
+    out: PrintStream,
+    path: Either[String, os.Path],
+    diag: bsp4j.Diagnostic
+  ): Unit = {
+    val isWarningOrError = diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR ||
+      diag.getSeverity == bsp4j.DiagnosticSeverity.WARNING
+    if (isWarningOrError) {
+      val red    = Console.RED
+      val yellow = Console.YELLOW
+      val reset  = Console.RESET
+      val prefix =
+        if (diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR) s"[${red}error$reset] "
+        else s"[${yellow}warn$reset] "
+
+      val line  = (diag.getRange.getStart.getLine + 1).toString + ":"
+      val col   = (diag.getRange.getStart.getCharacter + 1).toString + ":"
+      val msgIt = diag.getMessage.linesIterator
+
+      val path0 = path match {
+        case Left(source) => source
+        case Right(p) if p.startsWith(Os.pwd) =>
+          "." + File.separator + p.relativeTo(Os.pwd).toString
+        case Right(p) => p.toString
+      }
+      out.println(s"$prefix$path0:$line$col" + (if (msgIt.hasNext) " " + msgIt.next() else ""))
+      for (line <- msgIt)
+        out.println(prefix + line)
+      val codeOpt = Option(diag.getCode).orElse {
+        val lineOpt =
+          if (diag.getRange.getStart.getLine == diag.getRange.getEnd.getLine)
+            Option(diag.getRange.getStart.getLine)
+          else None
+        for {
+          line <- lineOpt
+          p    <- path.toOption
+          lines = os.read.lines(p)
+          line <- if (line < lines.length) Some(lines(line)) else None
+        } yield line
+      }
+      for (code <- codeOpt)
+        code.linesIterator.map(prefix + _).foreach(out.println(_))
+      val canPrintUnderline = diag.getRange.getStart.getLine == diag.getRange.getEnd.getLine &&
+        diag.getRange.getStart.getCharacter != null &&
+        diag.getRange.getEnd.getCharacter != null &&
+        codeOpt.nonEmpty
+      if (canPrintUnderline) {
+        val len =
+          math.max(1, diag.getRange.getEnd.getCharacter - diag.getRange.getStart.getCharacter)
+        out.println(
+          prefix + " " * diag.getRange.getStart.getCharacter + "^" * len
+        )
+      }
+    }
+  }
+
+}