ASYNC102 no longer raises warning for calls to .aclose() on objects (…

…#222)

* ASYNC102 no longer raises warning for calls to `.aclose()` on objects

* bump __version__

* fixes after review: allow aclose() in except clauses. Only allow if no args. Don't exempt asyncio-only code.

* lol I was so confused why this started failing

CHANGELOG.md

@@ -1,8 +1,11 @@
 # Changelog
 *[CalVer, YY.month.patch](https://calver.org/)*
 
+## 24.3.5
+- ASYNC102 (no await inside finally or critical except) no longer raises warnings for calls to `aclose()` on objects in trio/anyio code. See https://github.com/python-trio/flake8-async/issues/156
+
 ## 24.3.4
-- ASYNC110 (don't loop sleep) now also warns if looping `[trio/anyio].lowlevel.checkpoint()`
+- ASYNC110 (don't loop sleep) now also warns if looping `[trio/anyio].lowlevel.checkpoint()`.
 
 ## 24.3.3
 - Add ASYNC251: `time.sleep()` in async method.

flake8_async/__init__.py

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

flake8_async/visitors/visitor102.py

@@ -52,15 +52,35 @@ def __init__(self, *args: Any, **kwargs: Any):
         self._trio_context_managers: list[Visitor102.TrioScope] = []
         self.cancelled_caught = False
 
-    # if we're inside a finally, and we're not inside a scope that doesn't have
-    # both a timeout and shield
-    def visit_Await(self, node: ast.Await | ast.AsyncFor | ast.AsyncWith):
+    # if we're inside a finally or critical except, and we're not inside a scope that
+    # doesn't have both a timeout and shield
+    def async_call_checker(
+        self, node: ast.Await | ast.AsyncFor | ast.AsyncWith
+    ) -> None:
         if self._critical_scope is not None and not any(
             cm.has_timeout and cm.shielded for cm in self._trio_context_managers
         ):
             self.error(node, self._critical_scope)
 
-    visit_AsyncFor = visit_Await
+    def is_safe_aclose_call(self, node: ast.Await) -> bool:
+        return (
+            # don't mark calls safe in asyncio-only files
+            # a more defensive option would be `asyncio not in self.library`
+            self.library != ("asyncio",)
+            and isinstance(node.value, ast.Call)
+            # only known safe if no arguments
+            and not node.value.args
+            and not node.value.keywords
+            and isinstance(node.value.func, ast.Attribute)
+            and node.value.func.attr == "aclose"
+        )
+
+    def visit_Await(self, node: ast.Await):
+        # allow calls to `.aclose()`
+        if not (self.is_safe_aclose_call(node)):
+            self.async_call_checker(node)
+
+    visit_AsyncFor = async_call_checker
 
     def visit_With(self, node: ast.With | ast.AsyncWith):
         self.save_state(node, "_trio_context_managers", copy=True)
@@ -82,7 +102,7 @@ def visit_With(self, node: ast.With | ast.AsyncWith):
             break
 
     def visit_AsyncWith(self, node: ast.AsyncWith):
-        self.visit_Await(node)
+        self.async_call_checker(node)
         self.visit_With(node)
 
     def visit_Try(self, node: ast.Try):
@@ -94,6 +114,10 @@ def visit_Try(self, node: ast.Try):
         self.visit_nodes(node.body, node.handlers, node.orelse)
 
         self._trio_context_managers = []
+        # node.finalbody does not have a lineno, so we give the position of the try
+        # it'd be possible to estimate the lineno given the last except and the first
+        # statement in the finally, but it would be very hard to get it perfect with
+        # comments and empty lines and stuff.
         self._critical_scope = Statement("try/finally", node.lineno, node.col_offset)
         self.visit_nodes(node.finalbody)
 

tests/eval_files/async102.py

@@ -252,3 +252,16 @@ async def foo_nested_excepts():
             await foo()
         await foo()
     await foo()
+
+
+# double check that this works nested inside an async for
+async def foo_nested_async_for():
+
+    async for i in trio.bypasslinters:
+        try:
+            ...
+        except BaseException:
+            async for (  # error: 12, Statement("BaseException", lineno-1)
+                j
+            ) in trio.bypasslinters:
+                ...

tests/eval_files/async102_aclose.py

@@ -0,0 +1,22 @@
+# type: ignore
+
+# exclude finally: await x.aclose() from async102, with trio/anyio
+# ANYIO_NO_ERROR
+# TRIO_NO_ERROR
+# See also async102_aclose_args.py - which makes sure trio/anyio raises errors if there
+# are arguments to aclose().
+
+
+async def foo():
+    # no type tracking in this check, we allow any call that looks like
+    # `await [...].aclose()`
+    x = None
+
+    try:
+        ...
+    except BaseException:
+        await x.aclose()  # ASYNC102: 8, Statement("BaseException", lineno-1)
+        await x.y.aclose()  # ASYNC102: 8, Statement("BaseException", lineno-2)
+    finally:
+        await x.aclose()  # ASYNC102: 8, Statement("try/finally", lineno-6)
+        await x.y.aclose()  # ASYNC102: 8, Statement("try/finally", lineno-7)

tests/eval_files/async102_aclose_args.py

@@ -0,0 +1,24 @@
+# type: ignore
+
+# trio/anyio should still raise errors if there's args
+# asyncio will always raise errors
+
+# See also async102_aclose.py, which checks that trio/anyio marks arg-less aclose() as safe
+
+
+async def foo():
+    # no type tracking in this check
+    x = None
+
+    try:
+        ...
+    except BaseException:
+        await x.aclose(foo)  # ASYNC102: 8, Statement("BaseException", lineno-1)
+        await x.aclose(bar=foo)  # ASYNC102: 8, Statement("BaseException", lineno-2)
+        await x.aclose(*foo)  # ASYNC102: 8, Statement("BaseException", lineno-3)
+        await x.aclose(None)  # ASYNC102: 8, Statement("BaseException", lineno-4)
+    finally:
+        await x.aclose(foo)  # ASYNC102: 8, Statement("try/finally", lineno-8)
+        await x.aclose(bar=foo)  # ASYNC102: 8, Statement("try/finally", lineno-9)
+        await x.aclose(*foo)  # ASYNC102: 8, Statement("try/finally", lineno-10)
+        await x.aclose(None)  # ASYNC102: 8, Statement("try/finally", lineno-11)

tests/test_messages_documented.py

@@ -44,7 +44,9 @@ def test_messages_documented():
         if not file_path.is_file():
             continue
 
-        if m := re.search(r"async\d\d\d", str(file_path)):
+        # only look in the stem (final part of the path), so as not to get tripped
+        # up by [git worktree] directories with an exception code in the name
+        if m := re.search(r"^async\d\d\d", str(file_path.stem)):
             documented_errors["eval_files"].add(m.group().upper())
 
         with open(file_path) as file: