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

Rules documentation improvements #248

Merged
merged 15 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 2 additions & 2 deletions docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ flake8-async
############


A highly opinionated flake8 plugin for problems related to `Trio <https://github.com/python-trio/trio>`_, `AnyIO <https://github.com/agronholm/anyio>`_, or `asyncio <https://docs.python.org/3/library/asyncio.html>`_.
A highly opinionated flake8 plugin for problems related to `Trio <https://github.com/python-trio/trio>`__, `AnyIO <https://github.com/agronholm/anyio>`__, or `asyncio <https://docs.python.org/3/library/asyncio.html>`__.


This can include anything from outright bugs, to pointless/dead code,
Expand All @@ -20,7 +20,7 @@ The plugin may well be too noisy or pedantic depending on your requirements or o
Pairs well with flake8-bugbear.


Some rules are incorporated into `ruff <https://docs.astral.sh/ruff/rules/#flake8-async-async>`_.
Some rules are incorporated into `ruff <https://docs.astral.sh/ruff/rules/#flake8-async-async>`__.


We previously maintained separate flake8-async and flake8-trio plugins, but merged both into this plugin under the more general "flake8-async" name after flake8-trio grew support for anyio and asyncio and became a superset of the former flake8-async. All flake8-trio error codes were renamed from TRIOxxx to ASYNCxxx and the flake8-trio package is now deprecated.
Expand Down
130 changes: 130 additions & 0 deletions docs/rules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,140 @@
List of rules
****************

.. Esp when writing short descriptions it'd be very handy to link to a glossary, instead of saying stuff like ``except BaseException/trio.Cancelled/anyio.get_cancelled_exc_class()/asyncio.exceptions.CancelledError``
it also allows easier use of library-specific terminology without forcing people to know all libraries by heart.
It should probably have it's own page in the long run

Glossary
========

.. _timeout_context:

timeout context
---------------
Colloquially referred to as a "Timeout" or "CancelScope". A context manager that enforces a timeout on a block of code. Trio/AnyIO timeout functions are implemented with a ``CancelScope``, but you can also directly use ``CancelScope`` as a context manager.
jakkdl marked this conversation as resolved.
Show resolved Hide resolved

.. I find this to have excessive spacing before/after sublists. Probably requires CSS to fix?

* Trio

* `Documentation <https://trio.readthedocs.io/en/stable/reference-core.html#cancellation-and-timeouts>`__

* :class:`trio.CancelScope`, :func:`trio.move_on_after`, :func:`trio.move_on_at`, :func:`trio.fail_after`, :func:`trio.fail_at`

* AnyIO

* `Documentation <https://anyio.readthedocs.io/en/stable/cancellation.html>`__

* :class:`anyio.CancelScope`, :func:`anyio.move_on_after`, :func:`anyio.fail_after``

* asyncio

* `Documentation <https://docs.python.org/3/library/asyncio-task.html#timeouts>`__

* :func:`asyncio.timeout`, :func:`asyncio.timeout_at`

.. _taskgroup_nursery:

TaskGroup / Nursery
-------------------

A collection of child Tasks that can run concurrently.

* Trio

* `Documentation <https://trio.readthedocs.io/en/stable/reference-core.html#tasks-let-you-do-multiple-things-at-once>`__
* :class:`trio.Nursery`, created with :func:`trio.open_nursery`
* AnyIO

* `Documentation <https://anyio.readthedocs.io/en/stable/tasks.html>`__
* :class:`anyio.abc.TaskGroup`, created with :func:`anyio.create_task_group`.
* asyncio

* `Documentation <https://docs.python.org/3/library/asyncio-task.html#asyncio.TaskGroup>`__
* :class:`asyncio.TaskGroup` (since python 3.11)


.. _cancelled:

Cancelled / CancelledError
--------------------------

Handling cancellation is very sensitive, and you generally never want to catch a cancellation exception without letting it propagate to the library.

* Trio: :class:`trio.Cancelled`. `Documentation <https://trio.readthedocs.io/en/stable/reference-core.html#cancellation-and-timeouts>`__
* AnyIO: :func:`anyio.get_cancelled_exc_class`. `Documentation <https://anyio.readthedocs.io/en/stable/cancellation.html>`__
* asyncio: :class:`asyncio.CancelledError`. `Documentation <https://docs.python.org/3/library/asyncio-task.html#task-cancellation>`__


