Metals: "Rename symbol" inlines methods to their implicit parameters

[kubukoz]

Describe the bug

When renaming a value that's used as an implicit parameter, the value is inlined at the position of the call to the method taking it implicitly.

To Reproduce
Steps to reproduce the behavior:

1. Compile this:

object foo {
  implicit val x: Int = ???

  def foo[A](implicit a: A): (A, A) = (a, a)

  foo[Int]
}

1. Rename x to anything:

1. Watch the result:

object foo {
  implicit val y: Int = ???

  def foo[A](implicit a: A): (A, A) = (a, a)

  y
}

Expected behavior

The call site of the method taking implicits isn't changed (unless the parameter is passed explicitly).

Installation:

• Operating system: macOS/Windows/Linux
• Editor: Visual Studio Code/Atom/Vim/Sublime/Emacs
• Metals version: current master (eecfc753249b34ecf64e54d57f510eab1dd617e1)

Additional context

If the argument is passed explicitly, everything is fine. If it's passed both implicitly and explicitly, like in:

object foo {
  implicit val x: Int = ???

  def foo[A](a: A)(implicit a2: A): (A, A) = (a, a2)

  foo(x)
}

Renaming results in:

Search terms
Rename symbol, implicit, inline, parameter

[kubukoz]

I'd love to help fix this, but I'll need pointers as to where to look :)

[kubukoz]

I gave the code a brief look, and from what I see there's no easy way to tell whether the call-site is an implicitly inserted parameter. So the entire position of foo[Int] is replaced with the new name.

[gabro]

Thanks for reporting @kubukoz and thanks for the detailed issued with a minimal reproduction 🙏

The local rename expert is @tgodzik, so he can probably point to where to fix this

[tgodzik]

Thanks for reporting @kubukoz and thanks for the detailed issued with a minimal reproduction

The local rename expert is @tgodzik, so he can probably point to where to fix this

Not sure exactly what's going, I will take a look as soon as I back from my very short holiday :sweat_smile:

[gabro]

I suspect this might be a bug in semanticdb occurrences.

One simple way to verify this is to see whether foo[Int] appears as a result if you do "find references" on x. @kubukoz could you confirm this if you have a minute?

In the meantime, enjoy your holiday @tgodzik!

[kubukoz]

It does appear there indeed, and the highlighted position appears to be the entire method call:

[gabro]

@kubukoz thanks for confirming!

@olafurpg should we move this to scalameta/scalameta?

[olafurpg]

The output of SemanticDB seems to be correct, from the example with one explicit y and one implicit y

Occurrences:
...
[5:2..5:5) => _empty_/foo.foo().
[5:6..5:9) => scala/Int#
[5:11..5:12) => _empty_/foo.y.

Synthetics:
[5:2..5:13) => *(y)

Rename symbol should ignore all synthetics except when renaming .apply methods.

[gabro]

Ah, thanks for checking. That's actually good news and it should an easy fix. What do you think about the behavior of find references? It's a valid occurrence, although the range is not super intuitive. Working as intended?

[kubukoz]

It's showing up quite reasonably in "find references"... I think it would be best if we could keep that behavior and only change the renaming part.