Skip to content

proposal: mention that an async function has no await keyword in the function body #59454

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
stephane-archer opened this issue Apr 21, 2024 · 7 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@stephane-archer
Copy link

when you modify your code, you can remove the last await in a function so the function can become synchronous but nothing tells you that and you keep await something that doesn't need to.

@lrhn
Copy link
Member

lrhn commented Apr 21, 2024

An async functions always returns a Future.
Even if you remove the last await, the function is still asynchronous. You have to change the return type too, to make it synchronous.
At that point, it's a very different function, with a different signature.

I don't see a warning if an async functions has no await as being actionable in most cases, and sometimes it's deliberate.
Will probably have too many false positives.

@stephane-archer
Copy link
Author

stephane-archer commented Apr 22, 2024

At that point, it's a very different function, with a different signature.

From my understanding, if there is no await, the function is going to execute synchronously and finish, then return a "completed future" that the caller is going to revolve immediately when he await the future.
So I don't see much difference with the Sync version except the function signature.

I see this to be deliberated when implementing an interface for example but I imagine the linter can detect if you overwrite something.

I would be happy with false positives in my case. If I can remove a bunch of await and async functions that are just async because of old await calls that are no longer there, this will make my codebase cleaner.

Remember that linters are optional and activated manually.

@lrhn
Copy link
Member

lrhn commented Apr 22, 2024

much difference ... except the function signature.

From the outside, the signature is pretty much the function. So no difference except everything about how to use the function.

It's possibly a reasonable lint while developing, if you notice that a function that you made async because it needed to be, now no longer needs to be async. It's useful during the design phase, or for private helper functions, but not for public API.
If public API returns a future, it's presumably because it intends to return a future, and returning something else is a breaking change.

@stephane-archer
Copy link
Author

@lrhn I agree with you.
if the lint applies to private helper functions, I think it's going to be a quite useful lint.

@FMorschel
Copy link
Contributor

This lint could also come with a quick fix/assist that does the opposite of what "Convert to async function body" assist does.

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Aug 30, 2024
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 20, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 20, 2024
@pq pq added the P3 A lower priority bug or feature request label Nov 20, 2024
@FMorschel
Copy link
Contributor

From @DanTup's comment on #58862:

How could we have done a better job of letting you know about the assist?

I don't know if it's appropriate here, but I thought it may be useful to share this. In TS, sometimes there are recommendations that are diagnostics with a "hint" severity that have fixes. Unlike normal code actions, they are visible in the editor. Unlike errors/warnings/info, they do not show up in the Problems view, and instead of a full squiggle, they just have a very short grey marker.

info_diagnostic.mp4

These are easy to ignore, but they're more discoverable than general code actions that you might not see unless you happen to put your cursor in the right place.

I wonder if it would be useful to support "hint" as an option in analysis_options like this:

analyzer:
  errors:
    dead_code: hint

It could be a non-committal way of using a lint.. you're not forced to use it everywhere or put ignores all over your code, but you'll still see some minor prompts to use the fixes in your code.

I think this would be a great place to have a similar thing. Where it is not an "info" lint, but just a "nudge" (bwilkerson's nickname for this). If this was a thing, I would also probably expect that the only private functions/methods part of this could go away.

Also, this could trigger only at non-overridden methods, so this is even safer to use.

@FMorschel
Copy link
Contributor

I'd also like to mention dart-lang/language#870 related to the return of Futures without await.

@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants