Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actionable diagnostics #1229

Merged
merged 7 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -151,19 +151,25 @@ object ConsoleBloopBuildClient {
private val red = Console.RED
private val yellow = Console.YELLOW

def diagnosticPrefix(isError: Boolean): String =
if (isError) s"[${red}error$reset] "
else s"[${yellow}warn$reset] "
def diagnosticPrefix(severity: bsp4j.DiagnosticSeverity): String =
severity match {
case bsp4j.DiagnosticSeverity.ERROR => s"[${red}error$reset] "
case bsp4j.DiagnosticSeverity.WARNING => s"[${yellow}warn$reset] "
case bsp4j.DiagnosticSeverity.HINT => s"[${yellow}hint$reset] "
}

def diagnosticPrefix(severity: Severity): String = diagnosticPrefix(severity.toBsp4j)

def printFileDiagnostic(
logger: Logger,
path: Either[String, os.Path],
diag: bsp4j.Diagnostic
): Unit = {
val isWarningOrError = diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR ||
diag.getSeverity == bsp4j.DiagnosticSeverity.WARNING
if (isWarningOrError) {
val prefix = diagnosticPrefix(diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR)
val isWarningOrErrorOrHint = diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR ||
diag.getSeverity == bsp4j.DiagnosticSeverity.WARNING ||
diag.getSeverity == bsp4j.DiagnosticSeverity.HINT
if (isWarningOrErrorOrHint) {
val prefix = diagnosticPrefix(diag.getSeverity)

val line = (diag.getRange.getStart.getLine + 1).toString + ":"
val col = (diag.getRange.getStart.getCharacter + 1).toString + ":"
Expand Down Expand Up @@ -215,7 +221,7 @@ object ConsoleBloopBuildClient {
val isWarningOrError = true
if (isWarningOrError) {
val msgIt = message.linesIterator
val prefix = diagnosticPrefix(severity == Severity.Error)
val prefix = diagnosticPrefix(severity)
logger.message(prefix + (if (msgIt.hasNext) " " + msgIt.next() else ""))
msgIt.foreach(line => logger.message(prefix + line))

Expand Down
24 changes: 14 additions & 10 deletions modules/build/src/main/scala/scala/build/bsp/BspClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import java.net.URI
import java.nio.file.Paths
import java.util.concurrent.{ConcurrentHashMap, ExecutorService}

import ch.epfl.scala.bsp4j.Location

import scala.build.Position.File
import scala.build.errors.{BuildException, CompositeBuildException, Diagnostic, Severity}
import scala.build.postprocessing.LineConversion
Expand Down Expand Up @@ -187,18 +189,20 @@ class BspClient(
)(diag: Diagnostic): Seq[os.Path] =
diag.positions.flatMap {
case File(Right(path), (startLine, startC), (endL, endC)) =>
val id = new b.TextDocumentIdentifier(path.toNIO.toUri.toASCIIString)
val bDiag = {
val startPos = new b.Position(startLine, startC)
val endPos = new b.Position(endL, endC)
val range = new b.Range(startPos, endPos)
val id = new b.TextDocumentIdentifier(path.toNIO.toUri.toASCIIString)
val startPos = new b.Position(startLine, startC)
val endPos = new b.Position(endL, endC)
val range = new b.Range(startPos, endPos)
val bDiag =
new b.Diagnostic(range, diag.message)

diag.relatedInformation.foreach { relatedInformation =>
val location = new Location(path.toNIO.toUri.toASCIIString, range)
val related = new b.DiagnosticRelatedInformation(location, relatedInformation.message)
bDiag.setRelatedInformation(related)
}
val severity = diag.severity match {
case Severity.Error => b.DiagnosticSeverity.ERROR
case Severity.Warning => b.DiagnosticSeverity.WARNING
}
bDiag.setSeverity(severity)
bDiag.setSeverity(diag.severity.toBsp4j)
bDiag.setSource("scala-cli")
val params = new b.PublishDiagnosticsParams(
id,
targetId,
Expand Down
2 changes: 1 addition & 1 deletion modules/build/src/main/scala/scala/build/bsp/BspImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ final class BspImpl(

if (actionableDiagnostics.getOrElse(false)) {
val projectOptions = options0Test.orElse(options0Main)
projectOptions.logActionableDiagnostics(logger)
projectOptions.logActionableDiagnostics(persistentLogger)
}

PreBuildProject(mainScope, testScope, persistentLogger.diagnostics)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package scala.build.tests

import com.eed3si9n.expecty.Expecty.expect

import java.io.IOException
import scala.build.{BuildThreads, Directories, LocalRepo}
import scala.build.options.{BuildOptions, InternalOptions, MaybeScalaVersion}
import scala.build.tests.util.BloopServer
import build.Ops.EitherThrowOps
import scala.build.Position

class DirectiveTests extends munit.FunSuite {

val buildThreads = BuildThreads.create()

def bloopConfigOpt = Some(BloopServer.bloopConfig)

val extraRepoTmpDir = os.temp.dir(prefix = "scala-cli-tests-extra-repo-")
val directories = Directories.under(extraRepoTmpDir)

override def afterAll(): Unit = {
TestInputs.tryRemoveAll(extraRepoTmpDir)
buildThreads.shutdown()
}

val baseOptions = BuildOptions(
internal = InternalOptions(
localRepository = LocalRepo.localRepo(directories.localRepoDir),
keepDiagnostics = true
)
)

test("resolving position of lib directive ") {
val testInputs = TestInputs(
os.rel / "simple.sc" ->
"""//> using lib "com.lihaoyi::utest:0.7.10"
|""".stripMargin
)
testInputs.withBuild(baseOptions, buildThreads, bloopConfigOpt) {
(_, _, maybeBuild) =>
val build = maybeBuild.orThrow
val dep = build.options.classPathOptions.extraDependencies.toSeq.headOption
assert(dep.nonEmpty)

val position = dep.get.positions.headOption
assert(position.nonEmpty)

val (startPos, endPos) = position.get match {
case Position.File(_, startPos, endPos) => (startPos, endPos)
case _ => sys.error("cannot happen")
}

expect(startPos == (0, 15))
expect(endPos == (0, 40))
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -477,9 +477,9 @@ class SourcesTests extends munit.FunSuite {

expect(
javaOpts(0).value.value == "-Dfoo1",
javaOpts(0).positions == Seq(Position.File(Right(root / "something.sc"), (0, 20), (0, 20))),
javaOpts(0).positions == Seq(Position.File(Right(root / "something.sc"), (0, 20), (0, 24))),
javaOpts(1).value.value == "-Dfoo2=bar2",
javaOpts(1).positions == Seq(Position.File(Right(root / "something.sc"), (1, 20), (1, 20)))
javaOpts(1).positions == Seq(Position.File(Right(root / "something.sc"), (1, 20), (1, 29)))
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ final case class VerbosityOptions(
verbose: Int @@ Counter = Tag.of(0),
@HelpMessage("Interactive mode")
@Name("i")
interactive: Option[Boolean] = None
interactive: Option[Boolean] = None,
@HelpMessage("Enable actionable diagnostics")
actions: Option[Boolean] = None
lwronski marked this conversation as resolved.
Show resolved Hide resolved
) {
// format: on

Expand Down
5 changes: 4 additions & 1 deletion modules/cli/src/main/scala/scala/cli/commands/Bsp.scala
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ object Bsp extends ScalaCommand[BspOptions] {
CurrentParams.workspaceOpt = Some(inputs.workspace)
val configDb = ConfigDb.open(options.shared.directories.directories)
.orExit(logger)
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
)

BspThreads.withThreads { threads =>
val bsp = scala.build.bsp.Bsp.create(
Expand Down
5 changes: 4 additions & 1 deletion modules/cli/src/main/scala/scala/cli/commands/Compile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ object Compile extends ScalaCommand[CompileOptions] {
val compilerMaker = options.shared.compilerMaker(threads)
val configDb = ConfigDb.open(options.shared.directories.directories)
.orExit(logger)
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
)

if (options.watch.watchMode) {
val watcher = Build.watch(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ object DependencyUpdate extends ScalaCommand[DependencyUpdateOptions] {
val appliedDiagnostics = updateDependencies(file, sortedByLine)
os.write.over(file, appliedDiagnostics)
diagnostics.foreach(diagnostic =>
logger.message(s"Updated dependency to: ${diagnostic._2.to}")
logger.message(s"Updated dependency to: ${diagnostic._2.suggestion}")
)
case (Left(file), diagnostics) =>
diagnostics.foreach {
diagnostic =>
logger.message(s"Warning: Scala CLI can't update ${diagnostic._2.to} in $file")
logger.message(s"Warning: Scala CLI can't update ${diagnostic._2.suggestion} in $file")
}
}
}
Expand All @@ -106,7 +106,7 @@ object DependencyUpdate extends ScalaCommand[DependencyUpdateOptions] {
val startIndex = startIndicies(line) + column
val endIndex = startIndex + diagnostic.oldDependency.render.length()

val newDependency = diagnostic.to
val newDependency = diagnostic.suggestion
s"${fileContent.slice(0, startIndex)}$newDependency${fileContent.drop(endIndex)}"
}
}
Expand Down
5 changes: 4 additions & 1 deletion modules/cli/src/main/scala/scala/cli/commands/Doc.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ object Doc extends ScalaCommand[DocOptions] {

val configDb = ConfigDb.open(options.shared.directories.directories)
.orExit(logger)
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
)

val builds =
Build.build(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ object Metabrowse extends ScalaCommand[MetabrowseOptions] {
val compilerMaker = options.shared.compilerMaker(threads)
val configDb = ConfigDb.open(options.shared.directories.directories)
.orExit(logger)
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
)

val builds =
Build.build(
Expand Down
5 changes: 4 additions & 1 deletion modules/cli/src/main/scala/scala/cli/commands/Package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ object Package extends ScalaCommand[PackageOptions] {
val cross = options.compileCross.cross.getOrElse(false)
val configDb = ConfigDb.open(options.shared.directories.directories)
.orExit(logger)
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
)

if (options.watch.watchMode) {
var expectedModifyEpochSecondOpt = Option.empty[Long]
Expand Down
5 changes: 4 additions & 1 deletion modules/cli/src/main/scala/scala/cli/commands/Repl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ object Repl extends ScalaCommand[ReplOptions] {
val cross = options.compileCross.cross.getOrElse(false)
val configDb = ConfigDb.open(options.shared.directories.directories)
.orExit(logger)
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
)

if (inputs.isEmpty) {
val artifacts = initialBuildOptions.artifacts(logger, Scope.Main).orExit(logger)
Expand Down
5 changes: 4 additions & 1 deletion modules/cli/src/main/scala/scala/cli/commands/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ object Run extends ScalaCommand[RunOptions] {

val configDb = ConfigDb.open(options.shared.directories.directories)
.orExit(logger)
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
)

if (options.watch.watchMode) {
var processOpt = Option.empty[(Process, CompletableFuture[_])]
Expand Down
5 changes: 4 additions & 1 deletion modules/cli/src/main/scala/scala/cli/commands/Test.scala
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ object Test extends ScalaCommand[TestOptions] {
val cross = options.compileCross.cross.getOrElse(false)
val configDb = ConfigDb.open(options.shared.directories.directories)
.orExit(logger)
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
)

def maybeTest(builds: Builds, allowExit: Boolean): Unit = {
val optionsKeys = builds.map.keys.toVector.map(_.optionsKey).distinct
Expand Down
27 changes: 19 additions & 8 deletions modules/cli/src/main/scala/scala/cli/internal/CliLogger.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package scala.cli.internal

import ch.epfl.scala.{bsp4j => b}
import ch.epfl.scala.bsp4j.Location
import coursier.cache.CacheLogger
import coursier.cache.loggers.{FallbackRefreshDisplay, RefreshLogger}
import org.scalajs.logging.{Level => ScalaJsLevel, Logger => ScalaJsLogger, ScalaConsoleLogger}
Expand All @@ -9,6 +10,7 @@ import java.io.PrintStream

import scala.build.blooprifle.BloopRifleLogger
import scala.build.errors.{BuildException, CompositeBuildException, Diagnostic, Severity}
import scala.build.errors.Diagnostic.RelatedInformation
import scala.build.internal.CustomProgressBarRefreshDisplay
import scala.build.{ConsoleBloopBuildClient, Logger, Position}
import scala.collection.mutable
Expand All @@ -28,7 +30,8 @@ class CliLogger(
d.positions,
d.severity,
d.message,
hashMap
hashMap,
d.relatedInformation
)
}
}
Expand All @@ -54,11 +57,12 @@ class CliLogger(
positions: Seq[Position],
severity: Severity,
message: String,
contentCache: mutable.Map[os.Path, Seq[String]]
contentCache: mutable.Map[os.Path, Seq[String]],
relatedInformation: Option[RelatedInformation]
) =
if (positions.isEmpty)
out.println(
s"${ConsoleBloopBuildClient.diagnosticPrefix(Severity.Error == severity)} $message"
s"${ConsoleBloopBuildClient.diagnosticPrefix(severity)} $message"
)
else {
val positions0 = positions.distinct
Expand All @@ -75,10 +79,17 @@ class CliLogger(
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
})
diag.setSeverity(severity.toBsp4j)
diag.setSource("scala-cli")

for {
filePath <- f.path
info <- relatedInformation
} {
val location = new Location(filePath.toNIO.toUri.toASCIIString, range)
val related = new b.DiagnosticRelatedInformation(location, info.message)
diag.setRelatedInformation(related)
}

for (file <- f.path) {
val lines = contentCache.getOrElseUpdate(file, os.read(file).linesIterator.toVector)
Expand Down Expand Up @@ -112,7 +123,7 @@ class CliLogger(
for (ex <- c.exceptions)
printEx(ex, contentCache)
case _ =>
printDiagnostic(ex.positions, Severity.Error, ex.getMessage(), contentCache)
printDiagnostic(ex.positions, Severity.Error, ex.getMessage(), contentCache, None)
}

def log(ex: BuildException): Unit =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package scala.build.errors

import scala.build.Position
import scala.build.errors.Diagnostic.RelatedInformation

trait Diagnostic {
def message: String
def severity: Severity
def positions: Seq[Position]
def relatedInformation: Option[RelatedInformation] = None
}

object Diagnostic {

case class RelatedInformation(message: String)
object Messages {
val bloopTooOld =
"JVM that is hosting bloop is older than the requested runtime. Please run `scala-cli bloop exit`, and then use `--jvm` flag to restart Bloop"
Expand Down
Loading