Skip to content

Improve IDE IntelliSense/autocomplete for types with static/constants #35587

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

Open
gabrc52 opened this issue Jan 7, 2019 · 5 comments
Open

Improve IDE IntelliSense/autocomplete for types with static/constants #35587

gabrc52 opened this issue Jan 7, 2019 · 5 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-completion Issues with the analysis server's code completion feature P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@gabrc52
Copy link

gabrc52 commented Jan 7, 2019

Dart VM version: 2.1.0-dev.9.4.flutter-f9ebf21297 (Thu Nov 8 23:00:07 2018 +0100) on "linux_x64"

When using an IDE, when an argument of a certain type is required, classes with static getters or constants of that type could be suggested, for example:

class A {
  final bool belongsToB;
  const A({this.belongsToB});
}

class B {
  static const A staticMember = A(belongsToB: true);
}

void fnThatNeedsA({A obj}) {}

void main() {
  fnThatNeedsA(obj: );
//                 ^
}

Here, B.staticMember should be an autocomplete suggestion.

See this Flutter example:

ThemeData(
  primarySwatch: 
)

The IDE suggests MaterialColor(, but what the user probably needs is Colors., so this should be the first suggestion. (primarySwatch is of type MaterialColor and the Colors class includes many static constants of type MaterialColor).

Originally filed in Dart-Code/Dart-Code#1375

@vsmenon vsmenon added the legacy-area-analyzer Use area-devexp instead. label Jan 9, 2019
@stereotype441 stereotype441 added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Jan 11, 2019
@srawlins srawlins added the devexp-completion Issues with the analysis server's code completion feature label Jun 17, 2020
@scheglov scheglov self-assigned this Mar 8, 2024
@scheglov
Copy link
Contributor

scheglov commented Mar 8, 2024

Interesting.
Indeed, looking at suggestions, I tend to agree that the list is not optimal.

  1. mixHeight and screenHeight are just never useful. There is no way from them to a MaterialColor. This might be a new feature to consider for relevance computation.
  2. I think context is the same, but I don't know Flutter well enough.
  3. Null class is just never useful.
  4. MaterialColor class, don't know why we suggest classes. We already show static fields. So, maybe not prioritize classes as type matching? Only constructors (and fields, and static methods).
  5. MaterialColor constructor vs. Colors fields. I'm almost sure that here we want Colors fields. In general case? I don't know. In the analyzer code base we have either static fields / enum constants, or instances, I think. Maybe run another experiment, check if using static fields is better than constructors is general. If not - hardcode MaterialColor, or invent an annotation.
image

@bwilkerson
Copy link
Member

  1. mixHeight and screenHeight are just never useful. There is no way from them to a MaterialColor. This might be a new feature to consider for relevance computation.

It has been considered, and I'd love to have it. I just don't know any way to determine "can lead to a value of the right type" in a performant way. If you know of one I'd love to hear about it.

Also, your categorical statement isn't true. You can get to them if there's an extension method that allows it.

  1. Null class is just never useful.

True, but it's valid in lots of places, including here.

  1. MaterialColor class, don't know why we suggest classes. We already show static fields. So, maybe not prioritize classes as type matching? Only constructors (and fields, and static methods).

That's an aspect that I wanted to explore taking into account in the relevance tables. IIRC, the code that currently builds the relevance tables only looks at the first token in most cases (I think there's an exception for enum constants). I wanted to see whether we'd get better results if the table builder looked past the first token to identify static field and static method references. I haven't had time, but I still think it would be interesting to explore.

  1. MaterialColor constructor vs. Colors fields. I'm almost sure that here we want Colors fields. In general case?

This is an interestingly hard question. Most users do want to reference a field from Colors. So minimally we should boost the class Colors. But adding all of the fields floods the list with hundreds of wrong options, with no way for us to know which are wrong and which is right.

We've talked several times about whether there should be a limit; something along the line of "we'll add static fields if there are less than N of them". We've also talked about whether the relevance of class names should always be higher than the relevance of static members of the class. We haven't so far had time to compare the variations to find the right answer.

@scheglov
Copy link
Contributor

scheglov commented Mar 8, 2024

Null class is just never useful.
True, but it's valid in lots of places, including here.

I don't understand how the Null class can be valid here.
Yes, the property is nullable, but Type, the type of Null, is not a subtype of MaterialColor? primarySwatch.

MaterialColor constructor vs. Colors fields. I'm almost sure that here we want Colors fields. In general case?
This is an interestingly hard question. Most users do want to reference a field from Colors. So minimally we should boost the class Colors. But adding all of the fields floods the list with hundreds of wrong options, with no way for us to know which are wrong and which is right.

We already add all static fields. It is just that static fields of Colors get higher relevance, and fit the top 100 that the client gets. No flood so far :-) It is possible that there would be too many even type matching fields, but then the user can start typing the field name, and filter them out quickly. I think setting any limit of the number of fields would be harmful.

Yeah, not sure about class name vs. static fields. Tried to do completion in Flutter, and see surprising results - some static fields are suggested when I try Colors, some not. Possibly we give the same relevance, and so cut mostly arbitrary.
image
I would expect that if I type Color, I get then all static fields of Colors. OTOH, there might be another class like ColorBuilder that we could created instance of, and then invoke ColorBuilder().makeGreen(). But this is more steps than just a static field, so should have less relevance, right? Which returns us to (1), about a path to a compatible value.

@bwilkerson
Copy link
Member

I don't understand how the Null class can be valid here.

We made a decision, a long time ago, that we were going to suggest anything that could be valid no matter how unlikely it is to be what the user wants, and let the relevance take care of moving it to the bottom. Maybe that was the wrong choice, but that's a different conversation.

Null can be valid here if there's an extension method on Type. It isn't likely that anyone will do that (other than a couple of members of the language team as part of testing outlying cases in the language), but it isn't impossible.

We already add all static fields.

Yes. I obviously wasn't being clear. We currently add all fields. I think there's a case to be made that it might be better for users if we didn't include any of the fields of Colors in the list because then Colors might pop to the top much sooner, allowing users to select it, type '.' and then start typing the field name. I think it's possible that that might result in fewer keystrokes for users.

And be less surprising when some of the fields are displayed and others not.

But this is more steps than just a static field, so should have less relevance, right?

Not necessarily. If there were some top-level variable, like defaultColor that was always available without dereferencing anything, it isn't clear to me that it would be referenced more often than say a field on an instance of a style object. While I'm confident that long chains of member accesses are less common than shorter, that doesn't necessarily correlate to all short chains having higher relevance than longer chains.

@scheglov
Copy link
Contributor

scheglov commented Mar 8, 2024

Ah, yes, I agree that Null is valid, and a principle that we suggest anything that can be used.
Its the relevance of Null that is not good then :-)

I think it's possible that that might result in fewer keystrokes for users.

All is possible, but in this case I doubt it.
Sufficiently sophisticated user could type something like this - both Colors and the color name.
I don't believe that going through Colors, dot, color name would be faster.
image

it isn't clear to me that it would be referenced more often

Well, we could get the answer on this looking at existing code.
Specifically for MaterialColor.

In general - it depends on the code base.
We have to make a decision one way or another.
Or somehow infer this from the specific code base dynamically.

@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
@scheglov scheglov removed their assignment Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-completion Issues with the analysis server's code completion feature P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants