Skip to content

Wrong details and symbols returned by Completions #11941

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
tgodzik opened this issue Mar 29, 2021 · 6 comments
Closed

Wrong details and symbols returned by Completions #11941

tgodzik opened this issue Mar 29, 2021 · 6 comments
Assignees

Comments

@tgodzik
Copy link
Contributor

tgodzik commented Mar 29, 2021

Compiler version

3.0.0-RC2

Minimized code

For:

object A {
  java.nio.file.Files.readAttributes@@
}

when using:

Completion.completions(pos)

where pos is set at @@

Output

Completion(
  readAttributes,
  method readAttributes,
  List(method readAttributes, method readAttributes)
)

Expectation

Previously for 3.0.0-RC1 we were getting:

List(Completion(
  readAttributes,
 (x$0: java.nio.file.Path, x$1: String, x$2: java.nio.file.LinkOption*):  java.util.Map[String, Object],
 List(method readAttributes))
)
@prolativ
Copy link
Contributor

@tgodzik this is related to the issue that I have raised #11313 but I haven't received a final answer for.
The method readAttributes is overloaded but previously only one of the signatures was displayed in the description (the other was lost because of a bug in the previous implementation of completions) which is misleading, suggesting that there is indeed only one signature. So I made this more uniform with other cases when one name corresponds to multiple definitions (e.g. a class and a val) with the difference that the description is rendered as method readAttributes rather than method and method readAttributes

@prolativ
Copy link
Contributor

As agreed with @tgodzik to make this work properly in Metals (as it does for scala 2) descriptions of symbols should not be merged. Instead, in this case, the list should contain 2 separate Completion items, each having its full signature in its description and only one element in its list of symbols. I think Ammonite would benefit from this approach, am I right @alexarchambault?
@smarter is the freeze of 3.0.0 branch as strict as described in the blogpost? I believe this change would directly affect only Metals and Ammonite but all the users would benefit from this working correctly in these tools

@smarter
Copy link
Member

smarter commented Mar 30, 2021

this is related to the issue that I have raised #11313

Seems like this is a duplicate of #11313 and we should close this one indeed.

is the freeze of 3.0.0 branch as strict as described in the blogpost?

It is, we can discuss this particular case on Monday but given that #11313 was a known issue and it wasn't raised as a blocker for RC2, I'd say it's too late for 3.0.0 (but that's fine, it'll only be six weeks until 3.0.1).

@smarter smarter closed this as completed Mar 30, 2021
@smarter
Copy link
Member

smarter commented Mar 30, 2021

but I haven't received a final answer for.

Were you waiting for me to give an answer by the way? I didn't reply to this thread because I saw @tgodzik was participating already, and also I stopped maintaining dotty.tools.dotc.interactive ever since we abandoned the Dotty Language Server, so please don't rely on me for anything important!

@prolativ
Copy link
Contributor

@smarter Don't worry, I expected rather @tgodzik to make the decision as well

@alexarchambault
Copy link
Contributor

I think Ammonite would benefit from this approach, am I right @alexarchambault?

I guess it should help there too, yes.

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

No branches or pull requests

4 participants