[Scala3] Regression: Multiple error inlines are not shown starting 3.0.1

[deusaquilus]

Describe the bug

When using Scala 3.0.0 if you have code that has errors in multiple levels of inlining, there is a red underline for every single one:

---

You can hover over any of them and see the errors thrown at that point:

In 3.0.1 you only see the last position underlined: (although you can see that SBT still knows about the parent inline i.e. the one on line 5):

---

This functionality is badly needed because in ProtoQuill errors almost always happen in inline def quote { ... } blocks long before the run function finally splices them back into real code so having mouse-over insight onto the error in those sections is invaluable.

If we only have the error information in the run(...) site it is very difficult to track down the error.

---

To Reproduce

Create a simple example. Create the following macro:

object InlineMac {
  inline def matchOne(inline value: String): String = ${ matchOneImpl('value) }
  def matchOneImpl(value: Expr[String])(using Quotes): Expr[String] = {
    import quotes.reflect._
    value match
      case '{ "foo" } => '{ "foo2" }
      case _ => report.throwError("Invalid value", value)
  }

  inline def matchTwo(inline value: String): String = ${ matchTwoImpl('value) }
  def matchTwoImpl(value: Expr[String])(using Quotes): Expr[String] = {
    import quotes.reflect._
    value.asTerm.underlyingArgument.asExpr match
      case '{ "foo2" } => '{ "foo3" }
  }
}

Then use it like so:

object InlineUse {
  def main(args: Array[String]): Unit = {
    inline def splice1 = InlineMac.matchOne("foo")
    val splice2 = InlineMac.matchTwo(splice1)
  }
}

When SBT scalaVersion is 3.0.0 there will be a short red line underneath (a part of) both matchOne and matchTwo. If SBT scalaVersion is 3.0.1 or above there will only be a red line underneath matchTwo.

Code examples can be found in the following repo: https://github.com/getquill/protoquill-example/tree/multi-inline-example

The simple example above is here: https://github.com/getquill/protoquill-example/tree/multi-inline-example/src/main/scala/io/getquill/simple

Expected behavior All the places from which the code is inlined should have a red arrow underneath.

Installation:

• Operating system: macOS/Windows/Linux
• Editor: Visual Studio Code/Atom/Vim/Sublime/Emacs
• Metals version: v0.10.7

Search terms

Inline. Error Highlight. Underline. Mouse Hover.

[kpodsiad]

Thanks for reporting! At first glance, it looks like the build server error. AFAIK, Metals only forwards diagnostics which they got from it. I checked and for both sbt and bloop, those diagnostics differ depending on the scala version. For 3.0.0 I got:

and for 3.0.1:

I have to dig deeper into the codebase to prove my hypothesis and see what diagnostics we're getting from the build server, and what we're doing with them. Will inform about the results!

[dos65]

I think it should be related to this - https://github.com/lampepfl/dotty/pull/12425 that was a fix for this issue https://github.com/scalameta/metals/issues/2769

Originally there was a problem that diagnostic from inline error was received for a wrong file. We need to look if we can unline such diagnostic messages into several ones instead of into single last one.

[deusaquilus]

Thanks for the feedback. If there’s something that you’d like me to test please let me know.

[deusaquilus]

Curiously, is lampepfl/dotty#12425 still a blocker for this issue? Seems like it was merged in May. Is there still an upstream fix needed?

[dos65]

@deusaquilus yep it was merged and probably that was a reason of this regression.

[deusaquilus]

What does that mean? Does it mean there needs to be a new fix for something on epfl/dotty?

[dos65]

@deusaquilus

Does it mean there needs to be a new fix for something on epfl/dotty?

Yes, it should fixed there.

[deusaquilus]

What is the upstream fix that is needed?

[DamianReeves]

Being affected by this as well. Would be great to get it fixed.

[deusaquilus]

Hey guys, sorry to pester. What is the upstream fix that is needed?

[dos65]

@deusaquilus I'm going to look at it today.

[deusaquilus]

Hi @dos65. Now with the release of 3.1.3 it looks like they enhanced the information provided with inline information: https://scala-lang.org/blog/2022/06/21/scala-3.1.3-released.html#better-error-reporting-in-inlined-code

Is does this provided the feature set that you need for this kind of functionality to be implemented in Metals?

[dos65]

@deusaquilus it doesn't. It' requires more changes in a bunch of interfaces.

I've seen that @ckipp01 started working on that - https://contributors.scala-lang.org/t/revisiting-dotty-diagnostics-for-tooling/5649/17

[deusaquilus]

Interesting. Does metals require SBT changes to Problem.java in order to move forward?

[tanishiking]

@deusaquilus Yes, and it's already done by @ckipp01 :tada: here https://github.com/sbt/sbt/commit/f90b09f1ee3ffd46100773d302d7aff60369f741

[deusaquilus]

Looks like the SBT changes are done. Thanks @ckipp01!! Just curious, how does this affect Metals?

[ckipp01]

Looks like the SBT changes are done. Thanks @ckipp01!! Just curious, how does this affect Metals?

So the idea here is that the inlined positions can be listed as DiagnosticRelatedInformation. So the actual error message would be at the specific error site, and then all of the inlined positions would be linked so that a user could click and jump right to them from the actual diagnostic. You can see a POC of this here.

Right now the compiler just spits out a giant ball of text that has the error, the inlined positions, etc all just in a string. So in Metals and really any tooling utilizing that, they have no option but to somehow parse that info out of the string to do anything meaningful with it. An alternative would be what I think they had very early on where every position had it's own diagnostic, but they weren't related in any way.

[deusaquilus]

An alternative would be what I think they had very early on where every position had it's own diagnostic, but they weren't related in any way.

That totally makes sense and is in line with what I saw in early Dotty versions. Thanks for the explanation! I can’t wait for Metals and other build tools to take advantage of DiagnosticRelatedInformation!!