diff --git a/codeanalyzer/__main__.py b/codeanalyzer/__main__.py index 388edb4..41e82b6 100644 --- a/codeanalyzer/__main__.py +++ b/codeanalyzer/__main__.py @@ -1,3 +1,4 @@ +import json import os import sys from importlib.metadata import version as _pkg_version, PackageNotFoundError @@ -38,7 +39,7 @@ def _pin_hash_seed() -> None: from codeanalyzer.core import Codeanalyzer from codeanalyzer.utils import _set_log_level, logger from codeanalyzer.config import OutputFormat -from codeanalyzer.schema import model_dump_json +from codeanalyzer.schema import model_dump_json, strip_internal_only from codeanalyzer.options import AnalysisOptions, EmitTarget, ShardStrategy @@ -451,7 +452,13 @@ def main( emit_neo4j(artifacts, options) elif options.output is None: - print(model_dump_json(artifacts, exclude_none=True)) + print( + json.dumps( + strip_internal_only( + artifacts.model_dump(mode="json", exclude_none=True) + ) + ) + ) else: options.output.mkdir(parents=True, exist_ok=True) _write_output(artifacts, options.output, options.format) @@ -462,7 +469,11 @@ def _write_output(artifacts, output_dir: Path, format: OutputFormat): if format == OutputFormat.JSON: output_file = output_dir / "analysis.json" # Use Pydantic's model_dump_json() for compact output - json_str = model_dump_json(artifacts, indent=None, exclude_none=True) + # Strip internal-only fields here rather than with a field-level Pydantic + # `exclude`: the analysis cache shares the serializer and must keep them. + json_str = json.dumps( + strip_internal_only(artifacts.model_dump(mode="json", exclude_none=True)) + ) with output_file.open("w") as f: f.write(json_str) logger.info(f"Analysis saved to {output_file}") diff --git a/codeanalyzer/neo4j/project.py b/codeanalyzer/neo4j/project.py index b838b8a..33589cc 100644 --- a/codeanalyzer/neo4j/project.py +++ b/codeanalyzer/neo4j/project.py @@ -47,7 +47,7 @@ PyModule, PyVariableDeclaration, ) -from codeanalyzer.schema.py_schema import PyCallsite, PyDecorator +from codeanalyzer.schema.py_schema import PyDecorator def project(app: PyApplication, app_name: str, sig_to_id: dict, @@ -98,7 +98,7 @@ def project(app: PyApplication, app_name: str, sig_to_id: dict, # Level-3 CPG overlay: each callable's v2 body/cfg/cdg/ddg. Idempotent under # MERGE — a no-op when no callable carries L3 fields (levels 1/2). - _project_program_graphs(b, app) + _project_program_graphs(b, app, externals, sig_to_id) return b.finish() @@ -128,7 +128,9 @@ def _cfg_ref(callable_id: str, local_key: str) -> NodeRef: return NodeRef("PyCFGNode", "id", _global_ordinal(callable_id, local_key)) -def _project_program_graphs(b: RowBuilder, app: PyApplication) -> None: +def _project_program_graphs( + b: RowBuilder, app: PyApplication, externals: dict, sig_to_id: dict +) -> None: """Level-3 CPG overlay, projected off each callable's v2 ``body``/``cfg``/ ``cdg``/``ddg`` (populated by ``emit_l3_body`` at ``-a 3``; empty otherwise). @@ -173,11 +175,31 @@ def _project_program_graphs(b: RowBuilder, app: PyApplication) -> None: "end_line": span.end[0] if span else None, "var": node.of, "call_node": node.parent, + # Call-site detail (#120). The JSON emits one node per + # call site; the graph now does too, instead of a + # separate :PyCallSite under a third id scheme. + "method_name": node.method_name, + "receiver_expr": node.receiver_expr, + "receiver_type": node.receiver_type, + "return_type": node.return_type, + "is_constructor_call": node.is_constructor_call, + "arguments_json": _stringify_if(node.arguments), "_module": file_key, } ), ) b.edge("PY_HAS_CFG_NODE", owner, ref) + if node.kind == "call" and node.callee: + # `callee` is ALREADY a resolved can:// id (a declared callable + # or an @external home), so it must not go through + # `_symbol_ref`, which expects a dotted signature and would + # fall back to matching a `signature` property against an id -- + # emitting an edge that matches nothing at load time. + b.edge( + "PY_RESOLVES_TO", + ref, + _call_endpoint(b, node.callee, externals, sig_to_id), + ) for e in c.cfg or []: # kind-discriminated: a conditional's true/false pair between one # endpoint pair must stay two relationships, not one MERGE. @@ -400,19 +422,6 @@ def _project_callable( for d in c.decorators or []: _project_decorator(b, ref, d) - for s in c.call_sites or []: - # Key off the relative file (a call site lives in its callable's file) so ids stay portable. - cs_id = ( - f"{file_key}#{s.start_line}:{s.start_column}-{s.end_line}:{s.end_column}" - ) - cs = b.node(["PyCallSite"], "id", cs_id, _call_site_props(s, file_key)) - b.edge("PY_HAS_CALLSITE", ref, cs) - if s.callee_signature: - b.edge_to_symbol( - "PY_RESOLVES_TO", cs, - _symbol_ref(s.callee_signature, externals, sig_to_id), - ) - for v in c.local_variables or []: _project_variable(b, file_key, ref, c.signature, v) for ic in (c.callables or {}).values(): @@ -577,26 +586,6 @@ def _variable_props(v: PyVariableDeclaration, var_id: str, file_key: str) -> Pro ) -def _call_site_props(s: PyCallsite, file_key: str) -> Props: - cs_id = f"{file_key}#{s.start_line}:{s.start_column}-{s.end_line}:{s.end_column}" - return prune( - { - "id": cs_id, - "method_name": s.method_name, - "receiver_expr": s.receiver_expr, - "receiver_type": s.receiver_type, - "argument_types": list(s.argument_types or []), - "arguments_json": _stringify_if(s.arguments), - "return_type": s.return_type, - "callee_signature": s.callee_signature, - "is_constructor_call": s.is_constructor_call, - "start_line": s.start_line, - "start_column": s.start_column, - "end_line": s.end_line, - "end_column": s.end_column, - "_module": file_key, - } - ) def _call_edge_props(weight: int, prov: List[str]) -> Props: diff --git a/codeanalyzer/neo4j/schema.py b/codeanalyzer/neo4j/schema.py index a1fd2aa..272c311 100644 --- a/codeanalyzer/neo4j/schema.py +++ b/codeanalyzer/neo4j/schema.py @@ -145,27 +145,6 @@ class RelType: "name", {"name": "string", "qualified_name": "string"}, ), - NodeLabel( - "PyCallSite", - "PyCallSite", - "id", - { - "id": "string", - "method_name": "string", - "receiver_expr": "string", - "receiver_type": "string", - "argument_types": "string[]", - "arguments_json": "string", - "return_type": "string", - "callee_signature": "string", - "is_constructor_call": "boolean", - "start_line": "integer", - "start_column": "integer", - "end_line": "integer", - "end_column": "integer", - "_module": "string", - }, - ), NodeLabel( "PyAttribute", "PyAttribute", @@ -209,6 +188,14 @@ class RelType: "kind": "string", "var": "string", "call_node": "string", + # Call-site detail (#120): the graph emits one node per call site, + # matching analysis.json, instead of a separate :PyCallSite. + "method_name": "string", + "receiver_expr": "string", + "receiver_type": "string", + "return_type": "string", + "is_constructor_call": "boolean", + "arguments_json": "string", **_SPAN, "_module": "string", }, @@ -224,8 +211,7 @@ class RelType: RelType("PY_HAS_METHOD", ["PyClass"], ["PyCallable"]), RelType("PY_HAS_ATTRIBUTE", ["PyClass"], ["PyAttribute"]), RelType("PY_DECLARES_VAR", ["PyModule", "PyCallable"], ["PyVariable"]), - RelType("PY_HAS_CALLSITE", ["PyCallable"], ["PyCallSite"]), - RelType("PY_RESOLVES_TO", ["PyCallSite"], ["PyCallable", "PyExternal"]), + RelType("PY_RESOLVES_TO", ["PyCFGNode"], ["PyCallable", "PyExternal"]), RelType( "PY_CALLS", ["PyCallable", "PyExternal"], diff --git a/codeanalyzer/schema/__init__.py b/codeanalyzer/schema/__init__.py index 648a10e..9d13f32 100644 --- a/codeanalyzer/schema/__init__.py +++ b/codeanalyzer/schema/__init__.py @@ -62,6 +62,31 @@ ) Analysis.update_forward_refs(PyApplication=PyApplication) +# Fields the analyzer keeps in memory (and in the cache) but never emits. +# `call_sites` is the internal record `body{}` call nodes are derived from (#120); +# emitting both shipped the same fact twice under two identity schemes. +INTERNAL_ONLY_FIELDS = frozenset({"call_sites"}) + + +def strip_internal_only(data): + """Recursively drop `INTERNAL_ONLY_FIELDS` from a dumped payload. + + Applied at emit time rather than as a field-level Pydantic `exclude`, because + the analysis cache uses the same serializer -- excluding at the field would + drop these from the cache as well, and the next warm-cache run would rebuild + from a payload with no call sites. + """ + if isinstance(data, dict): + return { + k: strip_internal_only(v) + for k, v in data.items() + if k not in INTERNAL_ONLY_FIELDS + } + if isinstance(data, list): + return [strip_internal_only(v) for v in data] + return data + + # Compatibility helpers for Pydantic v1/v2 def model_dump_json(model, **kwargs): """Compatibility helper for JSON serialization.""" @@ -89,5 +114,7 @@ def model_validate_json(model_class, json_data): __all__.extend([ "PYDANTIC_V2", "model_dump_json", + "strip_internal_only", + "INTERNAL_ONLY_FIELDS", "model_validate_json" ]) \ No newline at end of file diff --git a/codeanalyzer/schema/l1_body.py b/codeanalyzer/schema/l1_body.py index 3c6f781..04153e8 100644 --- a/codeanalyzer/schema/l1_body.py +++ b/codeanalyzer/schema/l1_body.py @@ -9,7 +9,17 @@ def _do_callable(source: str, c: PyCallable) -> None: 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 - c.body[key] = BodyNode(kind="call", span=span, callee=None) + c.body[key] = BodyNode( + kind="call", + span=span, + callee=None, + method_name=cs.method_name, + receiver_expr=cs.receiver_expr, + receiver_type=cs.receiver_type, + return_type=cs.return_type, + is_constructor_call=cs.is_constructor_call, + arguments=list(cs.arguments or []), + ) for ic in (c.callables or {}).values(): _do_callable(source, ic) for icl in (c.types or {}).values(): diff --git a/codeanalyzer/schema/py_schema.py b/codeanalyzer/schema/py_schema.py index 90e5a56..d1b61b4 100644 --- a/codeanalyzer/schema/py_schema.py +++ b/codeanalyzer/schema/py_schema.py @@ -132,6 +132,18 @@ class BodyNode(BaseModel): callee: Optional[str] = None # only on `call` nodes; the sanctioned null→id slot of: Optional[str] = None # param vertices: the variable/return they carry parent: Optional[str] = None # actuals: owning callsite ordinal id + # Call-site detail (#120). Previously reachable only through the parallel + # `PyCallable.call_sites` list, which emitted the same fact a second time under + # an unrelated id scheme. `method_name` and `is_constructor_call` are carried + # rather than derived from `callee`: measured across `requests` and `flask`, + # 20-28% of call sites never resolve a callee, so deriving them would lose them + # on one call in four. + method_name: Optional[str] = None + receiver_expr: Optional[str] = None + receiver_type: Optional[str] = None + return_type: Optional[str] = None + is_constructor_call: Optional[bool] = None + arguments: List["PyCallArgument"] = [] @builder @@ -334,6 +346,12 @@ class PyCallable(BaseModel): end_line: int = -1 code_start_line: int = -1 accessed_symbols: List[PySymbol] = [] + # Internal (#120): the Jedi-produced record that `l1_body` derives `body{}` + # call nodes from, and that `call_graph.py`, `l2_callees.py` and the dataflow + # builder all read. It is stripped at EMIT time (see `wire_json`), not with a + # field-level `exclude`: the analysis cache round-trips through the same + # serializer, so excluding it would drop it from the cache too and a warm-cache + # run would rebuild with no call sites at all. call_sites: List[PyCallsite] = [] callables: Dict[str, "PyCallable"] = {} # nested callables (closures) types: Dict[str, "PyClass"] = {} # nested (local) classes diff --git a/docs/design/specs/call-site-body-convergence.md b/docs/design/specs/call-site-body-convergence.md new file mode 100644 index 0000000..aaad7a5 --- /dev/null +++ b/docs/design/specs/call-site-body-convergence.md @@ -0,0 +1,172 @@ +# Spec: converge `call_sites[]` into `body{}` — separating the IR from the wire format + +Status: draft for review +Date: 2026-08-19 +Scope: `codeanalyzer-python` schema v2, `analysis.json` + Neo4j projection +Related: #120 (converge `call_sites[]`/`accessed_symbols[]`/`local_variables[]` with `body{}`) + +--- + +## 1. The finding + +Every call site is emitted **twice**, under two unrelated identity schemes. +Verified on `main` (`6f02581`), fixture `return cls()` at line 34: + +| representation | id | Neo4j | +| --- | --- | --- | +| `PyCallable.call_sites[]` | `app.py#34:15-34:22` | `:PyCallSite` ← `PY_HAS_CALLSITE` | +| `PyCallable.body{"34:15"}`, `kind:"call"` | `@34:15` | `:PyCFGNode` ← `PY_HAS_CFG_NODE` | + +One call in source, two graph nodes, no edge between them. `PyCallSite`'s id +(`file#line:col-line:col`) belongs to neither identity tier the schema defines — +it is neither a durable `can://` id nor a `@` ordinal id. + +**They cannot disagree in content.** `schema/l1_body.py` derives one from the other +in the same pass: + +```python +for cs in c.call_sites or []: + key = f"{cs.start_line}:{cs.start_column}" + c.body[key] = BodyNode(kind="call", span=span, callee=None) +``` + +So this is not a correctness bug. It is redundancy by construction. + +## 2. Why it exists + +`call_sites[]` is **not** v1 debris left lying around. It is the analyzer's internal +working record, and it is load-bearing for every level above L1: + +| reader | uses | +| --- | --- | +| `semantic_analysis/call_graph.py:163-215` | `callee_signature`, `method_name`, `is_constructor_call` — builds the L2 call graph | +| `schema/l2_callees.py` | `callee_signature`, `start_line`, `start_column` — backfills `BodyNode.callee` | +| `dataflow/builder.py:368-420` | `callee_signature`, `is_constructor_call`, position — builds SDG call sites | +| `dataflow/summaries.py`, `dataflow/sdg.py` | consume the above | +| `neo4j/project.py:400` | projects `:PyCallSite` | + +`body{}` is the v2 wire view *derived from* that record. The duplication is an +**internal IR leaking into the wire format** — not two competing encodings of equal +standing. That reframing is what makes the fix tractable: the wire format can lose +`call_sites[]` without the internal passes losing anything, provided the record +survives as an internal structure. + +## 3. Design + +**`body{}` is the single emitted representation of a call site.** Consumers obtain the +call-site set by filtering `body` on `kind == "call"`, which works from **L1** — verified: + +``` +L1 body{"34:15"} kind=call callee=null +L2 body{"34:15"} kind=call callee="can://…/@external/app.Account/__init__" +``` + +The Jedi-produced call record stays **internal**: it is the input to L2 resolution and +L3/L4 dataflow, and is not part of the contract. `PyCallsite` leaves the emitted schema. + +### Field disposition + +| field | disposition | why | +| --- | --- | --- | +| `start_line` / `start_column` / `end_*` | **becomes the node key + `span`** | already how `body{}` is keyed | +| `callee_signature` | **internal only** | it is the *input* to resolution; `callee` (a resolved `can://` id) is what the wire carries | +| `is_constructor_call` | **moves onto the call `BodyNode`** | see § 4a — not recoverable often enough to drop | +| `method_name` | **moves onto the call `BodyNode`** | see § 4a — not recoverable often enough to drop | +| `argument_types` | **deleted** | deprecated in 0.3.1 (#86) with "will be removed in schema v2"; no internal reader; removal overdue | +| `arguments` | **moves onto the call `BodyNode`** | no internal reader; output-only detail worth keeping. Encoding is OPEN — see § 5 | +| `receiver_expr` | **moves onto the call `BodyNode`** | no internal reader; Jedi inference with no other home | +| `receiver_type` | **moves onto the call `BodyNode`** | as above | +| `return_type` | **moves onto the call `BodyNode`** | as above | + +### 4a. Why `method_name` and `is_constructor_call` stay on the wire + +An earlier draft had both as "derivable from `callee`". Measurement killed that. Callee +resolution fails far too often for it to be a reliable base: + +| corpus | call body nodes | `callee` resolved | `callee` null | +| --- | --- | --- | --- | +| `requests` | 842 | 72.2% | **27.8%** | +| `flask` | 1129 | 80.2% | **19.8%** | + +Every one of those unresolved sites has `callee_signature is None` — Jedi genuinely failed on an +attribute call whose receiver type it cannot infer (`jar.extract_cookies`, `new_jar.clear`). The +gap is not a backfill bug: `l2_callees.py` propagates every resolution it is given, and zero sites +had a resolved signature that failed to reach its body node. + +Nor can the call graph close it. PyCG supplies 1337 of 1862 call-graph edges in `requests` (71.8%), +but a `call_graph` edge is `{src, dst, prov, weight}` — callable-to-callable, with no call-site +position — so it can never refine a specific `body{}` node. `body.callee` is bounded by Jedi's +call-site-level resolution. + +So dropping `method_name` and `is_constructor_call` would lose them on one call in four. +`is_constructor_call` was `True` for **zero** unresolved sites in both corpora, so it is the +cheaper of the two to reconsider later — but on this evidence both stay. + +### Resulting `BodyNode` + +```python +class BodyNode(BaseModel): + kind: str # statement | call | entry | exit | formal_* | actual_* + span: Optional[Span] = None + callee: Optional[str] = None # call nodes; the sanctioned null→id slot at L2 + of: Optional[str] = None + parent: Optional[str] = None + # call-specific, Jedi inference, absent on every other kind + receiver_expr: Optional[str] = None + receiver_type: Optional[str] = None + return_type: Optional[str] = None +``` + +### Neo4j consequences + +- `:PyCallSite`, `PY_HAS_CALLSITE`, and the `file#line:col-line:col` id scheme are **removed**. +- One body-node label, keyed on the global ordinal id, carries every `body{}` entry. +- `PY_RESOLVES_TO` is re-sourced from `BodyNode.callee` instead of `callee_signature`. +- Merge groups drop from 9 to 8. +- **Rename the body label.** `PyCFGNode` names an L3 concept, but a `call` node exists from + L1 and is deliberately **never** on the CFG spine — verified: at L3 the call `34:15` carries + `parent="34:8"` and appears in no `cfg` edge, while `@entry`/`34:8`/`@exit` do. The current + label asserts CFG membership for a node that has none. A level-neutral name (`PyBodyNode`, + matching what `body{}` is called in the JSON) states what is true at every level. +- If `MATCH (:PyCallSite)` ergonomics are wanted, the writer already supports label layering + (`rows.py:98` merges on `labels[0]` and unions the rest; `cypher.py:95-97` renders it), so a + marker label costs no second node and no second id. Note `MARKER_LABELS` in `neo4j/schema.py` + is declaration-only today — the writer never reads it, and no call site passes >1 label. + +## 4. What this does not do + +- **Does not touch `accessed_symbols[]` or `local_variables[]`**, and that is a decision, not an + omission. #120 groups all three, and `codeanalyzer-typescript` drops all three from its wire. + But only `call_sites[]` is *redundant*: `l1_body.py` derives `body{}` call nodes from it, so the + same fact is emitted twice. `BodyNode` carries `kind`/`span`/`callee`/`of`/`parent` and nothing + else — there is no representation of accessed symbols or local variables in `body{}` at any + level. Dropping those two would delete data rather than deduplicate it. Converging them means + first deciding what represents them, which is a separate design question. +- Does not change the call graph, Jedi resolution, or any dataflow analysis — only which + representation is serialized. +- Does not settle the Neo4j merge-label strategy, the `can://` callable-signature grammar, or + the decorator shape (#128). Those are separate decisions. + +## 5. Open questions + +- **`arguments` encoding.** Inline objects (`{ast_kind, inferred_type}`, what Python does now + and what works at L1) or local-ids referencing argument body nodes. The latter requires + materializing argument nodes at L1, which is a much larger change to the body model and the + id space. Recommendation: keep inline. +- **Body label name.** `PyBodyNode` is the obvious candidate; anything level-neutral works. +- **Whether the internal record stays a Pydantic model** excluded from serialization, or becomes + a plain dataclass in the analysis passes. Purely internal; no contract impact. + +## 6. Caveats and risks + +- **Breaking for anyone reading `call_sites[]`.** That is the point of the change, but it is the + most visible field in the callable model and its removal should lead the release notes. +- **Three fields become wire-recoverable rather than wire-present** (`method_name`, + `is_constructor_call`, `callee_signature`). Recovering `method_name` from a `span` slice is + string work a consumer may not want to do. If that proves unpopular the honest fix is to put + `method_name` back on the node, not to restore `call_sites[]`. +- **`callee` must actually resolve** for `is_constructor_call` and `method_name` to be + recoverable. Where resolution fails, `callee` is null and both are lost. The size of that + set is unmeasured and should be measured before the fields are dropped. +- **Test surface.** Every test asserting on `call_sites[]` changes. They should be rewritten + against `body{}`, not deleted. diff --git a/schema.neo4j.json b/schema.neo4j.json index aaf97d8..17893b2 100644 --- a/schema.neo4j.json +++ b/schema.neo4j.json @@ -101,27 +101,6 @@ "qualified_name": "string" } }, - { - "label": "PyCallSite", - "merge_label": "PyCallSite", - "key": "id", - "properties": { - "id": "string", - "method_name": "string", - "receiver_expr": "string", - "receiver_type": "string", - "argument_types": "string[]", - "arguments_json": "string", - "return_type": "string", - "callee_signature": "string", - "is_constructor_call": "boolean", - "start_line": "integer", - "start_column": "integer", - "end_line": "integer", - "end_column": "integer", - "_module": "string" - } - }, { "label": "PyAttribute", "merge_label": "PyAttribute", @@ -161,6 +140,12 @@ "kind": "string", "var": "string", "call_node": "string", + "method_name": "string", + "receiver_expr": "string", + "receiver_type": "string", + "return_type": "string", + "is_constructor_call": "boolean", + "arguments_json": "string", "start_line": "integer", "end_line": "integer", "_module": "string" @@ -222,20 +207,10 @@ ], "properties": {} }, - { - "type": "PY_HAS_CALLSITE", - "from": [ - "PyCallable" - ], - "to": [ - "PyCallSite" - ], - "properties": {} - }, { "type": "PY_RESOLVES_TO", "from": [ - "PyCallSite" + "PyCFGNode" ], "to": [ "PyCallable", @@ -386,7 +361,6 @@ "CREATE CONSTRAINT pysymbol_id IF NOT EXISTS FOR (x:PySymbol) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT pypackage_name IF NOT EXISTS FOR (x:PyPackage) REQUIRE x.name IS UNIQUE", "CREATE CONSTRAINT pydecorator_name IF NOT EXISTS FOR (x:PyDecorator) REQUIRE x.name IS UNIQUE", - "CREATE CONSTRAINT pycallsite_id IF NOT EXISTS FOR (x:PyCallSite) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT pyattribute_id IF NOT EXISTS FOR (x:PyAttribute) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT pyvariable_id IF NOT EXISTS FOR (x:PyVariable) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT pycfgnode_id IF NOT EXISTS FOR (x:PyCFGNode) REQUIRE x.id IS UNIQUE" diff --git a/test/sample_graph_app.py b/test/sample_graph_app.py index 99e39c1..f3c600a 100644 --- a/test/sample_graph_app.py +++ b/test/sample_graph_app.py @@ -38,6 +38,7 @@ from codeanalyzer.schema import PyApplication, PyExternalSymbol from codeanalyzer.schema.assign_ids import assign_ids from codeanalyzer.schema.l1_body import populate_l1_body +from codeanalyzer.schema.l2_callees import backfill_callees from codeanalyzer.schema.py_schema import PyCallEdge from codeanalyzer.semantic_analysis.call_graph import ( iter_callables_in_symbol_table, @@ -150,6 +151,10 @@ def make_sample_app() -> Tuple[PyApplication, Dict[str, str]]: } sig_to_id["os.getcwd"] = ext_id populate_l1_body(app) + # Mirror the real pipeline (core.py runs this at -a 2+): body `call` nodes get + # their resolved `callee`. Without it PY_RESOLVES_TO never fires, since #120 + # sources that edge from the body node rather than from `call_sites[]`. + backfill_callees(app, sig_to_id) syntactic_infos, _func_asts = build_function_pdgs( app, k=3, oracle_factory=lambda c, fast: SyntacticOracle() ) diff --git a/test/test_call_site_body_convergence.py b/test/test_call_site_body_convergence.py new file mode 100644 index 0000000..fd7c3be --- /dev/null +++ b/test/test_call_site_body_convergence.py @@ -0,0 +1,86 @@ +"""Call-site facts live on the body{} call node, not in a parallel list (#120). + +`l1_body.py` derived `body{}` call nodes from `call_sites[]`, so the analyzer emitted +the same fact twice under two unrelated id schemes. The list stays in memory — it is +the producer that L2 resolution and L3/L4 dataflow read — but it leaves the wire, and +the detail it carried moves onto the call node. +""" +import json + +from codeanalyzer.schema import strip_internal_only +from codeanalyzer.schema.l1_body import populate_l1_body +from codeanalyzer.schema.py_schema import ( + PyApplication, PyCallable, PyCallArgument, PyCallsite, PyModule, +) + +SRC = "def f():\n return g(1, x=2)\n" + + +def _app() -> tuple[PyApplication, PyCallable]: + fn = PyCallable(name="f", path="a.py", signature="a.f") + fn.call_sites.append( + PyCallsite( + method_name="g", + receiver_expr="obj", + receiver_type="Obj", + return_type="int", + is_constructor_call=False, + arguments=[PyCallArgument(ast_kind="Constant", inferred_type="int")], + start_line=2, start_column=11, end_line=2, end_column=22, + ) + ) + mod = PyModule(file_path="a.py", module_name="a", source=SRC, functions={"f": fn}) + return PyApplication(symbol_table={"a.py": mod}), fn + + +def test_call_detail_lands_on_the_body_node(): + app, fn = _app() + populate_l1_body(app) + (node,) = [n for n in fn.body.values() if n.kind == "call"] + assert node.method_name == "g" + assert node.receiver_expr == "obj" and node.receiver_type == "Obj" + assert node.return_type == "int" + assert node.is_constructor_call is False + assert [a.ast_kind for a in node.arguments] == ["Constant"] + + +def test_call_sites_stays_in_memory_for_the_internal_passes(): + """L2 resolution and L3/L4 dataflow read this list; it must not be emptied.""" + app, fn = _app() + populate_l1_body(app) + assert len(fn.call_sites) == 1 + assert fn.call_sites[0].method_name == "g" + + +def test_call_sites_is_stripped_at_emit_but_kept_when_serialized(): + """Both halves of the mechanism. + + Stripping happens at emit time, NOT via a field-level Pydantic `exclude`, + because the analysis cache round-trips through the same serializer. Excluding + at the field would drop `call_sites` from the cache too, and the next + warm-cache run would rebuild from a payload with no call sites at all — + silently losing the producer that `l1_body`, `l2_callees`, the call graph and + the dataflow builder all read. + """ + app, fn = _app() + populate_l1_body(app) + dumped = app.model_dump(mode="json") + + # Serialized form keeps it — this is what the cache persists. + assert "call_sites" in dumped["symbol_table"]["a.py"]["functions"]["f"] + + # Emitted form drops it — this is what analysis.json carries. + emitted = strip_internal_only(dumped) + callable_json = emitted["symbol_table"]["a.py"]["functions"]["f"] + assert "call_sites" not in callable_json + (node,) = [n for n in callable_json["body"].values() if n["kind"] == "call"] + assert node["method_name"] == "g" + + +def test_accessed_symbols_and_local_variables_are_untouched(): + """Only call_sites is redundant; these have no body{} representation to converge into.""" + app, fn = _app() + populate_l1_body(app) + callable_json = strip_internal_only(app.model_dump(mode="json"))["symbol_table"]["a.py"]["functions"]["f"] + assert "accessed_symbols" in callable_json + assert "local_variables" in callable_json diff --git a/test/test_v2_l2.py b/test/test_v2_l2.py index cf7e206..eaa6154 100644 --- a/test/test_v2_l2.py +++ b/test/test_v2_l2.py @@ -77,8 +77,11 @@ def walk_type(t): for t in (mod.get("types") or {}).values(): walk_type(t) def walk_callable(c): - for cs in c.get("call_sites") or []: - sites.append(cs) + # #120: call sites are `body{}` entries with kind == "call"; the + # parallel `call_sites[]` list no longer reaches the wire. + for node in (c.get("body") or {}).values(): + if node.get("kind") == "call": + sites.append(node) for ic in (c.get("callables") or {}).values(): walk_callable(ic) for fn in (mod.get("functions") or {}).values(): @@ -87,26 +90,34 @@ def walk_callable(c): for m in (t.get("callables") or {}).values(): walk_callable(m) + def _callee_name(callee: str) -> str: + """The invoked name from a resolved callee id. + + `can://…/Type/name(args)` -> `name`; an `@external` home ends in a bare + name with no parameter list, which this handles too. + """ + return callee.rsplit("/", 1)[-1].split("(", 1)[0] + resolved = [ cs for cs in sites - if cs.get("callee_signature") and not cs.get("is_constructor_call") + if cs.get("callee") and not cs.get("is_constructor_call") ] assert resolved, "fixture must produce resolved non-constructor callsites" agree = sum( 1 for cs in resolved - if cs["callee_signature"].rsplit(".", 1)[-1] == cs["method_name"] + if _callee_name(cs["callee"]) == cs["method_name"] ) ratio = agree / len(resolved) assert ratio >= 0.95, ( f"only {agree}/{len(resolved)} resolved callsites bind to the invoked " - f"name: {[(cs['method_name'], cs['callee_signature']) for cs in resolved if cs['callee_signature'].rsplit('.', 1)[-1] != cs['method_name']]}" + f"name: {[(cs['method_name'], cs['callee']) for cs in resolved if _callee_name(cs['callee']) != cs['method_name']]}" ) # no class fallback when the exact method exists: a callee binding that # names a class which declares the invoked method is the defect's signature for cs in resolved: for cls_id, methods in class_methods.items(): cls_sig_name = cls_id.rsplit("/", 1)[-1] - if cs["callee_signature"].rsplit(".", 1)[-1] == cls_sig_name and cs["method_name"] in methods: + if _callee_name(cs["callee"]) == cls_sig_name.split("(", 1)[0] and cs["method_name"] in methods: raise AssertionError( f"callsite {cs['method_name']!r} fell back to class {cls_sig_name!r} " f"which declares the exact method" diff --git a/test/test_v2_two_projection_agreement.py b/test/test_v2_two_projection_agreement.py index 332e35c..e660906 100644 --- a/test/test_v2_two_projection_agreement.py +++ b/test/test_v2_two_projection_agreement.py @@ -6,6 +6,7 @@ from codeanalyzer.dataflow.builder import build_function_pdgs, emit_l3_body from codeanalyzer.dataflow.syntactic import SyntacticOracle from codeanalyzer.schema.l1_body import populate_l1_body +from codeanalyzer.schema.l2_callees import backfill_callees from codeanalyzer.syntactic_analysis.symbol_table_builder import SymbolTableBuilder @@ -31,6 +32,11 @@ def test_py_resolves_to_edge_targets_declared_callee_by_can_id(): functions={"f": caller, "g": callee}) app = PyApplication(symbol_table={"m.py": mod}) sig_to_id = assign_ids(app, "myapp") + # #120: PY_RESOLVES_TO now sources from the `body{}` call node's resolved + # `callee`, not from `call_sites[].callee_signature`. Drive the same pipeline + # the analyzer runs so the fixture has a body node to project from. + populate_l1_body(app) + backfill_callees(app, sig_to_id) rows = project(app, "myapp", sig_to_id) resolves = [e for e in rows.edges if e.type == "PY_RESOLVES_TO"] # the callsite must resolve to g's can:// id — edge kept, not dropped