Bloop: Publish diagnostics for syntax errors instead of logging messages

[olafurpg]

Bloop currently uses build/logMessage to report syntax errors instead of buildTarget/publishDiagnostics. Consider the following code

package example

class UserTest {
  List[String)]("")
}

Here are the three json responses bloop sends to the client

- obtained
+ expected
{
  "jsonrpc": "2.0",
  "method": "build/logMessage",
  "params": {
    "type": 3,
    "message": "Compiling 1 Scala source to /Users/ollie/dev/metals/test-workspace/.bloop/test-workspace/scala-2.12/test-classes ..."
  }
}
{
  "jsonrpc": "2.0",
-  "method": "build/logMessage",
+  "method": "buildTarget/publishDiagnostics",
  "params": {
    "type": 1,
    "message": "\u0027]\u0027 expected but \u0027)\u0027 found."
+   <position information>
  }
}
{
  "jsonrpc": "2.0",
  "method": "buildTarget/compileReport",
  "params": {
    "target": {
      "uri": "file:///Users/ollie/dev/metals/test-workspace/?id\u003dtest-workspace-test"
    },
    "errors": 1,
    "warnings": 0
  }
}

[olafurpg]

Interestingly, I am unable to reproduce with type errors, they use publishDiagnostics as expected

{
  "jsonrpc": "2.0",
  "method": "build/logMessage",
  "params": {
    "type": 3,
    "message": "Compiling 2 Scala sources to /Users/ollie/dev/metals/test-workspace/.bloop/test-workspace/scala-2.12/test-classes ..."
  }
}
{
  "jsonrpc": "2.0",
  "method": "build/publishDiagnostics",
  "params": {
    "uri": "file:/Users/ollie/dev/metals/test-workspace/src/test/scala/example/UserTest.scala",
    "diagnostics": [
      {
        "range": {
          "start": {
            "line": 4,
            "character": 12
          },
          "end": {
            "line": 4,
            "character": 14
          }
        },
        "severity": 1,
        "message": "type mismatch;\n found   : String(\"\")\n required: Int"
      }
    ]
  }
}
{
  "jsonrpc": "2.0",
  "method": "buildTarget/compileReport",
  "params": {
    "target": {
      "uri": "file:///Users/ollie/dev/metals/test-workspace/?id\u003dtest-workspace-test"
    },
    "errors": 1,
    "warnings": 0
  }
}

[pbuszka]

I had a similar problem when using intellij bsp integration when testing with master build (e7cbd165). Today I tried again and nailed it down to the function bloop.logging.BspServerLogger#diagnostic and this case in the match statement

case ZincInternals.ZincExistsPos(startLine, startColumn)

What actually came here was an instance of xsbt.DelegatingReporter.PositionImpl
which does not match the case and bloop falls back to default case which does not publish any diagnostic over BSP.

I'm not sure how to fix it as all startLine/Column endLine/Column fields in xsbti.Position are empty, probably one has to use line0 or something but I have no idea why is it so over complicated.

I "brute force" fixed it in my debugging session and it was the cause but I was unable to create local bloop version which worked correctly on my workstation with the latest master. I made it work when starting bloop server from sbt but I could not build local version which worked correctly with IDEA even though the sbt server worked correctly. Probably some configuration magic which I messed up. I don;t submit a PR because of it but in case it is any usful here is the function which worked:

` def diagnostic(problem: Problem): Unit = {
import sbt.util.InterfaceUtil.toOption
val message = problem.message
val problemPos = problem.position
val sourceFile = toOption(problemPos.sourceFile())

(problemPos, sourceFile) match {
  case (position: xsbti.Position, Some(file)) =>
    val pos = position match {
      case ZincInternals.ZincExistsPos(startLine, startColumn) =>
        val start = bsp.Position(startLine, startColumn)
        val endTuple = ZincInternals.ZincRangePos.unapply(position).getOrElse((startLine, startColumn))
        val end = bsp.Position(endTuple._1, endTuple._2)
        bsp.Range(start, end)
      case p if p.line.isPresent =>
        val pos = bsp.Position(p.line.get, p.pointer.orElse(1))
        bsp.Range(pos, pos)
    }

    val severity = problem.severity match {
      case Severity.Error => bsp.DiagnosticSeverity.Error
      case Severity.Warn => bsp.DiagnosticSeverity.Warning
      case Severity.Info => bsp.DiagnosticSeverity.Information
    }

    val uri = bsp.Uri(file.toURI)
    val diagnostic = bsp.Diagnostic(pos, Some(severity), None, None, message, None)
    val diagnostics = bsp.PublishDiagnosticsParams(uri, None, List(diagnostic))
    Build.publishDiagnostics.notify(diagnostics)

  case _ =>
    problem.severity match {
      case Severity.Error => error(message)
      case Severity.Warn => warn(message)
      case Severity.Info => info(message)
    }
}
()

}
`

[jvican]

What actually came here was an instance of xsbt.DelegatingReporter.PositionImpl
which does not match the case and bloop falls back to default case which does not publish any diagnostic over BSP.

Yes, our zinc bridge now contains the correct position information in its public API, we only need to update the implementation.

[jvican]

@pbuszka Your analysis was correct, thank you for spending the time with it. I've pushed a PR that fixes this issue.