Extension methods are shown too high in the list of autocomplete suggestions

[jacob314]

Extension methods are now sometimes shown before regular methods in the list of autocompletes.
Extension methods should be shown after all regular methods because it is easy for popular classes to end up with a lot of rarely used extension methods. Showing all these extension methods before regular members makes it hard to see the true shape of the class versus the shape of all the extensions. Using different icons for extension methods in the autocomplete list would also help clarify what completions are extensions and what are normal.

Example autocompletes for a BuildContext (commonly used Flutter framework base class). Notice that watch() is just an extension method from package:provider that is in scope because my file uses package:provider

Fyi @Hixie for thoughts on reducing the risk of extension methods cluttering Flutter apis.

[bwilkerson]

I'm trying to understand the impact of the current behavior on users, so I'm hoping for some more information, if you have it.

Extension methods are now sometimes shown before regular methods in the list of autocompletes.

Under what conditions is this occurring (in general, not just in the one case you gave)? I ask because I think it would be appropriate in some cases. For example, if a class C does not define any methods that return a String, there is an extension method that returns a String, and the context clearly indicates that a method returning a String should be preferred, then it seems like the extension method might well be what the user is most likely trying to complete.

Extension methods should be shown after all regular methods because it is easy for popular classes to end up with a lot of rarely used extension methods.

I agree that this is possible, but do we have evidence that this is happening in practice? That is, do we have evidence that there are a large number of extension methods being declared? Do we have evidence the extension methods are rarely invoked? For whichever of these is happening in practice, are users importing the libraries that contain these extension methods?

Showing all these extension methods before regular members makes it hard to see the true shape of the class versus the shape of all the extensions. Using different icons for extension methods in the autocomplete list would also help clarify what completions are extensions and what are normal.

I agree that this is a valid concern. Code completion is not only used to reduce typing, but is also used to explore the API surface. Unfortunately, we don't currently have any way to measure the quality of code completion from the perspective of the second use case. That isn't an argument that we should ignore the second use case, but an argument that we need to find a way to measure the quality for that use case (even if it's only by measuring a proxy).

That said, and recognizing that I have no data to support my opinion, I suspect that having a different icon for extension methods would be a better solution for this problem than to base the placement of the extension methods on the fact that they are extension methods.

I believe that this would be relatively easy to do in IntelliJ (@jwren to confirm), but as far as I can see we can't add new CompletionItemKinds to LSP, so I don't think this would be feasible there (@DanTup to confirm).

Notice that watch() is just an extension method from package:provider that is in scope because my file uses package:provider

If a user has imported a library that declares extension methods, it seems like this will sometimes be a signal that the user wants to use the extension methods (for example, in the case where the library only declares extension methods) and sometimes isn't (apparently such as in the example you gave, though I'm not familiar with package:provider). Do you have any thoughts about what other kinds of signals there might be to indicate whether or not a user is interested in using the imported extension methods?

[jacob314]

Under what conditions is this occurring (in general, not just in the one case you gave)? I ask because I think it would be appropriate in some cases. For example, if a class C does not define any methods that return a String, there is an extension method that returns a String, and the context clearly indicates that a method returning a String should be preferred, then it seems like the extension method might well be what the user is most likely trying to complete.

Occurs all the time. I was typing context. as a statement to get the list of autocompletes.

I agree that this is possible, but do we have evidence that this is happening in practice? That is, do we have evidence that there are a large number of extension methods being declared? Do we have evidence the extension methods are rarely invoked? For whichever of these is happening in practice, are users importing the libraries that contain these extension methods?

package:provider is extremely popular so is evidence of this if package:provider users aren't adopting these extension methods. @pq we would need to filter to only look at recently written code as these extension methods are probably fairly recent.
An example from C#: you end up with tons of mainly low value extension methods on core types.
https://www.extensionmethod.net/search/List

That said, and recognizing that I have no data to support my opinion, I suspect that having a different icon for extension methods would be a better solution for this problem than to base the placement of the extension methods on the fact that they are extension methods.

I believe that this would be relatively easy to do in IntelliJ (@jwren to confirm), but as far as I can see we can't add new CompletionItemKinds to LSP, so I don't think this would be feasible there (@DanTup to confirm).

Maybe kind isn't the right hook. For example, indicating the library the member was defined in would also be sufficient and provide better context if we end up living in a world were core types have mountains of extension methods. If I could tell which members were from package:provider that would help clarify the shape of the api suggested by the completions.

