From acaa07e827b399ec20ea4b6478b6c02db23f8afe Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 14 Sep 2026 13:05:48 -0400 Subject: [PATCH 1/2] docs(design): spec co-positioned call-site identity (#215) Two nested ast.Call nodes that share a start position collide on the body map key, so the dynamic invocation in getattr(self, x)(y) is lost from the payload, the L3/L4 graphs and the Neo4j projection. The local id grammar gains a disambiguator, adopted verbatim from codeanalyzer-typescript's callBodyKeys: line:col, then /2, /3 for each further call site at the same position, outermost first. Records why java is immune, why re-anchoring python onto java's name-token rule was rejected, and why python-sdk needs no change. --- ...-09-14-co-positioned-call-site-identity.md | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 docs/design/specs/2026-09-14-co-positioned-call-site-identity.md diff --git a/docs/design/specs/2026-09-14-co-positioned-call-site-identity.md b/docs/design/specs/2026-09-14-co-positioned-call-site-identity.md new file mode 100644 index 0000000..0d28242 --- /dev/null +++ b/docs/design/specs/2026-09-14-co-positioned-call-site-identity.md @@ -0,0 +1,187 @@ +# Co-positioned call sites: one body node per `ast.Call` + +**Date:** 2026-09-14 +**Status:** Approved (design dialogue in-session) +**Scope:** codeanalyzer-python; LOCAL ordinal id grammar, L1 `body`, and the Neo4j +call-site join. Schema v2, no version move; closes #215 +**Builds on:** call-site/body convergence (`call-site-body-convergence.md`, #120); +JSON == Graph parity (`2026-09-10-neo4j-json-parity.md`, #202/#203) +**Sibling status:** codeanalyzer-typescript already ships this spelling +(`src/schema/l1Body.ts`, `callBodyKeys`); codeanalyzer-java is structurally immune — +see *Cross-language position* below + +## Problem + +`emit_l1_body` keys a callable's `body` dict on the call site's start position +(`codeanalyzer/schema/l1_body.py:9`): + +```python +key = f"{cs.start_line}:{cs.start_column}" +``` + +Two nested `ast.Call` nodes can begin at the same position. `getattr(self, x)(y)` is +that shape exactly: the outer application and the inner `getattr` both start at the +`g`. `_iter_calls_in_scope` yields both (`symbol_table_builder.py:693`), so +`call_sites` carries both, and the dict then keeps one — the later write, which is +the inner `getattr`. The dynamic invocation is destroyed. + +`IdentityMap.for_function` uses the same key format (`dataflow/identity.py:36`), so +L3 and L4 inherit the collision, and the Neo4j projection iterates `c.body` +(`neo4j/project.py:194`) so it emits one `:PyBodyNode` for the position — resolving +to `builtins.getattr`. Measured on a merged Odoo graph: 193 `getattr` call sites, +all 193 carrying a `PY_RESOLVES_TO` edge to the builtin, and no node anywhere for +the invocation that follows. `getattr(self, x)` and `getattr(self, x)(y)` are +indistinguishable in the graph. One reads an attribute; the other performs a +dynamic call. + +A second defect sits at the same site. `_callee_anchor` returns +`node.lineno, node.col_offset` whenever the callee is not an `ast.Attribute` +(`symbol_table_builder.py:147`). The outer call's `func` is an `ast.Call`, so the +anchor lands on `getattr` and Jedi infers `builtins.getattr`: the outer call is +labelled as a call to `getattr`, when it calls whatever `getattr` returned. + +## Contract-impact triage + +| Question | Answer | +| --- | --- | +| Changes schema v2 output? | **Yes.** The LOCAL ordinal id grammar gains a disambiguator, and `body` gains a node that has never existed — which flows into `cfg`/`cdg`/`ddg` endpoints and the Neo4j `PyBodyNode` merge keys. | +| Analyzers affected | `codeanalyzer-python` only. `codeanalyzer-typescript` already emits the target grammar; `codeanalyzer-java` cannot hit the collision. | +| SDKs affected | `python-sdk`: **verified, no code change** — see *Consumer impact*. | +| Docs affected | This repo's `CLAUDE.md` identity section and `.claude/SCHEMA_DECISIONS.md`. | +| Schema version | Stays `2.0.0`. The 2.0.0 line has not left RC, and the payload shape (fields, node kinds, edge kinds) does not move — only the key space below the callable. | + +## Cross-language position + +The three analyzers anchor a call node's local key differently, and it is worth +stating plainly because it looks like drift and is not: + +- **python** (`l1_body.py`) and **typescript** (`l1Body.ts`) key a call at the **call + expression's start**. Nested calls that begin at the same column therefore collide, + which is the bug this spec fixes. +- **java** (`BodyNodeBuilder.anchorOfStatement`) keys a call at the **invoked name + token** — `MethodCallExpr.getName()` for a call, the type for a `new`. `a.b().c()` + yields two distinct anchors, so java has no collision to disambiguate. + +Re-anchoring python onto the java rule was considered and rejected: it would move +**every** existing call key, and it does not even solve the motivating case +(`getattr(self, x)(y)`'s outer callee is a `Call`, which has no name token, so the +anchor falls back to the expression start and the collision returns). + +## Locked decisions + +1. **The disambiguator is adopted verbatim from codeanalyzer-typescript: + `line:col`, then `/2`, `/3`, … for each subsequent call site sharing a start + position.** `callBodyKeys` (`src/schema/l1Body.ts:24`) already ships exactly this, + with the comment "disambiguated `/2`, `/3`, … when chained calls share a start + position". Under the parity clause a term coined twice is permanently wrong, so + python ports the spelling rather than inventing `#1` or an end-position key. The + `/` delimiter is already in the grammar (`/actual_in:0`), and the + two never collide: a param-vertex segment always begins `actual_`. + +2. **The bare `line:col` goes to the first call site recorded, which is the + outermost call.** `_iter_calls_in_scope` yields a `Call` before descending into it + (`symbol_table_builder.py:711`), exactly as the TypeScript walker calls + `h.onCall(node)` before `forEachChild` (`builders.ts:454`). Pre-order is + deterministic on unchanged source, so no separate "rank by width" rule is needed — + but the implementation asserts the property (widest span first among + co-positioned siblings) rather than leaving it implicit in traversal order. + + This is the one meaning change: for a colliding position, `11:18` today resolves + to the inner `getattr` and afterwards resolves to the outer invocation. Keys that + do not collide are untouched, which is the great majority of the corpus. + +3. **`IdentityMap` needs no new format.** CFG nodes are statements, one per position, + so `for_function` keeps minting `line:col`. The existing L1/L3 merge + (`dataflow/builder.py:288`) therefore lands the CFG statement on the **outer** + call's key, which is the correct pairing — the statement and the outermost call + are the same region of code. The inner call keeps its own `/2` node with no CFG + contact, reached through the `parent` anchoring #115 already built. + +4. **The graph joins `callee_signature` per call site, not per position.** + `sig_by_pos` (`neo4j/project.py:186`) is keyed `(start_line, start_column)` and + silently keeps one of two co-positioned sites. It becomes a per-body-key map built + from the same key sequence L1 used, so each `:PyBodyNode` gets its own call site's + signature or none. + +5. **`_callee_anchor` gives a call-of-call no callee.** When `node.func` is itself an + `ast.Call`, the call site carries `callee_signature=None` and `method_name` + stays ``, so the site is identifiable as a dynamic invocation instead of + masquerading as a call to the inner callee. No `PY_RESOLVES_TO` edge is emitted + for it. + +6. **Nothing resolves the dynamic target.** No string-value analysis over + `'_index_%s' % ftype`, no new `PY_CALLS` edge to `_index_pdf`, no inference from + the receiver. Recording *that* a dynamic call happens is the fact being restored; + guessing *where it goes* is a different, unsound change. + +## What the payload looks like after + +For the reproducer in #215 (`buf = getattr(self, '_index_%s' % ftype)(bin_data)` on +line 11, column 18): + +``` +body (2 keys): + '11:18' kind='call' method_name='' callee=None callee_signature=None + '11:18/2' kind='call' method_name='getattr' callee=None callee_signature='builtins.getattr' +``` + +At L3 the enclosing statement merges onto `11:18`; at L4 any actual vertices for the +outer call parent as `11:18/actual_in:0`. On the graph, both nodes exist as +`:PyBodyNode` under `@11:18` and `@11:18/2`, and only the +second carries a `PY_RESOLVES_TO` edge to `builtins.getattr`. + +`f()()()` yields three keys — `L:C`, `L:C/2`, `L:C/3` — outermost first, stable +across runs. + +## Consumer impact + +- **python-sdk: no change required, verified against the source.** + `body_key_column` (`cldk/analysis/commons/keys.py:183`) does + `key.split("/", 1)[0].partition(":")`, so `"11:18/2"` already yields column `18` — + the same value the suffix-free key yields, which is what its tie-break wants. The + rank tuples in `codeanalyzer.py:380` and `neo4j_backend.py:2191` are + `(line width, -column, key)`, so two co-positioned nodes tie on the first two + components and break on the key string, putting `"11:18"` (the outer call) ahead of + `"11:18/2"`. That is the right order for a "which node is this position" query. +- **Existing graphs do not gain the node.** The fix applies to new runs; a consumer + reading an older graph still sees one node per position and must tolerate its + absence. +- **Anything that treated a body key as `line:col` verbatim** (splitting on `:` and + taking two fields) now sees a third form. The SDK's helper is the reference parser; + it already handles it. + +## Test plan (the issue's DoD, made executable) + +- The #215 reproducer yields exactly the two keys above; the test names the expected + key set, not a count. +- The invocation node has `method_name=''`, `callee=None`, + `callee_signature=None`, and no `PY_RESOLVES_TO` edge; the `getattr` node keeps + `method_name='getattr'` and `callee_signature='builtins.getattr'`. +- Every body key in the existing fixtures is unchanged for callables with no + co-positioned calls — asserted against the current key sets, not their sizes. +- `IdentityMap.global_id` and the Neo4j `_global_ordinal` produce the same string for + both new nodes. +- `f()()()` yields three distinct keys, and two runs on one source yield the same + three. +- The five analyzer gates, plus the regenerated graph schema snapshot if any declared + property moves. + +## Release plan + +One PR on `codeanalyzer-python` closing #215, carried by the **next patch train**, +alongside the already-merged #207 fix (`5f5734f`). Analyzer-only: no SDK pin moves, +no lockstep with another repo, and `schema_version` does not change. Docs land in the +same PR — the identity paragraph in `CLAUDE.md` and an entry in +`.claude/SCHEMA_DECISIONS.md` recording that a local id may carry a `/N` +disambiguator and why the outermost call holds the bare key. + +## Deferred, deliberately + +- **Argument text.** `arguments_json` keeps `value` only for a JSON-safe `Constant` + and `name` only for a bare `Name` (`symbol_table_builder.py:786`), so + `'_index_%s' % ftype` still reaches a consumer as + `{"ast_kind": "BinOp", "name": null, "value": null}`. Carrying `ast.unparse(arg)` + for other shapes is its own issue. +- **Dynamic target resolution** — see locked decision 6. +- **Sibling work.** None falls out: typescript already emits this grammar, java + cannot collide. From a8e1a02839bb7deebe19a2f8eb948b23a3dca8d7 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 14 Sep 2026 13:17:56 -0400 Subject: [PATCH 2/2] fix(schema): one body node per call site, even when two share a start position (#215) `body` was keyed on the call site's start position, and two nested `ast.Call` nodes can start at the same one. `getattr(o, n)(x)` is that shape: the outer application and the inner `getattr` both begin at the `g`, so the dict kept one -- the inner `getattr`, written last -- and the dynamic invocation was gone from the payload, from the L3/L4 graphs that key off the same format, and from the Neo4j projection that iterates `body`. Measured on a merged Odoo graph: 193 `getattr` sites, 193 edges to the builtin, and no node for any of the invocations that followed. The key sequence gains a disambiguator -- `line:col`, then `/2`, `/3`, ... per further call site at that position, outermost first, since call sites are recorded pre-order. `schema/ids.py::call_body_keys` is the single definition; L1, L2, the dataflow builder, the defuse linker and the graph projection now re-derive the pairing from it instead of rebuilding a key from a position. The spelling is codeanalyzer-typescript's `callBodyKeys`, adopted verbatim. Two smaller defects at the same site: - The Neo4j projection joined `callee_signature` on `(line, column)`, so co-positioned sites shared whichever signature the dict kept. It joins on the body key now. - `_callee_anchor` fell back to the call expression's start for any non-attribute callee, so a call whose callee is itself a call was anchored on the inner call's name and labelled a call to it. It returns `None` there, and the site carries `callee_signature=None` with `method_name=""`. What the dynamic call reaches is still not inferred, by design: the site is recorded so a consumer can decide, and no `PY_CALLS` edge is invented. Spec: docs/design/specs/2026-09-14-co-positioned-call-site-identity.md --- .claude/SCHEMA_DECISIONS.md | 27 ++++++ CLAUDE.md | 6 +- codeanalyzer/dataflow/builder.py | 28 ++++-- codeanalyzer/neo4j/project.py | 19 ++-- codeanalyzer/schema/ids.py | 38 +++++++- codeanalyzer/schema/l1_body.py | 5 +- codeanalyzer/schema/l2_callees.py | 7 +- .../semantic_analysis/defuse_linker.py | 11 ++- .../symbol_table_builder.py | 30 ++++-- test/test_co_positioned_call_sites.py | 95 +++++++++++++++++++ 10 files changed, 228 insertions(+), 38 deletions(-) create mode 100644 test/test_co_positioned_call_sites.py diff --git a/.claude/SCHEMA_DECISIONS.md b/.claude/SCHEMA_DECISIONS.md index 2dd5dbf..211c370 100644 --- a/.claude/SCHEMA_DECISIONS.md +++ b/.claude/SCHEMA_DECISIONS.md @@ -609,3 +609,30 @@ Sibling halves: codeanalyzer-java#255/#256, codeanalyzer-typescript#201/#202. is where the comment model stops. Its closing comment names #203 as dropping its comment goals for the same reason. `python-sdk`'s `get_all_comments` raising on the Neo4j backend is now the permanent answer, not a workaround. + +## 2026-09-14 — co-positioned call sites get a key disambiguator (#215) + +- **A body key may carry `/N`.** `line:col` addressed a *position*, and two nested + `ast.Call` nodes can share one (`getattr(o, n)(x)`, `f()()()`), so one of them was + dropped from `body` and with it from `cfg`/`cdg`/`ddg` and the Neo4j projection. + The key sequence is now `line:col`, then `/2`, `/3`, … per further call site at + that position. `schema/ids.py::call_body_keys` is the one definition; L1, L2, the + dataflow builder, the defuse linker and the graph projection all re-derive the + pairing from it instead of rebuilding a key from a position. +- **The spelling is codeanalyzer-typescript's, adopted verbatim** (`callBodyKeys`, + `src/schema/l1Body.ts`): `/` and a 2-based counter, not `#` and not an + end-position key. A term coined twice is permanently wrong. +- **The outermost call keeps the bare key.** Call sites are recorded pre-order in + both analyzers, so this needs no extra rule — but it does change what a colliding + key resolves to: `11:18` used to be the inner `getattr`, and is now the + invocation. Non-colliding keys are untouched. +- **codeanalyzer-java is structurally immune.** `BodyNodeBuilder.anchorOfStatement` + keys a call at the invoked name token, so its nested calls never share an anchor. + Re-anchoring python that way was rejected: it moves every existing call key and + does not fix a call-of-call, which has no name token. +- **A call-of-call resolves to nothing.** `_callee_anchor` returns `None` when + `node.func` is an `ast.Call`, so the site carries `callee_signature=None` and + `method_name=""` instead of claiming to call the inner callee. What the + dynamic call reaches is deliberately not inferred. +- **Neither version moves** — the 2026-09-07 hold stands. The payload shape is + unchanged; only the key space below the callable widens. diff --git a/CLAUDE.md b/CLAUDE.md index a3d1500..91612d3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,7 +104,11 @@ declared callables by their `can://` tree id, imported/builtin targets by a intra-callable `cfg`/`cdg`/`ddg`/`summary` edge endpoint: `"line:col"` for real statements, `"@entry"`/`"@exit"` for the CFG bookends, `"@formal_in:0"` / `"@formal_out"` for formals, and `"/actual_in:0"` / - `"/actual_out"` for actuals (parented to their call site). The + `"/actual_out"` for actuals (parented to their call site). Two nested + calls can *start* at one position (`getattr(o, n)(x)`), so a call key may carry a + `/2`, `/3`, … disambiguator — outermost call first, the bare `line:col` (#215). + `codeanalyzer/schema/ids.py::call_body_keys` is the single definition of that + sequence; the spelling is codeanalyzer-typescript's, adopted verbatim. The bijection `(signature, int node_id) ↔ (can:// id, local id)` is built once by `codeanalyzer/dataflow/identity.py` and feeds **both** projections, keeping them in lockstep. diff --git a/codeanalyzer/dataflow/builder.py b/codeanalyzer/dataflow/builder.py index 34b855c..7d6262a 100644 --- a/codeanalyzer/dataflow/builder.py +++ b/codeanalyzer/dataflow/builder.py @@ -35,6 +35,7 @@ from __future__ import annotations import ast +from collections import defaultdict from pathlib import Path from typing import Callable, Dict, List, Optional, Set, Tuple @@ -43,7 +44,7 @@ from codeanalyzer.dataflow.pdg import build_pdg from codeanalyzer.dataflow.sdg import ProgramGraphsIR, assemble_sdg from codeanalyzer.dataflow.summaries import CallSite, FunctionInfo, compute_summaries -from codeanalyzer.schema.ids import stamp_body_ids +from codeanalyzer.schema.ids import call_body_keys, stamp_body_ids from codeanalyzer.schema.py_schema import PyApplication, PyCallable, PyClass, PyModule from codeanalyzer.utils import logger @@ -291,6 +292,11 @@ def _span_of(source: str, node) -> Optional["Span"]: continue pycallable.body[local] = BodyNode(kind=node.kind, span=span) + keys_at: Dict[str, List[str]] = defaultdict(list) + for key, n in pycallable.body.items(): + if n.kind == "call": + keys_at[key.split("/", 1)[0]].append(key) + # #115: anchor nested call vertices to their statement. A bare-call # statement shares its key with its CFG node (handled above); a call # nested inside a larger statement (`y = f(x)`) has its own key and @@ -301,12 +307,16 @@ def _span_of(source: str, node) -> Optional["Span"]: continue stmt_local = im.local(node.id) for call in _calls_in(node.ast_node): - call_key = f"{call.lineno}:{call.col_offset}" - child = pycallable.body.get(call_key) - if child is None or call_key == stmt_local: - continue - if child.kind == "call": - child.parent = stmt_local + # Every call node at this position, not just one: nested calls + # that share a start column are keyed `line:col`, `line:col/2`, + # ... and each of them needs the anchor (#215). + base = f"{call.lineno}:{call.col_offset}" + for call_key in keys_at.get(base, ()): + child = pycallable.body.get(call_key) + if child is None or call_key == stmt_local: + continue + if child.kind == "call": + child.parent = stmt_local if want_cfg: pycallable.cfg = [ @@ -378,7 +388,7 @@ def build_program_graphs( calls_by_pos.setdefault(pos, (node.id, call)) calls_by_line.setdefault(call.lineno, (node.id, call)) - for site in pycallable.call_sites or []: + for site_key, site in call_body_keys(pycallable.call_sites): # Prefer the callsite's body-backfilled callee over Jedi's own # callee_signature side channel: under cross-test parso/Jedi cache # pressure that inference can silently degrade (full-suite-only @@ -388,7 +398,7 @@ def build_program_graphs( # Only a resolved INTERNAL target counts (id_to_sig misses on an # external/unresolved callee); falls through to callee_signature # exactly as before whenever the body doesn't have an answer. - body_node = pycallable.body.get(f"{site.start_line}:{site.start_column}") + body_node = pycallable.body.get(site_key) target = (id_to_sig.get(body_node.callee) if body_node else None) or site.callee_signature if not target: continue diff --git a/codeanalyzer/neo4j/project.py b/codeanalyzer/neo4j/project.py index efc0c38..e6c2660 100644 --- a/codeanalyzer/neo4j/project.py +++ b/codeanalyzer/neo4j/project.py @@ -49,7 +49,9 @@ PyVariableDeclaration, ) from codeanalyzer.schema import model_dump -from codeanalyzer.schema.ids import application_id, external_id, global_ordinal, purl_pypi +from codeanalyzer.schema.ids import ( + application_id, call_body_keys, external_id, global_ordinal, purl_pypi, +) from codeanalyzer.schema.py_schema import PyDecorator, byte_offsets @@ -183,12 +185,14 @@ def _project_program_graphs( continue # unstamped callable — assign_ids must run first owner = _sym(c.id) # the :PyCallable node, keyed by its can:// id # ``callee_signature`` lives on ``PyCallable.call_sites``, not on the body - # node, so the graph joins the two on the call site's position (#203). + # node, so the graph joins the two on the call site's BODY KEY (#203, + # #215) -- re-derived from the same sequence L1 keyed ``body`` with, so + # two calls that start at one position keep their own signatures. # ``argument_types`` is deliberately not joined: it is the legacy field #86 # split into ``PyCallArgument``, already carried as ``arguments_json``. - sig_by_pos = { - (cs.start_line, cs.start_column): cs.callee_signature - for cs in (c.call_sites or []) + sig_by_key = { + key: cs.callee_signature + for key, cs in call_body_keys(c.call_sites) if cs.callee_signature } for local_key, node in (c.body or {}).items(): @@ -204,10 +208,7 @@ def _project_program_graphs( { "kind": node.kind, **_span_props(span), - "callee_signature": ( - sig_by_pos.get((span.start[0], span.start[1])) - if span else None - ), + "callee_signature": sig_by_key.get(local_key), "var": node.of, "call_node": node.parent, # Call-site detail (#120). The JSON emits one node per diff --git a/codeanalyzer/schema/ids.py b/codeanalyzer/schema/ids.py index d984268..5887d4b 100644 --- a/codeanalyzer/schema/ids.py +++ b/codeanalyzer/schema/ids.py @@ -10,7 +10,7 @@ an application named ``python`` mints ``can://python/python/...``, so a test for ``can://python/`` no longer means "a python id"; test the scheme instead.""" from __future__ import annotations -from typing import List, Optional +from typing import Iterable, Iterator, List, Optional, Tuple SCHEME = "can://" @@ -52,6 +52,42 @@ def external_id(app_id: str, module: Optional[str], name: str) -> str: return f"{base}/{module}/{name}" if module else f"{base}/{name}" +def call_body_keys(sites: Iterable) -> Iterator[Tuple[str, object]]: + """The body key of each call site, in recording order: ``line:col``, + disambiguated ``/2``, ``/3``, ... when nested calls share a start position + (#215). + + `getattr(o, n)(x)` begins the outer application and the inner `getattr` at the + same column, so a bare ``line:col`` key keeps one of the two and the dynamic + invocation is lost. Call sites are recorded pre-order, so the bare key goes to + the OUTERMOST call and the nested ones take the suffixes. The spelling is + codeanalyzer-typescript's (``callBodyKeys``, ``src/schema/l1Body.ts``), adopted + verbatim; the ``/`` never collides with a param-vertex segment, which always + begins ``actual_``. + + The SINGLE definition of the sequence -- L1 builds ``body`` with it, and L2, the + dataflow builder, the defuse linker and the Neo4j projection re-derive the same + pairing from it rather than re-deriving a key from a position. + """ + used = set() + for cs in sites or []: + base = f"{cs.start_line}:{cs.start_column}" + key = base + k = 2 + while key in used: + key = f"{base}/{k}" + k += 1 + used.add(key) + yield key, cs + +def call_body_key(callable_, site) -> Optional[str]: + """``site``'s body key within ``callable_`` — the pairing of + :func:`call_body_keys`, for a caller that holds one site rather than the list.""" + for key, cs in call_body_keys(callable_.call_sites): + if cs is site: + return key + return None + def global_ordinal(callable_id: str, local_key: str) -> str: """The GLOBAL ordinal id of a body node from its LOCAL key: synthetic keys (`@entry`, `@formal_in:0`) already carry the `@`; positional keys (`15:2`, diff --git a/codeanalyzer/schema/l1_body.py b/codeanalyzer/schema/l1_body.py index 74892c7..32fb28e 100644 --- a/codeanalyzer/schema/l1_body.py +++ b/codeanalyzer/schema/l1_body.py @@ -1,12 +1,11 @@ """L1 body population: materialize `call` nodes from existing call sites. `callee` is left None here — the sanctioned null→id refinement happens at L2.""" from __future__ import annotations -from codeanalyzer.schema.ids import stamp_body_ids +from codeanalyzer.schema.ids import call_body_keys, stamp_body_ids from codeanalyzer.schema.py_schema import PyApplication, PyClass, PyCallable, BodyNode, Span, byte_offsets def _do_callable(source: str, c: PyCallable) -> None: - for cs in c.call_sites or []: - key = f"{cs.start_line}:{cs.start_column}" + for key, cs in call_body_keys(c.call_sites): span = Span(start=(cs.start_line, cs.start_column), end=(cs.end_line, cs.end_column), bytes=byte_offsets(source, cs.start_line, cs.start_column, cs.end_line, cs.end_column)) if source else None diff --git a/codeanalyzer/schema/l2_callees.py b/codeanalyzer/schema/l2_callees.py index 7134fac..02257f7 100644 --- a/codeanalyzer/schema/l2_callees.py +++ b/codeanalyzer/schema/l2_callees.py @@ -5,17 +5,18 @@ Two resolution sources feed the backfill: Jedi's `callee_signature` on the call site itself, and the defuse linker's returned map (keyed by caller -signature + "line:col"). The linker's resolutions are deliberately NOT written +signature + the call site's body key, which carries a `/N` disambiguator when +nested calls share a start position -- #215). The linker's resolutions are deliberately NOT written into `callee_signature` — the symbol table round-trips through the analysis cache, and a persisted resolution would resurface on a warm run as a Jedi edge, silently changing provenance.""" from __future__ import annotations +from codeanalyzer.schema.ids import call_body_keys from codeanalyzer.schema.py_schema import PyApplication, PyClass, PyCallable def _do_callable(c: PyCallable, sig_to_id: dict, resolutions: dict) -> None: - for cs in c.call_sites or []: - key = f"{cs.start_line}:{cs.start_column}" + for key, cs in call_body_keys(c.call_sites): jedi_sig = cs.callee_signature if jedi_sig and jedi_sig.startswith("typing."): # A decorator-typed callable resolved to its annotation, not a diff --git a/codeanalyzer/semantic_analysis/defuse_linker.py b/codeanalyzer/semantic_analysis/defuse_linker.py index cc7a777..c11e0e5 100644 --- a/codeanalyzer/semantic_analysis/defuse_linker.py +++ b/codeanalyzer/semantic_analysis/defuse_linker.py @@ -40,11 +40,14 @@ import builtins as _py_builtins from typing import Dict, List, Optional, Tuple +from codeanalyzer.schema.ids import call_body_key from codeanalyzer.schema.py_schema import PyCallable, PyCallEdge, PyClass, PyModule __all__ = ["defuse_linker_edges"] -# (caller signature, "line:col" of the call site) -> resolved callee signature +# (caller signature, the call site's body key) -> resolved callee signature. +# The key is `call_body_key`, not a bare "line:col": two nested calls can start +# at one position and only one of them is the resolution being recorded (#215). Resolutions = Dict[Tuple[str, str], str] _MAX_CHAIN = 16 # assignment-chain hops before giving up (cycle safety net) @@ -1095,7 +1098,7 @@ def bump(src: str, dst: str) -> None: oracle.vote(sig, site) bump(caller.signature, sig) resolutions[ - (caller.signature, f"{site.start_line}:{site.start_column}") + (caller.signature, call_body_key(caller, site)) ] = sig # Calls Jedi's extractor never recorded as sites at all (with- @@ -1398,7 +1401,7 @@ def _method_on(t, method, qual): oracle.vote(sig, site) bump(caller.signature, sig) resolutions[ - (caller.signature, f"{site.start_line}:{site.start_column}") + (caller.signature, call_body_key(caller, site)) ] = sig made_progress = True else: @@ -1452,7 +1455,7 @@ def _method_on(t, method, qual): oracle.vote(sig, site) bump(caller.signature, sig) resolutions[ - (caller.signature, f"{site.start_line}:{site.start_column}") + (caller.signature, call_body_key(caller, site)) ] = sig made_progress = True remaining = still diff --git a/codeanalyzer/syntactic_analysis/symbol_table_builder.py b/codeanalyzer/syntactic_analysis/symbol_table_builder.py index ce346f4..9e52e27 100644 --- a/codeanalyzer/syntactic_analysis/symbol_table_builder.py +++ b/codeanalyzer/syntactic_analysis/symbol_table_builder.py @@ -130,8 +130,9 @@ def _infer_callee( return None, False @staticmethod - def _callee_anchor(node: ast.Call) -> Tuple[int, int]: - """Position of the callee *name* for Jedi inference. + def _callee_anchor(node: ast.Call) -> Optional[Tuple[int, int]]: + """Position of the callee *name* for Jedi inference, or ``None`` when the + callee is not a name at all. An ``ast.Call``'s own ``lineno``/``col_offset`` is the first character of the whole call expression — for an attribute call @@ -139,11 +140,17 @@ def _callee_anchor(node: ast.Call) -> Tuple[int, int]: would infer the receiver's type instead of the invoked method (issue #80). Anchor attribute calls inside the attribute name — its last character, so one-character names stay in range; other - callee shapes keep the call-expression start. + callee shapes keep the call-expression start. A callee that is itself a + call has no name to anchor on, so it yields ``None`` (#215). """ func_expr = node.func if isinstance(func_expr, ast.Attribute): return func_expr.end_lineno, func_expr.end_col_offset - 1 + if isinstance(func_expr, ast.Call): + # `getattr(o, n)(x)`: the callee IS a call, so the expression start is + # the INNER call's name and inferring there labels this site a call to + # `getattr` -- the thing that produced the callee, not the callee (#215). + return None return node.lineno, node.col_offset @staticmethod @@ -766,11 +773,18 @@ def _call_sites(self, fn_node: ast.FunctionDef, script: Script) -> List[PyCallsi func_expr = node.func method_name = "" - anchor_line, anchor_col = self._callee_anchor(node) - callee_signature, is_constructor = self._infer_callee( - script, anchor_line, anchor_col - ) - return_type = self._infer_call_return_type(script, anchor_line, anchor_col) + anchor = self._callee_anchor(node) + if anchor is None: + # A dynamic invocation: the site is recorded, and it resolves to + # nothing. Guessing a target here is what produced a graph full of + # calls to `builtins.getattr` (#215). + callee_signature, is_constructor, return_type = None, False, None + else: + anchor_line, anchor_col = anchor + callee_signature, is_constructor = self._infer_callee( + script, anchor_line, anchor_col + ) + return_type = self._infer_call_return_type(script, anchor_line, anchor_col) receiver_expr = None receiver_type = None diff --git a/test/test_co_positioned_call_sites.py b/test/test_co_positioned_call_sites.py new file mode 100644 index 0000000..f47cc91 --- /dev/null +++ b/test/test_co_positioned_call_sites.py @@ -0,0 +1,95 @@ +"""Two `ast.Call` nodes can begin at the same position (#215). + +`getattr(o, n)(x)` is that shape exactly: the outer application and the inner +`getattr` both start at the `g`. Keying `body` on the start position alone kept +one of them -- the inner `getattr`, since it is written last -- so the dynamic +invocation vanished from the payload, from the L3/L4 graphs that key off the same +format, and from the Neo4j projection that iterates `body`. + +The key gains a disambiguator: `line:col`, then `/2`, `/3`, ... for each further +call site at that position, outermost first. The spelling is adopted verbatim from +codeanalyzer-typescript's `callBodyKeys` (`src/schema/l1Body.ts`). + +Spec: docs/design/specs/2026-09-14-co-positioned-call-site-identity.md +""" +from codeanalyzer.core import Codeanalyzer +from codeanalyzer.neo4j.project import project +from codeanalyzer.options import AnalysisOptions +from codeanalyzer.schema.assign_ids import assign_ids + +# `getattr(o, n)(1)` starts at line 2, column 11 -- and so does the `getattr` call +# inside it. +DYNAMIC = "def f(o, n):\n return getattr(o, n)(1)\n" + + +def _analyze(tmp_path, source, level=1, name="m.py"): + proj = tmp_path / "p" + proj.mkdir(parents=True, exist_ok=True) + (proj / name).write_text(source) + app = Codeanalyzer(AnalysisOptions( + input=proj, analysis_level=level, no_venv=True, cache_dir=tmp_path / "c", + )).analyze().application + return app + + +def _callable(app, file_key="m.py", name="f"): + return app.symbol_table[file_key].functions[name] + + +def test_each_co_positioned_call_gets_its_own_body_node(tmp_path): + f = _callable(_analyze(tmp_path, DYNAMIC)) + assert sorted(f.body) == ["2:11", "2:11/2"] + + +def test_the_outermost_call_keeps_the_bare_key(tmp_path): + f = _callable(_analyze(tmp_path, DYNAMIC)) + outer, inner = f.body["2:11"], f.body["2:11/2"] + assert outer.kind == "call" and inner.kind == "call" + # The invocation is of whatever `getattr` returned -- not of `getattr`. + assert outer.method_name == "" + assert inner.method_name == "getattr" + + +def test_a_call_of_a_call_resolves_to_no_callee(tmp_path): + """`_callee_anchor` used to fall back to the call expression's start for any + non-attribute callee, so the outer call was anchored on `getattr` and Jedi + labelled it a call to `builtins.getattr`.""" + f = _callable(_analyze(tmp_path, DYNAMIC, level=2)) + by_pos = {(cs.start_line, cs.start_column, cs.method_name): cs for cs in f.call_sites} + outer = by_pos[(2, 11, "")] + inner = by_pos[(2, 11, "getattr")] + assert outer.callee_signature is None + assert inner.callee_signature == "builtins.getattr" + assert f.body["2:11"].callee is None + assert f.body["2:11/2"].callee.endswith("/@external/builtins/getattr") + + +def test_a_chain_of_applications_gets_one_key_each(tmp_path): + f = _callable(_analyze(tmp_path, "def f(g):\n return g()()()\n")) + assert sorted(f.body) == ["2:11", "2:11/2", "2:11/3"] + + +def test_keys_that_do_not_collide_are_untouched(tmp_path): + f = _callable(_analyze(tmp_path, "def f(a, b):\n return len(a) + str(b)\n")) + assert sorted(f.body) == ["2:11", "2:20"] + + +def test_the_key_sequence_is_stable_across_runs(tmp_path): + first = sorted(_callable(_analyze(tmp_path, DYNAMIC)).body) + second = sorted(_callable(_analyze(tmp_path / "again", DYNAMIC)).body) + assert first == second == ["2:11", "2:11/2"] + + +def test_the_graph_carries_both_nodes_and_joins_signatures_per_call_site(tmp_path): + """`sig_by_pos` joined `callee_signature` on (line, column), so two call sites + at one position shared whichever signature the dict kept last.""" + app = _analyze(tmp_path, DYNAMIC, level=2) + f = _callable(app) + rows = project(app, "p", assign_ids(app, "p")) + body = { + n.value: n.props for n in rows.nodes + if n.labels[0] == "PyBodyNode" and n.value.startswith(f.id + "@") + } + assert f.id + "@2:11" in body and f.id + "@2:11/2" in body + assert "callee_signature" not in body[f.id + "@2:11"] + assert body[f.id + "@2:11/2"]["callee_signature"] == "builtins.getattr"