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

False positive with await_only_futures #58492

Open
Cyp opened this issue Aug 26, 2021 · 2 comments
Open

False positive with await_only_futures #58492

Cyp opened this issue Aug 26, 2021 · 2 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-false-positive linter-set-core P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@Cyp
Copy link

Cyp commented Aug 26, 2021

Describe the issue
False positive with await_only_futures.

Also, even completely fails to compile valid code in some cases, so may not just be a linter bug.

To Reproduce

  • // Spurious warning with await_only_futures.
Future<void> f<T>(T x) async {
  if (x is Future) {
    await x; // warning: "'await' applied to 'T & Future<dynamic>', which is not a 'Future'."
  }
}
  • // Spurious warning with await_only_futures (almost same as f)
Future<void> g<T extends Future>(T x) async {
  if (x is Future) {
    await x; // warning: "'await' applied to 'T & Future<dynamic>', which is not a 'Future'."
  }
}

  • // No error.
Future<int> h<T>(T x, Future<int> y) async => x is Future<int> ? x : y;
  • // Spurious error, despite being logically identical to h!
    // Since a value of type T & Future<int> does have the required type int | Future<int>, the error message is inconsistent.
    // Doing return x as Future<int>; compiles, but (correctly) warns about an unneccesary cast.
Future<int> i<T>(T x, Future<int> y) async {
  if (x is Future<int>) {
    return x; // error: "A value of type 'T & Future<int>' can't be returned from the function 'i' because it has a return type of 'Future<int>'."
    return x as Future<int>; // warning: "Unnecessary cast."
  }
  return y;
}
  • // No error, despite being exactly identical to i except for async.
Future<int> j<T>(T x, Future<int> y) {
  if (x is Future<int>) {
    return x;
  }
  return y;
}

Expected behavior
No warnings or errors except for the valid warning: "Unnecessary cast.".

Additional context
None.

@bwilkerson
Copy link
Member

Thanks for the report.

The lint rule looks only at the static type of the expression following await when deciding whether or not to create a diagnostic.

In the first two cases the issue isn't the lint rule, it's the fact that the parameter x wasn't promoted to Future. That's a result of how promotion works in Dart. You might want to open an issue in the SDK repo to ask for more specifics of why it didn't promote.

The remaining cases don't have any await expressions in them, so they don't apply to this lint. Again, it sounds like you have questions about type promotion, and those would be best asked in the SDK repo.

@pq pq added linter-set-core linter-false-positive type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jul 18, 2022
@pq pq added the P2 A bug or feature request we're likely to work on label Nov 14, 2022
@lrhn
Copy link
Member

lrhn commented Jun 14, 2023

This is (still) a lint issue.

In the first two original example, x was not promoted to Future, but it was promoted to T & Future<dynamic> which is subtype of Future<dynamic>. The lint warning says "which is not a Future", and that's incorrect. It is a future. It should be awaited. (We should check whether unawaited_futures counts it too.)

The other cases do indeed not apply, since there is no await, but they show issues that also apply to await.

In general the lint does not recognize that something typed as a type varaible can be a future, but it can if the bound or promotion is a future-like type.

The lint currently allows await e if e has a static type which is "future-like", meaning:

  • dynamic
  • Never
  • FutureOr<Something>
  • class type which implements Future<Something> (is subtype of Future<Object?>)
  • T? where T is future-like.

It should add two more future-like types:

  • type variable X with bound T where T is future-like
  • promoted type variable X & T where T is future-like.

Those two types are also subtypes of Future if T is, and FutureOr-bounded if T is.
Both cases can also be awaited, and the type inference of await will assume that it actually waits for a future.

Example:

import "dart:async";
void main() {}
void foo<T, F extends Future<T>, R extends F, O extends FutureOr<T>>() async {
  (await expr<F>()).s<E<T>>; 
  (await expr<R>()).s<E<T>>; 
  (await expr<O>()).s<E<T>>; 
  {
    var x = expr<T>();
    if (x is Future<int>) {
      (await x).s<E<int>>;
    }
  }
  {
    var x = expr<T>();
    if (x is FutureOr<int>) {
      (await x).s<E<int>>;
    }
  }

}
T expr<T>() => throw "static only test";


typedef E<T> = T Function(T);
extension <T> on T {
  void s<X extends E<T>>(){}
}

(The example contains code to test the static type after the await, so it's clear that the await does something. You can change the type inside the .s<E<...>> to see that the static type is precisely that type.)

The lint gives the following warnings, one at each await:

info
line 4 • Uses 'await' on an instance of 'F', which is not a subtype of 'Future'.[ (view docs)](https://dart-lang.github.io/linter/lints/await_only_futures.html)
Try removing the 'await' or changing the expression.
info
line 5 • Uses 'await' on an instance of 'R', which is not a subtype of 'Future'.[ (view docs)](https://dart-lang.github.io/linter/lints/await_only_futures.html)
Try removing the 'await' or changing the expression.
info
line 6 • Uses 'await' on an instance of 'O', which is not a subtype of 'Future'.[ (view docs)](https://dart-lang.github.io/linter/lints/await_only_futures.html)
Try removing the 'await' or changing the expression.
info
line 10 • Uses 'await' on an instance of 'T & Future<int>', which is not a subtype of 'Future'.[ (view docs)](https://dart-lang.github.io/linter/lints/await_only_futures.html)
Try removing the 'await' or changing the expression.
info
line 16 • Uses 'await' on an instance of 'T & FutureOr<int>', which is not a subtype of 'Future'.[ (view docs)](https://dart-lang.github.io/linter/lints/await_only_futures.html)
Try removing the 'await' or changing the expression.

In three of the cases it's just plain wrong that the type is not a subtype of future. The types F, R and T & Future<int> are all subtypes of Future.

In the other two cases, the type is a subtype of FutureOr<T>, but not Future<T> (the type is FutureOr<T>-bounded). The lint should allow those to be awaited too.

In each case, awaiting is correct and does something - the type of the result of awaiting differs from the type before awaiting. Omitting the await would change the program's type, so removing it would be wrong.
(The type being the same after awaiting is a necessary, but not sufficient, requirement for an await being unnecessary.)

@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-false-positive linter-set-core P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

5 participants