If a user has imported a library that declares extension methods, it seems like this will sometimes be a signal that the user wants to use the extension methods (for example, in the case where the library only declares extension methods) and sometimes isn't (apparently such as in the example you gave, though I'm not familiar with package:provider). Do you have any thoughts about what other kinds of signals there might be to indicate whether or not a user is interested in using the imported extension methods?

Some signals: have I used the extension methods at all in my package? In this case, the extension methods are never used in my package. Perhaps we should lint to encourage package authors to put extension methods in separate imports so you don't get opted into lots of extension methods just by importing packages. For example, if you want the extension methods you could import 'package:provider/extensions.dart' but if you don't care about the syntactic sugar you can just import 'package:provider/provider.dart'.

[bwilkerson]

For example, indicating the library the member was defined in would also be sufficient and provide better context if we end up living in a world were core types have mountains of extension methods.

Possibly, but I think that assumes a certain familiarity on the part of a user. For example, if a user was attempting to complete to see the members available on a class they were not familiar with, and they were told that the members come from four different libraries, then without knowing which library the class was defined in they wouldn't have any information about which were non-extension methods and which were extension methods.

... have I used the extension methods at all in my package?

That's an interesting possibility, thanks. I'm not sure how we'd efficiently compute the required data, but it might be an even more general feature. It certainly seems likely that methods that are referenced frequently are more likely to be referenced again than methods that are referenced infrequently or never. I'll have to test that hypothesis some day.

Perhaps we should lint to encourage package authors to put extension methods in separate imports ...

I'm not sure whether lints are the right way to accomplish the goal, but I think the goal might be correct. While I see the argument for package authors to provide a single library that imports everything, I have long thought that that was an anti-pattern when the package reaches a certain size [1]. This might be another characteristic that sometimes makes it an anti-pattern.

[1] For example, we've had requests to have a single import for the analyzer package, but I think it's too big for that to be a good thing. I've also wished several times that Flutter were broken up into smaller libraries to reduce the size of the name scope in importing libraries (primarily because of the impact of a large name scope on code completion quality).

[Hixie]

I rarely use autocomplete, but when I do, I find the autocomplete is way too eager to give me thousands of things I would never want to use. I understand that that's because in principle anything can be the start of an expression that ends up returning the right type, but if I'm in a String context, as @bwilkerson says, I'd rather see things that return Strings than anything else. Only if I type enough that none of the things that return strings are possible matches would I want it to consider showing me anything else.

In the example in the OP I don't know what I'd expect because I don't know where this is being typed. Typically, when I type "context" I wouldn't expect any autocomplete, because I am probably typing it in a context that expects a BuildContext, and the next thing I want to type is a comma.

[jacob314]

As clarification my expression was buildContext. so I only expected members of buildContext at that point not 2000 results. This isn't the best example as the answer I verified by going to the dart docs was that none of the methods on buildContext could do what I was looking for. Lets instead take an example from extension methods on the List class which has a lot more generally useful methods.

