Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

TRIO100 false positive with an async CM (already fixed in flake8-trio - ruff is out of date) #9934

Closed
mikenerone opened this issue Feb 11, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@mikenerone
Copy link

Ruff 0.2.1

The following snippet illustrates a TRIO100 false positive when the scope contains an async context manager. Note that this bug is already fixed in flake8-trio (in python-trio/flake8-async/pull/176 I think). It's probably worth checking whether ruff needs to port other more recent fixes, as well.

from collections.abc import AsyncGenerator
from contextlib import asynccontextmanager

import trio

@asynccontextmanager
async def a_cm_can_start_a_nursery() -> AsyncGenerator[None]:
    async with trio.open_nursery() as nursery:
        nursery.start_soon(trio.sleep_forever)
        yield

async def main() -> None:
    # ↓↓↓↓↓ TRIO100 A `with trio.CancelScope(...):` context does not contain any `await` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint.
    with trio.CancelScope() as cancel_scope:
        async with a_cm_can_start_a_nursery():
            try:
                pass # Pretend there's some real logic.
            finally:
                # Body exiting. For all I know, the CM has nursery like this one does. If so, I need it cancelled.
                cancel_scope.cancel()

trio.run(main)

The above is a valid case for a cancel scope with no explicit awaits in the body. Without the cancel(), this hangs forever instead of exiting.

Note: if I put an async for in the try:, it's even more obvious that this yields, but TRIO100 still triggers.

@charliermarsh
Copy link
Member

Thanks, will fix this tonight.

@mikenerone
Copy link
Author

@charliermarsh I missed #9855. It looks like you may have already fixed this in main.

@charliermarsh
Copy link
Member

Wow, yeah, I fixed it this week and didn't even remember!

@charliermarsh
Copy link
Member

Will be included in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants