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

Occurrences not working correctly for super parameters #48281

Open
Tracked by #48067
bwilkerson opened this issue Feb 2, 2022 · 4 comments
Open
Tracked by #48067

Occurrences not working correctly for super parameters #48281

bwilkerson opened this issue Feb 2, 2022 · 4 comments
Labels
area-intellij Tracking issues for the Dart IntelliJ plugin. P2 A bug or feature request we're likely to work on

Comments

@bwilkerson
Copy link
Member

Given a file containing

class A {
  A(int x);
}

class B extends A {
  int y;

  B(super.x) : y = x * 2;

  B.x(this.y) : assert(y > 0), super(0);
}

If I select the reference to y in the assert, then that reference, the y in this.y, the y in y = x * 2, and the y in the field declaration are all highlighted. Basically, we've said that the this.y parameter and the field are the same, so any references to either one should all be highlighted at the same time.

On the other hand, if I select the x in super.x, then the field declaration (in A) is highlighted, but the x in x * 2 is not. I think that for consistency it needs to be.

@bwilkerson bwilkerson added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Feb 2, 2022
@srawlins srawlins added P2 A bug or feature request we're likely to work on analyzer-spec Issues with the analyzer's implementation of the language spec labels Feb 4, 2022
@scheglov
Copy link
Contributor

I believe that it already works in DAS, but I will add a test.
https://dart-review.googlesource.com/c/sdk/+/234223

I think it does not work because IntelliJ plugin does not support super-formal parameters yet.
I'm actually not sure if IntelliJ uses AnalysisService.OCCURRENCES.

@jwren

copybara-service bot pushed a commit that referenced this issue Feb 24, 2022
Bug: #48281
Change-Id: I137bf5976b9db9c0c511411abaed08fa59542d2b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/234223
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@scheglov scheglov added area-intellij Tracking issues for the Dart IntelliJ plugin. and removed area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-spec Issues with the analyzer's implementation of the language spec labels Apr 2, 2022
@asashour
Copy link
Contributor

IntelliJ plugin doesn't use AnalysisService.occurrences, but uses navigation for calculating the reference highlighting.

This case works fine

class A {
  int x;
  int y;

  A(this.x) : y = x * 2;
}

because the x in x * 2 targets x in the declaration int x, and not this.x.

However, in the case of

class A {
  A(int x);
}

class B extends A {
  int y;

  B(super.x) : y = x * 2;
}

The x in x * 2 targets super.x, and the latter then targets int x.

I find that targeting int x in the first example, while targeting super.x in the second one is inconsistent, or it least should be cleared before continuing with the IntelliJ part.

@scheglov
Copy link
Contributor

I agree that for field formal parameters (the first case) and super formal parameters (the second) navigation is inconsistent. I seems to me that for the super formal parameters it is more correct - x in x * 2 is a reference to the value passed to the constructor (where x is implicitly a final formal parameter). Similarly for the field formal parameter x in x * 2 does not read the field int x - it uses the formal parameter value (also implicitly final formal parameter).

@scheglov
Copy link
Contributor

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

copybara-service bot pushed a commit that referenced this issue May 16, 2022
In constructor initializers the reference is to the parameter itself,
not the field.

Bug: #48281
Change-Id: Ic875e68d9b0d4ebdc08b3a8e71ecd8f0e52c3c26
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244863
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-intellij Tracking issues for the Dart IntelliJ plugin. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

4 participants