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

"Make final" fix puts "final" in the wrong position in record destructuring #52820

Closed
DanTup opened this issue Jun 30, 2023 · 6 comments
Closed
Labels
analyzer-quick-fix analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request

Comments

@DanTup
Copy link
Collaborator

DanTup commented Jun 30, 2023

I have the prefer_final_locals lint enabled which triggers on this code individually for both kind and value:

    for (var (kind, value) in sensorValues) {

However the quick-fix inserts final before the variable name producing invalid code:

for (var (final kind, value) in sensorValues) {

I thought it should change var to final instead, although that affects more than just the variable where the fix was invoked. This led me to try the following:

for (var (kind, value) in sensorValues) {
  value = 1;

Here, the lint still fires for kind, but I don't know what I'd expect the fix to do here. Perhaps the lint should only fire if all declared variables can be changed to final? Or is there some way to destructure into a mix of final/non-final variables?

@lrhn lrhn added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Jun 30, 2023
@scheglov scheglov added P3 A lower priority bug or feature request analyzer-server analyzer-quick-fix labels Jul 3, 2023
@scheglov
Copy link
Contributor

scheglov commented Jul 3, 2023

@pq

@pq
Copy link
Member

pq commented Jul 6, 2023

Here, the lint still fires for kind, but I don't know what I'd expect the fix to do here. Perhaps the lint should only fire if all declared variables can be changed to final?

That's my instinct exactly. We do make that call elsewhere so I think this is a bug. (Filed: dart-lang/linter#4539.)

@DanTup
Copy link
Collaborator Author

DanTup commented Jul 6, 2023

If it's fixed in that way, I presume there's nothing to change in the fix (and should close this issue?)

@bwilkerson
Copy link
Member

Even in cases where all of the variables can be final, such as

void f(List<(int, int)> sensorValues) {
  for (var (kind, value) in sensorValues) {
    print(kind);
    print(value);
  }
}

the fix still puts the 'final' in the wrong place. I think the fix still needs to be fixed.

@DanTup
Copy link
Collaborator Author

DanTup commented Jul 6, 2023

Oh yeah, whoops. That was my original issue, I confused myself with the tangent 🙃

@pq
Copy link
Member

pq commented Jul 6, 2023

I'm grateful that you brought it up. It's a legitimate false positive in the lint. Thanks for catching it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-quick-fix analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

5 participants