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

quick fix for unawaited_futures inserts and invalid await in a cascade #54105

Closed
natebosch opened this issue Nov 20, 2023 · 4 comments
Closed
Labels
analyzer-quick-fix area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on

Comments

@natebosch
Copy link
Member

class C {
  Future<String> something() {
    return Future.value('hello');
  }
}

void main() async {
  C()..something(); // unawaited_futures
}

Running the quick fix "Add 'await' keyword" results in invalid code.

class C {
  Future<String> something() {
    return Future.value('hello');
  }
}

void main() async {
  C()await ..something(); // expected to find ';'
}

An alternative fix to suggest could be to ignore the future, but I'm not sure if it's a good idea to offer that. WDYT @lrhn

class C {
  Future<String> something() {
    return Future.value('hello');
  }
}

void main() async {
  C()..something().ignore();
}
@natebosch natebosch added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-quick-fix labels Nov 20, 2023
@natebosch natebosch changed the title quick fix for unawaited_future inserts and invalid await in a cascade quick fix for unawaited_futures inserts and invalid await in a cascade Nov 20, 2023
@lrhn
Copy link
Member

lrhn commented Nov 20, 2023

I think ignore may be the best we can do for now.

The problem with a cascade invocation of an async function, is that the value is never available as an expression, so there is no way to wrap in unawaited or await.

We could add something, but the real answer here is "if it hurts, stop doing it!".
You shouldn't call an async function that way, just make it a non-cascade, and treat the future properly.
Or use ignore, which is safer than unawaited if an error actually happens.

Or wait for us to finally add a suffix .await.

Today: use ignore, rewrite to not be a cascade, or refuse to do anything.
Either it's fine with me.

@bwilkerson
Copy link
Member

We could certainly write a fix (or extend the existing fix) to split the cascade appropriately so that an await could be added, but I suspect that this case won't come up very often and that we're better off just disabling the fix for invocations in cascades.

@bwilkerson bwilkerson added the P2 A bug or feature request we're likely to work on label Nov 20, 2023
@natebosch
Copy link
Member Author

we're better off just disabling the fix for invocations in cascades.

This SGTM as well.

@hugo555686
Copy link

We could certainly write a fix (or extend the existing fix) to split the cascade appropriately so that an await could be added, but I suspect that this case won't come up very often and that we're better off just disabling the fix for invocations in cascades.

I'm sorry, I have no idea about things here, I didn't mean to make a mistake on purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-quick-fix area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

4 participants