sdk icon indicating copy to clipboard operation
sdk copied to clipboard

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

Open apwilson opened this issue 7 years ago • 8 comments

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
}

apwilson avatar Jan 11 '18 18:01 apwilson

/cc @leafpetersen @floitschG @lrhn

zanderso avatar Jan 11 '18 18:01 zanderso

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.

zoechi avatar Jan 12 '18 08:01 zoechi

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

leafpetersen avatar Jan 12 '18 20:01 leafpetersen

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 avatar Jan 12 '18 22:01 lrhn

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.

lrhn avatar Jun 22 '18 11:06 lrhn

Should be covered by the linter rule unnecessary_statement https://github.com/dart-lang/sdk/issues/57718

zoechi avatar Jun 22 '18 11:06 zoechi

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 avatar Jun 17 '20 20:06 srawlins

Note that this is essentially the same request as https://github.com/dart-lang/sdk/issues/53537, but both are good examples.

srawlins avatar Jun 05 '25 19:06 srawlins