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

various small fixes #264

Merged
merged 5 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/rules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ ASYNC112 : useless-nursery
_`ASYNC113` : start-soon-in-aenter
Using :meth:`~trio.Nursery.start_soon`/:meth:`~anyio.abc.TaskGroup.start_soon` in ``__aenter__`` doesn't wait for the task to begin.
Consider replacing with :meth:`~trio.Nursery.start`/:meth:`~anyio.abc.TaskGroup.start`.
This will only warn about functions listed in :ref:`ASYNC114 <async114>` or known from Trio.
If you're starting a function that does not define `task_status`, then neither will trigger.

_`ASYNC114` : startable-not-in-config
Startable function (i.e. has a ``task_status`` keyword parameter) not in :ref:`--startable-in-context-manager <--startable-in-context-manager>` parameter list, please add it so ASYNC113 can catch errors when using it.
Expand Down
7 changes: 5 additions & 2 deletions docs/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,11 @@ Example
``startable-in-context-manager``
--------------------------------

Comma-separated list of methods which should be used with :meth:`trio.Nursery.start`/:meth:`anyio.abc.TaskGroup.start` when opening a context manager,
in addition to the default :func:`trio.run_process`, :func:`trio.serve_tcp`, :func:`trio.serve_ssl_over_tcp`, and :func:`trio.serve_listeners`.
Comma-separated list of functions which should be used with :meth:`trio.Nursery.start`/:meth:`anyio.abc.TaskGroup.start` when opening a context manager.
We then add known functions from Trio to this list, namely :func:`trio.run_process`, :func:`trio.serve_tcp`, :func:`trio.serve_ssl_over_tcp`, :func:`trio.serve_listeners`, and :meth:`trio.DTLSEndpoint.serve`.
AnyIO does not have any functions in its API that defines ``task_status``.
asyncio does not have an equivalent of :meth:`~trio.Nursery.start`, nor ``task_status``, but you could still add functions to this list that you want to be extra careful about when opening in an `asyncio.TaskGroup` in an ``__aenter__``

Names must be valid identifiers as per :meth:`str.isidentifier`.
Used by :ref:`ASYNC113 <async113>`, and :ref:`ASYNC114 <async114>` will warn when encountering methods not in the list.

Expand Down
4 changes: 1 addition & 3 deletions flake8_async/visitors/visitor102.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ def visit_With(self, node: ast.With | ast.AsyncWith):

# Check for a `with trio.<scope_creator>`
for item in node.items:
call = get_matching_call(
item.context_expr, "open_nursery", *cancel_scope_names
)
call = get_matching_call(item.context_expr, *cancel_scope_names)
if call is None:
continue

Expand Down
9 changes: 6 additions & 3 deletions flake8_async/visitors/visitor_utility.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ def visit_ClassDef(self, node: ast.ClassDef):
self.save_state(node, "variables", copy=True)

def visit_AnnAssign(self, node: ast.AnnAssign):
if not isinstance(node.target, ast.Name):
return
target = node.target.id
if not isinstance(node.target, (ast.Name, ast.Attribute)):
# target can technically be a subscript
return # pragma: no cover
target = ast.unparse(node.target)
typename = ast.unparse(node.annotation)
self.variables[target] = typename

Expand All @@ -87,6 +88,8 @@ def visit_Assign(self, node: ast.Assign):
self.variables[node.targets[0].id] = value

def visit_With(self, node: ast.With | ast.AsyncWith):
# TODO: it's actually the return type of
# `ast.unparse(item.context_expr.func).__[a]enter__()` that should be used
if len(node.items) != 1:
return
item = node.items[0]
Expand Down
37 changes: 23 additions & 14 deletions flake8_async/visitors/visitors.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def visit_While(self, node: ast.While):
class Visitor112(Flake8AsyncVisitor):
error_codes: Mapping[str, str] = {
"ASYNC112": (
"Redundant nursery {}, consider replacing with directly awaiting "
"Redundant {1} {0}, consider replacing with directly awaiting "
"the function call."
),
}
Expand All @@ -113,19 +113,22 @@ def visit_With(self, node: ast.With | ast.AsyncWith):
continue
var_name = item.optional_vars.id

# check for trio.open_nursery and anyio.create_task_group
nursery = get_matching_call(
item.context_expr, "open_nursery", base="trio"
) or get_matching_call(item.context_expr, "create_task_group", base="anyio")
start_methods: tuple[str, ...] = ("start", "start_soon")
if nursery is None:
# check for asyncio.TaskGroup
nursery = get_matching_call(
item.context_expr, "TaskGroup", base="asyncio"
)
if nursery is None:
continue
# check for trio.open_nursery and anyio.create_task_group
if get_matching_call(item.context_expr, "open_nursery", base="trio"):
nursery_type = "nursery"

elif get_matching_call(
item.context_expr, "create_task_group", base="anyio"
):
nursery_type = "taskgroup"
# check for asyncio.TaskGroup
elif get_matching_call(item.context_expr, "TaskGroup", base="asyncio"):
nursery_type = "taskgroup"
start_methods = ("create_task",)
else:
# incorrectly marked as not covered on py39
continue # pragma: no cover # https://github.com/nedbat/coveragepy/issues/198

