Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support LSP references for better-monadic-for implicits #393

Open
Z1kkurat opened this issue Feb 6, 2024 · 3 comments
Open

Support LSP references for better-monadic-for implicits #393

Z1kkurat opened this issue Feb 6, 2024 · 3 comments

Comments

@Z1kkurat
Copy link

Z1kkurat commented Feb 6, 2024

Describe the bug

Find references doesn't work for implicits declared with better-monadic-for. Only the reference to the implicit itself is returned. Tried in nvim and vscode.

Steps to reproduce:

  1. Example repository: https://github.com/Z1kkurat/bm4-metals-repro
  2. Open Hello.scala, put the cursor on line 6 (word bar) link
  3. Try to find references

Expected behavior

A list of references to classes/methods an implicit (Bar in the repro example) is passed to (constructors of classes Foo and Baz in the repro example)

Operating system

macOS

Editor/Extension

Nvim (nvim-metals)

Version of Metals

v.1.2.0

Extra context or search terms

Screenshot:
Screenshot 2024-02-06 at 23 01 22

@kasiaMarek
Copy link

Thanks for the report. It seems that bar is not the implicit synthetic symbol created by implicit0. Associating those symbols might require some special casing. We could take another look after scalameta/metals#5940 is merged and finding references for local symbols will be handled by pc.

@kasiaMarek
Copy link

There seem to be 2 problems here:

  1. the usages of bar are not found,
  2. an incorrect reference enclosing the whole for comprehension is found.

1. the usages of bar are not found

In the desugared tree implicits passed to Baz constructors have different symbols then the bar value visible in the code. I think this should be fixed upstream so either all occurrences have the same symbol or are connected to the source symbol via some established way (e.g. special attachment).

E.g. the following code:

// Util.scala
package example
class Baz(implicit b: Bar)
class Bar(i: Int)
// Demo.scala
package example
object Hello {
  for {
    _ <- Some(1)
    implicit0(bar: Bar) = new Bar(1)
    baz = new Baz
    _ <- Some(2)
    baz2 = new Baz
  } yield ()
}

gets desugared into:

// Demo.scala
package example {
  object Hello extends scala.AnyRef {
    def <init>(): example.Hello.type = {
      Hello.super.<init>;
      ()
    };
    scala.Some.apply[Int](1).map[(Int, example.Bar, example.Baz)](((x$1: Int) => {
      <synthetic> <artifact> private[this] val x$3: (example.Bar, example.Bar) = (new Bar.<init>(1): example.Bar @unchecked) match {
        case (x$2 @ (bar @ _)) => scala.Tuple2.apply[example.Bar, example.Bar](x$2, bar)
      };
      val x$2: example.Bar = x$3._1;
      val bar: example.Bar = x$3._2;
      implicit <synthetic> <artifact> val bar$implicit$1: example.Bar = bar;
      val baz: example.Baz = new Baz.<init>(bar$implicit$1);
      scala.Tuple3.apply[Int, example.Bar, example.Baz](x$1, x$2, baz)
    })).flatMap[Unit](((x$6: (Int, example.Bar, example.Baz)) => (x$6: (Int, example.Bar, example.Baz) @unchecked) match {
      case (_1: Int, _2: example.Bar, _3: example.Baz): (Int, example.Bar, example.Baz)(_, (bar @ _), (baz @ _)) => {
        implicit <synthetic> <artifact> val bar$implicit$2: example.Bar = bar;
        scala.Some.apply[Int](2).map[Unit](((x$4: Int) => {
          val baz2: example.Baz = new Baz.<init>(bar$implicit$2);
          ()
        }))
      }
    }))
  }
}

2. an incorrect reference enclosing the whole for comprehension is found

In the generated tree usage of bar ( in implicit <synthetic> <artifact> val bar$implicit$1: example.Bar = bar;) has an incorrect range, which encloses the whole for comprehension. Should be fixed upstream.

@tgodzik
Copy link
Contributor

tgodzik commented Jul 24, 2024

Thanks for investigating @kasiaMarek

I don't think we can do anything as from the compiler perspective the current behaviour is correct. There is no way for us to know bar$implicit$1 and bar are the same symbol from source code perspective. This might be quite hard to fix since it requires some changes in the plugin and possibly the compiler. As such, I will move it to a feature request.

@tgodzik tgodzik changed the title LSP references are not found for better-monadic-for implicits Support LSP references for better-monadic-for implicits Jul 24, 2024
@tgodzik tgodzik transferred this issue from scalameta/metals Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants