ASYNC912: timeout/cancelscope with only conditional checkpoints (#242)

* Add ASYNC912: timeout/cancelscope with only conditional checkpoints

* ASYNC100 is now part of Visitor91x

* rewrite `with_has_call` to add support for nested attributes

Author: jakkdl

CHANGELOG.md

@@ -1,6 +1,11 @@
 # Changelog
 *[CalVer, YY.month.patch](https://calver.org/)*
 
+## 24.5.1
+- Add ASYNC912: no checkpoints in with statement are guaranteed to run.
+- ASYNC100 now properly treats async for comprehensions as checkpoints.
+- ASYNC100 now supports autofixing on asyncio.
+
 ## 24.4.2
 - Add ASYNC119: yield in contextmanager in async generator.
 

README.md

@@ -64,6 +64,7 @@ Note: 22X, 23X and 24X has not had asyncio-specific suggestions written.
 - **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).
+- **ASYNC912**: Timeout/Cancelscope has no awaits that are guaranteed to run. If the scope has no checkpoints at all, then `ASYNC100` will be raised instead.
 
 ### Removed Warnings
 - **TRIOxxx**: All error codes are now renamed ASYNCxxx

docs/rules.rst

@@ -55,6 +55,7 @@ Optional rules disabled by default
 - **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).
+-- **ASYNC912**: A timeout/cancelscope has checkpoints, but they're not guaranteed to run. Similar to ASYNC100, but it does not warn on trivial cases where there is no checkpoint at all. It instead shares logic with ASYNC910 and ASYNC911 for parsing conditionals and branches.
 
 Removed rules
 ================

flake8_async/__init__.py

@@ -37,7 +37,7 @@
 
 
 # CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
-__version__ = "24.4.2"
+__version__ = "24.5.1"
 
 
 # taken from https://github.com/Zac-HD/shed

flake8_async/visitors/__init__.py

@@ -30,7 +30,6 @@
 from . import (
     visitor2xx,
     visitor91x,
-    visitor100,
     visitor101,
     visitor102,
     visitor103_104,

flake8_async/visitors/flake8asyncvisitor.py

@@ -98,7 +98,11 @@ def error(
             ), "No error code defined, but class has multiple codes"
             error_code = next(iter(self.error_codes))
         # don't emit an error if this code is disabled in a multi-code visitor
-        elif strip_error_subidentifier(error_code) not in self.options.enabled_codes:
+        elif (
+            (ec_no_sub := strip_error_subidentifier(error_code))
+            not in self.options.enabled_codes
+            and ec_no_sub not in self.options.autofix_codes
+        ):
             return
 
         self.__state.problems.append(
@@ -217,7 +221,11 @@ def error(
             error_code = next(iter(self.error_codes))
         # don't emit an error if this code is disabled in a multi-code visitor
         # TODO: write test for only one of 910/911 enabled/autofixed
-        elif strip_error_subidentifier(error_code) not in self.options.enabled_codes:
+        elif (
+            (ec_no_sub := strip_error_subidentifier(error_code))
+            not in self.options.enabled_codes
+            and ec_no_sub not in self.options.autofix_codes
+        ):
             return False  # pragma: no cover
 
         if self.is_noqa(node, error_code):
@@ -237,7 +245,7 @@ def error(
         return True
 
     def should_autofix(self, node: cst.CSTNode, code: str | None = None) -> bool:
-        if code is None:
+        if code is None:  # pragma: no cover
             assert len(self.error_codes) == 1
             code = next(iter(self.error_codes))
         # this does not currently need to check for `noqa`s, as error() does that

flake8_async/visitors/helpers.py

@@ -51,13 +51,19 @@ def error_class_cst(error_class: type[T_CST]) -> type[T_CST]:
 
 
 def disabled_by_default(error_class: type[T_EITHER]) -> type[T_EITHER]:
+    """Default-disables all error codes in a class."""
     assert error_class.error_codes  # type: ignore[attr-defined]
     default_disabled_error_codes.extend(
         error_class.error_codes  # type: ignore[attr-defined]
     )
     return error_class
 
 
+def disable_codes_by_default(*codes: str) -> None:
+    """Default-disables only specified codes."""
+    default_disabled_error_codes.extend(codes)
+
+
 def utility_visitor(c: type[T]) -> type[T]:
     assert not hasattr(c, "error_codes")
     c.error_codes = {}
@@ -317,30 +323,68 @@ class AttributeCall(NamedTuple):
     function: str
 
 
+# the custom __or__ in libcst breaks pyright type checking. It's possible to use
+# `Union` as a workaround ... except pyupgrade will automatically replace that.
+# So we have to resort to specifying one of the base classes.
+# See https://github.com/Instagram/LibCST/issues/1143
+def build_cst_matcher(attr: str) -> m.BaseExpression:
+    """Build a cst matcher structure with attributes&names matching a string `a.b.c`."""
+    if "." not in attr:
+        return m.Name(value=attr)
+    body, tail = attr.rsplit(".")
+    return m.Attribute(value=build_cst_matcher(body), attr=m.Name(value=tail))
+
+
+def identifier_to_string(attr: cst.Name | cst.Attribute) -> str:
+    if isinstance(attr, cst.Name):
+        return attr.value
+    assert isinstance(attr.value, (cst.Attribute, cst.Name))
+    return identifier_to_string(attr.value) + "." + attr.attr.value
+
+
 def with_has_call(
     node: cst.With, *names: str, base: Iterable[str] = ("trio", "anyio")
 ) -> list[AttributeCall]:
+    """Check if a with statement has a matching call, returning a list with matches.
+
+    `names` specify the names of functions to match, `base` specifies the
+    library/module(s) the function must be in.
+    The list elements in the return value are named tuples with the matched node,
+    base and function.
+
+    Examples_
+
+    `with_has_call(node, "bar", base="foo")` matches foo.bar.
+    `with_has_call(node, "bar", "bee", base=("foo", "a.b.c")` matches
+      `foo.bar`, `foo.bee`, `a.b.c.bar`, and `a.b.c.bee`.
+
+    """
+    if isinstance(base, str):
+        base = (base,)  # pragma: no cover
+
+    # build matcher, using SaveMatchedNode to save the base and the function name.
+    matcher = m.Call(
+        func=m.Attribute(
+            value=m.SaveMatchedNode(
+                m.OneOf(*(build_cst_matcher(b) for b in base)), name="base"
+            ),
+            attr=m.SaveMatchedNode(
+                oneof_names(*names),
+                name="function",
+            ),
+        )
+    )
+
     res_list: list[AttributeCall] = []
     for item in node.items:
-        if res := m.extract(
-            item.item,
-            m.Call(
-                func=m.Attribute(
-                    value=m.SaveMatchedNode(m.Name(), name="library"),
-                    attr=m.SaveMatchedNode(
-                        oneof_names(*names),
-                        name="function",
-                    ),
-                )
-            ),
-        ):
+        if res := m.extract(item.item, matcher):
             assert isinstance(item.item, cst.Call)
-            assert isinstance(res["library"], cst.Name)
+            assert isinstance(res["base"], (cst.Name, cst.Attribute))
             assert isinstance(res["function"], cst.Name)
-            if res["library"].value not in base:
-                continue
             res_list.append(
-                AttributeCall(item.item, res["library"].value, res["function"].value)
+                AttributeCall(
+                    item.item, identifier_to_string(res["base"]), res["function"].value
+                )
             )
     return res_list