Add option to return rendered message when compiling via BSP

[mrdziuban]

xsbti.Problem#rendered contains the full error message as it's rendered by the compiler, including the error code, file path and location, line of code and the carets highlighting which part of the line had the error.

For example, this is what xsbti.Problem#message looks like for a type mismatch:

Found:    ("test" : String)
Required: Int

and this is what xsbti.Problem#rendered looks like:

-- [E007] Type Mismatch Error: /path/to/my/project/src/main/scala/Example.scala:76:15 ------------
76 |  val x: Int = "test"
   |               ^^^^^^
   |               Found:    ("test" : String)
   |               Required: Int

For tools like bleep that display errors on the command line, being able to show the full rendered message is ideal.

To support this, I've added a --show-rendered-message flag as suggested in Discord. When it's passed, the BspServerLogger will use the rendered message (when present) in place of the standard message.

[mrdziuban]

Hey @tgodzik, re: test failures, I just pushed a fix to use File.separator instead of hardcoding /, but it looks like Windows also preserves the line breaks in the message:

 =======
=> Diff
=======
---?
+++?
@@ -4,3 +4,3 @@
1: a/src/main/scala/Foo.scala
-  -> List(Diagnostic(Range(Position(0,26),Position(0,27)),Some(Error),Some(_),Some(_),-- [E007] Type Mismatch Error: C:\Users\runneradmin\AppData\Local\Temp\bloop-test-workspace14714670476857937804\a\src\main\scala\Foo.scala:1:26 
 1 |def foo(s: String): Int = s
   |                          ^
   |                          Found:    (s : String)
   |                          Required: Int
   |
   | longer explanation available when compiling with `-explain`,None,None,Some({"actions":[]})))
+  -> List(Diagnostic(Range(Position(0,26),Position(0,27)),Some(Error),Some(_),Some(_),-- [E007] Type Mismatch Error: C:\Users\runneradmin\AppData\Local\Temp\bloop-test-workspace14714670476857937804/a/src/main/scala/Foo.scala:1:26  1 |def foo(s: String): Int = s   |                          ^   |                          Found:    (s : String)   |                          Required: Int   |   | longer explanation available when compiling with `-explain`,None,None,Some({"actions":[]})))
   -> reset = true

Should I try to remove the line breaks? Or is there any other approach you'd suggest?

[tgodzik]

Yeah, that's always tricky, I see that we use somewhere:

assertNoDiff(
        logger.renderErrors(exceptContaining = "Failed to compile"),
        s"""[E1] ${targetFoo}:2:18
           |     ';' expected but '=' found.
           |     L2:   al foo: String = 1
           |                          ^
           |${targetFoo}: L2 [E1]""".stripMargin
      )

maybe something along these lines?

[mrdziuban]

I tried using logger.renderErrors but the test that's currently doing that isn't working with BSP, so the logged messages are a lot more straightforward. In the BSP tests, the logged diagnostics look like this, so it's harder to extract and diff them:

[debug]
  --> header: Content-Length -> 1197
  --> content: {"method":"build/publishDiagnostics","params":{"textDocument":{"uri":"file:///private/var/folders/bk/hnp2xmcs6kjf5s62x771mmxh0000gn/T/bloop-test-workspace13632709552891212982/a/src/main/scala/Foo.scala"},"buildTarget":{"uri":"file:/private/var/folders/bk/hnp2xmcs6kjf5s62x771mmxh0000gn/T/bloop-test-workspace13632709552891212982/a/?id=a"},"diagnostics":[{"range":{"start":{"line":0,"character":26},"end":{"line":0,"character":27}},"severity":1,"code":"7","source":"bloop","message":"\u001b[31m\u001b[31m-- [E007] Type Mismatch Error: /private/var/folders/bk/hnp2xmcs6kjf5s62x771mmxh0000gn/T/bloop-test-workspace13632709552891212982/a/src/main/scala/Foo.scala:1:26 \u001b[0m\u001b[0m\n\u001b[31m1 |\u001b[0m\u001b[33mdef\u001b[0m \u001b[36mfoo\u001b[0m(\u001b[36ms\u001b[0m: \u001b[35mString\u001b[0m): \u001b[35mInt\u001b[0m = s\n\u001b[31m\u001b[31m  |\u001b[0m                          ^\u001b[0m\n\u001b[31m  |\u001b[0m                          Found:    (s : String)\n\u001b[31m  |\u001b[0m                          Required: Int\n\u001b[31m  |\u001b[0m\n\u001b[31m  |\u001b[0m longer explanation available when compiling with `-explain`","data":{"actions":[]}}],"reset":true},"jsonrpc":"2.0"}

I went ahead and extracted the logic in BspClientTest that replaces newlines to a method and updated my test to use that. I'll keep poking around for a better option but I confirmed with a GH Actions run in my fork that this makes the test pass on Windows.

[mrdziuban]

Okay this time I really think the tests are going to pass 😅. The logic to replace newlines wasn't accounting for standalone \r sequences even though the comment above END_OF_LINE_MATCHER suggests that it should. It was also using replace, which replaces strings literally, instead of replaceAll which accepts a regex.

[mrdziuban]

The most recent test failures look unrelated

X bloop.bsp.LocalBspLinkSpec.can link scala-native-04 cross project 31712ms 
  java.util.concurrent.TimeoutException: Futures timed out after [30 seconds]

[tgodzik]

LGTM!