Skip to content

Commit 1719919

Browse files
lwronskilwronski
authored
Actionable diagnostics (#1229)
* Fix counting end pos in using directives * Use persistent logger to forward diagnostics to bsp * Use hint severity in ActionableDiagnostic * Set source as scala-cli for diagnostic generated in scala-cli * Pass actionable diagnostics suggestion to related information in diagnostic * Fix printing hint diagnostic in command line * Fix scalafmt
1 parent 67569dc commit 1719919

File tree

24 files changed

+238
-67
lines changed

24 files changed

+238
-67
lines changed

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,19 +151,25 @@ object ConsoleBloopBuildClient {
151151
private val red = Console.RED
152152
private val yellow = Console.YELLOW
153153

154-
def diagnosticPrefix(isError: Boolean): String =
155-
if (isError) s"[${red}error$reset] "
156-
else s"[${yellow}warn$reset] "
154+
def diagnosticPrefix(severity: bsp4j.DiagnosticSeverity): String =
155+
severity match {
156+
case bsp4j.DiagnosticSeverity.ERROR => s"[${red}error$reset] "
157+
case bsp4j.DiagnosticSeverity.WARNING => s"[${yellow}warn$reset] "
158+
case bsp4j.DiagnosticSeverity.HINT => s"[${yellow}hint$reset] "
159+
}
160+
161+
def diagnosticPrefix(severity: Severity): String = diagnosticPrefix(severity.toBsp4j)
157162

158163
def printFileDiagnostic(
159164
logger: Logger,
160165
path: Either[String, os.Path],
161166
diag: bsp4j.Diagnostic
162167
): Unit = {
163-
val isWarningOrError = diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR ||
164-
diag.getSeverity == bsp4j.DiagnosticSeverity.WARNING
165-
if (isWarningOrError) {
166-
val prefix = diagnosticPrefix(diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR)
168+
val isWarningOrErrorOrHint = diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR ||
169+
diag.getSeverity == bsp4j.DiagnosticSeverity.WARNING ||
170+
diag.getSeverity == bsp4j.DiagnosticSeverity.HINT
171+
if (isWarningOrErrorOrHint) {
172+
val prefix = diagnosticPrefix(diag.getSeverity)
167173

168174
val line = (diag.getRange.getStart.getLine + 1).toString + ":"
169175
val col = (diag.getRange.getStart.getCharacter + 1).toString + ":"
@@ -215,7 +221,7 @@ object ConsoleBloopBuildClient {
215221
val isWarningOrError = true
216222
if (isWarningOrError) {
217223
val msgIt = message.linesIterator
218-
val prefix = diagnosticPrefix(severity == Severity.Error)
224+
val prefix = diagnosticPrefix(severity)
219225
logger.message(prefix + (if (msgIt.hasNext) " " + msgIt.next() else ""))
220226
msgIt.foreach(line => logger.message(prefix + line))
221227

‎modules/build/src/main/scala/scala/build/bsp/BspClient.scala

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import java.net.URI
77
import java.nio.file.Paths
88
import java.util.concurrent.{ConcurrentHashMap, ExecutorService}
99

10+
import ch.epfl.scala.bsp4j.Location
11+
1012
import scala.build.Position.File
1113
import scala.build.errors.{BuildException, CompositeBuildException, Diagnostic, Severity}
1214
import scala.build.postprocessing.LineConversion
@@ -187,18 +189,20 @@ class BspClient(
187189
)(diag: Diagnostic): Seq[os.Path] =
188190
diag.positions.flatMap {
189191
case File(Right(path), (startLine, startC), (endL, endC)) =>
190-
val id = new b.TextDocumentIdentifier(path.toNIO.toUri.toASCIIString)
191-
val bDiag = {
192-
val startPos = new b.Position(startLine, startC)
193-
val endPos = new b.Position(endL, endC)
194-
val range = new b.Range(startPos, endPos)
192+
val id = new b.TextDocumentIdentifier(path.toNIO.toUri.toASCIIString)
193+
val startPos = new b.Position(startLine, startC)
194+
val endPos = new b.Position(endL, endC)
195+
val range = new b.Range(startPos, endPos)
196+
val bDiag =
195197
new b.Diagnostic(range, diag.message)
198+
199+
diag.relatedInformation.foreach { relatedInformation =>
200+
val location = new Location(path.toNIO.toUri.toASCIIString, range)
201+
val related = new b.DiagnosticRelatedInformation(location, relatedInformation.message)
202+
bDiag.setRelatedInformation(related)
196203
}
197-
val severity = diag.severity match {
198-
case Severity.Error => b.DiagnosticSeverity.ERROR
199-
case Severity.Warning => b.DiagnosticSeverity.WARNING
200-
}
201-
bDiag.setSeverity(severity)
204+
bDiag.setSeverity(diag.severity.toBsp4j)
205+
bDiag.setSource("scala-cli")
202206
val params = new b.PublishDiagnosticsParams(
203207
id,
204208
targetId,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ final class BspImpl(
185185

186186
if (actionableDiagnostics.getOrElse(false)) {
187187
val projectOptions = options0Test.orElse(options0Main)
188-
projectOptions.logActionableDiagnostics(logger)
188+
projectOptions.logActionableDiagnostics(persistentLogger)
189189
}
190190

191191
PreBuildProject(mainScope, testScope, persistentLogger.diagnostics)

‎modules/build/src/test/scala/scala/build/tests/DirectiveTests.scala

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package scala.build.tests
2+
3+
import com.eed3si9n.expecty.Expecty.expect
4+
5+
import java.io.IOException
6+
import scala.build.{BuildThreads, Directories, LocalRepo}
7+
import scala.build.options.{BuildOptions, InternalOptions, MaybeScalaVersion}
8+
import scala.build.tests.util.BloopServer
9+
import build.Ops.EitherThrowOps
10+
import scala.build.Position
11+
12+
class DirectiveTests extends munit.FunSuite {
13+
14+
val buildThreads = BuildThreads.create()
15+
16+
def bloopConfigOpt = Some(BloopServer.bloopConfig)
17+
18+
val extraRepoTmpDir = os.temp.dir(prefix = "scala-cli-tests-extra-repo-")
19+
val directories = Directories.under(extraRepoTmpDir)
20+
21+
override def afterAll(): Unit = {
22+
TestInputs.tryRemoveAll(extraRepoTmpDir)
23+
buildThreads.shutdown()
24+
}
25+
26+
val baseOptions = BuildOptions(
27+
internal = InternalOptions(
28+
localRepository = LocalRepo.localRepo(directories.localRepoDir),
29+
keepDiagnostics = true
30+
)
31+
)
32+
33+
test("resolving position of lib directive ") {
34+
val testInputs = TestInputs(
35+
os.rel / "simple.sc" ->
36+
"""//> using lib "com.lihaoyi::utest:0.7.10"
37+
|""".stripMargin
38+
)
39+
testInputs.withBuild(baseOptions, buildThreads, bloopConfigOpt) {
40+
(_, _, maybeBuild) =>
41+
val build = maybeBuild.orThrow
42+
val dep = build.options.classPathOptions.extraDependencies.toSeq.headOption
43+
assert(dep.nonEmpty)
44+
45+
val position = dep.get.positions.headOption
46+
assert(position.nonEmpty)
47+
48+
val (startPos, endPos) = position.get match {
49+
case Position.File(_, startPos, endPos) => (startPos, endPos)
50+
case _ => sys.error("cannot happen")
51+
}
52+
53+
expect(startPos == (0, 15))
54+
expect(endPos == (0, 40))
55+
}
56+
}
57+
58+
}

‎modules/build/src/test/scala/scala/build/tests/SourcesTests.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,9 +477,9 @@ class SourcesTests extends munit.FunSuite {
477477

478478
expect(
479479
javaOpts(0).value.value == "-Dfoo1",
480-
javaOpts(0).positions == Seq(Position.File(Right(root / "something.sc"), (0, 20), (0, 20))),
480+
javaOpts(0).positions == Seq(Position.File(Right(root / "something.sc"), (0, 20), (0, 24))),
481481
javaOpts(1).value.value == "-Dfoo2=bar2",
482-
javaOpts(1).positions == Seq(Position.File(Right(root / "something.sc"), (1, 20), (1, 20)))
482+
javaOpts(1).positions == Seq(Position.File(Right(root / "something.sc"), (1, 20), (1, 29)))
483483
)
484484
}
485485
}

‎modules/cli-options/src/main/scala/scala/cli/commands/VerbosityOptions.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ final case class VerbosityOptions(
1212
verbose: Int @@ Counter = Tag.of(0),
1313
@HelpMessage("Interactive mode")
1414
@Name("i")
15-
interactive: Option[Boolean] = None
15+
interactive: Option[Boolean] = None,
16+
@HelpMessage("Enable actionable diagnostics")
17+
actions: Option[Boolean] = None
1618
) {
1719
// format: on
1820

‎modules/cli/src/main/scala/scala/cli/commands/Bsp.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ object Bsp extends ScalaCommand[BspOptions] {
7272
CurrentParams.workspaceOpt = Some(inputs.workspace)
7373
val configDb = ConfigDb.open(options.shared.directories.directories)
7474
.orExit(logger)
75-
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
75+
val actionableDiagnostics =
76+
options.shared.logging.verbosityOptions.actions.orElse(
77+
configDb.get(Keys.actions).getOrElse(None)
78+
)
7679

7780
BspThreads.withThreads { threads =>
7881
val bsp = scala.build.bsp.Bsp.create(

‎modules/cli/src/main/scala/scala/cli/commands/Compile.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ object Compile extends ScalaCommand[CompileOptions] {
8282
val compilerMaker = options.shared.compilerMaker(threads)
8383
val configDb = ConfigDb.open(options.shared.directories.directories)
8484
.orExit(logger)
85-
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
85+
val actionableDiagnostics =
86+
options.shared.logging.verbosityOptions.actions.orElse(
87+
configDb.get(Keys.actions).getOrElse(None)
88+
)
8689

8790
if (options.watch.watchMode) {
8891
val watcher = Build.watch(

‎modules/cli/src/main/scala/scala/cli/commands/DependencyUpdate.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ object DependencyUpdate extends ScalaCommand[DependencyUpdateOptions] {
8383
val appliedDiagnostics = updateDependencies(file, sortedByLine)
8484
os.write.over(file, appliedDiagnostics)
8585
diagnostics.foreach(diagnostic =>
86-
logger.message(s"Updated dependency to: ${diagnostic._2.to}")
86+
logger.message(s"Updated dependency to: ${diagnostic._2.suggestion}")
8787
)
8888
case (Left(file), diagnostics) =>
8989
diagnostics.foreach {
9090
diagnostic =>
91-
logger.message(s"Warning: Scala CLI can't update ${diagnostic._2.to} in $file")
91+
logger.message(s"Warning: Scala CLI can't update ${diagnostic._2.suggestion} in $file")
9292
}
9393
}
9494
}
@@ -106,7 +106,7 @@ object DependencyUpdate extends ScalaCommand[DependencyUpdateOptions] {
106106
val startIndex = startIndicies(line) + column
107107
val endIndex = startIndex + diagnostic.oldDependency.render.length()
108108

109-
val newDependency = diagnostic.to
109+
val newDependency = diagnostic.suggestion
110110
s"${fileContent.slice(0, startIndex)}$newDependency${fileContent.drop(endIndex)}"
111111
}
112112
}

‎modules/cli/src/main/scala/scala/cli/commands/Doc.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,10 @@ object Doc extends ScalaCommand[DocOptions] {
3636

3737
val configDb = ConfigDb.open(options.shared.directories.directories)
3838
.orExit(logger)
39-
val actionableDiagnostics = configDb.get(Keys.actions).getOrElse(None)
39+
val actionableDiagnostics =
40+
options.shared.logging.verbosityOptions.actions.orElse(
41+
configDb.get(Keys.actions).getOrElse(None)
42+
)
4043

4144
val builds =
4245
Build.build(

0 commit comments

Comments
 (0)