add async124 async-function-could-be-sync

[jakkdl]

fixes #253

given that converting an async test to a sync test will give weird errors agronholm/anyio#803 pytest-dev/pytest#10839 we should probably err on the side of not erroring, at least until pytest/anyio is fixed, which would extend to 910 and 911.

[Zac-HD]

Implementation looks good, but I'm pretty concerned about the false-alarm rate.

• Let's also exclude methods on a class; in those cases it's reasonably likely that it 'has to be async' in order to match the interface of some other type
• If there's a convenient way to try it out we could get some empirical feedback? I can't think of one though.

[jakkdl]

Implementation looks good, but I'm pretty concerned about the false-alarm rate.

• Let's also exclude methods on a class; in those cases it's reasonably likely that it 'has to be async' in order to match the interface of some other type

Hmhm, I also feel like this would lose a bunch of the upside of it. Maybe a separate error code?

• If there's a convenient way to try it out we could get some empirical feedback? I can't think of one though.

push a release and wait for complaints? 😆
We could push it as default-disabled and mention in changelog + ask on anyio/trio gitter and python discord (started hanging out a bit in the async channel) for people to try it out and give feedback if it should be default-enabled or not.
I should also add the linter to the trio/ repo

[jakkdl]

after messing around in trio:

1. this is implemented in ruff preview as https://docs.astral.sh/ruff/rules/unused-async/
   • their tests are in https://github.com/astral-sh/ruff/blob/f4263490513ff1f5283b6801de4744a28f36df98/crates/ruff_linter/resources/test/fixtures/ruff/RUF029.py and include a couple cases I missed, e.g. (x async for y in z)
2. We definitely need to suppress it for methods, cray false alarm rate. If one wanted to be very specific you could specifically look for __aiter__ and stuff but it's probably not worth it. RUF029 is disabled for class methods
3. RUF029 further has lots of edge cases brought up in RUF029 (unneeded async) needs nuance for class methods astral-sh/ruff#10980, but we probably don't need to be as thorough.
   • EDIT: the best way to go about it would probably be no-checkpoint-warning-decorators... actually yeah I should maybe just use that directly

[Zac-HD]

Looks good to me; optional nitpick below.

[Zac-HD]

I'm also inclined to include FastAPI support in the default no-checkpoint-decorators; reducing the low-value-alarm rate for users is more important than my personal distaste for missing checkpoints 😅. That can easily go in a future PR though.

[jakkdl]

I'm also inclined to include FastAPI support in the default no-checkpoint-decorators; reducing the low-value-alarm rate for users is more important than my personal distaste for missing checkpoints 😅. That can easily go in a future PR though.

hrm. suppressing it for async124 makes lots of sense, but less so for async9xx... if they got suppressed for cases where async124 would get suppressed >_<
I guess that's a point in favor of default-disabling it for async124 in particular regardless of no-checkpoint-decorator

[jakkdl]

I'm also inclined to include FastAPI support in the default no-checkpoint-decorators; reducing the low-value-alarm rate for users is more important than my personal distaste for missing checkpoints 😅. That can easily go in a future PR though.

hrm. suppressing it for async124 makes lots of sense, but less so for async9xx... if they got suppressed for cases where async124 would get suppressed >_< I guess that's a point in favor of default-disabling it for async124 in particular regardless of no-checkpoint-decorator

opened #313 to procrastinate on this while I have way-too-many-balls in the air simultaneously 😇

[jakkdl]

So this one fell by the wayside for.. no real reason. But I've merged main and done some minor edits, I'll merge it in a couple days unless somebody has any comments