body_call = node.body[0].value
if isinstance(body_call, ast.Await):
Expand All @@ -142,7 +145,7 @@ def visit_With(self, node: ast.With | ast.AsyncWith):
for n in self.walk(*body_call.args, *body_call.keywords)
)
):
self.error(item.context_expr, var_name)
self.error(item.context_expr, var_name, nursery_type)

visit_AsyncWith = visit_With

Expand All @@ -168,8 +171,11 @@ class Visitor113(Flake8AsyncVisitor):

def __init__(self, *args: Any, **kwargs: Any):
super().__init__(*args, **kwargs)
# this is not entirely correct, it's trio.open_nursery.__aenter__()->trio.Nursery
# but VisitorTypeTracker currently does not work like that.
self.typed_calls["trio.open_nursery"] = "trio.Nursery"
self.typed_calls["anyio.create_task_group"] = "anyio.TaskGroup"
self.typed_calls["asyncio.TaskGroup"] = "asyncio.TaskGroup"

self.async_function = False
self.asynccontextmanager = False
Expand All @@ -196,7 +202,10 @@ def is_startable(n: ast.expr, *startable_list: str) -> bool:
return False

def is_nursery_call(node: ast.expr):
if not isinstance(node, ast.Attribute) or node.attr != "start_soon":
if not isinstance(node, ast.Attribute) or node.attr not in (
"start_soon",
"create_task",
):
return False
var = ast.unparse(node.value)
return ("trio" in self.library and var.endswith("nursery")) or (
Expand Down
16 changes: 13 additions & 3 deletions tests/eval_files/async102.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,19 @@ async def foo():
pass
finally:
myvar = True
with trio.open_nursery(10) as s:
s.shield = myvar
await foo() # safe in theory, error: 12, Statement("try/finally", lineno-6)
with trio.CancelScope(deadline=10) as cs:
cs.shield = myvar
# safe in theory, but we don't track variable values
await foo() # error: 12, Statement("try/finally", lineno-7)
try:
pass
finally:
# false alarm, open_nursery does not block/checkpoint on entry.
async with trio.open_nursery() as nursery: # error: 8, Statement("try/finally", lineno-4)
nursery.cancel_scope.deadline = trio.current_time() + 10
nursery.cancel_scope.shield = True
# false alarm, we currently don't handle nursery.cancel_scope.[deadline/shield]
await foo() # error: 12, Statement("try/finally", lineno-8)
try:
pass
finally:
Expand Down
18 changes: 9 additions & 9 deletions tests/eval_files/async112.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,34 +9,34 @@
import trio as noterror

# error
with trio.open_nursery() as n: # error: 5, "n"
with trio.open_nursery() as n: # error: 5, "n", "nursery"
n.start(...)

with trio.open_nursery(...) as nurse: # error: 5, "nurse"
with trio.open_nursery(...) as nurse: # error: 5, "nurse", "nursery"
nurse.start_soon(...)

with trio.open_nursery() as n: # error: 5, "n"
with trio.open_nursery() as n: # error: 5, "n", "nursery"
n.start_soon(n=7)


async def foo():
async with trio.open_nursery() as n: # error: 15, "n"
async with trio.open_nursery() as n: # error: 15, "n", "nursery"
n.start(...)


# weird ones with multiple `withitem`s
# but if split among several `with` they'd all be treated as error (or ASYNC111), so
# treating as error for now.
with trio.open_nursery() as n, trio.open("") as n: # error: 5, "n"
with trio.open_nursery() as n, trio.open("") as n: # error: 5, "n", "nursery"
n.start(...)

with open("") as o, trio.open_nursery() as n: # error: 20, "n"
with open("") as o, trio.open_nursery() as n: # error: 20, "n", "nursery"
n.start(o)

with trio.open_nursery() as n, trio.open_nursery() as nurse: # error: 31, "nurse"
with trio.open_nursery() as n, trio.open_nursery() as nurse: # error: 31, "nurse", "nursery"
nurse.start(n.start(...))

with trio.open_nursery() as n, trio.open_nursery() as n: # error: 5, "n" # error: 31, "n"
with trio.open_nursery() as n, trio.open_nursery() as n: # error: 5, "n", "nursery" # error: 31, "n", "nursery"
n.start(...)

# safe if passing variable as parameter
Expand Down Expand Up @@ -83,7 +83,7 @@ async def foo():

# body is a call to await n.start
async def foo_1():
with trio.open_nursery(...) as n: # error: 9, "n"
with trio.open_nursery(...) as n: # error: 9, "n", "nursery"
await n.start(...)


Expand Down
4 changes: 2 additions & 2 deletions tests/eval_files/async112_anyio.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ async def bar(*args): ...


async def foo():
async with anyio.create_task_group() as tg: # error: 15, "tg"
async with anyio.create_task_group() as tg: # error: 15, "tg", "taskgroup"
await tg.start_soon(bar())

async with anyio.create_task_group() as tg:
await tg.start(bar(tg))

async with anyio.create_task_group() as tg: # error: 15, "tg"
async with anyio.create_task_group() as tg: # error: 15, "tg", "taskgroup"
tg.start_soon(bar())

async with anyio.create_task_group() as tg:
Expand Down
2 changes: 1 addition & 1 deletion tests/eval_files/async112_asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ async def bar(*args): ...


async def foo():
async with asyncio.TaskGroup() as tg: # error: 15, "tg"
async with asyncio.TaskGroup() as tg: # error: 15, "tg", "taskgroup"
tg.create_task(bar())

async with asyncio.TaskGroup() as tg:
Expand Down
80 changes: 80 additions & 0 deletions tests/eval_files/async113.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# mypy: disable-error-code="arg-type,attr-defined"
# ARG --startable-in-context-manager=my_startable
from __future__ import annotations
from contextlib import asynccontextmanager

import anyio
Expand All @@ -22,6 +24,84 @@ async def foo():
boo.start_soon(trio.run_process) # ASYNC113: 4

boo_anyio: anyio.TaskGroup = ... # type: ignore
# false alarm - anyio.run_process is not startable
# (nor is asyncio.run_process)
boo_anyio.start_soon(anyio.run_process) # ASYNC113: 4

yield


async def my_startable(task_status: trio.TaskStatus[object] = trio.TASK_STATUS_IGNORED):
task_status.started()
await trio.lowlevel.checkpoint()


# name of variable being [xxx.]nursery triggers it
class MyCm_named_variable:
def __init__(self):
self.nursery_manager = trio.open_nursery()
self.nursery = None

async def __aenter__(self):
self.nursery = await self.nursery_manager.__aenter__()
self.nursery.start_soon(my_startable) # ASYNC113: 8

async def __aexit__(self, *args):
assert self.nursery is not None
await self.nursery_manager.__aexit__(*args)


# call chain is not tracked
# trio.open_nursery -> NurseryManager
# NurseryManager.__aenter__ -> nursery
class MyCm_calls:
async def __aenter__(self):
self.nursery_manager = trio.open_nursery()
self.moo = None
self.moo = await self.nursery_manager.__aenter__()
self.moo.start_soon(my_startable)

async def __aexit__(self, *args):
assert self.moo is not None
await self.nursery_manager.__aexit__(*args)


# types of class variables are not tracked across functions
class MyCm_typehint_class_variable:
def __init__(self):
self.nursery_manager = trio.open_nursery()
self.moo: trio.Nursery = None # type: ignore

async def __aenter__(self):
self.moo = await self.nursery_manager.__aenter__()
self.moo.start_soon(my_startable)

async def __aexit__(self, *args):
assert self.moo is not None
await self.nursery_manager.__aexit__(*args)


# type hint with __or__ is not picked up
class MyCm_typehint:
async def __aenter__(self):
self.nursery_manager = trio.open_nursery()
self.moo: trio.Nursery | None = None
self.moo = await self.nursery_manager.__aenter__()
self.moo.start_soon(my_startable)

async def __aexit__(self, *args):
assert self.moo is not None
await self.nursery_manager.__aexit__(*args)


# only if the type hint is exactly trio.Nursery
class MyCm_typehint_explicit:
async def __aenter__(self):
self.nursery_manager = trio.open_nursery()
self.moo: trio.Nursery = None # type: ignore
self.moo = await self.nursery_manager.__aenter__()
self.moo.start_soon(my_startable) # ASYNC113: 8

async def __aexit__(self, *args):
assert self.moo is not None
await self.nursery_manager.__aexit__(*args)
12 changes: 11 additions & 1 deletion tests/eval_files/async113_anyio.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
# mypy: disable-error-code="arg-type"
# ARG --startable-in-context-manager=my_startable
from contextlib import asynccontextmanager

import anyio


async def my_startable(
task_status: anyio.abc.TaskStatus[object] = anyio.TASK_STATUS_IGNORED,
):
task_status.started()
await anyio.lowlevel.checkpoint()


@asynccontextmanager
async def foo():
# create_task_group only exists in anyio
async with anyio.create_task_group() as bar_tg:
bar_tg.start_soon(my_startable) # ASYNC113: 8
# false alarm - anyio.run_process is not startable
bar_tg.start_soon(anyio.run_process) # ASYNC113: 8
yield
yield
17 changes: 17 additions & 0 deletions tests/eval_files/async113_asyncio.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# BASE_LIBRARY asyncio
# ARG --startable-in-context-manager=bar
# TRIO_NO_ERROR
# ANYIO_NO_ERROR

from contextlib import asynccontextmanager
import asyncio


async def bar(): ...


@asynccontextmanager
async def my_cm():
async with asyncio.TaskGroup() as tg: # type: ignore[attr-defined]
tg.create_task(bar) # ASYNC113: 8
yield
4 changes: 3 additions & 1 deletion tests/test_flake8_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,9 @@ def test_eval(
)

if only_check_not_crash:
return
# mark it as skipped to indicate we didn't actually test anything in particular
# (it confused me once when I didn't notice a file was marked with NOTRIO)
pytest.skip()

# Check that error messages refer to current library, or to no library.
if test not in (
Expand Down
Loading