Skip to content

No analyzer warning or any other errors when I use a timer wrong with =>. #57669

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

Open
apwilson opened this issue Jan 11, 2018 · 7 comments
Open
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@apwilson
Copy link
Contributor

I wrote the following code and was surprised when _updateTime was never called:

new Timer.periodic(
  const Duration(seconds: 1),
  (_) => _updateTime,
);

What the code should have looked like was the following:

new Timer.periodic(
  const Duration(seconds: 1),
  (_) => _updateTime(),
);

The surprising part about this is in the first code set I was returning a function instead of calling it which is invalid as the Timer callback is a void function but I received no run-time or analysis-time errors.

Why I'd expect an error is because this does generate an analysis error which I think is synonymous to the first code set:

new Timer.periodic(
  const Duration(seconds: 1),
  (_) {
    return _updateTime;
  },
);

_updateTime's signature looks like this:

void _updateTime() {
 // do stuff
}
@zanderso
Copy link
Member

/cc @leafpetersen @floitschG @lrhn

@zoechi
Copy link
Contributor

zoechi commented Jan 12, 2018

I don't think there is anything the analyzer can do, is there?
Timer.periodic expects a function with one parameter and (_) => _updateTime exactly matches that requirement.

There is also nothing wrong with the function

(_) => _updateTime

A function is allowed to contain arbitrary expressions and _updateTime is a perfectly valid expression, it's just not the expression the developer actually wanted, but how what should that be inferred from?

dart-lang/linter#866 might be related.

@leafpetersen
Copy link
Member

This is an unfortunate side effect of the decision to allow arrow functions with a void return type to return values. If you rewrite that callback as a block bodied function with return _updateTime you'll get an error.

I don't immediately see a language fix for this, given that we did decide to allow returning values from void typed arrow functions.

This feels like a really solid case for a hint or a lint though. To me, it seems clear that:

  • using a function typed variable as a statement (without calling it) is almost certainly an error
  • returning a function typed variable from void typed arrow function is almost certainly an error
    • especially if the function typed variable has return type void itself

cc @munificent @pq @bwilkerson

@lrhn
Copy link
Member

lrhn commented Jan 12, 2018

One helping hand could be that the analyzer recognizes that you have a body of a void function with no side-effects. I don't know if the analyzer catches "useless operations" like print; or {x:y}.toList;, generally closurization of a method (which is one thing we know has no side-effect) as the last operation of a statement - but if it does, then the return position of a void .. => function could be treated the same.

@lrhn
Copy link
Member

lrhn commented Jun 22, 2018

This is not something we are planning to change in the language right now.
If we revisit void in the future, we might reassess that part as well, but that's not currently planned.

@zoechi
Copy link
Contributor

zoechi commented Jun 22, 2018

Should be covered by the linter rule unnecessary_statement #57718

@srawlins srawlins transferred this issue from dart-lang/sdk Jun 17, 2020
@srawlins
Copy link
Member

CC @bwilkerson and @pq; we've recently had a really good discussion on this one, and a lint rule idea. Not sure if another issue for the lint rule idea exists yet.

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 29, 2024
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
@pq pq added the P3 A lower priority bug or feature request label Nov 20, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

9 participants