scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.08k stars 330 forks source link

Cannot rename symbol in for comprehension for `F[_]` #4187

Open kpodsiad opened 2 years ago

kpodsiad commented 2 years ago

Describe the bug

  1. Try to rename renameMe in foo and foo_2
  2. Renaming only works in the seconds case
//> using scala "2.13.8"
//> using lib "org.typelevel::cats-core:2.7.0"

import cats.Monad
import cats.syntax.all._

object Renaming {
  // doesn't work
  def foo[F[_]: Monad] = {
    val renameMe = Monad[F].pure(5)
    for {
      first <- Monad[F].pure(5)
      second <- renameMe
    } yield first + second
  }

  // works
  def foo_2 = {
    val renameMe = Option(2)
    for {
      first <- Option(5)
      second <- renameMe
    } yield first + second
  }
}

Expected behavior

Renaming work for both cases.

I took a quick look at semanticDB and there is no occurrence for the usage of <- renameMe for type F.

local0 => val local renameMe: F[Int]
...
local3 => val local renameMe: Option[Int]
...
(occurrences)
[8:8..8:16) <= local0
...
[16:8..16:16) <= local3
...
[19:16..19:24) => local3
...

Operating system

macOS

Editor/Extension

VS Code

Version of Metals

0.11.7+59-4322516c-SNAPSHOT

Extra context or search terms

renaming refactor

tgodzik commented 2 years ago

Thanks for reporting. This might need a fix in scalameta :thinking:

jkciesluk commented 1 year ago

It was fixed by implementing PcRenameProvider for local symbols. @tgodzik do you think we can close?

tgodzik commented 1 year ago

That does improve the situation, however foo is not local, so unfortunately this will not work :/

jkciesluk commented 1 year ago

But we are renaming renameMe inside method foo, which is local

tgodzik commented 1 year ago

But we are renaming renameMe inside method foo, which is local

Ach, sure, but if we move it outside it will stop working for example:

//> using scala "2.13.10"
//> using lib "org.typelevel::cats-core:2.7.0"

import cats.Monad
import cats.syntax.all._

class Renaming[F[_]: Monad] {
  // doesn't work
  val renameMe = Monad[F].pure(5)
  def foo = {
    for {
      first <- Monad[F].pure(5)
      second <- renameMe
    } yield first + second
  }

  // works
  def foo_2 = {
    val renameMe = Option(2)
    for {
      first <- Option(5)
      second <- renameMe
    } yield first + second
  }
}

I would keep this open since there is still something weird going on.

jkciesluk commented 1 year ago

Oh right, I though the issue was connected to local# symbols. For now the only solution I see is to collect occurrences from both presentation compiler and semantic db for rename and not fallback to PC only for local symbols

tgodzik commented 1 year ago

Most likely though this needs a fix in semanticdb unfortunately to work.