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

Idea: lint when passing async functions into forEach #891

Open
greglittlefield-wf opened this issue Feb 12, 2018 · 7 comments
Open

Idea: lint when passing async functions into forEach #891

greglittlefield-wf opened this issue Feb 12, 2018 · 7 comments
Labels
lint-request type-enhancement A request for a change that isn't a bug

Comments

@greglittlefield-wf
Copy link

I recently ran into a few instances of code that used the following:

values.forEach((value) async {
  // ...
  doSomethingSync();
  await value.doSomethingAsync();
});
map.forEach((key, value) async {
  doSomethingSync();
  await value.doSomethingAsync();
});

The intended behavior in at least one of the cases was to wait for all of the awaits to be run before proceeding, which can be done via Future.forEach or a for loop with awaits.

Even if the intended behavior is what is actually happening, it might not be immediately obvious to someone unfamiliar with how async functions are executed in Dart 1.x.

The first example would be avoided by using the prefer_foreach rule (unless a tearoff were passed in instead), but the second would not. Still, for the first example, it might be good to have a more specific lint to catch this case.

Are Iterable.forEach/Map.forEach common-enough special cases where using async functions should result in a lint?

Another idea, though it would have far more impact and may be too noisy, would be to lint when an async function is being used where a void function is accepted.

CallbackSubscription registerVoidCallback(void callback()) { /*...*/ }

// Good
registerVoidCallback((() { /* ... */ });
// Would lint
registerVoidCallback((() async { /* ... */ });
@matanlurey
Copy link
Contributor

Async functions will be executed synchronously in Dart 2, FWIW.

@greglittlefield-wf
Copy link
Author

Good point! This lint would definitely have more impact in Dart 1.

@pq pq added type-enhancement A request for a change that isn't a bug and removed type-enhancement A request for a change that isn't a bug labels Jun 29, 2018
@natebosch
Copy link
Member

natebosch commented Oct 23, 2018

See also #1123

Should we merge these issues?

@jamesderlin
Copy link

I see this confusion all the time on StackOverflow. It would be nice if there were a warning about passing Future-returning callbacks to Iterable.forEach/Map.forEach.

@pq
Copy link
Member

pq commented Sep 23, 2020

@jamesderlin: the next time you see this come up on SO, could you add a link? That'd be great for motivation. Thanks!

@pq
Copy link
Member

pq commented Sep 25, 2020

Amazing. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lint-request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants