Skip to content

Representation of code completion suggestions #11313

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

Closed
prolativ opened this issue Feb 4, 2021 · 2 comments · Fixed by #12029
Closed

Representation of code completion suggestions #11313

prolativ opened this issue Feb 4, 2021 · 2 comments · Fixed by #12029

Comments

@prolativ
Copy link
Contributor

prolativ commented Feb 4, 2021

There is some discrepancy between how code completion works with Metals for scala 2 and for scala 3 if a suggested name refers to multiple definitions. Consider the following examples

object Test1 {
  object Foo {
    def xxxx(i: Int): Int = 123
    def xxxx(s: String): String = ""
  }
  val x = Foo.xx
}

object Test2 {
  object Foo {
    def xxxx(i: Int): Int = 123
    def xxxx(s: String): String = ""
  }
  import Foo.xx
}

object Test3 {
  object Foo {
    type xxxx = Int
    def xxxx(i: Int): Int = 123
  }
  import Foo.xx
}

object Test4 {
  object Foo {
    type xxxx = Int
    def xxxx(i: Int): Int = 123
    def xxxx(s: String): String = ""
  }
  import Foo.xx
}

The completions suggested for Foo.xx are show in the table

test case scala 2 scala 3
Test1 xxxx(i: Int): Int xxxx(s: String): String
xxxx(s: String): String
Test2 xxxx(i: Int): Int xxxx(s: String): String
xxxx(s: String): String
Test3 xxxx(i: Int): Int type and method xxx
Test4 xxxx(i: Int): Int type and method xxx
xxxx(s: String): String

Beside some apparent bugs (completely ignoring some definitions) that will need to be reported separately, we can see here 2 strategies of dealing with overloaded symbols: either displaying them as separate entries or merging them into one. The former approach gives users more information about the meanings of a name in a given context (which is especially useful when dealing with overloaded methods), the latter is more concise (having to go through multiple entries with the same name might get frustrating for TAB completion in REPL).

Was this discrepancy introduced on purpose or would we rather like to preserve consistency between the two versions of the language?

Personally I would go for taking scala 2's behaviour, which seems to work well for IDEs with LSP, and for the REPL this could be later improved by doing what Ammonite does: merging the identically named entries into one for TAB completion but then showing full signatures with alternatives on pressing TAB again.

@tgodzik @smarter what do you think?

@tgodzik
Copy link
Contributor

tgodzik commented Feb 4, 2021

I think having multiple symbols in one completion should be fine. The current behaviour is just very basic and we haven't changed it almost at all from the Dotty LSP Server. We still have work to do to have it all on the same level as Scala 2 features.

@prolativ
Copy link
Contributor Author

prolativ commented Feb 4, 2021

I'm more interested in what would be the the best behaviour rather than whether we can live with what we have now. As it turned out that I need to slightly change the way how code completions are generated internally to do the reworks requested for my PR https://github.com/lampepfl/dotty/pull/11187/files it looks like splitting multiple definitions with the same name into separate entries like it's done for scala 2 seems trivial so I wondered whether this could be changed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants