Skip to content

Commit

Permalink
WIP - set hint message in relatedInformation parameter in diagnostic
Browse files Browse the repository at this point in the history
  • Loading branch information
lwronski committed Aug 3, 2022
1 parent cae4078 commit 4938397
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class ConsoleBloopBuildClient(
.getOrElse((Right(path), diag))
if (keepDiagnostics)
diagnostics0 += updatedPath -> updatedDiag
ConsoleBloopBuildClient.printFileDiagnostic(logger, updatedPath, updatedDiag)
ConsoleBloopBuildClient.printFileDiagnostic(logger, updatedPath, updatedDiag, None)
}
}

Expand Down Expand Up @@ -151,19 +151,27 @@ 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
diag: bsp4j.Diagnostic,
hint: Option[String]
): Unit = {
val isWarningOrError = diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR ||
diag.getSeverity == bsp4j.DiagnosticSeverity.WARNING
if (isWarningOrError) {
val prefix = diagnosticPrefix(diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR)
val isHint = diag.getSeverity == bsp4j.DiagnosticSeverity.HINT
val isWarningOrErrorOrHint = diag.getSeverity == bsp4j.DiagnosticSeverity.ERROR ||
diag.getSeverity == bsp4j.DiagnosticSeverity.WARNING ||
isHint
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 @@ -199,9 +207,14 @@ object ConsoleBloopBuildClient {
if (canPrintUnderline) {
val len =
math.max(1, diag.getRange.getEnd.getCharacter - diag.getRange.getStart.getCharacter)
logger.message(
prefix + " " * diag.getRange.getStart.getCharacter + "^" * len
)
if (isHint)
logger.message(
prefix + " " * (diag.getRange.getStart.getCharacter - 4) + hint.get
)
else
logger.message(
prefix + " " * diag.getRange.getStart.getCharacter + "^" * len
)
}
}
}
Expand All @@ -215,7 +228,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
26 changes: 16 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,22 @@ 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)

bDiag.setSource("scala-cli")
bDiag.setSeverity(diag.severity.toBsp4j)

diag.hint.foreach { hintMsg =>
val location = new Location(path.toNIO.toUri.toASCIIString, range)
val related = new b.DiagnosticRelatedInformation(location, hintMsg)
bDiag.setRelatedInformation(related)
}
val severity = diag.severity match {
case Severity.Error => b.DiagnosticSeverity.ERROR
case Severity.Warning => b.DiagnosticSeverity.WARNING
}
bDiag.setSeverity(severity)

val params = new b.PublishDiagnosticsParams(
id,
targetId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class DirectiveTests extends munit.FunSuite {
def bloopConfigOpt = Some(BloopServer.bloopConfig)

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

override def afterAll(): Unit = {
TestInputs.tryRemoveAll(extraRepoTmpDir)
Expand All @@ -38,21 +38,21 @@ class DirectiveTests extends munit.FunSuite {
)
testInputs.withBuild(baseOptions, buildThreads, bloopConfigOpt) {
(_, _, maybeBuild) =>
val build = maybeBuild.orThrow
val dep = build.options.classPathOptions.extraDependencies.toSeq.headOption
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")
case _ => sys.error("cannot happen")
}

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

}
}
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.hint}")
)
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.hint} 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.hint
s"${fileContent.slice(0, startIndex)}$newDependency${fileContent.drop(endIndex)}"
}
}
Expand Down
29 changes: 20 additions & 9 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 Down Expand Up @@ -28,7 +29,8 @@ class CliLogger(
d.positions,
d.severity,
d.message,
hashMap
hashMap,
d.hint
)
}
}
Expand All @@ -54,11 +56,12 @@ class CliLogger(
positions: Seq[Position],
severity: Severity,
message: String,
contentCache: mutable.Map[os.Path, Seq[String]]
contentCache: mutable.Map[os.Path, Seq[String]],
hint: Option[String]
) =
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 +78,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
hintMsg <- hint
} {
val location = new Location(filePath.toNIO.toUri.toASCIIString, range)
val related = new b.DiagnosticRelatedInformation(location, hintMsg)
diag.setRelatedInformation(related)
}

for (file <- f.path) {
val lines = contentCache.getOrElseUpdate(file, os.read(file).linesIterator.toVector)
Expand All @@ -88,7 +98,8 @@ class CliLogger(
ConsoleBloopBuildClient.printFileDiagnostic(
this,
f.path,
diag
diag,
hint
)
}

Expand All @@ -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
Expand Up @@ -6,6 +6,7 @@ trait Diagnostic {
def message: String
def severity: Severity
def positions: Seq[Position]
def hint: Option[String] = None
}

object Diagnostic {
Expand All @@ -17,12 +18,14 @@ object Diagnostic {
private case class ADiagnostic(
message: String,
severity: Severity,
positions: Seq[Position]
positions: Seq[Position],
override val hint: Option[String]
) extends Diagnostic

def apply(
message: String,
severity: Severity,
positions: Seq[Position] = Nil
): Diagnostic = ADiagnostic(message, severity, positions)
positions: Seq[Position] = Nil,
hint: Option[String] = None
): Diagnostic = ADiagnostic(message, severity, positions, hint)
}
17 changes: 14 additions & 3 deletions modules/core/src/main/scala/scala/build/errors/Severity.scala
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
package scala.build.errors

sealed abstract class Severity extends Product with Serializable
import ch.epfl.scala.bsp4j as b
import ch.epfl.scala.bsp4j.DiagnosticSeverity
sealed abstract class Severity extends Product with Serializable {
def toBsp4j: b.DiagnosticSeverity
}

object Severity {
case object Error extends Severity
case object Warning extends Severity
case object Error extends Severity {
override def toBsp4j: DiagnosticSeverity = b.DiagnosticSeverity.ERROR
}
case object Warning extends Severity {
override def toBsp4j: DiagnosticSeverity = b.DiagnosticSeverity.WARNING
}
case object Hint extends Severity {
override def toBsp4j: DiagnosticSeverity = b.DiagnosticSeverity.HINT
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ abstract class ActionableDiagnostic {

/** Provide the new content which will be replaced by actionable diagnostic
*/
def to: String
def hint: String

def positions: Seq[Position]

final def toDiagnostic: Diagnostic = Diagnostic(
message = s"""|$message
| To: $to""".stripMargin,
severity = Severity.Warning,
positions = positions
message = message,
severity = Severity.Hint,
positions = positions,
hint = Some(hint)
)
}

Expand All @@ -32,7 +33,7 @@ object ActionableDiagnostic {
oldDependency: AnyDependency,
newVersion: String
) extends ActionableDiagnostic {
override def to: String = oldDependency.copy(version = newVersion).render
override def hint: String = oldDependency.copy(version = newVersion).render
}

}

0 comments on commit 4938397

Please sign in to comment.