Skip to content

Dartanalyzer unused-variable hint is incorrect #29478

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

Closed
mkustermann opened this issue Apr 26, 2017 · 5 comments
Closed

Dartanalyzer unused-variable hint is incorrect #29478

mkustermann opened this issue Apr 26, 2017 · 5 comments
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@mkustermann
Copy link
Member

Here is an example where a variable is used but the analyzer thinks it is not used:

class Foo {
  Foo operator |(Foo _) {
    return this;
  }
}
main() {
  var f = new Foo();
  f |= new Foo();
}
$ dartanalyzer test.dart
Analyzing [test.dart]...
[hint] The value of the local variable 'f' isn't used. (test.dart, line 9, col 7)
1 hint found.
$ dartanalyzer --version
 dartanalyzer version 1.22.1
@mkustermann mkustermann added the legacy-area-analyzer Use area-devexp instead. label Apr 26, 2017
@lrhn
Copy link
Member

lrhn commented Apr 26, 2017

I think the analyzer is correct. The variable is used, but the value of the variable, assigned by |=, is not.
It means that the assignment is superfluous, and you could replace the line with f | new Foo();.

@mkustermann
Copy link
Member Author

That is an interesting observation, but:

  1 class Foo {
  2   Foo operator |(Foo _) {
  3     return this;
  4   }
  5 }
  6 
  7 main() {
  8   var f = new Foo();
  9   f |= new Foo();
 10 }
$ dartanalyzer test.dart
Analyzing [test.dart]...
[hint] The value of the local variable 'f' isn't used. (test.dart, line 8, col 7)
1 hint found.

The location the analyzer is pointing to is the variable itself (line 8) not the location of the |= assignment.

Clearly the variable f, as well as one of it's assignments, is used, namely as the receiver of f.|(new Foo()).

A variable can have several different assignments. If the value after a variable assignment is not used until the next assignment, then that specific assignment should be marked as being unnecessary, but not the variable itself.

@lrhn Would you agree?

@bwilkerson bwilkerson added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Apr 26, 2017
@bwilkerson
Copy link
Member

Having some analysis to point out when a local variable is not referenced after an assignment (and hence the assignment is not necessary) would be useful. But it is different from what this was attempting to find, which is local variables that are not used at all. The point is not that you can eliminate the assignment, the point is that you can eliminate the variable by replacing both lines with new Foo() | new Foo().

@mkustermann
Copy link
Member Author

the point is that you can eliminate the variable by replacing both lines with new Foo() | new Foo()

Sure, but that can be said for this code as well:

var x = new Foo();
x.bar()

Here we can just do new Foo().bar() instead, but shouldn't we let the programmer decide he wants to use a variable or not?

@srawlins
Copy link
Member

srawlins commented Dec 5, 2018

Duplicate of #27705

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

4 participants