Elements names are shadowed on autocomplete and usability #56821
Comments
|
Summary: The issue reports that Dart's autocomplete doesn't clearly indicate name shadowing, making it difficult for users to identify and resolve conflicts between variables, imports, and extensions. The user suggests improving autocomplete by differentiating shadowed elements, potentially by adding a visual cue or a "this." prefix. |
There's no completion suggestion because there's no way to access it. That is, there's no valid code you can write at that location to get around the fact that the local declaration hides the top-level declaration.
Yes, we could add a case for that. I would only want to suggest that form when the extension method can't be reached because it's hidden. It's not a form of code that we want to promote. I'm not sure code completion is written in such as way as to make it performant to add this support. Something that would require some investigation.
I'd be ok with that, but again, only in cases where the |
Yes, I know. The point I was trying to bring here was that we could maybe think of a way for us to tell the user things are being hidden. I'm not sure how, but that is the gist of this issue. Also, if someone can think of a better name for this please feel free to update it, I could not think of a name that would satisfy me. |
|
I like the idea of including I'm less sure there's a simple solution to accessing the local function though. I think there is a way it could work, but it's weird and I don't know if it would be used enough to be worthwhile making it work. You can use I'm not sure if it would be very obvious to users if they saw |
|
If the problem is shadowing, then we could potentially introduce a lint to flag all shadowing (we do have lints for some specific kinds of shadowing). Or we could add a highlight modifier to allow users to color declarations that shadow another declaration. But I don't think that code completion is the right way to solve this particular problem. |
|
I like both of those ideas - a lint would allow you to forbid having conflicting names if you wanted, or you could use colouring if you just wanted them to be more visible in code :-) |
It could solve part of this problem, as we discussed, but probably you're right that it will not solve all of it. Maybe a lint, as you said, when the user writes FYI:
There is a discussion on using this kind of workarounds here: Should "self" importing trigger unnecessary_import |
I found only |
|
I was thinking about Dart-Code/Dart-Code#5242 where we added the two suggestions (both prefixes) on completion: import 'my_class_name.dart' as i;
import 'second.dart' as s;
void main() {
f2(My^);
}
void f1(i.MyClassName c) {}
void f2(s.MyClassName c) {}Something I'd like to suggest for the above case, could we also add import 'a.dart' as bar;
import 'thing.dart';
class C extends A {
void foo(int value) {
this.bar.isEven;
bar^
}
}I'd vote for it to also happen when shadowed by local variables, too. I'm not sure it would be easy, but it would tell the user this is something that exists and you can access it. Of course, it would go away if the user writes |
By "potentially introduce" I meant that we could implement a new lint to cover this case.
I think that would be reasonable. And yes, it should be offered no matter what kind of declaration is shadowing the member. |
|
I'm not sure if this was ever considered, but what about cases like this: abstract class A {
int value = 0;
void m() {
value^;
}
}
extension FirstExtension on A {
int value() => 0;
}
extension SecondExtension on A {
int get value => 0;
}
void f(A Function() a) {
a().value;
}I'm sure it is pretty unusual, and if we did that, we might want to handle inside This would be especially great because if we remove Edit: I'm not even sure this is possible for auto-complete, just a suggestion. I think @DanTup once talked to me about not being able to edit anything written before it, so I don't think it is. |
After a bit more thinking, I think for the example I gave above, it is possible. When nothing is written before, not even |
We already have fixes to resolve this issue, I don't think that using code completion to correct errors is a good path forward. As for all the rest of the suggestions, (a) we have to consider how common these cases are and decide whether it's worth the opportunity cost of not doing something else that might help more users, and (b) the more 'uncommon' cases we add to code completion the more likely it is that one or more of them will degrade the performance for the more common cases. While I completely agree that part of the value of tooling is to help users learn parts of the language and framework that they're not familiar with, sometimes the cost of doing so is too high, and I think this might be one such time. |
I don't recall the specifics of that conversation, but: When we produce a completion for LSP, we can attach to it a "replacement range", which is the range that will be replaced when accepting the completion, if completion mode is "replace" (which is out default). So for: The replacement range here would be There is an exception... We can provide additional text edits with a completion that are applied as a best effort by VS Code and may silently be discarded (because VS Code might not be able to rewrite those edits to account for changes made to the document since completion was computed). We use these for adding |
Partially inspired by Dart-Code/Dart-Code#5242.
If you have the following code:
There is no current indication that either the top declaration
barexists or that we could doEC(this).barto use the declaration ofbarfor the extension.This is all working as intended, of course, but I'd like to see a differentiation on the autocomplete options so the user can easily see that there is name shadowing here.
This is even more crucial in cases like:
a.dart
thing.dart
other.dart
This is the current output of the autocomplete:
If I add the
.In this case, we could easily surpass this by adding the
this.at the start similar to what we do with aliased imports and its elements, but we have no indication of it currently.As a reference, I stumbled upon this issue in the last case, so it took me a while to figure out that my variable was being shadowed by the import.
I'm not sure what we could do for:
Since both are valid things but in this case if I was trying to use the import inside
CI would never be able to unless I renamed it (which we would probably want) but then it is not easy to see that this is what the user is supposed to do.@DanTup
The text was updated successfully, but these errors were encountered: