dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
633 stars 170 forks source link

Inaccurate unnecessary_await_in_return #1674

Closed ThinkDigitalSoftware closed 6 months ago

ThinkDigitalSoftware commented 5 years ago

Describe the issue Linter shows unnecessary_await_in_return if in an expression body, but not when the statement is a block body.

To Reproduce

 onInitialBuild: (store) async {
              await Future.wait(
                  widget.student.guardians.map(
                    (guardian) => guardian.inflate(),
                  ),
                );
            },

No issue here, but when converted to an expression body, it shows a warning.

StoreConnector<AppState, ViewStudentViewModel>(
            onInitialBuild: (store) async => await Future.wait( // warning here.
                  widget.student.guardians.map(
                    (guardian) => guardian.inflate(),
                  ),
                ),

Expected behavior if it's a simple refactor, then it shouldn't change the correctness of the statement. We should either not offer the refactor, or not show a warning here. I know the fat arrow is short for return, but that's not always desirable nor correct, especially when using void functions.

Screen Shot 2019-08-09 at 5 06 07 PM Screen Shot 2019-08-09 at 5 06 17 PM
pq commented 5 years ago

Good catch. Thanks for the report!

ThinkDigitalSoftware commented 5 years ago

My pleasure!

pq commented 5 years ago

/cc @a14n

a14n commented 5 years ago

I don't see this as a bug.

In the first snippet there's no return and async/await are needed.

I rather think the refactoring should be changed to have convertions (block/expression) as follow:

Future f() => aFuture;
Future f() async {
  return await aFuture;
}

Future<void> v() => aVoidFuture;
Future<void> v() async {
  await aVoidFuture;
}
ThinkDigitalSoftware commented 5 years ago

In my case, the callback definition isn't async, but it supports it. Just like onPressed() callbacks. so I can't define that it's a Future. Not sure if that makes a difference

typedef OnInitialBuildCallback<ViewModel> = void Function(ViewModel viewModel)
srawlins commented 6 months ago

I agree with @a14n; the code in question doesn't have a return statement, so the lint rule does not apply.