Skip to content

Code completion at the top of non-empty maps in analysis_options.yaml and pubspec.yaml shows incorrect completions #59827

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

Open
FMorschel opened this issue Dec 30, 2024 · 10 comments
Labels
analyzer-analysis-options area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-completion Issues with the analysis server's code completion feature P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@FMorschel
Copy link
Contributor

Working as intented

If you have on analysis_options.yaml the following:

include: package:lints/recommended.yaml

linter:
  rules:
    - ^

analyzer:
  enable-experiment:
    - ^

On either ^ it will show you the possible rules for lints or possible experiments we have.

If you trigger auto-complete (CTRL + Space) on the left of the file, it will show you the sections; analyzer, linter, code-style, etc.


NOT Working as intended

If you do that with one indentation inside analyzer it still shows the same as the previous paragraph.

If you do that on ^ below:

analyzer:
  language:
    ^

Here is the output:

image

This is a mix of the possible entries for the analyzer section and the entire file-valid sections and (at the end because this is alphabetically sorted) the valid options for the analyzer.language section (strict-inference, strict-casts, etc).


Also, on another note, we can't do with the analyzer.language section what we can with the linter.rules section of posting them on a list. We need to add each one with : true (again, the auto-complete here is totally lost when it would show true/false options it shows a lot more).


Related to #57034.

@FMorschel FMorschel added the legacy-area-analyzer Use area-devexp instead. label Dec 30, 2024
@FMorschel
Copy link
Contributor Author

FYI @srawlins @DanTup @bwilkerson

@keertip keertip added type-enhancement A request for a change that isn't a bug analyzer-analysis-options P3 A lower priority bug or feature request labels Jan 2, 2025
@DanTup
Copy link
Collaborator

DanTup commented Jan 6, 2025

Do you have a complete file sample that triggers this? When I try reproducing, I see this:

image

@DanTup
Copy link
Collaborator

DanTup commented Jan 6, 2025

Part of the difference might be due to me having "editor.wordBasedSuggestions": "off" off. By default, VS Code will add suggestions for all the words it finds in the document (I think this is poor behaviour myself). However, even with that enabled, the case above works correctly (but all the words from the document do show up in some other places).

@FMorschel
Copy link
Contributor Author

Not on the same OS, will test it out later today on that same project. But there is still this problem with the parts for the analyzer scope:

image


Also,wordBasedSuggestions is set to off on Dart -> User

image

But I'm unsure of what it means. Would it only be set to off on .dart files?

@DanTup
Copy link
Collaborator

DanTup commented Jan 6, 2025

But I'm unsure of what it means. Would it only be set to off on .dart files?

Correct. I have it set to off in my VS Code User Settings so it's disabled for all languages, because IMO it's never useful. I think if you turned it off globally (or for YAML), you might be seeing empty results in the places where you see those (I think it might only appear as a fallback if the server doesn't return anything?).

I can reproduce this one:

image

@FMorschel
Copy link
Contributor Author

Part of the difference might be due to me having "editor.wordBasedSuggestions": "off"

I`ve tested it again and I can confirm that was it.

I've also confirmed that the case you managed to repro above only happens if you already have one of the internal options for analyzer later.

image

If you trigger autocomplete after, it still thinks you're at the last spot:

image

See that there is no auto-complete option for true/false here and as I mentioned at the start, for some reason, this is not coherent with the linter->rules section where we can use a yaml list.

image

image

@DanTup
Copy link
Collaborator

DanTup commented Feb 11, 2025

I had a quick look at this thinking it'd be a trivial fix, but I don't think it is. The reason it fails is that in code like:

analyzer:
  ^

The AST for the file looks like {analyzer: null} and we return the null node which allows us to provide completion for the 1analyzer` section.

However, in the caseof:

analyzer:
  ^
  foo: bar

Here, the AST looks like { analyzer: { foo: bar } } and therefore there is no equivalent of the null node above, so we don't have a good node to base the completion on (we can't return analyzer, because completion for analyzer would still be the parent completions, which include analyzer but are not children of it).

So I think it'll need a little reworking of YamlCompletionResults._producerForPath so that we're not just trying to compute completion based on some node, but support producing completion for the children of a node (which probably also means representing this in some way in the path.. perhaps as a synthetic key?).

In the meantime, I did open a fix for the missing booleans for the strict- language options at https://dart-review.googlesource.com/c/sdk/+/409140 though.

@FMorschel
Copy link
Contributor Author

Doing so would probably help with #57034, too.

Is your change enough for me to use the strict- language options as a list? Or is that something the team would like to avoid for some meaning @bwilkerson? I feel like these are close enough to how lints work today so they could be added as a list that would mean true for all.

copybara-service bot pushed a commit that referenced this issue Feb 11, 2025
…tions in analysis_options

Fixes part of #59827, but not the main issue.

Change-Id: I7f9d6cfd5163a1bf6c72f9eea4bb71872d3dfa15
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409140
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
@bwilkerson
Copy link
Member

I believe that the medium to long term goal is to replace the language options with lints. I don't know what the time frame is for that, so I don't know whether it's worth the effort.

@srawlins

@DanTup
Copy link
Collaborator

DanTup commented Feb 12, 2025

Is your change enough for me to use the strict- language options as a list?

No, all I did was make the completion for the existing (map) entries provide boolean completions instead of no completions.

Since it just occurred to me, I'll also note the original issue described above also affects Pubspec (eg. you get "dependencies:" in completion here instead of package names):

Image

@DanTup DanTup changed the title Auto complete on analysis option not working correctly Code completion at the top of non-empty maps in analysis_options.yaml and pubspec.yaml shows incorrect completions Feb 12, 2025
@bwilkerson bwilkerson added devexp-completion Issues with the analysis server's code completion feature area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-analysis-options area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-completion Issues with the analysis server's code completion feature P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants