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

Issue completing static members #2982

Closed
safield opened this issue Dec 4, 2020 · 4 comments
Closed

Issue completing static members #2982

safield opened this issue Dec 4, 2020 · 4 comments
Labels
in lsp/analysis server Something to be fixed in the Dart analysis server is bug
Milestone

Comments

@safield
Copy link

safield commented Dec 4, 2020

VsCode Info
Version: 1.51.1
Commit: e5a624b788d92b8d34d1392e4c4d9789406efe8f
Date: 2020-11-10T23:31:29.624Z
Electron: 9.3.3
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Linux x64 5.4.0-56-generic

Dart Extension Version: 3.17.0

Observe the following code...

class TestClass2 {

}

enum TestEnum {

}

class TestClass {
  static Test
}

When typing "static Te...", no completion options are proposed.

Expected behaviour is take the class and enum starting with Test would be offered as suggestions.

@safield safield added the is bug label Dec 4, 2020
@DanTup DanTup added this to the v3.18.0 milestone Dec 7, 2020
@DanTup DanTup added the in lsp/analysis server Something to be fixed in the Dart analysis server label Dec 7, 2020
@DanTup
Copy link
Member

DanTup commented Dec 17, 2020

@bwilkerson I had a look at this and can repro like this:

class MyClass1 {}

class MyClass2 {
  static MyCl^
}

If I insert the word final after static then the type names show up.

The difference seems to be that the targetNode here is a VariableDeclarationListImpl in the case that works (it gets includeTypeNameSuggestions set to true here) but it's a FieldDeclarationImpl in the case that doesn't (which skips setting includeTypeNameSuggestions here because the check offset <= node.fields.offset fails - the caret offset is 49 and node.fields.offset is 45).

If I understand correctly, it may be interpreting MyCl as the field name and not a type? Although I'm not sure whether it's valid to just write static foo without a type (it seems to produce missing_const_final_var_or_type). Any ideas?

@DanTup DanTup modified the milestones: v3.18.0, v3.19.0 Dec 17, 2020
@bwilkerson
Copy link

Interesting. I suspect that it is related to recovery. I don't know what's going on specifically, but you might be right that the parser is inserting a synthetic var after static and taking MyCl to be the name of the field.

We should probably change the completion engine to set includeTypeNameSuggestions in cases where the FieldDeclaration has neither a type nor a keyword, but we'll want to ensure that this situation can only occur when there's an error in user code.

@DanTup
Copy link
Member

DanTup commented Jan 18, 2021

@bwilkerson I think I have a fix here:

https://dart-review.googlesource.com/c/sdk/+/179771/

Although I'm not sure if there's anything specific needed for this:

we'll want to ensure that this situation can only occur when there's an error in user code

There were some existing tests that cover some similar situations - for example this test seems to ensure we don't suggest the type name when it's already there, and also goes through the FieldDeclaration path.

If you can think of anything that might be tripped up by this, let me know. I think all the tests pass, though when I try to run all of the tests I'm hitting a VM crash (I'll file a separate issue about that) so I'm not completely positive just yet.

@DanTup
Copy link
Member

DanTup commented Jan 19, 2021

This is fixed by dart-lang/sdk@d901213. The change is in the analysis server so will show up in an SDK update rather than a Dart-Code update.

@DanTup DanTup closed this as completed Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in lsp/analysis server Something to be fixed in the Dart analysis server is bug
Projects
None yet
Development

No branches or pull requests

3 participants