python-trio / flake8-async

Highly opinionated linter for Trio code
https://flake8-async.readthedocs.io
MIT License
17 stars 2 forks source link

False alarms in `libcst` backend #174

Closed Zac-HD closed 1 year ago

Zac-HD commented 1 year ago
import trio

# Seems like the inner context manager 'hides' the checkpoint.
async def does_contain_checkpoints():
    with trio.fail_after(1):  # false-alarm TRIO100
        with trio.CancelScope():  # or any other context manager
            await trio.sleep_forever()

# -------------------------

async def elems():
    await trio.lowlevel.checkpoint()
    print("here")
    return ["elem1", "elem2"]

async def await_in_comp_target():
    return (print(x) for x in await elems())  # false-alarm TRIO910

trio.run(await_in_comp_target)  # prints "here", although not any elements
Zac-HD commented 1 year ago

As an unrelated issue, I spent a while fiddling with autofixing but didn't get that to work. Can you add a simple example to the documentation, and ensure that there's a simple "fix everything you can" option (e.g. bare --autofix would be neat).

jakkdl commented 1 year ago
async def await_in_comp_target():
    return (print(x) for x in await elems())  # false-alarm TRIO910

This is due to: https://github.com/Zac-HD/flake8-trio/blob/dd8dd3caf9669237d0b2a4c7d86ba2864f65edaa/flake8_trio/visitors/visitor91x.py#L772-L775

But if you rewrite the original code with a list comprehension instead, it will not raise an alarm.

async def await_in_comp_target():
    return [print(x) for x in await elems()]  # now raises no error

Maybe unintuitive (and could maybe add a special error message for this case), but I think the logic is sound?

jakkdl commented 1 year ago

I was hoping to open a fifth PR, but alas :grin:

Zac-HD commented 1 year ago

No, you can still open another PR - from the Python docs:

the iterable expression in the leftmost for clause is immediately evaluated, so that an error produced by it will be emitted at the point where the generator expression is defined, rather than at the point where the first value is retrieved

_ = (await x async for y in await z)
#    ^       ^              ^ this always runs!
#    ^       ^ this might not run
#    ^ this might not run

So visit_GeneratorExp does need to be non-trivial 😁