Ideas for flake8-trio checks

[Zac-HD]

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 yield inside a nursery or cancel scope
  Trio101: Never yield inside a nursery or cancel scope #4
• Anything that awaits inside a finally: clause needs shielding and a timeout, e.g. with trio.fail_after(1, shield=True): (source) (also applies to async for or async with)
  Trio102: await inside finally needs shielding and timeout #5
• Never have an except Cancelled or except BaseException block with a code path that doesn't re-raise the 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 an await. We don't need to check async for or async with because 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 trio namespace in a way that isn't just import 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, or async 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 to await inside an except Cancelled/BaseException block, not just finally:
  Extend 102 to excepts, 104 also catch yield #18
• Async function definitions should not have a timeout parameter - encourage use of trio.fail_after() or trio.move_on_after() instead. (httpx does 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.Event rather than sleeping in a loop. Concretely I think matching while 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 outermost innermost. (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"), unless nursery is passed as an argument to the start-soon'd function.
  TRIO112: replace one-line nursery with regular function call #34

[jakkdl]

For #1, python-trio/trio#264 has a trio.open_cancel_scope which 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 @contextmanager or an @asynccontextmanager?

[Zac-HD]

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]

Possibly support import trio as <foo> and from trio import * with varying levels of sophistication (assume everything called e.g. open_nursery is from trio, vs parse the imports) depending on need.
Warning if user tries to import trio as <foo> or from trio import <> since that is unsupported.

[jakkdl]

Async functions should not have a timeout parameter - use trio.fail_after() or trio.move_on_after() instead. (httpx does OK here, but most uses are IMO a design mistake)

Is this about looking at the function signature

async def foo(timeout): # error
  ...

or function calls with a timeout keyword? (excluding calls to the trio library)

foo(timeout=15) # error

or both?

[jakkdl]

Prefer waiting on a trio.Event rather than sleeping in a loop. Concretely I think matching while not <expr>: await trio.sleep(...) might work well here?

is the not in the loop condition significant? Or should any calls to await trio.sleep(...) inside loops be errors?
the latter feels like it might send a lot of false alarms?

for i in range(...):
  ...
  if ...:
    await trio.sleep(...)
  ...

[Zac-HD]

Is this about looking at the function signature [...] or function calls with a timeout keyword?

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.

Prefer waiting on a trio.Event rather than sleeping in a loop. Concretely I think matching while not <expr>: await trio.sleep(...) might work well here?

is the not in the loop condition significant? Or should any calls to await trio.sleep(...) inside loops be errors? the latter feels like it might send a lot of false alarms?

Let's go with "any while loop where the body consists entirely of await 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]

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)

Should there be a checkpoint before the first yield?

[Zac-HD]

That's allowed but not required.

Oops, yes, there should be; see #17 (review)

[jakkdl]

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]

Oops, will do!

[jakkdl]

All current Tasks now have pull requests associated with them, and #8 is merged already.

[Zac-HD]

Added one last task based on a suggestion in the upstream issue 🙂

[Zac-HD]

All done 🎉