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

Expand 105 to check that nursery.start(...) is awaited #56

Closed
Zac-HD opened this issue Nov 16, 2022 · 2 comments · Fixed by #59
Closed

Expand 105 to check that nursery.start(...) is awaited #56

Zac-HD opened this issue Nov 16, 2022 · 2 comments · Fixed by #59

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Nov 16, 2022

I think we can just assume that almost all nurseries are named nursery and also that ~everything named nursery is in fact a nursery, at which point this is a pretty simple addition change:

https://github.com/Zac-HD/flake8-trio/blob/a900cffc2e47c36a9d0576cd384eb20ca33289b4/flake8_trio.py#L820-L821

and (
    (node.func.value.id == "trio" and node.func.attr in trio_async_functions)
    or (node.func.value.id == "nursery" and node.func.attr == "start")
)

plus tests etc.

@jakkdl
Copy link
Member

jakkdl commented Nov 17, 2022

Could also make a check that nurseries only get named nursery, and that nothing else gets named nursery.

@Zac-HD
Copy link
Member Author

Zac-HD commented Nov 17, 2022

Respectively no because there are sometimes good reasons not to (eg nested nurseries), and doesn't seem worth it (implementation complexity, false alarms, small benefits).

charliermarsh pushed a commit to astral-sh/ruff that referenced this issue Nov 5, 2023
## Summary

Adds `TRIO105` from the [flake8-trio
plugin](https://github.com/Zac-HD/flake8-trio). The `MethodName` logic
mirrors that of `TRIO100` to stay consistent within the plugin.

It is at 95% parity with the exception of upstream also checking for a
slightly more complex scenario where a call to `start()` on a
`trio.Nursery` context should also be immediately awaited. Upstream
plugin appears to just check for anything named `nursery` judging from
[the relevant issue](python-trio/flake8-async#56).

Unsure if we want to do so something similar or, alternatively, if there
is some capability in ruff to check for calls made on this context some
other way

## Test Plan

Added a new fixture, based on [the one from upstream
plugin](https://github.com/Zac-HD/flake8-trio/blob/main/tests/eval_files/trio105.py)

## Issue link

Refers: #8451
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants