Metals: Renaming class hierarchy inside a method produces incorrect results.

[tigerinus]

Describe the bug

To Reproduce Steps to reproduce the behavior:

1. Have following code ready in some Scala function

    sealed trait Symbol
    case class Note(name: String, duration: String, octave: Int) extends Symbol
    case class Rest(duration: String) extends Symbol

    val symbol1: Symbol = Note("C", "Quarter", 3)
    val symbol2: Symbol = Rest("Whole")

1. Right click on Symbol, select _Rename Symbol_ and give it a new name, say S

Got

    sealed trait S
    case class S(name: String, duration: String, octave: Int) extends S
    case class S(duration: String) extends S

    val symbol1: S = Note("C", "Quarter", 3)
    val symbol2: S = Rest("Whole")

Expected behavior

    sealed trait S
    case class Note(name: String, duration: String, octave: Int) extends S
    case class Rest(duration: String) extends S

    val symbol1: S= Note("C", "Quarter", 3)
    val symbol2: S= Rest("Whole")

Installation:

Version: 1.43.1 (user setup)
Commit: fe22a9645b44368865c0ba92e2fb881ff1afce94
Date: 2020-03-18T07:01:20.184Z
Electron: 7.1.11
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Windows_NT x64 10.0.18363

• VSCode extension version: 1.8.4
• Metals version: 0.8.3

[tgodzik]

Hi, thanks for reporting! I can't seem to reproduce, are you maybe using some kind of macro?

If you can provide some additional data or even github repo with a reproduction it would most helpful

[tigerinus]

Hi, thanks for reporting! I can't seem to reproduce, are you maybe using some kind of macro?

If you can provide some additional data or even github repo with a reproduction it would most helpful

Can you try rename the Symbol in the first line? That's what I did.

[tigerinus]

[tgodzik]

Ok, the missing information was the fact it was inside a method. We haven't tested renaming hierarchy inside a method it seem.

I can confirm. Thanks!

[tgodzik]

It should work if you move the class definitions outside the method for now.

[Fabszn]

Hi,
I going to start working on this issue. It the first for me on this project.

[tgodzik]

@Fabszn Thanks! I think the issue might be that we are looking for the implementations of the class, which most likely shouldn't be needed.

My guess is that the issue is here

That condition might not be valid for local symbols. We most likely need to:

• if the symbol is local then we need to check semanticDb for the definition
• if the signature is ClassSignature then we don't look for the implementations

Please take a look at https://scalameta.org/metals/docs/contributors/getting-started.html

Best workflow would be to create a test case in RenameLspSuite and run it in the background via:
sbt ~unit/testOnly tests.RenameLspSuite -- *exact-test-name*

[Fabszn]

thanks @tgodzik for your message. Before post my first message above, I reproduced with success the issue on local test project. I setted up all environment (change code, publish code and test modifications locally). So I am ready to search and fix the issue. Thanks for your tips.

[tgodzik]

@Fabszn Any luck? Do you need some pointers?

[Fabszn]

@tgodzik My apologizes. I started a new mission with my company. Since I started, I was really busy. I want try to fix this issue. I am really sorry for the delay.

[tgodzik]

@Fabszn No worries, I was just wondering if you are still interested or maybe there were some bigger issues. This can wait until you have time for sure.