search

dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
628 stars 172 forks source link

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

Open stephane-archer opened 2 months ago

stephane-archer commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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