Repository navigation
Ideas for flake8-trio checks #1
Description
As a "list of ideas" issue, any of the ideas below might turn out to be too difficult to implement, or in fact counterproductive if implemented. Nonetheless, we need somewhere to dump ideas and this issue is that place. Items are in rough priority order by a combination of estimated importance and tractability.
- Never
yieldinside a nursery or cancel scope
Trio101: Never yield inside a nursery or cancel scope #4 - Anything that
awaits inside afinally:clause needs shielding and a timeout, e.g.with trio.fail_after(1, shield=True):(source) (also applies toasync fororasync with)
Trio102: await inside finally needs shielding and timeout #5 - Never have an
except Cancelledorexcept BaseExceptionblock with a code path that doesn't re-raisethe error.
Trio103: except Cancelled/BaseException must be re-raised #8 - Calling a Trio async function without immediately
awaiting it. Per the discussion here typecheckers have some support for the (difficult!) general case, but it's easy and useful to make a list of all Trio functions and check for e.g.trio.sleep(0)without anawait. We don't need to checkasync fororasync withbecause using the sync syntax is a noisy error at runtime, unlike the checkpoint warnings.
TRIO105: Calling trio async function without await #10 - Importing things from the main
trionamespace in a way that isn't justimport trio(this is really just to make the linter easier to implement, but it's consistent with ~99% of existing usage anyway)
Add 106, don't fancy import trio #11 - Async functions must have at least one checkpoint (i.e.
await,async for, orasync with) on every code path, unless an exception is raised (the same rule that Trio internals follow)
6: async checkpoints #16 - Async iterables (i.e. with a
yield) should have at least one checkpoint in each loop iteration, and an additional checkpoint after the last iteration (unless an exception is raised, again like the Trio internals)
7: async iterables must checkpoint after yields #17 - Expanding
TRIO102- you need a shielded cancel scope toawaitinside anexcept Cancelled/BaseExceptionblock, not justfinally:
Extend 102 to excepts, 104 also catch yield #18 - Async function definitions should not have a
timeoutparameter - encourage use oftrio.fail_after()ortrio.move_on_after()instead. (httpxdoes OK here, but most uses are IMO a design mistake)
Add trio[300], async functions should not have a timeout parameter #20 - Prefer waiting on a
trio.Eventrather than sleeping in a loop. Concretely I think matchingwhile not <expr>: await trio.sleep(...)might work well here?
10: add trio[301], don't await trio.sleep inside loop #21 - When nesting async context managers, nurseries should be
outermostinnermost. (actually pretty complicated?)
Add trio111, passing variables from context managers to nurseries opened outside them #22 - Warn if we see a nursery body that contains exactly one statement, and that statement is a call to
nursery.start[_soon]("consider using a regular function call instead"), unlessnurseryis passed as an argument to the start-soon'd function.
TRIO112: replace one-line nursery with regular function call #34
Activity
jakkdl commentedon Jul 17, 2022
For #1, python-trio/trio#264 has a
trio.open_cancel_scopewhich I can't find in the docs. So I'm guessing that's removed, and as such we only care about yielding in nurseries?Must this include a check that it's not within a
@contextmanageror an@asynccontextmanager?Zac-HD commentedon Jul 17, 2022
Nurseries, and
trio.{fail,move_on}_{at,after}(), yeah. And yep, we'll want to check for the presence of a context-manager decorator to avoid false alarms 👍jakkdl commentedon Jul 27, 2022
Possibly supportimport trio as <foo>andfrom trio import *with varying levels of sophistication (assume everything called e.g.open_nurseryis from trio, vs parse the imports) depending on need.Warning if user tries to
import trio as <foo>orfrom trio import <>since that is unsupported.jakkdl commentedon Jul 30, 2022
Is this about looking at the function signature
or function calls with a timeout keyword? (excluding calls to the trio library)
or both?
jakkdl commentedon Jul 30, 2022
is the
notin the loop condition significant? Or should any calls toawait trio.sleep(...)inside loops be errors?the latter feels like it might send a lot of false alarms?
Zac-HD commentedon Jul 30, 2022
Function definitions - nothing wrong with using the API available to you, but for user-controlled code Trio's idiomatic cancellation scopes are so much nicer.
Let's go with "any
whileloop where the body consists entirely ofawait trio.sleep[_until](...)". It's a pretty narrow check that way, but it's part of a useful category I think of as 'docs presented as you need them'.jakkdl commentedon Jul 30, 2022
Should there be a checkpoint before the first yield?
Zac-HD commentedon Jul 31, 2022
That's allowed but not required.Oops, yes, there should be; see #17 (review)
jakkdl commentedon Aug 2, 2022
Would be nice if you made a comment here (or somewhere) when updating the task list - I get no notification when you edit it otherwise.
Zac-HD commentedon Aug 2, 2022
Oops, will do!
jakkdl commentedon Aug 5, 2022
All current Tasks now have pull requests associated with them, and #8 is merged already.
Zac-HD commentedon Aug 5, 2022
Added one last task based on a suggestion in the upstream issue 🙂
Zac-HD commentedon Aug 11, 2022
All done 🎉