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

await quickfix overwrite FutureOr type declaration #59838

Open
stephane-archer opened this issue Jan 3, 2025 · 2 comments
Open

await quickfix overwrite FutureOr type declaration #59838

stephane-archer opened this issue Jan 3, 2025 · 2 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.

Comments

@stephane-archer
Copy link

Input code:

class _A extends AsyncNotifier<img.Image?> {
  @override
  FutureOr<img.Image?> build() {
    var a =  await   ref.watch(refImageCompatiblePathProvider.future);
    // TODO: implement build
    throw UnimplementedError();
  }
}

The await expression can only be used in an async function. Try marking the function body with either 'async' or 'async*'.

Apply quick fix:

class _A extends AsyncNotifier<img.Image?> {
  @override
  Future<img.Image?> build() async {
    var a =  await   ref.watch(refImageCompatiblePathProvider.future);
    // TODO: implement build
    throw UnimplementedError();
  }
}

Expected code:

class _A extends AsyncNotifier<img.Image?> {
  @override
  FutureOr<img.Image?> build() async {
    var a = await ref.watch(refImageCompatiblePathProvider.future);
    // TODO: implement build
    throw UnimplementedError();
  }
}
@stephane-archer stephane-archer added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Jan 3, 2025
@lrhn
Copy link
Member

lrhn commented Jan 3, 2025

The Future return type is correct and precise, the async function will return a Future, so I don't think it's unreasonable to update the return type.

If you still want a FutureOr return type for some reason, it should be easy to change it back, but most likely you won't need that. The Future return type is a valid override of a superinterface FutureOr type, so unless you expect subclasses to override again and return a FutureOr, it shouldn't matter.

It is a little inconsistent, though.

If you look at the quick-fix used on the following examples:

int foo() {
  await Future.value();
  return 1;
}

Future<int> bar() {
  await Future.value();
  return 1;
}

FutureOr<int> baz() {
  await Future.value();
  return 1;
}

then it converts the return type to Future<int> in all three cases (trivially in the middle one).
That's necessary for foo, and unnecessary for baz.

It's inconsistent because using the quick-fix on qux here does not remove the ?:

Future<int>? qux() {
  await Future.value();
  return 1;
}

And it's downright questionable what it does to:

FutureOr<int>? biff() {
  await Future.value();
  return 1;
}

where it changes the return type to Future<int?>.
(Which may be reasonable from the perspective that any existing valid return v; statement will stay valid, including return null, but that does not explain that qux stays Future<int>?.)

Something is inconsistent. One of qux and biff should behave like the other (or the type should just not be changed if it already has a future value type).

@stephane-archer
Copy link
Author

@lrhn Because the user has already specified "FutureOr" in the input I did not expect the linter to replace it with another type, but that true that FutureOr is not necessary here so this is fine. that would not be good if FutureOr were necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.
Projects
None yet
Development

No branches or pull requests

2 participants