Skip to content

Ranking/sorting of code completion is worse with suggestion sets #38739

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
DanTup opened this issue Oct 7, 2019 · 30 comments
Closed

Ranking/sorting of code completion is worse with suggestion sets #38739

DanTup opened this issue Oct 7, 2019 · 30 comments
Labels
devexp-completion Issues with the analysis server's code completion feature legacy-area-analyzer Use area-devexp instead. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@DanTup
Copy link
Collaborator

DanTup commented Oct 7, 2019

This was raised in Dart-Code, but I think needs to be fixed in the server. I thought there was already an open issue about this here but cannot locate it. I don't know if it's easily fixable, but seems like we should have an issue tracking it here.

This is what completion looked like before:

Screenshot 2019-05-29 at 10 18 53 am

And this is what it looks like using the new auto-import completions:

Screenshot 2019-05-29 at 10 24 04 am

Based on comments from @scheglov I think the issue here is that the declared type of the property does not match the one the user wants (because it just uses the parsed AST) and therefore doesn't get ranked at the top.

@maRci002
Copy link

maRci002 commented Oct 13, 2019

To sum up there was a community request to enable completion for non imported identifiers and problems occur if the client subscribe for completion service:

request: {
  "id": "some_id"
  "method": "completion.setSubscriptions"
  "params": {
    "subscriptions": ["AVAILABLE_SUGGESTION_SETS"]
  }
}

Thanks to @scheglov the problem was helped a bit on the dev channel but the relevance is still high in the local file for all the identifiers.

  • current behavior on dev channel
    1. list all identifiers from current file
    2. list every other identifiers from other files sorted by relevance order
  • old behavior
    1. list all identifiers from current file
    2. list every other identifiers from other files sorted by alphabetical order

local 1

Current workarounds (if you are satisfied with the manual import):

  • Visual Studio Code:
    • uncheck: Auto Import Completions ( "dart.autoImportCompletions": false, )
  • Android studio /IntelliJ
    • make sure you are using at most IntelliJ IDEA 2018.3.6 / Android Studio 3.4.2
    • you can use newest Android Studio / IntelliJ but you have to compile Dart plugin yourself with a little modification: comment this line
      startedServer.completion_setSubscriptions(ImmutableList.of(CompletionService.AVAILABLE_SUGGESTION_SETS));

@NikolnikovK
Copy link

Any follow up on this issue ?

@bwilkerson
Copy link
Member

Yes, we are actively discussing how best to address this situation and I think we're getting close to having a plan. I don't know how long it will take to implement, but it should get better eventually.

@NikolnikovK
Copy link

It's been quite some time, and it's getting worse.
Now, even with auto-import on 'OFF' in VS Code, the code completion does not work.
Maybe I missed a news, would be great to at least have a work-around.

@DanTup
Copy link
Collaborator Author

DanTup commented Jan 7, 2021

Edit: Moved this comment to #43657 (comment), since that issue seemed to better describe my repro, whereas this one was specifically about suggestion sets ranking.

@fisforfaheem
Copy link

