From c0b7731b98f5c74647c0a5cfe294e604e62ebc0e Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Thu, 20 Aug 2026 16:48:33 -0400 Subject: [PATCH] fix(dataflow): recognise staticmethod by identity, not spelling (#135) `build_scope` decided whether a callable has a receiver with exact string membership against unparsed source: `"staticmethod" not in {ast.unparse(d) ...}`. Only the bare spelling matched, so `@builtins.staticmethod` -- or `from builtins import staticmethod as sm` -- fell through, `scope.self_name` was set, and the first parameter was treated as a receiver. That later adds `scope.self_name` as a definition (`access_paths.py:472-473`), putting a spurious def into the L3/L4 dataflow. Measured on real symbol-table records: dotted resolved={'builtins.staticmethod'} self_name fixed=None old='self' method resolved=set() self_name fixed='self' old='self' #128 made this cheap: every decorator now carries a Jedi-resolved `qualified_name`, and every spelling of the builtin resolves to `builtins.staticmethod`. `build_scope` takes an optional `decorator_names` set and checks identity; `build_pdg` passes it through, and `dataflow/builder.py` supplies it from the `PyCallable` it already holds. The parameter is optional and the old written-spelling match remains as the fallback, so a caller with no resolved records keeps its current behaviour rather than being forced to thread records through. The pipeline caller does supply them, which is where the defect actually bit. Trigger is narrow -- the first parameter must be named `self` or `cls` -- which is why no bug report prompted this. --- codeanalyzer/dataflow/access_paths.py | 30 ++++++++-- codeanalyzer/dataflow/builder.py | 7 +++ codeanalyzer/dataflow/pdg.py | 9 ++- test/test_access_paths_receiver_binding.py | 67 ++++++++++++++++++++++ 4 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 test/test_access_paths_receiver_binding.py diff --git a/codeanalyzer/dataflow/access_paths.py b/codeanalyzer/dataflow/access_paths.py index e053124..3533d42 100644 --- a/codeanalyzer/dataflow/access_paths.py +++ b/codeanalyzer/dataflow/access_paths.py @@ -245,15 +245,37 @@ def _names_loaded(node: ast.AST) -> Set[str]: return out -def build_scope(func: ast.AST, enclosing_locals: Set[str]) -> FunctionScope: +#: Jedi resolves every spelling of the builtin -- ``@staticmethod``, +#: ``@builtins.staticmethod``, ``from builtins import staticmethod as sm`` -- +#: to this one name (#135). +_STATICMETHOD_QUALIFIED = "builtins.staticmethod" + + +def build_scope( + func: ast.AST, + enclosing_locals: Set[str], + decorator_names: Optional[Set[str]] = None, +) -> FunctionScope: """Classify every base name the callable touches. ``enclosing_locals`` is the union of locals/params of all enclosing callables (for capture vs - global disambiguation).""" + global disambiguation). + + ``decorator_names`` are the callable's Jedi-resolved decorator + ``qualified_name``s. When supplied, staticmethod detection is by identity, + so a dotted or aliased spelling is recognised (#135). When omitted -- a + caller with no resolved records -- it falls back to matching the written + source, which only recognises the bare ``@staticmethod``. + """ params = _param_names(func) scope = FunctionScope(params=params) if params and isinstance(func, (ast.FunctionDef, ast.AsyncFunctionDef)): - decorators = {ast.unparse(d) for d in func.decorator_list} - if params[0] in ("self", "cls") and "staticmethod" not in decorators: + if decorator_names is None: + is_static = "staticmethod" in { + ast.unparse(d) for d in func.decorator_list + } + else: + is_static = _STATICMETHOD_QUALIFIED in decorator_names + if params[0] in ("self", "cls") and not is_static: scope.self_name = params[0] scope.globals_ = _declared(func, ast.Global) nonlocals = _declared(func, ast.Nonlocal) diff --git a/codeanalyzer/dataflow/builder.py b/codeanalyzer/dataflow/builder.py index 844910a..8520848 100644 --- a/codeanalyzer/dataflow/builder.py +++ b/codeanalyzer/dataflow/builder.py @@ -186,6 +186,13 @@ def build_function_pdgs( oracle=oracle, k=k, global_qualifier=module.module_name, + # Resolved decorator names, so staticmethod detection works for a + # dotted or aliased spelling and not just bare `@staticmethod` (#135). + decorator_names={ + d.qualified_name + for d in (pycallable.decorators or []) + if d.qualified_name + }, ) infos[pycallable.signature] = FunctionInfo( signature=pycallable.signature, pdg=pdg, oracle=oracle diff --git a/codeanalyzer/dataflow/pdg.py b/codeanalyzer/dataflow/pdg.py index 09c0e59..e997b3f 100644 --- a/codeanalyzer/dataflow/pdg.py +++ b/codeanalyzer/dataflow/pdg.py @@ -66,10 +66,15 @@ def build_pdg( oracle: TypeBasedAliasOracle, k: int = 3, global_qualifier: Optional[str] = None, + decorator_names: Optional[Set[str]] = None, ) -> FunctionPDG: - """CFG → dominance → def-use → PDG for one callable.""" + """CFG → dominance → def-use → PDG for one callable. + + ``decorator_names`` are the callable's resolved decorator qualified names, + used for staticmethod detection by identity rather than spelling (#135). + """ cfg = build_cfg(func) - scope = build_scope(func, enclosing_locals) + scope = build_scope(func, enclosing_locals, decorator_names=decorator_names) facts = statement_facts(cfg, func, scope, k, global_qualifier) edges: List[PDGEdge] = [ diff --git a/test/test_access_paths_receiver_binding.py b/test/test_access_paths_receiver_binding.py new file mode 100644 index 0000000..1fcf910 --- /dev/null +++ b/test/test_access_paths_receiver_binding.py @@ -0,0 +1,67 @@ +"""Receiver binding recognises staticmethod by identity, not spelling (#135). + +`build_scope` decided whether a callable has a receiver with +`"staticmethod" not in {ast.unparse(d) for d in decorator_list}` — exact string +membership against written source. `@builtins.staticmethod`, or an aliased +import, failed that test, so a static method got a receiver it does not have and +`scope.self_name` was later added as a definition, producing a spurious def in +the L3/L4 dataflow. + +#128 gives every decorator a Jedi-resolved `qualified_name`, so the check can be +made on identity: both `@staticmethod` and `@builtins.staticmethod` resolve to +`builtins.staticmethod`. +""" +import ast + +from codeanalyzer.dataflow.access_paths import build_scope + +SRC = """\ +import builtins +from builtins import staticmethod as sm + + +class C: + @builtins.staticmethod + def dotted(self, x): + return x + + @sm + def aliased(self, x): + return x + + @staticmethod + def plain(self, x): + return x + + def method(self, x): + return x +""" + +_BY_NAME = {n.name: n for n in ast.parse(SRC).body[2].body} +_RESOLVED = "builtins.staticmethod" + + +def test_dotted_spelling_has_no_receiver(): + scope = build_scope(_BY_NAME["dotted"], set(), decorator_names={_RESOLVED}) + assert scope.self_name is None + + +def test_aliased_import_has_no_receiver(): + scope = build_scope(_BY_NAME["aliased"], set(), decorator_names={_RESOLVED}) + assert scope.self_name is None + + +def test_bare_spelling_still_has_no_receiver(): + scope = build_scope(_BY_NAME["plain"], set(), decorator_names={_RESOLVED}) + assert scope.self_name is None + + +def test_an_ordinary_method_keeps_its_receiver(): + scope = build_scope(_BY_NAME["method"], set(), decorator_names=set()) + assert scope.self_name == "self" + + +def test_unresolved_decorators_fall_back_to_the_written_spelling(): + """Callers that cannot supply resolved names keep the old behaviour.""" + assert build_scope(_BY_NAME["plain"], set()).self_name is None + assert build_scope(_BY_NAME["method"], set()).self_name == "self"