-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Move unawaited to package:meta? #58351
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
Comments
When we turn on |
We attempted to move the Since then a proposal was made to add an extension method (getter) to |
Taking it further, I wonder if we might consider landing this in the SDK proper? /cc @lrhn |
No reason to put it into the SDK. It doesn't do anything. The |
Thanks @lrhn! I guess my thinking is that an increasing number of people are depending on it and more still if Alternatively, I'm drawn to the idea of fire-and-forget annotations on the declaration side (#58348); curious how that sits with you. (And @davidmorgan, if that would make |
Putting more annotations in the platform libraries sits badly with me, unsurprisingly. The fewer the better, and with I don't think annotations on the declarations is the way to go. It assumes that the declaration point knows how the result will be used, which is a stretch. It's probably sometimes true that an async function can say that it's return value is FYI only, and only if it can guarantee that the future will never complete with an error, but most such async functions are written in user libraries. The platform library async functions are generally so generic and user-data dependent that they it's hard to say anything universal about them. Since we are recommending this lint, we might want to expose the same tool in other places as well, possibly re-exporting it from whichever package we distribute the recommended lints using. I still think |
I believe @goderbauer had examples of framework async functions that do know if the return value is FYI or not? |
Ack. To me, framework libraries are also "user libraries" 😁. I meant it as non-platform libraries. |
Two additional thoughts. What if we stopped recommending Note that in some other issue there is discussion of adding an annotation to mark functions whose returned future can safely be ignored. I have to wonder whether that would cover most or all of the places where this lint is currently being ignored (using either mechanism) and hence largely negate the need for a way to silence the lint, making the function even less useful/necessary. |
I hypothesize that something like this is true. Something like: 80% of |
For google3 we have gone pretty strongly for never advising use of '// ignore' comments; I don't think that will change for 'unawaited' even if it's only 20% or 5% of uses. I don't mind if the external lint message / doc changes, though, since we can now set a google3-specific one if we want. |
To be clear, I wasn't suggesting changes to internal usage, only changing the messaging externally. And external users would still be able to use Although if my hypothesis is true, and we could annotate the appropriate internal APIs as not requiring an |
After deprecating of pedantic and introducing flutter_lints, how about moving unawaited to some core library? Upd. Already done https://api.dart.dev/stable/2.14.2/dart-async/unawaited.html |
@subzero911 you may be looking for dart:async's unawaited. I think this issue can be closed. Edit: Oops I missed your update. 😄 |
The
unawaited_futures
lint is part of the core.yaml set of lints [1] that will be distributed as part of a new package. It is a little odd that in order to fully use it and get access to theunawaited
function recommended by the lint you have to also depend on package:pedantic, which is yet another unrelated package bundling opinionated linter rules.Maybe the
unawaited
function recommended by the lint should live somewhere else, like the meta package? That package is home to most other annotations that modify how lint rules and the analyzer behave./cc @pq
[1] https://github.com/dart-lang/linter/blob/bf95e79bccaedaa17824e364fad8c291a1543bd6/tool/canonical/core.yaml
The text was updated successfully, but these errors were encountered: