Skip to content
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

Make analysis server/IDEs show the distinction between fields and getters accurately and consistently #55956

Open
DanTup opened this issue Jun 7, 2024 · 0 comments
Labels
analyzer-lsp Issues with analysis server's support of Language Server Protocol analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@DanTup
Copy link
Collaborator

DanTup commented Jun 7, 2024

In Dart-Code/Dart-Code#5011 it was highlighted that in some places in VS Code (via LSP) we are inconsistent about when we map things to a "Property" kind vs a "Field" kind. In some places (for example code completion) we use "Field" where the static type has an implicit getter that just points at a field, but in others (like semantic tokens) we do not.

We feel the right thing to do here is to follow the language accurately, so when something is a reference to a getter, we should always show it as such (a "Property" in LSP) even if in the static type it's just a field. Showing something as a Field is inconsistent with the spec and could lead to incorrect assumptions (since it could actually be an overridden getter at runtime).

For LSP, this means we should only map to the "Field" kinds for actual references to fields (like constructor initializers, field formals, and the field declaration in something like document outline) and other places (semantic tokens, hovers, code completion) should use "Property".

(@jwren I suspect all changes here are either LSP-specific, or in base server functionality that will be updated in order to fix this for LSP, but FYI just in case there might be things in the IntelliJ plugin that might need reviewing).

(fyi @bwilkerson @stereotype441)

@DanTup DanTup added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-server analyzer-lsp Issues with analysis server's support of Language Server Protocol labels Jun 7, 2024
@srawlins srawlins added type-enhancement A request for a change that isn't a bug P3 A lower priority bug or feature request labels Jun 7, 2024
copybara-service bot pushed a commit that referenced this issue Sep 11, 2024
Also fixes top-level variable declarations to be treated as variables and not properties (see #55956).

Fixes #56567

Change-Id: I899ce4fc5950369acae70517d76a2830fe082731
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384746
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-lsp Issues with analysis server's support of Language Server Protocol analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. 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

2 participants