Hope it gets fixed soon :(

@gaetschwartz
Copy link

Any updates @DanTup ?

@waleedf112
Copy link

Current workarounds (if you are satisfied with the manual import):

  • Visual Studio Code:

    • uncheck: Auto Import Completions ( "dart.autoImportCompletions": false, )

the workaround doesn't work after Flutter 1.20.4.
I've kept using 1.20.4 hoping that with the release of Flutter 2.0.0 it gets resolved, unfortunately it didn't :(

@DanTup
Copy link
Collaborator Author

DanTup commented Mar 4, 2021

It's a bug that the dart.autoImportCompletions setting in VS Code is being ignored for LSP - I've filed Dart-Code/Dart-Code#3177 to fix that.

@waleedf112
Copy link

It's a bug that the dart.autoImportCompletions setting in VS Code is being ignored for LSP - I've filed Dart-Code/Dart-Code#3177 to fix that.

I've tried to set LSP and "dart.autoImportCompletions" to false.
didn't work either.

@DanTup
Copy link
Collaborator Author

DanTup commented Mar 4, 2021

@waleedf112 sorry, you seem to be correct - disabling LSP and auto-import-completions does still result in worse ranking than the screenshot shown at the top here.

@bwilkerson I'm not sure if this is known or a new/different bug. My understanding was that the ranking with SuggestionSets wasn't as good because of less type information, though with suggestion sets disabled the padding example above is ranking EdgeInsets further down that it did:

Screenshot 2021-03-04 at 17 48 30

{
	"kind": "INVOCATION",
	"relevance": 517,
	"completion": "EdgeInsets.all",

{
	"kind": "INVOCATION",
	"relevance": 564,
	"completion": "hashCode",

@bwilkerson
Copy link
Member

I'm not sure if this is known or a new/different bug.

Me either.

My understanding was that the ranking with SuggestionSets wasn't as good because of less type information ...

It's true that suggestion sets don't currently contain enough information to use type information, so that seems like a likely cause for the relevance results we were seeing.

This certainly suggests that there's also a problem when we aren't using suggestion sets. Unfortunately, because of having two code paths for suggesting EdgeInsets.all there could easily be multiple issues.

@fisforfaheem
Copy link

fisforfaheem commented Mar 4, 2021 via email

@bwilkerson
Copy link
Member

I'm sorry the tools are causing frustration, and I completely understand why.

I've done some digging and I have found two issues that impact this behavior:

  • It's giving too much weight to members defined on the implied target of this.
  • It's not using the type of the parameter to give higher priority to static members.

I have a couple of ideas for how to solve the first issue, and I have just landed a CL to address the second issue (https://dart-review.googlesource.com/c/sdk/+/190281).

@fisforfaheem
Copy link

fisforfaheem commented Mar 14, 2021 via email

@bwilkerson
Copy link
Member

bwilkerson commented Mar 15, 2021

I have attempted to improve the first part of the issue with https://dart-review.googlesource.com/c/sdk/+/190581. Unfortunately I am not yet seeing any improvements in IntelliJ, which uses available declaration support.

@fisforfaheem
Copy link

fisforfaheem commented Mar 18, 2021 via email

@waleedf112
Copy link

new progress in the master channel, code completion is better now, yet not perfect.

when viewing code suggestions, now I see the appropriate values:

image

a small and minor problem, if the suggested value was a function call, for example:

image
image

it completes it as an Enum or a Type, not a function call like this:

image
image

Flutter version: 2.1.0-13.0.pre.390
Dart version: 2.13.0 (build 2.13.0-190.0.dev)
Flutter extension version: 3.21.0

dart settings:

  "dart.previewLsp": true,
  "dart.autoImportCompletions": false

finally, I can upgrade from Flutter 1.20.4 😂.
thank you guys, I hope it will get better and better.

@fisforfaheem
Copy link

fisforfaheem commented Apr 2, 2021 via email

@fisforfaheem
Copy link

if u goto flutter master channel things do get better just try it

@rajeshzmoke
Copy link

new progress in the master channel, code completion is better now, yet not perfect.

when viewing code suggestions, now I see the appropriate values:

image

a small and minor problem, if the suggested value was a function call, for example:

image
image

it completes it as an Enum or a Type, not a function call like this:

image
image

Flutter version: 2.1.0-13.0.pre.390
Dart version: 2.13.0 (build 2.13.0-190.0.dev)
Flutter extension version: 3.21.0

dart settings:

  "dart.previewLsp": true,
  "dart.autoImportCompletions": false

finally, I can upgrade from Flutter 1.20.4 😂.
thank you guys, I hope it will get better and better.

can u post the full settings.json ?

@fisforfaheem
Copy link

fisforfaheem commented May 10, 2021 via email

@srawlins
Copy link
Member

@bwilkerson did you do work on this recently?

@bwilkerson
Copy link
Member

Yes. We're no longer using available suggestions data for libraries that are explicitly imported by the library in which completion is occurring. We are still using it for libraries that could be imported but aren't yet. The recent work should improve the situation but doesn't necessarily solve the whole problem.

@fisforfaheem
Copy link

fisforfaheem commented May 27, 2021 via email

@fisforfaheem
Copy link

fisforfaheem commented Jun 19, 2021 via email

@srawlins srawlins added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Nov 16, 2021
@KlausJokisuo
Copy link

Any news on working fix for IntelliJ also?

@bwilkerson
Copy link
Member

@scheglov Is experimenting with some changes that ought to improve code completion response time on IntelliJ, and this might have also have some benefits in terms of the ranking. Given the end-of-year holidays it isn't clear when we'll have those results, but we are continuing to work on improving code completion.

@DanTup
Copy link
Collaborator Author

DanTup commented Dec 15, 2022

The original example at the top is now much better (at least in Flutter master), although similar to a comment I just added in #43657, Null is oddly near the top, and the constructors might be a better choice than the types (especially since any named constructors are already in the list fully-qualified).

Screenshot 2022-12-15 at 18 34 28

@DanTup
Copy link
Collaborator Author

DanTup commented Sep 13, 2023

I believe this issue is redundant now, as we don't use suggestion sets at all. Not-yet-imported completions are included (and ranked) using the same mechanism as other completions now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-completion Issues with the analysis server's code completion feature legacy-area-analyzer Use area-devexp instead. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
No open projects
Development

No branches or pull requests