completion should prioritize constructors over types

[pq]

For example:

The first match is not what you want nor is it on the way to what you want -- StringToken has no named constructors or static factory methods.

I'm not sure how smart we need to be but in general I think we want to propose constructors and static instances over type names.

But maybe there's some logic/rationale I'm unaware of?

/cc @scheglov @bwilkerson

/fyi @devoncarew

[pq]

For discussion.

The current sorted ranking of suggestions for a simple completion,

class A {
  void operator &(A other) { 
    ^
  }
}

is the following:

[1059] other • PARAMETER INVOCATION /home/test/lib/test.dart
[1057] & • METHOD INVOCATION /home/test/lib/test.dart
[1055] super •  KEYWORD 
[1055] this •  KEYWORD 
[1055] assert •  KEYWORD 
[1055] const •  KEYWORD 
[1055] do •  KEYWORD 
[1055] final •  KEYWORD 
[1055] for •  KEYWORD 
[1055] if •  KEYWORD 
[1055] return •  KEYWORD 
[1055] switch •  KEYWORD 
[1055] throw •  KEYWORD 
[1055] try •  KEYWORD 
[1055] var •  KEYWORD 
[1055] void •  KEYWORD 
[1055] while •  KEYWORD 
[1000] bool.fromEnvironment • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] noSuchMethod • METHOD INVOCATION /sdk/lib/core/core.dart
[1000] A • CONSTRUCTOR INVOCATION /home/test/lib/test.dart
[1000] runtimeType • GETTER INVOCATION /sdk/lib/core/core.dart
[1000] toString • METHOD INVOCATION /sdk/lib/core/core.dart
[1000] A • CLASS INVOCATION /home/test/lib/test.dart
[1000] deprecated • GETTER INVOCATION /sdk/lib/core/core.dart
[1000] override • GETTER INVOCATION /sdk/lib/core/core.dart
[1000] proxy • GETTER INVOCATION /sdk/lib/core/core.dart
[1000] identical • FUNCTION INVOCATION /sdk/lib/core/core.dart
[1000] print • FUNCTION INVOCATION /sdk/lib/core/core.dart
[1000] bool • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] hashCode • GETTER INVOCATION /sdk/lib/core/core.dart
[1000] Comparable • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] DateTime • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] DateTime • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Deprecated • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Deprecated • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] pragma • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] pragma • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] double • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Duration • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Duration • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Error • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Error • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Exception • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Exception • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Function • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Function • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] int • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] int.fromEnvironment • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Invocation • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Iterable • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Iterator • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] List • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] List • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] List.from • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] List.filled • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] List.of • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] List.generate • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] List.unmodifiable • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Map • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Stream.fromIterable • CONSTRUCTOR INVOCATION /sdk/lib/async/stream.dart
[1000] Map.fromIterable • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Null • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] num • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Object • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Object • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Pattern • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] RegExp • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] RegExp • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Set • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Set • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Set.identity • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Set.from • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Set.of • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] StackTrace • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] StackTrace • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] String • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] String.fromCharCodes • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Symbol • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Symbol • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Type • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Type • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Uri • CLASS INVOCATION /sdk/lib/core/core.dart
[1000] Uri • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] Future • CLASS INVOCATION /sdk/lib/async/async.dart
[1000] Future • CONSTRUCTOR INVOCATION /sdk/lib/async/async.dart
[1000] Future.delayed • CONSTRUCTOR INVOCATION /sdk/lib/async/async.dart
[1000] Future.microtask • CONSTRUCTOR INVOCATION /sdk/lib/async/async.dart
[1000] Future.value • CONSTRUCTOR INVOCATION /sdk/lib/async/async.dart
[1000] Stream • CLASS INVOCATION /sdk/lib/async/stream.dart
[1000] Map • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart

Notice how CLASS and CONSTRUCTOR invocations are both ranked 1000 (the default).

Proposal: demote CLASS to 990 (or anything sub-default).

@bwilkerson @jwren: thoughts?

[bwilkerson]

I think it's going to be very difficult to reason well about the relative relevance of items if we're looking at them in a pairwise fashion. I would like to use data to figure out the appropriate relevance ranking for each kind of suggestion, possibly based on the context in which the completion is occurring.

[jwren]

I agree with both of you here. We should chat in person today. Completion is hard for a number of reasons as we have discussed in person as it tries to serve many roles through as single UX.

In this case Brian, even if you showed me data to show that for some

Foo foo = Fo^

the user typically is not trying to complete a constructor, I think there is subjectively enough of this expectation that it is surprising (at least to new dart users), that we should make the change that Phil is proposing here.

I'd like to see the following in the next few weeks:

1) If you have a decent idea of what such an algorithm would look like to model out the relevance data, I'd be happy to take this on Brian. 2) We should refactor all of the relevance computation to a single method, or set of methods, in the server so that we can iterate with more confidence in the future. I'd be happy to take this on as well.

[devoncarew]

What if we were to do something simple like improve the importance of constructors by 1?

[1001] StackTrace • CONSTRUCTOR INVOCATION /sdk/lib/core/core.dart
[1000] StackTrace • CLASS INVOCATION /sdk/lib/core/core.dart

It's very ad hoc, but we could verify by hand if we thought this was a net improvement. We could - longer term - also work to build a rigorous corpus, and better understanding of the normal statistics around the types of symbols, and which should be suggested more often, and in which contexts.

[bwilkerson]

That's effectively what we decided on in a meeting we had this morning.

[pq]

Some progress in https://dart-review.googlesource.com/c/sdk/+/131424. The value at the moment is limited, I think, by our relevance getting overruled by "available suggestions" on the client side.

For example:

Notice that the local constructor AbstractCompletionDomainTest pops to the top as desired, but for the other suggestions, types rank first (AnalysisClosingLabelsParams, etc).

(The tests in the linked PR suggest that we're properly setting the relevance which is why I'm speculating that our relevance values are getting overridden in the non-local type case.)

[devoncarew]

Hmm, where to the priorities get set for the available suggestions? It's odd that only the very first suggestion is rendered with trailing parans (()).

[pq]

It's odd that only the very first suggestion is rendered with trailing parans (()).

That one is a local type and not coming from available suggestions.

[bwilkerson]

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