Skip to content

Commit

Permalink
basic Asyncio support (#210)
Browse files Browse the repository at this point in the history
* initial support for asyncio: 106, 200, 21x, and 9xx are working

* partial support for 102, 103, 104 which now detect asyncio.exceptions.CancelledError as a critical exception

* add --asyncio

* update error descriptions in README to better reflect which errors work with which library, and other minor rewrites.

* test improvements: Fix import/arg detection of asyncio. add asyncio102_asyncio. add async103_all_imported. Finally make test_anyio_from_config autodetect the correct line number. Fix BASE_LIBRARY marker being interpreted as a bool. Make #NOTRIO/#NOASYNCIO/#NOANYIO run the visitor but ignore the result, instead of skipping, to check it doesn't crash. Generalize error-message-library-check.

* Replace all old URLs (ZacHD/flake8-trio and python-trio/flake8-trio) to point to python-trio/flake8-async

---------

Co-authored-by: Zac Hatfield-Dodds <[email protected]>
  • Loading branch information
jakkdl and Zac-HD authored Mar 2, 2024
1 parent eb2de73 commit 5c6879a
Show file tree
Hide file tree
Showing 57 changed files with 390 additions and 82 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
release:
runs-on: ubuntu-latest
needs: [pyright, test]
if: github.repository == 'Zac-HD/flake8-trio' && github.ref == 'refs/heads/main'
if: github.repository == 'python-trio/flake8-async' && github.ref == 'refs/heads/main'
steps:
- uses: actions/checkout@v4
- name: Set up Python 3
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@

## Future
- Removed TRIO117, MultiError removed in trio 0.24.0
- Renamed the library from flake8-trio to flake8-async, to indicate the checker supports more than just `trio`.
- Renamed all error codes from TRIOxxx to ASYNCxxx
- Renamed the binary from flake8-trio to flake8-async
- Lots of internal renaming.
- Added asyncio support for ASYNC106
- added `--library`

## 23.5.1
- TRIO91X now supports comprehensions
Expand Down
50 changes: 25 additions & 25 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[![pre-commit.ci status](https://results.pre-commit.ci/badge/github/python-trio/flake8-trio/main.svg)](https://results.pre-commit.ci/latest/github/python-trio/flake8-trio/main)
[![pre-commit.ci status](https://results.pre-commit.ci/badge/github/python-trio/flake8-async/main.svg)](https://results.pre-commit.ci/latest/github/python-trio/flake8-async/main)
[![Checked with pyright](https://microsoft.github.io/pyright/img/pyright_badge.svg)](https://microsoft.github.io/pyright/)
# flake8-async

Expand All @@ -23,47 +23,47 @@ pip install flake8-async
```

## List of warnings

- **ASYNC100**: A `with trio.fail_after(...):` or `with trio.move_on_after(...):`
- **ASYNC100**: A `with [trio|anyio].fail_after(...):` or `with [trio|anyio].move_on_after(...):`
context does not contain any `await` statements. This makes it pointless, as
the timeout can only be triggered by a checkpoint.
- **ASYNC101**: `yield` inside a nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling.
- **ASYNC102**: It's unsafe to await inside `finally:` or `except BaseException/trio.Cancelled` unless you use a shielded
cancel scope with a timeout.
- **ASYNC103**: `except BaseException`, `except trio.Cancelled` or a bare `except:` with a code path that doesn't re-raise. If you don't want to re-raise `BaseException`, add a separate handler for `trio.Cancelled` before.
- **ASYNC104**: `Cancelled` and `BaseException` must be re-raised - when a user tries to `return` or `raise` a different exception.
- **ASYNC105**: Calling a trio async function without immediately `await`ing it.
- **ASYNC106**: `trio`/`anyio` must be imported with `import trio`/`import anyio` for the linter to work.
- **ASYNC109**: Async function definition with a `timeout` parameter - use `trio.[fail/move_on]_[after/at]` instead
- **ASYNC110**: `while <condition>: await trio.sleep()` should be replaced by a `trio.Event`.
- **ASYNC101**: `yield` inside a trio/anyio nursery or cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling.
- **ASYNC102**: It's unsafe to await inside `finally:` or `except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError` unless you use a shielded cancel scope with a timeout. This is currently not able to detect asyncio shields.
- **ASYNC103**: `except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError`, or a bare `except:` with a code path that doesn't re-raise. If you don't want to re-raise `BaseException`, add a separate handler for `trio.Cancelled`/`anyio.get_cancelled_exc_class()`/`asyncio.exceptions.CancelledError` before.
- **ASYNC104**: `trio.Cancelled`/`anyio.get_cancelled_exc_class()`/`asyncio.exceptions.CancelledError`/`BaseException` must be re-raised. The same as ASYNC103, except specifically triggered on `return` or a different exception being raised.
- **ASYNC105**: Calling a trio async function without immediately `await`ing it. This is only supported with trio functions, but you can get similar functionality with a type-checker.
- **ASYNC106**: `trio`/`anyio`/`asyncio` must be imported with `import trio`/`import anyio`/`import asyncio` for the linter to work.
- **ASYNC109**: Async function definition with a `timeout` parameter - use `[trio/anyio].[fail/move_on]_[after/at]` instead.
- **ASYNC110**: `while <condition>: await [trio/anyio].sleep()` should be replaced by a `[trio|anyio].Event`.
- **ASYNC111**: Variable, from context manager opened inside nursery, passed to `start[_soon]` might be invalidly accessed while in use, due to context manager closing before the nursery. This is usually a bug, and nurseries should generally be the inner-most context manager.
- **ASYNC112**: Nursery body with only a call to `nursery.start[_soon]` and not passing itself as a parameter can be replaced with a regular function call.
- **ASYNC113**: Using `nursery.start_soon` in `__aenter__` doesn't wait for the task to begin. Consider replacing with `nursery.start`.
- **ASYNC114**: Startable function (i.e. has a `task_status` keyword parameter) not in `--startable-in-context-manager` parameter list, please add it so ASYNC113 can catch errors when using it.
- **ASYNC115**: Replace `trio.sleep(0)` with the more suggestive `trio.lowlevel.checkpoint()`.
- **ASYNC116**: `trio.sleep()` with >24 hour interval should usually be `trio.sleep_forever()`.
- **ASYNC115**: Replace `[trio|anyio].sleep(0)` with the more suggestive `[trio|anyio].lowlevel.checkpoint()`.
- **ASYNC116**: `[trio|anyio].sleep()` with >24 hour interval should usually be `[trio|anyio].sleep_forever()`.
- **ASYNC118**: Don't assign the value of `anyio.get_cancelled_exc_class()` to a variable, since that breaks linter checks and multi-backend programs.

### Warnings for blocking sync calls in async functions
- **ASYNC200**: User-configured error for blocking sync calls in async functions. Does nothing by default, see [`trio200-blocking-calls`](#trio200-blocking-calls) for how to configure it.
- **ASYNC210**: Sync HTTP call in async function, use `httpx.AsyncClient`.
Note: 22X, 23X and 24X has not had asyncio-specific suggestions written.
- **ASYNC200**: User-configured error for blocking sync calls in async functions. Does nothing by default, see [`async200-blocking-calls`](#async200-blocking-calls) for how to configure it.
- **ASYNC210**: Sync HTTP call in async function, use `httpx.AsyncClient`. This and the other ASYNC21x checks look for usage of `urllib3` and `httpx.Client`, and recommend using `httpx.AsyncClient` as that's the largest http client supporting anyio/trio.
- **ASYNC211**: Likely sync HTTP call in async function, use `httpx.AsyncClient`. Looks for `urllib3` method calls on pool objects, but only matching on the method signature and not the object.
- **ASYNC212**: Blocking sync HTTP call on httpx object, use httpx.AsyncClient.
- **ASYNC220**: Sync process call in async function, use `await nursery.start(trio.run_process, ...)`.
- **ASYNC221**: Sync process call in async function, use `await trio.run_process(...)`.
- **ASYNC222**: Sync `os.*` call in async function, wrap in `await trio.to_thread.run_sync()`.
- **ASYNC230**: Sync IO call in async function, use `trio.open_file(...)`.
- **ASYNC231**: Sync IO call in async function, use `trio.wrap_file(...)`.
- **ASYNC232**: Blocking sync call on file object, wrap the file object in `trio.wrap_file()` to get an async file object.
- **ASYNC240**: Avoid using `os.path` in async functions, prefer using `trio.Path` objects.
- **ASYNC220**: Sync process call in async function, use `await nursery.start([trio|anyio].run_process, ...)`.
- **ASYNC221**: Sync process call in async function, use `await [trio|anyio].run_process(...)`.
- **ASYNC222**: Sync `os.*` call in async function, wrap in `await [trio|anyio].to_thread.run_sync()`.
- **ASYNC230**: Sync IO call in async function, use `[trio|anyio].open_file(...)`.
- **ASYNC231**: Sync IO call in async function, use `[trio|anyio].wrap_file(...)`.
- **ASYNC232**: Blocking sync call on file object, wrap the file object in `[trio|anyio].wrap_file()` to get an async file object.
- **ASYNC240**: Avoid using `os.path` in async functions, prefer using `[trio|anyio].Path` objects.

### Warnings disabled by default
- **ASYNC900**: Async generator without `@asynccontextmanager` not allowed.
- **ASYNC910**: Exit or `return` from async function with no guaranteed checkpoint or exception since function definition.
- **ASYNC900**: Async generator without `@asynccontextmanager` not allowed. You might want to enable this on a codebase since async generators are inherently unsafe and cleanup logic might not be performed. See https://github.com/python-trio/flake8-async/issues/211 and https://discuss.python.org/t/using-exceptiongroup-at-anthropic-experience-report/20888/6 for discussion.
- **ASYNC910**: Exit or `return` from async function with no guaranteed checkpoint or exception since function definition. You might want to enable this on a codebase to make it easier to reason about checkpoints, and make the logic of ASYNC911 correct.
- **ASYNC911**: Exit, `yield` or `return` from async iterable with no guaranteed checkpoint since possible function entry (yield or function definition)
Checkpoints are `await`, `async for`, and `async with` (on one of enter/exit).

### Removed Warnings
- **TRIOxxx**: All error codes are now renamed ASYNCxxx
- **TRIO107**: Renamed to TRIO910
- **TRIO108**: Renamed to TRIO911
- **TRIO117**: Don't raise or catch `trio.[NonBase]MultiError`, prefer `[exceptiongroup.]BaseExceptionGroup`. `MultiError` was removed in trio==0.24.0.
Expand Down
14 changes: 14 additions & 0 deletions flake8_async/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,19 @@ def add_options(option_manager: OptionManager | ArgumentParser):
" suggestions with [anyio|trio]."
),
)
add_argument(
"--asyncio",
# action=store_true + parse_from_config does seem to work here, despite
# https://github.com/PyCQA/flake8/issues/1770
action="store_true",
required=False,
default=False,
help=(
"Change the default library to be asyncio instead of trio."
" If anyio/trio is imported it will assume that is also available and"
" print suggestions with [asyncio|anyio/trio]."
),
)

@staticmethod
def parse_options(options: Namespace):
Expand Down Expand Up @@ -342,6 +355,7 @@ def get_matching_codes(
startable_in_context_manager=options.startable_in_context_manager,
trio200_blocking_calls=options.trio200_blocking_calls,
anyio=options.anyio,
asyncio=options.asyncio,
disable_noqa=options.disable_noqa,
)

Expand Down
1 change: 1 addition & 0 deletions flake8_async/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Options:
startable_in_context_manager: Collection[str]
trio200_blocking_calls: dict[str, str]
anyio: bool
asyncio: bool
disable_noqa: bool


Expand Down
3 changes: 3 additions & 0 deletions flake8_async/visitors/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,9 @@ def has_exception(node: ast.expr) -> str | None:
"trio.Cancelled",
"anyio.get_cancelled_exc_class()",
"get_cancelled_exc_class()",
"asyncio.exceptions.CancelledError",
"exceptions.CancelledError",
"CancelledError",
):
return name
return None
Expand Down
34 changes: 32 additions & 2 deletions flake8_async/visitors/visitor103_104.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,30 @@
_suggestion_dict: dict[tuple[str, ...], str] = {
("anyio",): "anyio.get_cancelled_exc_class()",
("trio",): "trio.Cancelled",
("asyncio",): "asyncio.exceptions.CancelledError",
}
_suggestion_dict[("anyio", "trio")] = "[" + "|".join(_suggestion_dict.values()) + "]"
# TODO: ugly
for a, b in (("anyio", "trio"), ("anyio", "asyncio"), ("asyncio", "trio")):
_suggestion_dict[(a, b)] = (
"[" + "|".join((_suggestion_dict[(a,)], _suggestion_dict[(b,)])) + "]"
)
_suggestion_dict[
(
"anyio",
"asyncio",
"trio",
)
] = (
"["
+ "|".join(
(
_suggestion_dict[("anyio",)],
_suggestion_dict[("asyncio",)],
_suggestion_dict[("trio",)],
)
)
+ "]"
)

_error_codes = {
"ASYNC103": _async103_common_msg,
Expand Down Expand Up @@ -56,6 +78,7 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler):
marker = critical_except(node)

if marker is None:
# not a critical exception handler
return

# If previous excepts have handled trio.Cancelled, don't do anything - namely
Expand All @@ -69,14 +92,21 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler):
):
error_code = "ASYNC103"
self.cancelled_caught.add("anyio")
elif marker.name in (
"asyncio.exceptions.CancelledError",
"exceptions.CancelledError",
"CancelledError",
):
error_code = "ASYNC103"
self.cancelled_caught.add("asyncio")
else:
if self.cancelled_caught:
return
if len(self.library) < 2:
error_code = f"ASYNC103_{self.library_str}"
else:
error_code = f"ASYNC103_{'_'.join(sorted(self.library))}"
self.cancelled_caught.update("trio", "anyio")
self.cancelled_caught.update("trio", "anyio", "asyncio")

# Don't save the state of cancelled_caught, that's handled in Try and would
# reset it between each except
Expand Down
21 changes: 17 additions & 4 deletions flake8_async/visitors/visitor91x.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ def copy(self):


def checkpoint_statement(library: str) -> cst.SimpleStatementLine:
# logic before this should stop code from wanting to insert the non-existing
# asyncio.lowlevel.checkpoint
assert library != "asyncio"
return cst.SimpleStatementLine(
[cst.Expr(cst.parse_expression(f"await {library}.lowlevel.checkpoint()"))]
)
Expand All @@ -111,6 +114,7 @@ def __init__(self):
self.noautofix: bool = False
self.add_statement: cst.SimpleStatementLine | None = None

# used for inserting import if there's none
self.explicitly_imported_library: dict[str, bool] = {
"trio": False,
"anyio": False,
Expand Down Expand Up @@ -145,8 +149,11 @@ def leave_SimpleStatementLine(
# possible TODO: generate an error if transforming+visiting is done in a
# single pass and emit-error-on-transform can be enabled/disabled. The error can't
# be generated in the yield/return since it doesn't know if it will be autofixed.
if self.add_statement is None or not self.should_autofix(original_node):
if self.add_statement is None:
return updated_node

# methods setting self.add_statement should have called self.should_autofix
assert self.should_autofix(original_node)
curr_add_statement = self.add_statement
self.add_statement = None

Expand Down Expand Up @@ -250,8 +257,12 @@ def __init__(self, *args: Any, **kwargs: Any):
self.try_state = TryState()

def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool:
return not self.noautofix and super().should_autofix(
node, "ASYNC911" if self.has_yield else "ASYNC910"
return (
not self.noautofix
and super().should_autofix(
node, "ASYNC911" if self.has_yield else "ASYNC910"
)
and self.library != ("asyncio",)
)

def checkpoint_statement(self) -> cst.SimpleStatementLine:
Expand Down Expand Up @@ -359,7 +370,9 @@ def leave_Return(
) -> cst.Return:
if not self.async_function:
return updated_node
if self.check_function_exit(original_node):
if self.check_function_exit(original_node) and self.should_autofix(
original_node
):
self.add_statement = self.checkpoint_statement()
# avoid duplicate error messages
self.uncheckpointed_statements = set()
Expand Down
12 changes: 10 additions & 2 deletions flake8_async/visitors/visitor_utility.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,13 @@ def __init__(self, *args: Any, **kwargs: Any):
# see imports
if self.options.anyio:
self.add_library("anyio")
if self.options.asyncio:
self.add_library("asyncio")

def visit_Import(self, node: ast.Import):
for alias in node.names:
name = alias.name
if name in ("trio", "anyio") and alias.asname is None:
if name in ("trio", "anyio", "asyncio") and alias.asname is None:
self.add_library(name)


Expand All @@ -134,11 +136,17 @@ def __init__(self, *args: Any, **kwargs: Any):
# see imports
if self.options.anyio:
self.add_library("anyio")
if self.options.asyncio:
self.add_library("asyncio")

def visit_Import(self, node: cst.Import):
for alias in node.names:
if m.matches(
alias, m.ImportAlias(name=m.Name("trio") | m.Name("anyio"), asname=None)
alias,
m.ImportAlias(
name=m.Name("trio") | m.Name("anyio") | m.Name("asyncio"),
asname=None,
),
):
assert isinstance(alias.name.value, str)
self.add_library(alias.name.value)
Expand Down
6 changes: 4 additions & 2 deletions flake8_async/visitors/visitors.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
if TYPE_CHECKING:
from collections.abc import Mapping

LIBRARIES = ("trio", "anyio", "asyncio")


@error_class
class Visitor106(Flake8AsyncVisitor):
Expand All @@ -19,12 +21,12 @@ class Visitor106(Flake8AsyncVisitor):
}

def visit_ImportFrom(self, node: ast.ImportFrom):
if node.module in ("trio", "anyio"):
if node.module in LIBRARIES:
self.error(node, node.module)

def visit_Import(self, node: ast.Import):
for name in node.names:
if name.name in ("trio", "anyio") and name.asname is not None:
if name.name in LIBRARIES and name.asname is not None:
self.error(node, name.name)


Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ ignore = [
"COM", # flake8-comma, handled by black
"ANN", # annotations, handled by pyright/mypy
"T20", # flake8-print
"TID252", # relative imports from parent modules https://github.com/python-trio/flake8-trio/pull/196#discussion_r1200413372
"TID252", # relative imports from parent modules https://github.com/python-trio/flake8-async/pull/196#discussion_r1200413372
"D101",
"D102",
"D103",
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def local_file(name: str) -> Path:
author="Zac Hatfield-Dodds, John Litborn, and Contributors",
author_email="[email protected]",
packages=find_packages(include=["flake8_async", "flake8_async.*"]),
url="https://github.com/python-trio/flake8-trio",
url="https://github.com/python-trio/flake8-async",
license="MIT",
description="A highly opinionated flake8 plugin for Trio-related problems.",
zip_safe=False,
Expand Down
1 change: 1 addition & 0 deletions tests/autofix_files/async100.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# type: ignore
# AUTOFIX
# NOASYNCIO

import trio

Expand Down
1 change: 1 addition & 0 deletions tests/autofix_files/async100_simple_autofix.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# NOASYNCIO
# AUTOFIX
import trio

Expand Down
4 changes: 2 additions & 2 deletions tests/autofix_files/async910.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ async def foo_try_7(): # safe
pass


# https://github.com/Zac-HD/flake8-trio/issues/45
# https://github.com/python-trio/flake8-async/issues/45
async def to_queue(iter_func, queue):
async with iter_func() as it:
async for x in it:
Expand Down Expand Up @@ -499,7 +499,7 @@ async def foo_range_5(): # error: 0, "exit", Statement("function definition", l
await trio.lowlevel.checkpoint()


# https://github.com/Zac-HD/flake8-trio/issues/47
# https://github.com/python-trio/flake8-async/issues/47
async def f():
while True:
if ...:
Expand Down
2 changes: 1 addition & 1 deletion tests/autofix_files/async910.py.diff
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
+ await trio.lowlevel.checkpoint()


# https://github.com/Zac-HD/flake8-trio/issues/47
# https://github.com/python-trio/flake8-async/issues/47
@@ x,6 x,7 @@
# should error
async def foo_comprehension_2(): # error: 0, "exit", Statement("function definition", lineno)
Expand Down
1 change: 1 addition & 0 deletions tests/autofix_files/noqa.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# AUTOFIX
# NOANYIO # TODO
# NOASYNCIO
# ARG --enable=ASYNC100,ASYNC911
from typing import Any

Expand Down
3 changes: 3 additions & 0 deletions tests/eval_files/anyio_trio.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# type: ignore
# ARG --enable=ASYNC220
# NOTRIO
# NOASYNCIO
# set base library so trio doesn't get replaced when running with anyio
# BASE_LIBRARY anyio

# anyio eval will automatically prepend this test with `--anyio`
import trio # isort: skip
Expand Down
Loading

0 comments on commit 5c6879a

Please sign in to comment.