General rules
=============



.. For some reason using :ref:`timeout_context` fails to find the reference, but :ref:`timeout_context <timeout_context>` works. I have no clue why

.. list-table::
:widths: 1 18 40
:header-rows: 1

* - Code
- Name
- Message
* - ASYNC100
- scope-no-checkpoint
- A :ref:`timeout_context <timeout_context>` does not contain any ``await`` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. This check also allows ``yield`` statements, since checkpoints can happen in the caller we yield to.
* - ASYNC101
- yield-in-taskgroup-or-scope
- ``yield`` inside a :ref:`TaskGroup/Nursery <taskgroup_nursery>` or :ref:`timeout_context <timeout_context>` is only safe when implementing a context manager - otherwise, it breaks exception handling.
Zac-HD marked this conversation as resolved.
Show resolved Hide resolved
* - ASYNC102
- await-in-finally-or-cancelled
- ``await`` inside ``finally`` or :ref:`cancelled-catching <cancelled>` ``except:`` must have shielded cancelscope with timeout.
* - ASYNC103
- no-reraise-cancelled
- :ref:`cancelled <cancelled>`-catching exception that does not reraise the exception.
* - ASYNC104
- cancelled-not-raised
- :ref:`cancelled <cancelled>`-catching exception does not raise the exception. Triggered on ``return`` or raising a different exception.
* - ASYNC105
- missing-await
- async trio function called without using ``await``
Zac-HD marked this conversation as resolved.
Show resolved Hide resolved
* - ASYNC106
- bad-async-library-import
- trio/anyio/asyncio must be imported with ``import xxx`` for the linter to work.
* - ASYNC109
- async-function-with-timeout
- Async function definition with a ``timeout`` parameter. In structured concurrency the caller should instead use :ref:`timeout context managers <timeout_context>`.
* - ASYNC110
- busy-wait
- ``while ...: await [trio/anyio].sleep()`` should be replaced by a :class:`trio.Event`/:class:`anyio.Event`.
* - ASYNC111
- variable-from-cm-in-start-soon
- 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
- useless-nursery
- 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
- start-soon-in-aenter
- Using ``nursery.start_soon`` in ``__aenter__`` doesn't wait for the task to begin. Consider replacing with ``nursery.start``.
* - ASYNC114
- startable-not-in-config
- 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
- sleep-zero
- Replace ``[trio/anyio].sleep(0)`` with the more suggestive ``[trio/anyio].lowlevel.checkpoint()``.
* - ASYNC116
- long-sleep-not-forever
- ``[trio/anyio].sleep()`` with >24 hour interval should usually be ``[trio/anyio].sleep_forever()``.
* - ASYNC118
- cancelled-class-saved
- Don't assign the value of ``anyio.get_cancelled_exc_class()`` to a variable, since that breaks linter checks and multi-backend programs.
* - ASYNC119
- yield-in-cm-in-async-gen
- ``yield`` in context manager in async generator is unsafe, the cleanup may be delayed until ``await`` is no longer allowed.
jakkdl marked this conversation as resolved.
Show resolved Hide resolved

- **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. This check also allows ``yield`` statements, since checkpoints can happen in the caller we yield to.

- **ASYNC101**: ``yield`` inside a :class:`trio.Nursery`/:class:`anyio.abc.TaskGroup`/:py:class:`asyncio.TaskGroup`, or in a timeout/cancel scope is only safe when implementing a context manager - otherwise, it breaks exception handling. See `this thread <https://discuss.python.org/t/preventing-yield-inside-certain-context-managers/1091/23>`_ for discussion of a future PEP. This has substantial overlap with :ref:`ASYNC119 <async119>`, which will warn on almost all instances of ASYNC101, but ASYNC101 is about a conceptually different problem that will not get resolved by `PEP 533 <https://peps.python.org/pep-0533/>`_.
- **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`` :class:`BaseException`/:class:`trio.Cancelled`/:func:`anyio.get_cancelled_exc_class`/:class:`asyncio.CancelledError`, or a bare ``except:`` with a code path that doesn't re-raise. If you don't want to re-raise :class:`BaseException`, add a separate handler for :class:`trio.Cancelled`/:func:`anyio.get_cancelled_exc_class`/:class:`asyncio.CancelledError` before.
Expand Down
Loading