mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-28 01:06:36 +00:00
fix(extract): affected traverses in-function dynamic imports; Python member-call gating; ObjC resolver arm; bump to 0.9.39 (#2584, #2586, #2589, #2591)
These land together because they are interleaved in extract.py/engine.py. #2584 (PR #2588, thanks @phudayyy): the 0.9.38 dynamic-import dedupe keyed only on target, so an in-function import() suppressed the file-level edge affected follows. Dedupe now keys on the importing file, emitting one file-level dynamic_import edge per file/target while keeping the call-site edge. #2586 / #2417 (PR #2586, thanks @EZZEASY): a Python member call on an untyped receiver (x.get(...)) no longer binds by name to a same-named module function. walk_calls now defers non-self/cls/super Python member calls to the evidence-gated resolver; super().method() still resolves. Known trade: same-file x = Thing(); x.method() loses its evidence-free edge (precision over recall, per #2553). #2589/#2591 resolver arm (in _resolve_objc_member_calls): the @protocol exclusion and the self.field/_ivar receiver resolution (paired with the objc.py extractor changes committed separately). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
ba8254b3ab
commit
50556baaea
+9
-1
@@ -2,7 +2,15 @@
|
||||
|
||||
Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases)
|
||||
|
||||
## 0.9.38 (unreleased)
|
||||
## 0.9.39 (unreleased)
|
||||
|
||||
- Fix: `affected` now traverses a dynamic `import('…')` made inside a function or at module scope (#2584, thanks @phudayyy). The 0.9.38 dedupe keyed only on the target, so an in-function dynamic import (whose symbol-level edge is anchored on the enclosing function) suppressed the file-level edge `affected` follows; the dedupe now keys on the importing file, emitting one file-level `dynamic_import` edge per file/target while keeping the call-site edge.
|
||||
- Fix: a Python member call on an untyped receiver (`x.get(...)`) no longer binds by name alone to a same-named module-level function, fabricating a false high-confidence `calls` edge and a god node (#2417, #2586, thanks @EZZEASY). Such a call is now resolved only with receiver-type, import, or `self`/`cls`/`super` evidence, matching the TypeScript fix from 0.9.37; `super().method()` still resolves.
|
||||
- Fix: fuzzy dedup no longer over-merges two distinct entities in the same file whose long labels differ by a content word (#2576, thanks @wilyan09007). A one-token difference is judged on the differing tokens rather than the prefix-weighted whole-label similarity, so `asset contribution flow` and `asset consumption flow` stay separate while genuine typo and whitespace/case variants still collapse.
|
||||
- Fix: `graphify watch` now rebuilds on a documentation-only deletion batch instead of only flagging it (#2580, thanks @angmeng), so a deleted doc's nodes are evicted immediately rather than waiting for the next code-file event. (The general deleted-file leak was already fixed in 0.9.10; this closes the live-watcher residual.)
|
||||
- Fix: Objective-C member-call resolution (#2589, #2590, #2591, thanks @xiongjianxu). A `@protocol` declaration is no longer treated as a receiver type (it collided with a same-named class); a category or class-extension interface (`@interface Foo (Bar)`) now folds into the base class instead of minting a duplicate node; and a message send to a `@property` or ivar receiver (`[self.svc run]`, `[_svc run]`) now resolves through the property/ivar's declared type.
|
||||
|
||||
## 0.9.38 (2026-08-09)
|
||||
|
||||
- Fix: the 0.9.37 callback-body fix (#2552) no longer lets a local declared in one callback suppress a call in a sibling callback (#2568, thanks @imagineers-tyler). Each callback body's local names are now scoped to that body instead of unioned under the shared declaration, so a real `indirect_call` in one sibling closure is no longer dropped because another sibling declared a same-named local. This can only restore dropped edges, never fabricate.
|
||||
- Fix: Kotlin calls in a property initializer are now collected (#2565, thanks @kskchaitanya1993). A class property (`val repo = createRepo()`), a `by lazy { ... }` delegate, a companion-object property, and a top-level property initializer now produce `calls` edges attributed to the enclosing class (or file), including fully-qualified calls. A plain literal initializer produces no edge.
|
||||
|
||||
+97
-4
@@ -1252,7 +1252,10 @@ def _rescue_js_dynamic_imports(path: Path, result: dict) -> None:
|
||||
as an ``imports_from`` edge marked ``deferred`` (``_dynamic_import_js``).
|
||||
Re-emitting it here as a second ``dynamic_import`` edge would state the
|
||||
same fact twice, so a match whose resolved target already has a deferred
|
||||
edge is skipped.
|
||||
edge FROM THIS FILE'S NODE is skipped. The source check matters: the AST
|
||||
pass anchors the edge on the enclosing function when the ``import()`` is
|
||||
written inside one, and that is a different fact from "this file depends on
|
||||
that module" — the only one file-level traversal can use (#2584).
|
||||
|
||||
Regex false positives in comments/strings are the precedented trade of
|
||||
the Svelte/Vue rescues; a ``//``-prefix guard covers the common case.
|
||||
@@ -1268,8 +1271,28 @@ def _rescue_js_dynamic_imports(path: Path, result: dict) -> None:
|
||||
base_url = _load_tsconfig_base_url(path.parent)
|
||||
deferred_ids: set[str] = set()
|
||||
deferred_files: set[str] = set()
|
||||
rescued_targets: set[str] = set()
|
||||
for e in result.get("edges", []):
|
||||
if e.get("deferred") and e.get("relation") == "imports_from":
|
||||
# Only a FILE-level deferred edge makes the rescue redundant (#2584).
|
||||
#
|
||||
# `_dynamic_import_js` emits `caller_nid -> target`, and `caller_nid` is this
|
||||
# file's node only when the `import()` sits at module scope. Written inside a
|
||||
# function it is that function's node — a different fact, at a granularity
|
||||
# `affected` does not walk. Matching on target alone treated the two as one and
|
||||
# skipped the rescue, so a dynamic import inside a function ended up with no
|
||||
# file-level edge at all. The reverse walk then reached the enclosing function
|
||||
# and stopped: the only edge pointing at it is `contains`, deliberately kept out
|
||||
# of DEFAULT_AFFECTED_RELATIONS.
|
||||
#
|
||||
# Measured on a ~700-file TS repo: `affected --depth 3` returned 39 of 49 truly
|
||||
# affected files (recall 0.80, precision 1.00) and deeper traversal did not help,
|
||||
# which is a dead end rather than a depth limit. It stayed hidden because the
|
||||
# usual case still resolves — when the next importer imports that exact symbol
|
||||
# by name there IS an edge into the function. Switch that importer to
|
||||
# `import * as ns` or a side-effect `import './dyn'` and the same graph goes
|
||||
# silent.
|
||||
if (e.get("deferred") and e.get("relation") == "imports_from"
|
||||
and e.get("source") == file_node_id):
|
||||
deferred_ids.add(e.get("target"))
|
||||
tf = e.get("target_file")
|
||||
if tf:
|
||||
@@ -1305,6 +1328,16 @@ def _rescue_js_dynamic_imports(path: Path, result: dict) -> None:
|
||||
continue
|
||||
except OSError:
|
||||
pass
|
||||
# One file depending on one module is one file-level fact, however many
|
||||
# call sites defer it. Pre-existing (two module-scope `import('./x')` in one
|
||||
# file already emitted two identical edges on v8), but #2584 routes every
|
||||
# in-function dynamic import through here too, which would turn an edge case
|
||||
# into the common one — a hub module deferred from eight functions of the same
|
||||
# file would carry eight identical arrows.
|
||||
emit_key = str(resolved_file.resolve()) if resolved_file is not None else raw
|
||||
if emit_key in rescued_targets:
|
||||
continue
|
||||
rescued_targets.add(emit_key)
|
||||
_emit_rescued_import(
|
||||
result, existing_ids, file_node_id, path, raw,
|
||||
"dynamic_import", aliases, base_url,
|
||||
@@ -3296,9 +3329,18 @@ def _resolve_objc_member_calls(
|
||||
* ``self`` / ``super`` — the caller's own enclosing class -> EXTRACTED.
|
||||
* Capitalized receiver (``[Foo new]``) — the type named explicitly -> EXTRACTED.
|
||||
* ``[f doThing]`` — ``f`` typed via the file's ``Foo *f`` local table -> INFERRED.
|
||||
* ``[self.bar doIt]`` / ``[_ivarBar doIt]`` — the field typed via the class's
|
||||
``@property``/ivar table (locals shadow fields for the bare-identifier
|
||||
form) -> INFERRED. Only the exact ``self.<field>`` receiver shape is
|
||||
captured; a dotted receiver like ``Foo.shared`` is never passed through,
|
||||
because ``_key`` would strip the dot and collide with a real ``FooShared``.
|
||||
An uninferable receiver is SKIPPED (no guess), so an ambiguous selector across
|
||||
classes never fans out. ``_merge_decl_def_classes`` folds each @interface/@impl
|
||||
pair into one node, so a paired class clears the single-definition guard.
|
||||
``@protocol`` declarations are excluded from the receiver-type index: a protocol
|
||||
is a contract, not a message receiver, and ObjC keeps protocol and class names in
|
||||
separate namespaces, so a same-named pair used to both mis-bind a message to the
|
||||
protocol's declaration and, when a real class existed, trip the god-node guard.
|
||||
|
||||
Must run after id-disambiguation so node ids and caller_nids are final.
|
||||
"""
|
||||
@@ -3308,16 +3350,49 @@ def _resolve_objc_member_calls(
|
||||
if tt and tt.get("path"):
|
||||
type_table_by_file[tt["path"]] = tt.get("table", {})
|
||||
|
||||
# #1556: cross-file `field -> ClassName` tables merged per class nid (the
|
||||
# .h/.m pair share one id, preserved by _merge_decl_def_classes, so the header's
|
||||
# @property entries and the impl's ivar entries land in one table). A cross-file
|
||||
# conflict on the same (class, field) drops the entry — no guess.
|
||||
field_types_by_class: dict[str, dict[str, str]] = {}
|
||||
field_conflicts: set[tuple[str, str]] = set()
|
||||
for result in per_file:
|
||||
ft = result.get("objc_field_types")
|
||||
if not ft:
|
||||
continue
|
||||
for cls_nid, tbl in (ft.get("tables") or {}).items():
|
||||
merged = field_types_by_class.setdefault(cls_nid, {})
|
||||
for field, tname in tbl.items():
|
||||
if (cls_nid, field) in field_conflicts:
|
||||
continue
|
||||
prev = merged.get(field)
|
||||
if prev is None:
|
||||
merged[field] = tname
|
||||
elif prev != tname:
|
||||
del merged[field]
|
||||
field_conflicts.add((cls_nid, field))
|
||||
|
||||
def _key(label: str) -> str:
|
||||
return re.sub(r"[^a-zA-Z0-9]+", "", str(label)).lower()
|
||||
|
||||
contained = {e.get("target") for e in all_edges if e.get("relation") == "contains"}
|
||||
|
||||
def _is_protocol_declaration(n: dict) -> bool:
|
||||
"""A ``@protocol`` declaration, which the ObjC extractor labels ``<Name>``.
|
||||
|
||||
A protocol is a contract, never a message receiver, so it must not be a
|
||||
receiver-typing candidate. It stays a valid target for `implements`; only
|
||||
this pass's type index excludes it.
|
||||
"""
|
||||
label = str(n.get("label", "")).strip()
|
||||
return label.startswith("<") and label.endswith(">")
|
||||
|
||||
type_def_nids: dict[str, list[str]] = {}
|
||||
node_by_id: dict[str, dict] = {}
|
||||
for n in all_nodes:
|
||||
node_by_id[n.get("id")] = n
|
||||
if n.get("source_file") and n.get("id") in contained and _is_type_like_definition(n):
|
||||
if (n.get("source_file") and n.get("id") in contained
|
||||
and _is_type_like_definition(n) and not _is_protocol_declaration(n)):
|
||||
type_def_nids.setdefault(_key(n.get("label", "")), []).append(n["id"])
|
||||
|
||||
method_index: dict[tuple[str, str], str] = {}
|
||||
@@ -3349,7 +3424,20 @@ def _resolve_objc_member_calls(
|
||||
src_file = rc.get("source_file", "")
|
||||
if rc.get("lang") != "objc":
|
||||
continue
|
||||
if receiver in ("self", "super"):
|
||||
if rc.get("receiver_kind") == "self_field":
|
||||
# `[self.bar doIt]`: the extractor stamped the BARE field name; type it
|
||||
# via the caller's own class's @property/ivar table. Checked before the
|
||||
# capitalized arm so a capitalized field never reads as a class name.
|
||||
cls = enclosing_type.get(caller)
|
||||
type_name = field_types_by_class.get(cls, {}).get(receiver) if cls else None
|
||||
if not type_name:
|
||||
continue
|
||||
type_defs = type_def_nids.get(_key(type_name), [])
|
||||
if len(type_defs) != 1: # ambiguous or absent -> bail (god-node guard)
|
||||
continue
|
||||
type_nid = type_defs[0]
|
||||
type_qualified = False
|
||||
elif receiver in ("self", "super"):
|
||||
type_nid = enclosing_type.get(caller)
|
||||
if not type_nid:
|
||||
continue
|
||||
@@ -3361,7 +3449,12 @@ def _resolve_objc_member_calls(
|
||||
type_nid = type_defs[0]
|
||||
type_qualified = True
|
||||
else:
|
||||
# Locals shadow fields: the file's `Foo *f` local table first, then the
|
||||
# enclosing class's @property/ivar table (covers `[_ivarBar doIt]`).
|
||||
type_name = type_table_by_file.get(src_file, {}).get(receiver)
|
||||
if not type_name:
|
||||
cls = enclosing_type.get(caller)
|
||||
type_name = field_types_by_class.get(cls, {}).get(receiver) if cls else None
|
||||
if not type_name:
|
||||
continue
|
||||
type_defs = type_def_nids.get(_key(type_name), [])
|
||||
|
||||
@@ -4778,6 +4778,21 @@ def _extract_generic(
|
||||
obj = func_node.child_by_field_name(config.call_accessor_object_field)
|
||||
if obj is not None and obj.type == "identifier":
|
||||
member_receiver = _read_text(obj, source)
|
||||
elif (
|
||||
config.ts_module == "tree_sitter_python"
|
||||
and obj is not None
|
||||
and obj.type == "call"
|
||||
):
|
||||
# ``super().method()`` has a call node as its
|
||||
# receiver. Preserve it as a known intra-class
|
||||
# receiver instead of treating it as unresolved.
|
||||
receiver_func = obj.child_by_field_name("function")
|
||||
if (
|
||||
receiver_func is not None
|
||||
and receiver_func.type == "identifier"
|
||||
and _read_text(receiver_func, source) == "super"
|
||||
):
|
||||
member_receiver = "super"
|
||||
elif (obj is not None
|
||||
and obj.type in config.call_accessor_node_types
|
||||
and config.call_accessor_object_field):
|
||||
@@ -4792,12 +4807,17 @@ def _extract_generic(
|
||||
callee_name = _read_text(func_node, source)
|
||||
|
||||
if callee_name and callee_name not in _LANGUAGE_BUILTIN_GLOBALS:
|
||||
# A capitalized-receiver member call (`ClassName.method()`) must defer
|
||||
# to receiver-based cross-file resolution: the bare method name can
|
||||
# collide with an in-file node — even the calling method itself, when a
|
||||
# viewset action delegates to a same-named service action — which would
|
||||
# match `tgt_nid == caller_nid` and silently drop the call (#1446). The
|
||||
# captured receiver is resolved later in _resolve_python_member_calls.
|
||||
# Python member calls defer to receiver-based resolution unless the
|
||||
# receiver is known to stay in the current class. Falling back to a
|
||||
# bare method name for an unresolved/lowercase receiver (`d.get()` or
|
||||
# `self.store.get()`) can bind to an unrelated module function and
|
||||
# inflate it into a god node (#2417). Qualified class/module calls are
|
||||
# recovered later by _resolve_python_member_calls when the receiver
|
||||
# supplies enough evidence (#1446/#1883). Known recall trade (#2586):
|
||||
# a same-file `x = Thing(); x.method()` no longer gets an edge — it
|
||||
# came from the same evidence-free bare-name map and could bind wrong
|
||||
# under label collision; local-instantiation receiver typing is a
|
||||
# separate follow-up.
|
||||
# C#: ANY member call with a captured receiver defers to the
|
||||
# receiver-typed resolver — a bare method-name match ignores the
|
||||
# receiver's declared type and mis-binds to an unrelated same-named
|
||||
@@ -4807,10 +4827,15 @@ def _extract_generic(
|
||||
config.ts_module == "tree_sitter_c_sharp"
|
||||
and is_member_call and member_receiver
|
||||
)
|
||||
_python_defer = (
|
||||
config.ts_module == "tree_sitter_python"
|
||||
and is_member_call
|
||||
and member_receiver not in {"self", "cls", "super"}
|
||||
)
|
||||
_java_defer = (
|
||||
config.ts_module == "tree_sitter_java" and is_member_call
|
||||
)
|
||||
if _java_defer or (
|
||||
if _python_defer or _java_defer or (
|
||||
is_member_call
|
||||
and member_receiver
|
||||
and (
|
||||
|
||||
+1
-1
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
|
||||
|
||||
[project]
|
||||
name = "graphifyy"
|
||||
version = "0.9.38"
|
||||
version = "0.9.39"
|
||||
description = "AI coding assistant skill (Claude Code, CodeBuddy, Codex, OpenCode, Kilo Code, Cursor, Gemini CLI, Aider, OpenClaw, Factory Droid, Trae, Hermes, Kiro, Pi, Devin CLI, Google Antigravity) - turn any folder of code, docs, papers, images, or videos into a queryable knowledge graph"
|
||||
readme = "README.md"
|
||||
license = "Apache-2.0"
|
||||
|
||||
+126
-3
@@ -1136,13 +1136,15 @@ def test_python_relative_import_out_of_root_target_id_is_portable(tmp_path):
|
||||
|
||||
def test_python_module_qualified_call_resolves_extracted(tmp_path):
|
||||
"""`module.func()` where `module` is imported resolves to the callable that
|
||||
module contains, with an EXTRACTED `calls` edge (#1883). A lowercase module
|
||||
receiver was previously dropped alongside instance calls."""
|
||||
module contains, with an EXTRACTED `calls` edge (#1883), even when the caller
|
||||
file contains a same-named function that bare-name lookup could select."""
|
||||
mathlib = tmp_path / "mathlib.py"
|
||||
caller = tmp_path / "caller.py"
|
||||
mathlib.write_text("def compute(x):\n return x * 2\n")
|
||||
caller.write_text(
|
||||
"import mathlib\n\n"
|
||||
"def compute(x):\n"
|
||||
" return x + 1\n\n"
|
||||
"def use_qualified(n):\n"
|
||||
" return mathlib.compute(n)\n"
|
||||
)
|
||||
@@ -1153,9 +1155,9 @@ def test_python_module_qualified_call_resolves_extracted(tmp_path):
|
||||
if e["relation"] == "calls"
|
||||
and "use_qualified" in nodes[e["source"]]["label"]
|
||||
and "compute" in nodes[e["target"]]["label"]
|
||||
and "mathlib.py" in (nodes[e["target"]].get("source_file") or "")
|
||||
]
|
||||
assert len(edges) == 1, f"expected one use_qualified->compute edge, got {edges}"
|
||||
assert "mathlib.py" in (nodes[edges[0]["target"]].get("source_file") or "")
|
||||
assert edges[0]["confidence"] == "EXTRACTED"
|
||||
|
||||
|
||||
@@ -1464,6 +1466,127 @@ def test_python_instance_member_call_not_overconnected(tmp_path):
|
||||
assert bad == [], f"instance member call must not connect cross-file: {bad}"
|
||||
|
||||
|
||||
def test_python_unresolved_member_calls_do_not_bind_to_bare_function(tmp_path):
|
||||
"""#2417: unresolved attribute calls must not bind by bare method name.
|
||||
|
||||
``d.get()`` and ``self.store.get()`` do not identify the module-level
|
||||
``get()`` definition, so only the direct ``get()`` call is a real edge.
|
||||
The result must also survive a warm cache extraction.
|
||||
"""
|
||||
fixture = tmp_path / "fixture.py"
|
||||
fixture.write_text(
|
||||
"def get(k):\n"
|
||||
" return k\n\n"
|
||||
"class Store:\n"
|
||||
" def __init__(self):\n"
|
||||
" self.store = {}\n\n"
|
||||
" def read(self, k):\n"
|
||||
" return self.store.get(k)\n\n"
|
||||
"def other(d):\n"
|
||||
" return d.get('x')\n\n"
|
||||
"def real():\n"
|
||||
" return get('real')\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
def _get_callers(result):
|
||||
nodes = {node["id"]: node for node in result["nodes"]}
|
||||
target_ids = {
|
||||
node["id"] for node in result["nodes"]
|
||||
if node.get("label") == "get()" and node.get("source_file")
|
||||
}
|
||||
return sorted(
|
||||
nodes[edge["source"]]["label"]
|
||||
for edge in result["edges"]
|
||||
if edge.get("relation") == "calls" and edge.get("target") in target_ids
|
||||
)
|
||||
|
||||
cold = extract([fixture], cache_root=tmp_path, root=tmp_path)
|
||||
assert _get_callers(cold) == ["real()"]
|
||||
warm = extract([fixture], cache_root=tmp_path, root=tmp_path)
|
||||
assert _get_callers(warm) == ["real()"]
|
||||
|
||||
|
||||
def test_python_known_member_receivers_keep_local_call_edges(tmp_path):
|
||||
"""Preserve self/cls/super calls while deferring other call receivers."""
|
||||
fixture = tmp_path / "known_receivers.py"
|
||||
fixture.write_text(
|
||||
"class Base:\n"
|
||||
" def inherited(self):\n"
|
||||
" return 1\n\n"
|
||||
"class Worker(Base):\n"
|
||||
" def local(self):\n"
|
||||
" return 2\n\n"
|
||||
" @classmethod\n"
|
||||
" def class_local(cls):\n"
|
||||
" return 3\n\n"
|
||||
" def via_self(self):\n"
|
||||
" return self.local()\n\n"
|
||||
" @classmethod\n"
|
||||
" def via_cls(cls):\n"
|
||||
" return cls.class_local()\n\n"
|
||||
" def via_super(self):\n"
|
||||
" return super().inherited()\n\n"
|
||||
"def via_factory(factory):\n"
|
||||
" return factory().local()\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract([fixture], cache_root=tmp_path, root=tmp_path)
|
||||
nodes = {node["id"]: node for node in result["nodes"]}
|
||||
call_pairs = {
|
||||
(nodes[edge["source"]]["label"], nodes[edge["target"]]["label"])
|
||||
for edge in result["edges"]
|
||||
if edge.get("relation") == "calls"
|
||||
}
|
||||
|
||||
for caller, callee in (
|
||||
("via_self", "local"),
|
||||
("via_cls", "class_local"),
|
||||
("via_super", "inherited"),
|
||||
):
|
||||
assert any(
|
||||
caller in source_label and callee in target_label
|
||||
for source_label, target_label in call_pairs
|
||||
), f"missing {caller} -> {callee} call edge: {call_pairs}"
|
||||
assert not any(
|
||||
"via_factory" in source_label and "local" in target_label
|
||||
for source_label, target_label in call_pairs
|
||||
), f"unresolved factory() receiver must not bind by bare name: {call_pairs}"
|
||||
|
||||
|
||||
def test_python_unresolved_receiver_never_crosses_modules(tmp_path):
|
||||
"""#2417 cross-file guard: `client.fetch('x')` must not bind to `util.fetch`
|
||||
just because the caller's file imports that name — the receiver `client`
|
||||
supplies no evidence it is the `util` module. A plain `fetch('y')` call to
|
||||
the imported name still resolves."""
|
||||
util = tmp_path / "util.py"
|
||||
caller = tmp_path / "app.py"
|
||||
util.write_text("def fetch(url):\n return url\n")
|
||||
caller.write_text(
|
||||
"from util import fetch\n\n"
|
||||
"def via_receiver(client):\n"
|
||||
" return client.fetch('x')\n\n"
|
||||
"def via_name():\n"
|
||||
" return fetch('y')\n"
|
||||
)
|
||||
result = extract([caller, util], cache_root=tmp_path)
|
||||
nodes = {n["id"]: n for n in result["nodes"]}
|
||||
fetch_edges = [
|
||||
(nodes[e["source"]]["label"], e)
|
||||
for e in result["edges"]
|
||||
if e["relation"] == "calls"
|
||||
and "fetch" in nodes[e["target"]]["label"]
|
||||
and "util.py" in (nodes[e["target"]].get("source_file") or "")
|
||||
]
|
||||
callers = sorted(label for label, _ in fetch_edges)
|
||||
assert not any("via_receiver" in c for c in callers), (
|
||||
f"unresolved receiver bound cross-module by bare name: {callers}"
|
||||
)
|
||||
assert any("via_name" in c for c in callers), (
|
||||
f"imported-name call lost its edge: {callers}"
|
||||
)
|
||||
|
||||
|
||||
def test_python_qualified_call_ambiguous_class_bails(tmp_path):
|
||||
"""When the class name is defined in 2+ files, the qualified call must not
|
||||
resolve — single-definition god-node guard (#1446)."""
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
"""`affected` must traverse a dynamic `import('…')` written inside a function — #2584.
|
||||
|
||||
The edge was already emitted (#2575), so an edge-existence assertion passes while the
|
||||
answer users actually read is short. The reason is a granularity mismatch: a static import
|
||||
emits ``file -> target``, but ``_dynamic_import_js`` emitted ``caller_nid -> target``, and
|
||||
``caller_nid`` is the file node ONLY at module level. Written inside a function it is that
|
||||
function's node, so the graph held ``load() --imports_from--> target`` with no file-level
|
||||
edge, and the reverse walk stopped at ``load()`` — the one edge pointing at it is
|
||||
``contains``, which is deliberately not in DEFAULT_AFFECTED_RELATIONS.
|
||||
|
||||
Measured on a ~700-file TS repo: recall 0.80 at depth 3, precision 1.00, and raising the
|
||||
depth did not help. It stayed hidden because the common case works — if the next importer
|
||||
imports that exact symbol by name, there IS an edge into ``load()``. So the tests below
|
||||
pin the cases where nothing points at the enclosing symbol: a namespace import and a
|
||||
side-effect import. Those are the ones that were silent.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import networkx as nx
|
||||
|
||||
from graphify.affected import affected_nodes
|
||||
from graphify.extract import _file_node_id, extract
|
||||
|
||||
|
||||
def _write(path: Path, text: str) -> Path:
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(text, encoding="utf-8")
|
||||
return path
|
||||
|
||||
|
||||
TARGET = "export const value = 1\n"
|
||||
|
||||
# The dynamic import lives INSIDE a function — the case that regressed.
|
||||
DYN_IN_FUNCTION = (
|
||||
"export async function load() {\n"
|
||||
" const m = await import('./target')\n"
|
||||
" return m.value\n"
|
||||
"}\n"
|
||||
"export const other = 2\n"
|
||||
)
|
||||
|
||||
TOP = "import { run } from './mid'\nexport const go = () => run()\n"
|
||||
|
||||
|
||||
def _build(tmp_path: Path, mid_src: str, dyn_src: str = DYN_IN_FUNCTION):
|
||||
files = [
|
||||
_write(tmp_path / "src/target.ts", TARGET),
|
||||
_write(tmp_path / "src/dyn.ts", dyn_src),
|
||||
_write(tmp_path / "src/mid.ts", mid_src),
|
||||
_write(tmp_path / "src/top.ts", TOP),
|
||||
]
|
||||
result = extract(files, cache_root=tmp_path, root=tmp_path)
|
||||
graph = nx.DiGraph()
|
||||
for n in result["nodes"]:
|
||||
graph.add_node(n["id"], **n)
|
||||
for e in result["edges"]:
|
||||
graph.add_edge(e["source"], e["target"], **e)
|
||||
return result, graph
|
||||
|
||||
|
||||
def _fid(rel: str) -> str:
|
||||
return _file_node_id(Path(rel))
|
||||
|
||||
|
||||
def _reaches(graph: nx.DiGraph, seed: str, wanted: str, depth: int = 3) -> bool:
|
||||
hits = affected_nodes(graph, _fid(seed), depth=depth)
|
||||
return _fid(wanted) in {h.node_id for h in hits}
|
||||
|
||||
|
||||
def _file_edges(result: dict, source: str, target: str) -> list[dict]:
|
||||
return [
|
||||
e for e in result["edges"]
|
||||
if e["source"] == _fid(source) and e["target"] == _fid(target)
|
||||
]
|
||||
|
||||
|
||||
def test_dynamic_import_in_function_emits_a_file_level_edge(tmp_path: Path):
|
||||
"""The file owning the `import()` depends on the target, whoever wrote the call."""
|
||||
result, _ = _build(tmp_path, "import './dyn'\nexport const run = () => 1\n")
|
||||
|
||||
edges = _file_edges(result, "src/dyn.ts", "src/target.ts")
|
||||
assert edges, "no file-level edge for a dynamic import written inside a function"
|
||||
# `dynamic_import`, not `imports_from`: it keeps the deferred nature legible, it is
|
||||
# already in DEFAULT_AFFECTED_RELATIONS, and find_import_cycles reads only
|
||||
# `imports_from`/`re_exports` — so the phantom file cycle of #1241 cannot come back
|
||||
# through this edge the way a second `imports_from` might.
|
||||
assert all(e["relation"] == "dynamic_import" for e in edges)
|
||||
|
||||
|
||||
def test_affected_reaches_through_a_side_effect_importer(tmp_path: Path):
|
||||
"""`import './dyn'` binds no symbol, so nothing points at the enclosing function."""
|
||||
_, graph = _build(tmp_path, "import './dyn'\nexport const run = () => 1\n")
|
||||
|
||||
assert _reaches(graph, "src/target.ts", "src/dyn.ts", depth=1)
|
||||
assert _reaches(graph, "src/target.ts", "src/top.ts", depth=3)
|
||||
|
||||
|
||||
def test_affected_reaches_through_a_namespace_importer(tmp_path: Path):
|
||||
"""`import * as ns` binds the module, not the function that defers the load."""
|
||||
_, graph = _build(tmp_path, "import * as ns from './dyn'\nexport const run = () => ns.load()\n")
|
||||
|
||||
assert _reaches(graph, "src/target.ts", "src/top.ts", depth=3)
|
||||
|
||||
|
||||
def test_affected_reaches_when_importer_names_a_different_symbol(tmp_path: Path):
|
||||
"""`other` is a sibling export; the edge into `load()` that used to rescue this is absent."""
|
||||
_, graph = _build(tmp_path, "import { other } from './dyn'\nexport const run = () => other\n")
|
||||
|
||||
assert _reaches(graph, "src/target.ts", "src/top.ts", depth=3)
|
||||
|
||||
|
||||
def test_call_site_precision_is_preserved(tmp_path: Path):
|
||||
"""The symbol-level edge must survive: `explain` still has to name the deferring function."""
|
||||
result, _ = _build(tmp_path, "import { load } from './dyn'\nexport const run = () => load()\n")
|
||||
|
||||
tgt = _fid("src/target.ts")
|
||||
symbol_edges = [
|
||||
e for e in result["edges"]
|
||||
if e["target"] == tgt and e["source"] != _fid("src/dyn.ts")
|
||||
and e.get("deferred") is True
|
||||
]
|
||||
assert symbol_edges, "the symbol-level dynamic-import edge was replaced instead of added"
|
||||
assert symbol_edges[0]["relation"] == "imports_from"
|
||||
|
||||
|
||||
def test_module_level_dynamic_import_emits_no_duplicate(tmp_path: Path):
|
||||
"""At module level `caller_nid` IS the file node — emitting again would double-count."""
|
||||
result, _ = _build(
|
||||
tmp_path,
|
||||
"import './dyn'\nexport const run = () => 1\n",
|
||||
dyn_src="export const p = import('./target')\n",
|
||||
)
|
||||
|
||||
assert len(_file_edges(result, "src/dyn.ts", "src/target.ts")) == 1
|
||||
|
||||
|
||||
def test_one_file_deferring_the_same_module_twice_emits_one_file_edge(tmp_path: Path):
|
||||
"""Two functions, one dependency. The file-level edge dedupes on its own key."""
|
||||
result, _ = _build(
|
||||
tmp_path,
|
||||
"import './dyn'\nexport const run = () => 1\n",
|
||||
dyn_src=(
|
||||
"export async function a() {\n"
|
||||
" return (await import('./target')).value\n"
|
||||
"}\n"
|
||||
"export async function b() {\n"
|
||||
" return (await import('./target')).value\n"
|
||||
"}\n"
|
||||
),
|
||||
)
|
||||
|
||||
assert len(_file_edges(result, "src/dyn.ts", "src/target.ts")) == 1
|
||||
@@ -93,10 +93,26 @@ def test_module_scope_dynamic_import_edges(tmp_path: Path):
|
||||
assert any(e["relation"] == "dynamic_import" for e in edges)
|
||||
|
||||
|
||||
def test_ast_captured_dynamic_import_is_one_fact_not_two(tmp_path: Path):
|
||||
"""Dedupe: a dynamic import the AST pass already emitted (as a deferred
|
||||
imports_from edge) must not be restated by the rescue as a second
|
||||
dynamic_import edge to the same target."""
|
||||
def test_ast_captured_dynamic_import_still_gets_a_file_level_edge(tmp_path: Path):
|
||||
"""One import() inside a function, two granularities — #2584.
|
||||
|
||||
This test used to assert `len(edges) == 1`, on the reasoning that the rescue would be
|
||||
restating what the AST pass had already said. The two edges are not the same statement:
|
||||
the AST pass anchors on `caller_nid`, which is the enclosing FUNCTION when the import()
|
||||
is written inside one, while the rescue anchors on the FILE. Only the second is a fact
|
||||
`affected` can walk, since it traverses file to file.
|
||||
|
||||
Suppressing it cost real recall: on a ~700-file TS repo `affected --depth 3` returned 39
|
||||
of 49 truly affected files, precision 1.00, and more depth did not help — a dead end, not
|
||||
a depth limit. The reverse walk reached the enclosing function and stopped there, because
|
||||
the only edge pointing at it is `contains`, deliberately not in
|
||||
DEFAULT_AFFECTED_RELATIONS.
|
||||
|
||||
The dedupe is still right, just keyed wrong: it now matches on (source file, target)
|
||||
rather than target alone, so the genuinely redundant case — a module-scope import(),
|
||||
where `caller_nid` IS the file node — still collapses to one edge. That case is pinned
|
||||
by `test_module_scope_dynamic_import_is_still_one_fact` below.
|
||||
"""
|
||||
_write(tmp_path / "src/dep.ts", "export const dep = 1\n")
|
||||
importer = _write(
|
||||
tmp_path / "src/page.ts",
|
||||
@@ -109,9 +125,35 @@ def test_ast_captured_dynamic_import_is_one_fact_not_two(tmp_path: Path):
|
||||
result = extract([tmp_path / "src/dep.ts", importer], root=tmp_path)
|
||||
|
||||
edges = _edges_to(result, "src/dep.ts")
|
||||
assert len(edges) == 1, f"one import(), one edge — got {edges}"
|
||||
assert edges[0]["relation"] == "imports_from"
|
||||
assert edges[0].get("deferred") is True
|
||||
page = _file_node_id(Path("src/page.ts"))
|
||||
|
||||
# The precise fact: which function defers the load. `explain` reads this one.
|
||||
symbol_level = [e for e in edges if e["source"] != page]
|
||||
assert symbol_level, f"lost the call-site edge — got {edges}"
|
||||
assert symbol_level[0]["relation"] == "imports_from"
|
||||
assert symbol_level[0].get("deferred") is True
|
||||
|
||||
# The traversable fact: this file depends on that module. `affected` reads this one.
|
||||
file_level = [e for e in edges if e["source"] == page]
|
||||
assert file_level, f"no file-level edge — `affected` cannot traverse this: {edges}"
|
||||
assert file_level[0]["relation"] == "dynamic_import"
|
||||
|
||||
|
||||
def test_module_scope_dynamic_import_is_still_one_fact(tmp_path: Path):
|
||||
"""At module scope `caller_nid` IS the file node, so the rescue is genuinely redundant.
|
||||
|
||||
This is the half of the old dedupe that was always correct, kept as its own test so a
|
||||
future change cannot quietly restore double-counting here while fixing #2584.
|
||||
"""
|
||||
_write(tmp_path / "src/dep.ts", "export const dep = 1\n")
|
||||
importer = _write(
|
||||
tmp_path / "src/boot2.ts",
|
||||
"const { dep } = await import('./dep')\nexport const booted = dep\n",
|
||||
)
|
||||
|
||||
result = extract([tmp_path / "src/dep.ts", importer], root=tmp_path)
|
||||
|
||||
assert len(_edges_to(result, "src/dep.ts")) == 1
|
||||
|
||||
|
||||
def test_static_import_alongside_dynamic_is_untouched(tmp_path: Path):
|
||||
@@ -130,7 +172,15 @@ def test_static_import_alongside_dynamic_is_untouched(tmp_path: Path):
|
||||
|
||||
a_edges = _edges_to(result, "src/a.ts", "imports_from")
|
||||
assert a_edges and not any(e.get("deferred") for e in a_edges)
|
||||
assert len(_edges_to(result, "src/b.ts")) == 1
|
||||
|
||||
# The point of this test is the STATIC edge above: the rescue must leave it alone, and
|
||||
# in particular must not mark it deferred. The dynamic side is asserted only for the
|
||||
# property that concerns this test — every edge to b.ts is flagged as deferred one way
|
||||
# or another, so no arrow into b.ts can be mistaken for a static dependency. Its COUNT
|
||||
# is #2584's subject, pinned in test_ast_captured_dynamic_import_still_gets_a_file_level_edge.
|
||||
b_edges = _edges_to(result, "src/b.ts")
|
||||
assert b_edges
|
||||
assert all(e.get("deferred") or e["relation"] == "dynamic_import" for e in b_edges)
|
||||
|
||||
|
||||
def test_tsconfig_aliased_dynamic_import_edges(tmp_path: Path):
|
||||
|
||||
Reference in New Issue
Block a user