Fix ASYNC200 regression on foo("x").bar patterns (#26.4.2)

docs/changelog.rst

@@ -4,6 +4,10 @@ Changelog
 
 `CalVer, YY.month.patch <https://calver.org/>`_
 
+26.4.2
+======
+- Fixed a regression in canonical-qualname resolution where a call nested inside an attribute chain (e.g. ``foo("x").bar``) was silently elided into a dotted name (``"foo.bar"``). This caused :ref:`ASYNC200 <async200>` false alarms for patterns like ``*session.get`` matching ``read_session("a").get(...)``, where ``.get`` is a method on the *return value* of ``read_session()``.
+
 26.4.1
 ======
 - Rules resolve function/class references via the canonical qualname, so checks fire regardless of import style (``import trio``, ``import trio as t``, ``from trio import open_nursery [as on]``, …). Only module-level imports are tracked. `(issue #132) <https://github.com/python-trio/flake8-async/issues/132>`_

docs/usage.rst

@@ -33,7 +33,7 @@ adding the following to your ``.pre-commit-config.yaml``:
    minimum_pre_commit_version: '2.9.0'
    repos:
    - repo: https://github.com/python-trio/flake8-async
-     rev: 26.4.1
+     rev: 26.4.2
      hooks:
        - id: flake8-async
          # args: ["--enable=ASYNC100,ASYNC112", "--disable=", "--autofix=ASYNC"]

flake8_async/__init__.py

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

flake8_async/visitors/_canonical.py

@@ -19,24 +19,39 @@
 # Resolve a Name/Attribute/Call node to a dotted qualname via `imports`
 # (local-name -> canonical dotted qualname). The root Name falls back to its own
 # identifier, so `trio.open_nursery()` resolves to "trio.open_nursery" even when
-# nothing was imported. Returns None for shapes we can't resolve (subscripts, etc.).
+# nothing was imported. Returns None for shapes we can't resolve (subscripts,
+# Calls nested inside an Attribute chain like `foo("x").bar`, etc.).
 def resolve_canonical_ast(node: ast.AST, imports: Mapping[str, str]) -> str | None:
+    # A Call only collapses to its callee at the *outermost* position. Inside
+    # an Attribute chain (e.g. `foo("x").bar`), the call's return value is what
+    # `.bar` is bound on, and we can't determine that statically — so don't
+    # silently elide the call into a dotted name like "foo.bar".
+    if isinstance(node, ast.Call):
+        return resolve_canonical_ast(node.func, imports)
+    return _resolve_attr_chain_ast(node, imports)
+
+
+def _resolve_attr_chain_ast(node: ast.AST, imports: Mapping[str, str]) -> str | None:
     if isinstance(node, ast.Name):
         return imports.get(node.id, node.id)
     if isinstance(node, ast.Attribute):
-        prefix = resolve_canonical_ast(node.value, imports)
+        prefix = _resolve_attr_chain_ast(node.value, imports)
         return None if prefix is None else f"{prefix}.{node.attr}"
-    if isinstance(node, ast.Call):
-        return resolve_canonical_ast(node.func, imports)
     return None
 
 
 def resolve_canonical_cst(node: cst.CSTNode, imports: Mapping[str, str]) -> str | None:
+    if isinstance(node, cst.Call):
+        return resolve_canonical_cst(node.func, imports)
+    return _resolve_attr_chain_cst(node, imports)
+
+
+def _resolve_attr_chain_cst(
+    node: cst.CSTNode, imports: Mapping[str, str]
+) -> str | None:
     if isinstance(node, cst.Name):
         return imports.get(node.value, node.value)
     if isinstance(node, cst.Attribute):
-        prefix = resolve_canonical_cst(node.value, imports)
+        prefix = _resolve_attr_chain_cst(node.value, imports)
         return None if prefix is None else f"{prefix}.{node.attr.value}"
-    if isinstance(node, cst.Call):
-        return resolve_canonical_cst(node.func, imports)
     return None

tests/eval_files/async200.py

@@ -2,7 +2,7 @@
 # specify command-line arguments to be used when testing this file.
 # Test spaces in options, and trailing comma
 # Cannot test newlines, since argparse splits on those if passed on the CLI
-# ARG --async200-blocking-calls=bar -> BAR, bee-> SHOULD_NOT_BE_PRINTED,bonnet ->SHOULD_NOT_BE_PRINTED,bee.bonnet->BEEBONNET,*.postwild->POSTWILD,prewild.*->PREWILD,*.*.*->TRIPLEDOT,
+# ARG --async200-blocking-calls=bar -> BAR, bee-> SHOULD_NOT_BE_PRINTED,bonnet ->SHOULD_NOT_BE_PRINTED,bee.bonnet->BEEBONNET,*.postwild->POSTWILD,prewild.*->PREWILD,*.*.*->TRIPLEDOT,*session.get->SESSIONGET,
 
 
 # don't error in sync function
@@ -63,3 +63,8 @@ async def bar3():
 
     # check that errors are enabled again
     bar()  # ASYNC200: 4, "bar", "BAR"
+
+    # `foo("x").bar` calls a method on the *return value* of foo(), so the
+    # canonical name should not collapse to "foo.bar".
+    session.get("k")  # ASYNC200: 4, "*session.get", "SESSIONGET"
+    read_session("a").get("k")

tests/test_flake8_async.py

@@ -26,6 +26,10 @@
 from flake8_async import Plugin
 from flake8_async.base import Error, Statement
 from flake8_async.visitors import ERROR_CLASSES, ERROR_CLASSES_CST
+from flake8_async.visitors._canonical import (
+    resolve_canonical_ast,
+    resolve_canonical_cst,
+)
 from flake8_async.visitors.visitor4xx import EXCGROUP_ATTRS
 
 if sys.version_info < (3, 11):
@@ -891,6 +895,49 @@ def test_async400_excgroup_attributes():
         assert attr in EXCGROUP_ATTRS
 
 
+def test_resolve_canonical_does_not_elide_nested_calls():
+    """A Call inside an Attribute chain must not collapse to a dotted name.
+
+    Regression test: previously `foo("x").bar` resolved to "foo.bar" because
+    the recursive case for ``ast.Call`` / ``cst.Call`` fired on calls nested
+    inside an Attribute chain, not just at the outermost position. That made
+    ``*session.get`` fnmatch ``read_session("a").get`` in ASYNC200.
+    """
+    imports: dict[str, str] = {}
+
+    # outermost-Call unwrapping is preserved: trio.open_nursery() -> "trio.open_nursery"
+    ast_call = ast.parse("trio.open_nursery()", mode="eval").body
+    assert isinstance(ast_call, ast.Call)
+    assert resolve_canonical_ast(ast_call, imports) == "trio.open_nursery"
+
+    # plain attribute chain still resolves
+    ast_attr = ast.parse("session.get", mode="eval").body
+    assert resolve_canonical_ast(ast_attr, imports) == "session.get"
+
+    # the regression: Call nested inside Attribute should NOT collapse
+    ast_nested = ast.parse('foo("x").bar', mode="eval").body
+    assert resolve_canonical_ast(ast_nested, imports) is None
+
+    # also when the outer node is itself a call (`foo("x").bar()`):
+    # outer call unwraps to `foo("x").bar`, which then can't resolve.
+    ast_nested_call = ast.parse('foo("x").bar()', mode="eval").body
+    assert isinstance(ast_nested_call, ast.Call)
+    assert resolve_canonical_ast(ast_nested_call, imports) is None
+
+    # cst variant: same expectations
+    cst_call = cst.parse_expression("trio.open_nursery()")
+    assert resolve_canonical_cst(cst_call, imports) == "trio.open_nursery"
+
+    cst_attr = cst.parse_expression("session.get")
+    assert resolve_canonical_cst(cst_attr, imports) == "session.get"
+
+    cst_nested = cst.parse_expression('foo("x").bar')
+    assert resolve_canonical_cst(cst_nested, imports) is None
+
+    cst_nested_call = cst.parse_expression('foo("x").bar()')
+    assert resolve_canonical_cst(cst_nested_call, imports) is None
+
+
 # from https://docs.python.org/3/library/itertools.html#itertools-recipes
 def consume(iterator: Iterable[Any]):
     deque(iterator, maxlen=0)