Variable override auto-complete doesn't differenciate between getter and setter

[FMorschel]

If you have code like:

class A {
  A(this.value);
  int value;
}

class B extends A {
  B(super.value);
  @override^
}

And you ask for code completion at ^ and continue writing value this is the output:

---

If you ask for something that is not in scope, it'll show on the right side what is the library that will be imported:

---

Could we use that place for differentiating between getter and setter?

CC @DanTup

[DanTup]

I added this to the end of the detail field (since the detail on the right is always the URI right now). It looks like this:

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

[bwilkerson]

It seems a bit inconsistent that we mark an override suggestion with @override if the user has started typing the annotation explicitly, but not when it's going to be implicitly added. It seems more useful in the second case than in the first case; without it it looks like we might be adding just the name of the member.

[FMorschel]

Yeah, that would be another point for #59930. Maybe fixing that here would be fine.

@DanTup can you take a look into why is it that auto-complete for value (fully written) on B won't show the suggestion too? Not sure if this is too hard to fix (last comment, currently, on the issue above has more details on this).

[DanTup]

It seems a bit inconsistent that we mark an override suggestion with @override if the user has started typing the annotation explicitly, but not when it's going to be implicitly added.

Yeah, I agree. I wonder if we could add it to the label but leave it off the filterText so that it should show up the same, but not affect the client-side ranking. Let me try this out..

[DanTup]

Here's how it looks with a prefix:

I don't know how I feel about the matched text not being left-most in the label though, it does make it harder to scan. Thoughts?

[DanTup]

btw, I'm not sure if wen ca represent this in the legacy protocol. To do this, we have three strings:

1. The visible label in the completion
2. The text to match when filtering/ranking (if the user has not typed "override" we should not rank badly because the display label starts with "override "
3. The text to insert (the actual valid override code)

LSP supports all of these (label, filterText, insertText/edits) but legacy only has a displayText and completion. For the completions when you're typing "override" it's fine because it can be included in both the visible label and the filter/rank text, but when you're not, I don't think it can/should.

[FMorschel]

I do agree it makes it a bit harder to scan but I don't think placing it somewhere else would make this coherent with writing @override. I'd vote for consistency here (keeping it on the left) since moving override somewhere else would probably feel too much when overriding a big FuntionType or something with a big class name.

Also, on that note, can we move get/set before the type? Considering the above paragraph it may not have enough space to write that down in some cases?

[DanTup]

I do agree it makes it a bit harder to scan but I don't think placing it somewhere else would make this coherent with writing @override

I wonder if we should also always put @override instead of override in the label? (again, this might be complicated with legacy protocol, but should be fine for LSP).

Also, on that note, can we move get/set before the type?

I originally did it this way because it was how you'd write int get foo =>... but then I realised that for set we don't write the type there. I don't feel strongly either way so I'm happy to swap it around if no objections.

[FMorschel]

I wonder if we should also always put @OverRide instead of override in the label?

I don't feel strongly either way here but currently we never show @ but this may be more consistent for new users?

[FMorschel]

Just to illustrate what I said above for small space with long declarations, see noSuchMethod on the images below:

[bwilkerson]

.. but legacy only has a displayText and completion.

I'd have to try using it to see what IntelliJ would do with display text that doesn't start with the prefix.

I wonder if we should also always put @OverRide instead of override in the label?

I think not. We don't put the ampersand in when completing the annotation so I think we're fine to not include it when completing the name of the member to be overridden.

I originally did it this way because it was how you'd write int get foo =>... but then I realised that for set we don't write the type there.

I suppose we could include the parameter type in a setter (inside parens), but no, I'd omit the return type.

[DanTup]

Here's how it looks with set ($returnType) for setters:

I've updated the CL at https://dart-review.googlesource.com/c/sdk/+/405007 with this.