diff --git a/docs/changelog.rst b/docs/changelog.rst index 79fa885..48be201 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,10 @@ Changelog `CalVer, YY.month.patch `_ +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 ` 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) `_ diff --git a/docs/usage.rst b/docs/usage.rst index dbad5ea..7b71ee6 100644 --- a/docs/usage.rst +++ b/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"] diff --git a/flake8_async/__init__.py b/flake8_async/__init__.py index 26722f8..fc296eb 100644 --- a/flake8_async/__init__.py +++ b/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 diff --git a/flake8_async/visitors/_canonical.py b/flake8_async/visitors/_canonical.py index 0a37a24..0648408 100644 --- a/flake8_async/visitors/_canonical.py +++ b/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 diff --git a/tests/eval_files/async200.py b/tests/eval_files/async200.py index 419a8ba..61d535d 100644 --- a/tests/eval_files/async200.py +++ b/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") diff --git a/tests/test_flake8_async.py b/tests/test_flake8_async.py index c85e564..2b8a365 100644 --- a/tests/test_flake8_async.py +++ b/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)