Skip to content
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

code completion ranks type names over constructors for named params #43854

Open
pq opened this issue Oct 20, 2020 · 5 comments
Open

code completion ranks type names over constructors for named params #43854

pq opened this issue Oct 20, 2020 · 5 comments
Assignees
Labels
analyzer-completion Issues with the analysis server's code completion feature analyzer-ux area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@pq
Copy link
Member

pq commented Oct 20, 2020

Example:

//a.dart
class A { 
  A child;
  A({this.child});
}
''');
import 'a.dart';

void main(List<String> args) {
  var a = A(
    child: ^
  );  
}

In this case, the type name A ranks higher (57) than the constructor A() (40).

@pq pq added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-completion Issues with the analysis server's code completion feature analyzer-ux labels Oct 20, 2020
@pq pq self-assigned this Oct 20, 2020
@bwilkerson
Copy link
Member

It isn't entirely clear to me that this is a bug, so I'll explain why.

The relevance of these two elements is largely controlled by the element_kind feature. That feature uses the location in which an element is being referenced and the kind of element being referenced in order to look up the relevance score in a table. Given the location in the example above, the portion of the relevance table being used is here (https://github.com/dart-lang/sdk/blob/master/pkg/analysis_server/lib/src/services/completion/dart/relevance_tables.g.dart#L47). According to the table, a constructor occurs in this location approximately 13% of the time and a class occurs approximately 17% of the time. So, either the table is correct and classes should appear higher in the list or the table is incorrect.

The table is generated by performing a statistical analysis of a corpus of code. I can think of a couple of reasons why the table might be incorrect.

First, it's possible that the corpus of code we used is not representative of real code which could lead to the probabilities encoded in the table being wrong. It's been a while since the table was generated, so it might be worthwhile regenerating the table from an updated (and possibly expanded) corpus. If that changes the relative order of those two element types, then the problem will be solved. If not, then we might consider using a different and more representative corpus of code.

Second, the quality of the table is strongly influenced by the selection of the locations. Right now the location of interest is "the first element in a named argument in a constructor invocation". There's no distinction being made as to whether the constructor is the constructor for a Flutter widget. I believe that we ran an experiment in which such a distinction was made and found that it didn't change the relative ordering of elements, but I might be mistaken and it's possible that with an updated or more appropriate corpus the results would be different. That would need to be investigated.

Third, it's possible that the statistical analysis code that determines which element to record at each location has a bug that causes a class to be recorded more often than is appropriate. It's worth investigating this option as well.

@pq
Copy link
Member Author

pq commented Oct 20, 2020

Thanks for the clarification! I do remember you explaining this a while back when I brought this up before but it didn't completely stick. From an end-user's perspective, I think what makes the ranking feel wrong is that A (the class) makes no sense in this context. Accepting the first completion will give you a static type error. Moreover, since A has no static members that produce an instance assignable to A, there is no way to use the class to get to something that makes sense.

Does ranking do any look-ahead? That is, is there any way to rank classes w/ members (or members that produce) an object of an assignable type higher than ones that don't?

For example:

class A {
  static a = A();
  static A create() => A();
}

vs.

class A {
}

Does ranking know about type assignability?

@bwilkerson
Copy link
Member

Does ranking do any look-ahead? That is, is there any way to rank classes w/ members (or members that produce) an object of an assignable type higher than ones that don't?

Not currently, no. My expectation (which might be wrong) is that it will be too expensive. In part because we might have to traverse and unbounded number of "links". You might have a class A that has a static member that returns a B, which has a static member that returns a C, ..., that has a static member that returns an A.

But just for the record, in an in-person discussion we decided that it would be worth exploring a new feature based on whether a candidate class has any non-constructor static members. Those for which there are no static members seem less likely to be useful completions than any constructors that those classes might declare. The feature would probably need to only apply in places where a type annotation isn't allowed, but I think that's also doable.

Does ranking know about type assignability?

Yes, there's a feature for that (context_type).

@CMMT20
Copy link

CMMT20 commented Mar 17, 2022

Hello, i have the same issue, so i put it here, but i think i made more elaborated explanation, i hope it helps to go to the solution.

you can see it here
#48600 (comment)

@pq pq removed their assignment May 5, 2023
@scheglov scheglov self-assigned this Mar 11, 2024
@scheglov
Copy link
Contributor

I completely agree, in many cases there is no reason to suggest type names.
This is especially true in the Flutter example above.
So, I'm going to send some time to gather statistics to improve our relevance computing heuristics.

@srawlins srawlins added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-completion Issues with the analysis server's code completion feature analyzer-ux area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
Development

No branches or pull requests

5 participants