astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
30.94k stars 1.02k forks source link

Implement flake8-async rules #8451

Open charliermarsh opened 10 months ago

charliermarsh commented 10 months ago

General Rules

Blocking sync calls in async functions

Asyncio-specific rules

Optional rules disabled by default

Resources

qdegraaf commented 10 months ago

TRIO118 sounds like it is not needed in Ruff, similar to TRIO106

The TRIO2XX rules are partially covered by the port of flake8_async.

The recommended fixes in flake8-trio are TRIO specific though. How do we want to combine those two?

jakkdl commented 10 months ago

whoa, this is super cool!! As the primary author of flake8-trio I'm super honored to be included in ruff :heart: Feel free to ping me about any questions of functionality or implementation details @Zac-HD might also have opinions or suggestions about stuff

jakkdl commented 10 months ago

The recommended fixes in flake8-trio are TRIO specific though. How do we want to combine those two?

There's some anyio support as well in flake8-trio and for a while we considered renaming the library. If flake8-trio sees import anyio and no import trio in the code, or --anyio is passed it will suggest autofixes from the anyio library instead of trio.

jakkdl commented 10 months ago

TRIO118 sounds like it is not needed in Ruff, similar to TRIO106

TRIO106 definitely isn't needed by itself, and solely a consequence of implementation at the time. TRIO118 is only partially a weakness in implementation, the part about multi-backend programs still holds though.

The TRIO2XX rules are partially covered by the port of flake8_async.

Yeah there's overlap there, though I gave the implementation in flake8-trio quite a bit more love and the suggestions/error messages are more detailed/specific since it can know what the backend is.

TRIO105 is mostly redundant as of the recently released trio 0.23 which included types into the main package, and type checkers will complain about async functions not being awaited. I suppose it could be useful for somebody not using type-checking, but enabling this check, but I think that's a pretty slim proportion and the rule requires constantly keeping up with the API of anyio/trio.

Zac-HD commented 10 months ago

🥳 I'm delighted to see this going into ruff!

TRIO105 doesn't seem redundant to me - many people aren't using type annotations (everywhere), and typecheckers also don't offer autofixers the way that ruff can. Those fixers would mostly be unsafe, but still remarkably useful IMO!

charliermarsh commented 10 months ago

Thank you both so much for chiming in here, for all the helpful commentary, and for all your work on flake8-trio (among so much else)!

I already gated one rule behind "require an import trio to be present before firing" so we now have some precedent and infrastructure for that, which will likely be helpful with a few of the other rules and their associated fixes.

alex007sirois commented 9 months ago

I already gated one rule behind "require an import trio to be present before firing"

Most of the rules for trio would apply for anyio too:

The recommended fixes in flake8-trio are TRIO specific though. How do we want to combine those two?

There's some anyio support as well in flake8-trio and for a while we considered renaming the library. If flake8-trio sees import anyio and no import trio in the code, or --anyio is passed it will suggest autofixes from the anyio library instead of trio.

jakkdl commented 5 months ago

This issue should be renamed and all the rules in the checklist renamed to match upstream merging with flake8-async. See #10303

Also TRIO117 is removed (because trio.MultiError is removed), and there's two new rules ASYNC250 and ASYNC251

MichaReiser commented 2 months ago

I updated the issue to reflect the flake8-trio to flake8-async rename, crossed of the new rules implemented in #10416, and added new rules to the list to match flake8-async's documentation

Zac-HD commented 4 weeks ago

Hey all - I just tried moving some code from flake8-async to ruff check for that delicious massive speedup, and ended up back on this issue 🙂

The good news: https://github.com/astral-sh/ruff/pull/10416 also implemented ASYNC251!

The sad-for-me news: lots of other rules still to implement. If you feel inspired but not so inspired as to finish closing out this issue, ASYNC900 and ASYNC200 seem pretty simple to implement, and would be really nice to have...