The popular (2000+ star) get package (https://github.com/jonataslaw/getx) adds a number of extensions to the core List type. If I was using this package I would probably like to see the obs getter high up in my list of auto-complete matches but the other extensions make me nervous as they are more a grab bag of nice to convenience helpers on List that 20 different packages could easily have 20 redundant versions of. Most likely if I was using this package I would be using the obs extension somewhere in my code but would not be using the other extensions. So it would be great if I saw obs high up in my list of autocompletes but saw the other extensions bellow relevant members on the core List type.

extension ListExtension<E> on List<E> {
  RxList<E> get obs => RxList<E>(this);

  /// Add [item] to [List<E>] only if [item] is not null.
  void addNonNull(E item) {
    if (item != null) add(item);
  }

  /// Add [Iterable<E>] to [List<E>] only if [Iterable<E>] is not null.
  void addAllNonNull(Iterable<E> item) {
    if (item != null) addAll(item);
  }

  /// Add [item] to [List<E>] only if [condition] is true.
  void addIf(dynamic condition, E item) {
    if (condition is Condition) condition = condition();
    if (condition is bool && condition) add(item);
  }

  /// Adds [Iterable<E>] to [List<E>] only if [condition] is true.
  void addAllIf(dynamic condition, Iterable<E> items) {
    if (condition is Condition) condition = condition();
    if (condition is bool && condition) addAll(items);
  }

  /// Replaces all existing items of this list with [item]
  void assign(E item) {
    if (this is RxList) {
      (this as RxList)._value ??= <E>[];
    }

    clear();
    add(item);
  }

  /// Replaces all existing items of this list with [items]
  void assignAll(Iterable<E> items) {
    if (this is RxList) {
      (this as RxList)._value ??= <E>[];
    }
    clear();
    addAll(items);
  }
}

Here are all extension methods on package:flutter and dart:core classes from a popular (2000+ star) package (https://github.com/jonataslaw/getx)

lib/get_connect/http/src/http/io/http_request_io.dart:// extension FileExt on io.FileSystemEntity {
lib/get_instance/src/extension_instance.dart:extension Inst on GetInterface {
lib/get_navigation/src/extension_navigation.dart:extension ExtensionSnackbar on GetInterface {
lib/get_navigation/src/extension_navigation.dart:extension ExtensionDialog on GetInterface {
lib/get_navigation/src/extension_navigation.dart:extension ExtensionBottomSheet on GetInterface {
lib/get_navigation/src/extension_navigation.dart:extension GetNavigation on GetInterface {
lib/get_rx/src/rx_types/rx_core/rx_impl.dart:extension StringExtension on String {
lib/get_rx/src/rx_types/rx_core/rx_impl.dart:extension IntExtension on int {
lib/get_rx/src/rx_types/rx_core/rx_impl.dart:extension DoubleExtension on double {
lib/get_rx/src/rx_types/rx_core/rx_impl.dart:extension BoolExtension on bool {
lib/get_rx/src/rx_types/rx_core/rx_impl.dart:extension RxT<T> on T {
lib/get_rx/src/rx_types/rx_iterables/rx_list.dart:extension ListExtension<E> on List<E> {
lib/get_rx/src/rx_types/rx_iterables/rx_map.dart:extension MapExtension<K, V> on Map<K, V> {
lib/get_rx/src/rx_types/rx_iterables/rx_set.dart:extension SetExtension<E> on Set<E> {
lib/get_state_manager/src/rx_flutter/rx_notifier.dart:extension ReactiveT<T> on T {
lib/get_state_manager/src/rx_flutter/rx_notifier.dart:extension StateExt<T> on StateMixin<T> {
lib/get_state_manager/src/simple/get_state.dart:// extension WatchEtx on GetxController {
lib/get_state_manager/src/simple/get_state.dart:// extension FindExt on BuildContext {
lib/get_state_manager/src/simple/get_state.dart:// extension ObserverEtx on BuildContext {
lib/get_utils/src/extensions/context_extensions.dart:extension ContextExtensionss on BuildContext {
lib/get_utils/src/extensions/double_extensions.dart:extension Precision on double {
lib/get_utils/src/extensions/duration_extensions.dart:extension GetDurationUtils on Duration {
lib/get_utils/src/extensions/dynamic_extensions.dart:extension GetDynamicUtils<T> on T {
lib/get_utils/src/extensions/event_loop_extensions.dart:extension LoopEventsExt on GetInterface {
lib/get_utils/src/extensions/internacionalization.dart:extension Trans on String {
lib/get_utils/src/extensions/internacionalization.dart:extension LocalesIntl on GetInterface {
lib/get_utils/src/extensions/num_extensions.dart:extension GetNumUtils on num {
lib/get_utils/src/extensions/string_extensions.dart:extension GetStringUtils on String {
lib/get_utils/src/extensions/widget_extensions.dart:extension WidgetPaddingX on Widget {
lib/get_utils/src/extensions/widget_extensions.dart:extension WidgetMarginX on Widget {
lib/get_utils/src/extensions/widget_extensions.dart:extension WidgetSliverBoxX on Widget {

[Hixie]

I guess what I'm saying is that what the expression is matters less than where the expression is.

For example, if the expression is:

  final Title? ancestorTitle = context.|

...where | is the cursor position, then I would not expect to see debugDoingBuild, hashCode, owner, runtimeType, size, widget, dependOnInheritedElement, describeElement, etc in the list. I might expect to see dependOnInheritedWidgetOfExactType<Title>, findAncestorRenderObjectOfType<Title>, etc (things that return Title).

Now if the expression was:

  final Title? ancestorTitle = context.w|

...then I would expect to see widget on the list, since now there's nothing that starts with w that could return a Title? so clearly the developer has something else in mind.

[jacob314]

That matches my expectations for completions as well. To be clear, in my case the expression was just

Widget build(BuildContext context) {
  context.|;
}

so the return type was unconstrained.

[Hixie]

In cases like that I'd probably start with things that return void, but I see what you're saying.

[bwilkerson]

As clarification my expression was buildContext. so I only expected members of buildContext at that point not 2000 results.

Did you really see 2000 results? We should only be suggesting members defined on BuildContext or its superclasses at that point. I tried to reproduce your results, but I only saw 29 suggestions. Of course, I don't have any other package dependencies in my small test package other than those provided by Flutter, so I suppose that might account for the difference. Were the others all from extensions, or are we producing invalid suggestions? If the latter, could you please open a separate issue describing what suggestions we're making and how to reproduce what you're seeing? Thanks.

I would probably like to see the obs getter high up in my list of auto-complete matches but the other extensions make me nervous ...

I'm already planning to look into the frequency_of_reference feature, but I'm curious whether there are any other statically discoverable indicators that you can think of that would distinguish the obs getter from other extensions that we might be able to use as a signal to the ranking algorithm.

I guess what I'm saying is that what the expression is matters less than where the expression is.

Our research confirms that hypothesis. We currently have two signals related to "where" that we use to rank suggestions. One, of course, is the type required by the context in which completion is being requested. Unfortunately, there are several places in Dart where there is no context type information that we can use. The other is the syntactic structure surrounding the completion location. Completions in the argument list of a constructor have a different frequency pattern than completions in the argument list of a method, for example.

[jacob314]

Did you really see 2000 results? We should only be suggesting members defined on BuildContext or its superclasses at that point. I tried to reproduce your results, but I only saw 29 suggestions. Of course, I don't have any other package dependencies in my small test package other than those provided by Flutter, so I suppose that might account for the difference. Were the others all from extensions, or are we producing invalid suggestions? If the latter, could you please open a separate issue describing what suggestions we're making and how to reproduce what you're seeing? Thanks.

No I only saw around 33 suggestions (ones you mentioned plus the extension methods from provider).

I'm already planning to look into the frequency_of_reference feature, but I'm curious whether there are any other statically discoverable indicators that you can think of that would distinguish the obs getter from other extensions that we might be able to use as a signal to the ranking algorithm.

One signal that I would like to see is whether the extension method uses types from the package defining the extension method (for example obs) or if it is just standalone syntactic sugar that any package author could have added (for example addNonNull and addAllNonNull). There is nothing wrong with pure generic syntactic sugar extension methods but they are the area where there is the most risk of a cluttered list of low value completions.

[bwilkerson]

I'm not sure whether we have enough samples to get an accurate statistical analysis, but I've added the idea to the list of features to experiment with. If we don't have enough samples yet we should be able to run the experiment after the use of extension methods has increased.

[DanTup]

as far as I can see we can't add new CompletionItemKinds to LSP, so I don't think this would be feasible there (@DanTup to confirm).

We can send custom completion kinds (for clients that use newer versions of the spec), but unless they're kinds the client specifically knows (and has icons for), they would just ignore the kind and pick a default. There's no way for us to provide icons to the client.

We could possibly add a note to the "detail" field which we currently use for auto-import completions:

Something like "extension from package:foo/foo.dart". Although these only currently show for the selected item in VS Code (although I think there's work happening to support showing them for all visible items).

One thing worth noting for VS Code, is that the server ranking is only used for the initial list of completions when there is no prefix, for example:

buildContext.| // completions here will be ranked by the server

Once there is any prefix at all, VS Code will do its own ranking:

buildContext.w| // completions here will be re-ranked by VS Code

This means that even if extension methods were pushed down based on some ranking, once the user types any character (or if they invoke completion in an existing identifier), that knowledge goes out of the window (for VS Code) unfortunately.

[bwilkerson]

Once there is any prefix at all, VS Code will do its own ranking ...

Do you know what kind of re-ranking is done (both the reasons and the extent)? IntelliJ also does re-ranking, such as moving any suggestion that exactly matches the prefix to the top, but leaves suggestions for which it doesn't have a heuristic in the original order.

[jacob314]

Being able to show extension methods as Extension from package:foo sgtm as the visual display across all IDEs. In my case, even being able to see it as I selected through the list of extensions would have saved me time

The good news is that the buildContext.| // completions here will be ranked by the server case is the most problematic for extension methods so I'm not too worried that is the only spot our ranking is respected.