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

Method calls on unresolved/invalid variables are coloured inconsistently #3412

Closed
ykmnkmi opened this issue Jun 17, 2021 · 4 comments
Closed
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug
Milestone

Comments

@ykmnkmi
Copy link

ykmnkmi commented Jun 17, 2021

Describe the bug
Extension method color depends on list type.

To Reproduce

void main() {
  var list = <dynamic>[1, 2, 3];
  list[0].equals(1); // <- blue

  var list2 = <int>[1, 2, 3];
  list2[0].equals(1); // <- yellow
}

extension ExpectExtension on Object? {
  void equals(Object? value) {
    assert(this == value);
  }
}

Expected behavior
Same colors.

Screenshots
image

Versions (please complete the following information):

  • VS Code version: 1.57.0
  • Dart extension version: v3.23.0
  • Dart/Flutter SDK version: 2.14.0-152.0.dev
@ykmnkmi ykmnkmi added the is bug label Jun 17, 2021
@DanTup DanTup changed the title Extension method color depends on list type. Methods called on dynamic are not coloured as methods Jun 17, 2021
@DanTup DanTup added in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server labels Jun 17, 2021
@DanTup DanTup added this to the v3.24.0 milestone Jun 17, 2021
@DanTup
Copy link
Member

DanTup commented Jun 17, 2021

Thanks for the report! This appears to be due to the dynamic - all members are coloured as variables even when they're invocations:

Screenshot 2021-06-17 at 11 44 36

@DanTup
Copy link
Member

DanTup commented Jun 17, 2021

I've a fix out for review at https://dart-review.googlesource.com/c/sdk/+/203940/. With that, it looks like this:

Screenshot 2021-06-17 at 11 47 29

The fix is in the Dart SDK, so after merging, it'll show up in a future Dart/Flutter SDK release (rather than a VS Code extension release).

@DanTup DanTup changed the title Methods called on dynamic are not coloured as methods Methods called on dynamic are not coloured correctly Jun 17, 2021
@DanTup
Copy link
Member

DanTup commented Jun 17, 2021

After some back and forth, I'm no longer certain this is a bug (or at least, the originally described bug). There were some weird inconsistencies after making this change when it came to invalid code. Invalid methods are intended not to be coloured. Given the code:

int a;
a.foo().foo();

The first foo is known to be invalid and should be uncoloured. The second foo is unresolved (we don't know what type it is). It seems incorrect to colour the second one like a method but the first one uncoloured.

So I think for consistency we should fix both unresolved and invalid members to be uncoloured (this was already sort-of happening for some, but the textmate grammar was filling them in and hiding this). This means methods that are definitely wrong or might be wrong (eg. the compiler can't validate them, and you also can't "jump to definition" etc.) will be uncoloured, like this:

Screenshot 2021-06-17 at 19 58 03

If there is sufficient desire to colour unknown and unresolved methods like normal resolved methods, this may be something to look at - although it does mean that known invalid methods (eg. you try to call foo on an int) would also be coloured - making the highlighting a little less semantic.

dart-bot pushed a commit to dart-lang/sdk that referenced this issue Jun 17, 2021
…SP semantic tokens

See Dart-Code/Dart-Code#3412.

Change-Id: Icc44ff0537122d6d2211d64037666ecb58a28bd6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203940
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
@DanTup DanTup changed the title Methods called on dynamic are not coloured correctly Method calls on unresolved/invalid variables are coloured inconsistently Jun 28, 2021
@DanTup
Copy link
Member

DanTup commented Jun 28, 2021

I repurposed this slightly to better match the new behaviour - that is, unresolved/invalid types will be left uncoloured (note: this requires an SDK update).

This means dynamic items will still be uncoloured (which was the original complaint), but with semantic tokens I think this makes sense - the compiler is unable to resolve them and leaving them coloured is a good indicator of that.

If you want the original regex-based colouring, you can disable semantic tokens (using the editor.semanticHighlighting.enabled setting). Generally, I'd try to avoid using dynamic where possible though - it's easy to create code that would be easy to make fail at runtime without any analysis errors.

Hope this makes sense!

@DanTup DanTup closed this as completed Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug
Projects
None yet
Development

No branches or pull requests

2 participants