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

unnecessary_statements and no_self_assignments discussion #59578

Open
FMorschel opened this issue Nov 21, 2024 · 5 comments
Open

unnecessary_statements and no_self_assignments discussion #59578

FMorschel opened this issue Nov 21, 2024 · 5 comments
Labels
analyzer-warning Issues with the analyzer's Warning codes 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) type-enhancement A request for a change that isn't a bug

Comments

@FMorschel
Copy link
Contributor

If you have a code like the following we should be receiving unnecessary_statements triggers:

void f(int a) {
  a = a;  // <-- No lint
}

The background for this is that while working on #57090 I ended with a code similar to:

int i;
if (other case int? i?) {  // It was an actual case but this is not the point
  i = i;   // shadowed i self-assigning 
} else {
  return;
}
i; // <-- The non-nullable local variable 'i' must be assigned before it can be used.

In my case the variable in question was even the same name as a propriety for the class case I was testing. So by doing like MyClass(: var i) I completely shadowed my variable and was left wondering why I got that error bellow.

@dart-github-bot
Copy link
Collaborator

Summary: Self-assignment (a = a) doesn't trigger unnecessary_statements, but should. This impacts shadowed variables, causing confusion with null safety.

@dart-github-bot dart-github-bot added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Nov 21, 2024
@lrhn lrhn added type-enhancement A request for a change that isn't a bug and removed triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Nov 21, 2024
@bwilkerson
Copy link
Member

That's valid for local variables (including parameters), but isn't safe to conclude for getters and setters, both of which can have side-effects. They shouldn't have side-effects, but they can, so we can't assume that they won't.

@parlough
Copy link
Member

Note that there is a separate no_self_assignments rule. Is that sufficient?

@FMorschel
Copy link
Contributor Author

Yes, I think it would be. Then two questions:

  • What about the concerns that @bwilkerson raised? The examples on the site do suggest it would warn for getters-setters.
  • Why aren't these on the recommended set? It feels like they are simple enough that they should.

@pq pq added analyzer-warning Issues with the analyzer's Warning codes P3 A lower priority bug or feature request labels Nov 21, 2024
@bwilkerson
Copy link
Member

Why aren't these on the recommended set?

I don't know, but my guess is that it's probably because the lint flags the use of getters and setters, and as a result there was probably a feeling that there's too much potential for false positives.

@FMorschel FMorschel changed the title Self-assigning should trigger unnecessary_statements unnecessary_statements and no_self_assignments discussion Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-warning Issues with the analyzer's Warning codes 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) type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants