diff --git a/CHANGELOG.md b/CHANGELOG.md index 359d00c14..da6baf83d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases) +## 0.9.39 (unreleased) + +- Fix: a PHP **function call never binds to a method** across files (`lawnstarter/graphify#52`). In PHP a bare `name(...)` — a `function_call_expression` — can only invoke a global or namespaced *function*; reaching a method requires `$obj->`, `Class::` or first-class-callable syntax, each of which takes a different path through the extractor. The shared cross-file pass matched by normalized label, and that normalization (`raw.strip("()").lstrip(".")`) erases the member marker the engine writes — a method labeled `.event()` and a function labeled `event()` both key as `event` — so Laravel's `event(...)` helper bound to whichever class happened to declare an `event()` method. On the pinned 46,406-node api.lawnstarter.com corpus that is **848 fabricated inbound `calls` edges on `GetProviderBillingEventsTest::event()`**, a test method credited as the callee of 368 distinct source files, 308 of them not tests. It was not even an ambiguity the god-node tie-breakers could catch: the three `Event` *classes* key as `Event`, so the method was the **sole** exact-case candidate and bound through the single-candidate path at INFERRED 0.8, before any tie-break could apply. The refusal is language-semantic rather than heuristic — a method is simply not a candidate at a function-call site — so it is applied to the candidate **list**, not by re-keying the indexes, which covers every consumer at once: the single-candidate bind, the symbol/module import-evidence disambiguation, and the god-node tie-break. The same language-semantic argument extends past methods: a **class-like declaration is not invocable either** — `report($e)` where only `class Report` exists is a "Call to undefined function" fatal, not a constructor call — so class, interface, enum and trait nodes are refused at a function-call site alongside methods, leaving only plausible function targets. That half also fixes a pre-existing bind this release would otherwise have inherited: `foo(...)` matching a cross-file `class Foo` through the case-insensitive fold has always produced a `calls` edge into the class. Both index paths are covered, and the folded lookup is *retried* after the method/class refusal rather than skipped: the case-insensitive fallback only fires on an empty exact-case list, so a method shadowing the exact-case key would otherwise have hidden a real `function Event()` reachable solely under the folded key, and the site would have resolved to nothing at all. The retry has to carry the refusal with it, or it hands the folded index a candidate the exact-case path never offered — a capitalized class under the shadowed lowercase key, which is exactly the **7 `report(...)` → `class Checkr\Resources\Report` edges** measured on the rebuilt 46.6k-node corpus. The class discriminator is the `_callable_class` marker the `indirect_call` guard already reads, chosen for the same reason as the label one: it is replayed onto `resolution_context_nodes` by both `graphify update` and `watch`, so it still classifies unchanged-corpus candidates on an incremental rebuild. The call site is identified by a marker the PHP extractor stamps on the raw call, which is load-bearing rather than convenience: a `scoped_call_expression` is also not a member call, so refusing on "PHP and not a member call" alone would have silently widened the policy to `Class::method()`. The method-vs-function discriminator is the engine's own label convention (`.name()` for a member, `name()` for a top-level function), chosen because a label persists into `graph.json` and so still classifies the unchanged-corpus candidates an incremental rebuild hands back as resolution context (#2406) — an edge-derived test would not, since the shared pass never receives those edges. PHP-only by construction: a Ruby or Python bare call reaches a method through implicit self, so their candidate filtering is deliberately untouched, as is the member-call resolver (`_resolve_php_member_calls`). One residual is documented rather than chased: a `event(...)` call in the *same file* as a class declaring an `event()` method still binds to it, because the extractor resolves that raw call in-file and it never reaches the cross-file pass — unchanged behavior, and the reason the measured fan-in shrinks to near-zero rather than provably zero. Extraction-side, so a PHP corpus must be re-extracted (`graphify update .`) to shed these edges, and re-extraction genuinely requires the release's version bump to roll the version-namespaced AST cache (`graphify-out/cache/ast/v{version}/`): entries are keyed by content hash *within* that namespace, so a same-version rebuild replays the pre-fix raw calls untouched. +- Fix: `graphify query` and `graphify path` / `shortest_path` no longer resolve an endpoint to a **sourceless stub** when a real declaration carries the same label (`lawnstarter/graphify#54`, closing the gap `#49` left behind). `#49` taught `_find_node_tiers` and `affected`'s `resolve_seed` to drop stubs from a mixed exact tier, so `explain` and `affected` agree on the sourced declaration — but the scored path those two commands do not share, the `_score_nodes` ranking that feeds `_pick_scored_endpoint` (path) and `_pick_seeds` (query), never learned the rule. On the two-node repro — `FooRepository` at `source_file: ""` beside `FooRepository` at `app/bindings.php` — both nodes score **5619.081861425897**, identically, so the ranking fell through to its lexicographic node-id tie-break and answered with whichever id sorted first: `explain` named the declaration while `query` traversed the stub, reinstating for `explain`-vs-`query` exactly the divergence `#49` closed for `explain`-vs-`affected`, and violating the design intent the scorer's own comment states — that `path` and `query` resolve the same node `explain` does. The stub is rarely a merely cosmetic mismatch, because it is usually the *disconnected* node: `shortest_path` answered a reachable query with a false "No directed path found", and `query` spent its traversal on the stub's neighborhood instead of the declaration's. The fix is a **tie-break, not a scoring penalty**, and the distinction is load-bearing: a sourceless node can never *out*-score an otherwise-identical sourced one, since the only source-derived scoring term is additive and source hits do not count toward term coverage, so a shadowing stub always arrives at the sort as an exact tie and a tie-break is the complete and minimal intervention. The sourced preference is inserted directly under the score in both tie-break keys — the combined ranking sort and the per-term singleton-winner key that feeds the seed guarantee — above the label-length and node-id keys, which are arbitrary with respect to which node is real. No live score changes, so every score-sensitive baseline holds unedited. `#49`'s carve-out survives by construction rather than by a special case: an all-sourceless field has nothing to prefer, so a lone stub still wins its query and nothing that answered before now returns nothing. One asymmetry with `_find_node` is deliberate and left in place, since `_find_node` is unchanged here: it demotes stubs in its *exact* tier only, while a score tie can also arise from the prefix and substring tiers — on those shapes the scored path now prefers the sourced node where `_find_node` still answers by graph-iteration order, which is the more deterministic of the two answers, and closing the remaining gap would mean changing `_find_node` itself. Serve-side only: nothing in extraction, merge or the graph format changes, so an existing graph picks this up with **no rebuild and no re-extraction**. +- Fix: a PHP receiver typed with an `interface`, `enum` or `trait` name now binds to that declaration's own method instead of being refused outright (`lawnstarter/graphify#53`). `_resolve_php_member_calls` consulted a corpus-wide pre-scan of the non-class declaration names (`_php_non_class_types`) and skipped every such receiver — correct while the three minted no definition node, because the only thing such a receiver could land on was a same-short-named CLASS: Laravel's `App\Contracts\Notifier` interface beside an unrelated `App\Support\Notifier`, or `App\Enums\Status` beside an Eloquent `App\Models\Status`, each leaving exactly ONE definition under the short name and so satisfying the single-definition guard with the wrong answer. `#47` (0.9.38 above) gave all three kinds canonical sourced nodes that their own methods attach to, which makes the refusal redundant: the collision now presents TWO definitions and is refused by the guard unaided, while a `use`-imported name is decided by the declared-FQN match. What the refusal was still costing is the rest — an interface or enum named by exactly one declaration, and a `use`-claimed one whose import says which of the namesakes it means — so `$this->notifier->send()` on a `private Notifier $notifier` now lands on `Notifier::send()` itself. Implementations are still never guessed: a `MailNotifier implements Notifier` gets nothing from an interface-typed receiver, and the same-short-named stranger class gets nothing on any path. +- The lift required extending the declared-FQN pre-scan to the three kinds, and that half is a fix in its own right: `_php_pre_scan_class_namespaces` recorded `class_declaration` only, so an interface/enum/trait node NEVER carried a declared FQN — not on a full build, not through the `_php_class_fqns` marker (`#23`). `PhpNameResolver`'s `use`-claim guard then fell back to comparing the node's PSR-4 PATH, and `_php_fqn_names_another_class` treats a path with FEWER segments than the written name as *no evidence* and keeps the edge. On a full build the paths are still absolute at resolver time, so the comparison ran and refused; on an incremental rebuild the replayed context node carries the relativized `app/Contracts/Notifier.php` (3 segments), so `use Illuminate\Contracts\Notifications\Notifier;` (4) tripped the bail-out and bound the in-corpus interface that the vendor import provably does not name — a wrong edge of the `#16` class, appearing ONLY under `graphify update`/`watch`, which is the normal operating mode. `Illuminate\Contracts\\` is exactly 4 segments and Laravel apps routinely declare `App\Contracts\{Repository,Factory,Guard,Mailer,…}` beside imports of the framework contracts with those same short names, so the shape is ordinary rather than exotic. All four declaration kinds are now read into `php_class_fqns` / `_php_class_fqns` / `fqn_def_nid`, which makes the guard whole-name and decisive on both build paths and closes the full-vs-incremental parity hole; the `tests/test_php_member_calls.py` cases assert the two builds' verdicts TOGETHER, since a full-build-only assertion is green on the broken code. +- The `_php_non_class_types` / `_php_interfaces` channel is kept intact — pre-scan, per-file payload, file-node stamp, `graphify update`/`watch` replay, both spellings read back — so graphs written by earlier versions still round-trip. Nothing consumes it for resolution any more; it remains the only record of declaration KIND that survives into a graph. +- Extraction-side, so a PHP corpus must be re-extracted (`graphify update .`) to pick this up: the new edges come from the receiver-typing pass and the declared FQNs come from a pre-scan, and AST cache entries are keyed by content hash *within* the version namespace, so a same-version rebuild replays the pre-fix payloads untouched. + ## 0.9.38 (unreleased) - Fix: a PHP `interface`, `trait` or `enum` now mints a declaration node, exactly as a `class` does (`lawnstarter/graphify#47`, RC1 of `#46`). `_PHP_CONFIG.class_types` held `class_declaration` alone, so **no node was ever created for any of the three** — 142 interfaces, 30 traits and 119 enums (291 declarations) on the pinned 46,406-node api.lawnstarter.com corpus. Every resolution pass that could canonicalize an edge therefore had nothing to land on, and the fan-in scattered three ways: the `implements`/`extends`/trait-`use` base minted a bare *sourceless* stub (`balanceitemrepository`) which, having an empty source key, kept the un-salted id and so shadowed the real name — `graphify explain "BalanceitemRepository"` answered from that degree-1 stub while `graphify affected` on the same name refused with "No unique node match"; `Foo::CONST` fan-in fragmented across one salted per-file stub per referencing file (17 of them, holding 20 `references_constant` edges, for that one interface); and `imports`/parameter-type `references` parked on the *file* node by the PSR-4 id-collision accident that makes `_make_id(FQN)` equal the file id — or, when the filename differs from the type name (a trait in `Extras.php`), on a sourceless FQN-labeled stub instead. The three kinds join `class_declaration` in `class_types`, mirroring Java and Groovy, which have always had `interface_declaration`. Two grammar details ride along: an enum's body is an `enum_declaration_list` rather than the `declaration_list` every other PHP declaration uses, so `body_fallback_child_types` learns it; and the `_resolve_php_type_references` raw-scan — which reads the written extends/implements/`use` text so a qualified name is resolved as written instead of guessed — scanned `class_declaration` bodies only, so `interface Reader extends Sub\Repo` and `enum Status { use Sub\Describes; }` recorded nothing and fell through to the same-namespace fallback, silently binding the rival `App\Contracts\Repo` / `App\Enums\Describes`. It now scans all four declaration kinds and both body shapes. diff --git a/graphify/cli.py b/graphify/cli.py index fa9b5499c..7d7f6b794 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -3215,14 +3215,16 @@ def _ctx_identity(source_file) -> str | None: "type": _node.get("type"), } # `_php_non_class_types` (#11, #12) rides the same - # marker channel as the callability flags: without it an - # unchanged PHP file declaring an interface, enum or - # trait stops refusing such a receiver and a stranger - # class gets the edge. `_php_interfaces` is the pre-#12 - # spelling, still carried for older graphs. - # `_php_class_fqns` (#23) is the positive counterpart: - # the declared FQNs that let a claimed `use` import keep - # binding into an unchanged defining file (#22). + # marker channel as the callability flags: it records + # which of an unchanged PHP file's declarations are + # interfaces, enums or traits rather than classes. It + # drove a receiver refusal until #53 lifted it, and is + # still carried; `_php_interfaces` is the pre-#12 + # spelling, kept for older graphs. `_php_class_fqns` + # (#23) is what resolution reads today: the declared + # FQNs that let a claimed `use` import bind into an + # unchanged defining file (#22), and let the guard + # refuse a vendor import that only shares a short name. for _marker in ("_callable", "_callable_class", "_php_non_class_types", "_php_interfaces", "_php_class_fqns"): diff --git a/graphify/extract.py b/graphify/extract.py index db5d85137..e1251f9f9 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -3289,23 +3289,29 @@ def key(label: str) -> str: def _php_context_interface_entry(context_nodes: list[dict] | None) -> dict | None: """Recover the unchanged corpus's PHP interface/enum/trait names (#11, #12). - None of the three mints a definition node, so the extractor stamps the names a - file declared on that file's own node as ``_php_non_class_types`` — a persisted - marker, like ``_callable`` (#2438) — and ``watch``/``graphify update`` hand it - back on the resolution-context nodes. Returns a synthetic ``per_file``-shaped - entry carrying just those names (or None when there are none), which extends - the resolver's existing single channel instead of adding a second one. + The extractor stamps the names a file declared on that file's own node as + ``_php_non_class_types`` — a persisted marker, like ``_callable`` (#2438) — + and ``watch``/``graphify update`` hand it back on the resolution-context + nodes. Returns a synthetic ``per_file``-shaped entry carrying just those + names (or None when there are none), which extends the resolver's existing + single channel instead of adding a second one. ``_php_interfaces`` is the pre-#12 spelling of the same marker, carrying interfaces alone; it is still read so a graph.json written before enums and - traits joined the set keeps refusing the names it does carry, rather than - losing the refusal outright until its files are re-extracted. + traits joined the set keeps handing back the names it does carry. + + The MEMBER-CALL RESOLVER no longer consults these names: #5/#12 used them to + refuse an interface/enum/trait-typed receiver outright, and #53 lifted that + refusal once #47 gave the three declarations canonical nodes of their own + (`_resolve_php_member_calls`). The channel itself — pre-scan, stamp, replay — + is kept whole: it is what a graph written by an older version still carries, + and it is the only record of which corpus names are non-class declarations + that survives a rebuild in which the declaring file is never dispatched. Read from the RAW context list rather than off the merged resolution nodes: a changed caller that does `use App\\Contracts\\Notifier;` mints a sourceless import stub whose id IS the interface file node's id, and the merge drops the - colliding context node (fresh wins) — taking the marker with it, exactly in the - case the refusal is needed. + colliding context node (fresh wins) — taking the marker with it. """ names = sorted({ str(name) @@ -3365,6 +3371,22 @@ def _resolve_php_member_calls( (``$obj->method(...)``): the method is referenced, not invoked, so it resolves by exactly the rules above but is emitted as ``indirect_call`` (#15). + + A receiver typed with an ``interface``, ``enum`` or ``trait`` name resolves + by those same rules (#53). Until #47 the three minted no definition node, so + such a receiver could only ever land on a same-short-named CLASS — the + Laravel Contracts collision (``App\\Contracts\\Notifier`` vs + ``App\\Support\\Notifier``) or the enum-beside-model one — and #5/#12 + refused every one of them outright, off a corpus-wide pre-scan of the + declared names. Now that each declaration mints a canonical sourced node + that its own methods attach to, the same two rules that keep a CLASS-typed + receiver honest cover them: a calling file that names one through a ``use`` + import gets ``PhpNameResolver``'s verdict — the declared-FQN match when the + import names an in-corpus declaration, a refusal when it names a vendor type + that merely shares the short name — and a file that claims nothing falls to + the single-definition guard, which sees the collision as the two definitions + it now is. What the blanket refusal was costing is the rest: an interface or + enum named by exactly one declaration binds to that declaration's method. """ def key(label: str) -> str: # PHP class and method names are case-insensitive. @@ -3400,33 +3422,14 @@ def key(label: str) -> str: enclosing_type.setdefault(method, owner) method_index.setdefault((owner, key(method_node.get("label", ""))), set()).add(method) - # Names declared as `interface`, `enum` or `trait` anywhere in the corpus. - # None of the three mints a definition node, so without this such a receiver - # would bind to whatever same-named CLASS happens to exist — the Laravel - # Contracts collision (`App\Contracts\Notifier` vs `App\Support\Notifier`) - # or the enum-beside-model one (`App\Enums\Status` vs `App\Models\Status`), - # neither of which the single-definition guard can see because there IS only - # one definition. `php_interfaces` is the pre-#12 spelling of the same fact, - # still read so an AST-cache entry written before enums and traits joined - # the set keeps refusing interfaces. - # - # `per_file` aligns 1:1 with the files dispatched THIS run, so an incremental - # rebuild that leaves the declaring file untouched used to see no such names - # at all and mint a wrong edge into the same-short-named class (#11). - # extract() closes that hole by appending the unchanged corpus's persisted - # names as one extra entry, so this single channel still covers the whole - # corpus — see `_php_context_interface_entry`. - non_class_type_names = { - key(name) - for result in per_file - for keyname in ("php_non_class_types", "php_interfaces") - for name in result.get(keyname, []) - } - - # Fully qualified class names as each defining file DECLARES them (#14), so + # Fully qualified type names as each defining file DECLARES them (#14), so # the inline-`new` corroboration below compares the written name against the # real one instead of against the file's path, which PSR-4 only conventionally - # agrees with. Keyed by defining file, then by short class name. + # agrees with. Keyed by defining file, then by short name. Covers every + # namespaced declaration, `interface`/`enum`/`trait` included since they + # became bindable receiver types (#53) — a declaration without a declared + # FQN is one the guards can only judge by its path, and a replayed context + # node's path is short enough to make that judgement abstain. class_fqn_by_file: dict[str, dict[str, str]] = {} for result in per_file: declared = result.get("php_class_fqns") @@ -3438,7 +3441,7 @@ def key(label: str) -> str: # call site in `_resolve_csharp_member_calls`. A name the calling file # CLAIMS through a `use` import or writes out qualified is decided here and # never falls back: the refusal is the whole fix for #16, and the claimed - # FQN binding to the class whose file declares exactly that name is the + # FQN binding to the type whose file declares exactly that name is the # recall counterpart (#22) — see PhpNameResolver. resolver = PhpNameResolver(all_nodes, all_edges, type_def_nids, class_fqn_by_file) @@ -3482,11 +3485,6 @@ def declared_fqn(type_node: dict | None) -> str | None: type_name = raw_call.get("receiver_type") if not type_name: continue # untyped / union-typed / unknown receiver: refuse - if key(type_name) in non_class_type_names: - # An interface names no implementation, a trait is not a - # type, and an enum's methods live on no definition node: - # refuse rather than bind a same-short-named stranger. - continue resolved, decisive = resolver.resolve_type_name( type_name, raw_call.get("receiver_type_qualified"), @@ -3523,8 +3521,8 @@ def declared_fqn(type_node: dict | None) -> str | None: "target": method_nid, # A first-class callable NAMES the method without invoking it, so it # is the repo's `indirect_call`, not `calls` (#15). Everything else - # above — receiver typing, the single-definition and interface/enum/ - # trait refusals, the confidence ladder — is deliberately shared. + # above — receiver typing, the claimed-name and single-definition + # refusals, the confidence ladder — is deliberately shared. "relation": "indirect_call" if raw_call.get("fcc") else "calls", "context": "call", "confidence": "EXTRACTED" if exact else "INFERRED", @@ -5267,10 +5265,11 @@ def extract( `_callable_class` markers, #2438), and the member-call resolvers run by `run_language_resolvers` (#2437) — so a changed caller can still bind `foo()`, `obj.method()`, or `submit(handler)` to an - unchanged callee. They also carry the PHP resolver's interface, - enum and trait names, stamped as `_php_non_class_types` on each PHP - file node, so an unchanged declaring file keeps its refusal (#11, - #12). They are never + unchanged callee. They also carry the unchanged corpus's PHP + interface, enum and trait names, stamped as `_php_non_class_types` + on each PHP file node (#11, #12) — a persisted channel kept whole + for older graphs, though the member-call resolver stopped consuming + it when #53 lifted the refusal it fed. They are never parsed, mutated, or returned; raw_calls come only from `paths`, so only edges sourced by the re-extracted files are emitted. resolution_context_edges: the `contains`/`method` edges of the same @@ -6012,11 +6011,22 @@ def _looks_like_bash(result: object) -> bool: n for n in resolution_context_nodes if n.get("id") and n["id"] not in _fresh_ids ] + # #52: ids of nodes whose label marks them as a MEMBER of a type — the + # engine labels a method `.name()` and a property `.name`, against a + # top-level function's `name()`. The normalization below erases that marker + # (`.event()` and `event()` both key as `event`), which is what let a PHP + # `event(...)` function call bind to a class's `event()` METHOD. The label + # is persisted in graph.json, so the discriminator holds just as well for + # candidates handed back as unchanged-corpus resolution context (#2406) — + # unlike an id-shape or edge-derived test. + member_labelled_nids: set[str] = set() for n in resolution_nodes: if n.get("file_type") == "rationale" or n.get("type") == "namespace": continue raw = n.get("label", "") normalised = raw.strip("()").lstrip(".") + if raw.strip("()").startswith("."): + member_labelled_nids.add(n["id"]) if normalised: # Case is semantic in most languages, so index (and match, below) by exact # case — folding collapses `Path` (class) into `PATH` (env var) and makes a @@ -6039,6 +6049,20 @@ def _looks_like_bash(result: object) -> bool: # them from the indirect_call guard below to avoid false edges (#2137). class_nids = {n["id"] for n in resolution_nodes if n.get("_callable_class")} + def _is_php_function_target(nid: str) -> bool: + """Can a bare PHP ``name(...)`` reach this candidate? (#52) + + Only a global or namespaced FUNCTION. A METHOD needs ``$obj->`` / + ``Class::`` / callable syntax, and a CLASS-LIKE declaration (class, + interface, enum, trait) is not invocable at all — ``Report($e)`` where + only ``class Report`` exists is a "Call to undefined function" fatal, + not a constructor call. Both discriminators survive an incremental + rebuild: the member marker rides the persisted LABEL, and + ``_callable_class`` is one of the markers `graphify update` / `watch` + replay onto their `resolution_context_nodes` (cli.py, watch.py). + """ + return nid not in member_labelled_nids and nid not in class_nids + # Build evidence index from import edges so cross-file calls backed by an # explicit import statement can be promoted from INFERRED to EXTRACTED. # Direct symbol imports (`import { foo }` / `const { foo } = require()`) are @@ -6142,6 +6166,35 @@ def _looks_like_bash(result: object) -> bool: candidates = global_label_to_nids_ci.get(callee.lower(), []) if not candidates: continue + # #52: a PHP `function_call_expression` — a bare `name(...)` — can only + # invoke a global/namespaced FUNCTION, so only plausible function targets + # stay candidates (`_is_php_function_target`). Laravel's `event(...)` + # helper was binding to whichever class declared an `event()` METHOD — + # 848 incoming edges on one test method in the measured corpus — because + # the label index erases the `.` member marker and the method was then + # the sole candidate → single-candidate bind. Filtering the candidate + # list (rather than the index) covers every consumer below at once — + # single-candidate bind, import-evidence disambiguation and the god-node + # tie-break. PHP-only by design: Ruby and Python bare calls reach methods + # through implicit self, so their candidate filtering is unchanged. + if rc.get("php_function_call"): + candidates = [c for c in candidates if _is_php_function_target(c)] + # Refusing the exact-case candidates re-opens the folded fallback, + # which only ever fires on an EMPTY exact-case list above. A method + # shadowing the exact-case key (`.event()` → `event`) otherwise hid a + # real `function Event()` — reachable solely under the folded key — + # and the site resolved to nothing at all. Same folded index, same + # case-insensitive-caller gate, same refusal applied to it: without + # the refusal here the retry reaches a capitalized CLASS under the + # folded key (`report(...)` → `class Report`) that the exact-case + # path never offered — 7 such edges on the measured corpus. + if not candidates and _lang_is_case_insensitive(rc.get("source_file")): + candidates = [ + c for c in global_label_to_nids_ci.get(callee.lower(), []) + if _is_php_function_target(c) + ] + if not candidates: + continue # Cross-language guard: never bind a call to a definition in a different # language family. Name-only matching was resolving a TSX callback passed # by name to a same-named Kotlin method in the Android half of the repo @@ -6296,12 +6349,12 @@ def _has_import_evidence(candidate_id: str) -> bool: # unchanged file is ever emitted, and the ambiguity guards count the same # candidates a full build would (the context is the whole unchanged corpus). # - # #11/#12: nodes and edges are not the whole story — the PHP resolver also - # needs the unchanged corpus's INTERFACE, ENUM and TRAIT names, which mint no - # node of their own. They ride in on the context nodes' `_php_non_class_types` - # marker; hand them over as one extra `per_file` entry (scratch list, the real - # `per_file` is untouched) so an unchanged declaring file keeps refusing such - # a receiver instead of letting it bind to a same-short-named class. + # #11/#12: the unchanged corpus's INTERFACE, ENUM and TRAIT names ride in on + # the context nodes' `_php_non_class_types` marker and are handed over as one + # extra `per_file` entry (scratch list, the real `per_file` is untouched). + # The member-call resolver stopped consuming them in #53 — its refusal was + # made redundant by the declaration nodes #47 mints — but the channel is kept + # whole for the graphs still carrying it; see `_php_context_interface_entry`. if resolution_context_nodes or resolution_context_edges: _rl_nodes = list(resolution_nodes) _rl_edges = all_edges + list(resolution_context_edges or []) @@ -6456,11 +6509,12 @@ def _canon(nid: str) -> str: # label (that would reintroduce the #1566/#2137 data-symbol false positives); # a graph written before the markers existed simply fails closed until its # files are re-extracted. - # `_php_non_class_types` (#11, #12) is kept for the same reason and with the - # opposite failure direction: a pre-marker graph simply loses the refusal on - # an incremental rebuild until the declaring file is re-extracted. A graph - # carrying only the pre-#12 `_php_interfaces` spelling keeps refusing the - # interfaces it names — both spellings are read. + # `_php_non_class_types` (#11, #12) persists for the same reason: it is the + # only record of which corpus names are interface/enum/trait declarations + # that survives a rebuild leaving the declaring file undispatched. Both + # spellings are read back, the pre-#12 `_php_interfaces` included. Nothing + # consumes it for resolution since #53 lifted the receiver refusal it fed; + # the channel stays because the graphs written by earlier versions carry it. # `_php_class_fqns` (#23) persists likewise, failing in the safe direction # too: a pre-marker graph loses the declared-FQN BINDING (#22) on an # incremental rebuild — an absent edge, never a guessed one — until the diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 1d4396af1..19f7eb82e 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -144,12 +144,13 @@ def _csharp_pre_scan_interfaces(root_node, source: bytes) -> set[str]: stack.extend(n.children) return out -# PHP declaration kinds whose names the member-call resolver refuses to bind a -# receiver to (#1682). All three ARE in `_PHP_CONFIG.class_types` and do mint -# definition nodes as of #47, so this is no longer "the kinds outside -# class_types" — it is the refusal policy, stated for its own sake. The name is -# historical and kept because the marker it feeds (`_php_non_class_types`) is -# persisted in graph.json and read back on incremental rebuilds (#11/#12). +# The PHP declaration kinds that are not `class`. All three ARE in +# `_PHP_CONFIG.class_types` and mint definition nodes as of #47, so this is no +# longer "the kinds outside class_types"; it was the member-call resolver's +# refusal policy (#1682) until #53 lifted that refusal, and what remains is a +# record of declaration KIND. The name and the marker it feeds +# (`_php_non_class_types`) are kept as spelled: the marker is persisted in +# graph.json and read back on incremental rebuilds (#11/#12). _PHP_NON_CLASS_DECLARATIONS = frozenset({ "interface_declaration", "enum_declaration", @@ -160,21 +161,21 @@ def _csharp_pre_scan_interfaces(root_node, source: bytes) -> set[str]: def _php_pre_scan_non_class_declarations(root_node, source: bytes) -> set[str]: """Return names declared as `interface`, `enum` or `trait` in this PHP file (#1682). - The member-call resolver refuses to bind a receiver typed with one of these - names. Laravel's conventions make the collision that motivates the refusal - realistic: an `App\\Contracts\\Notifier` interface beside an unrelated - `App\\Support\\Notifier` class — or an `App\\Enums\\Status` enum beside an - Eloquent `App\\Models\\Status` — used to leave exactly ONE definition under - that short name, which satisfied the single-definition guard and bound the - receiver to a total stranger. The names are threaded to the resolver so it - can refuse instead. - - Since #47 all three kinds mint definition nodes, so that same collision now - presents TWO definitions and the single-definition guard would refuse on its - own. The pre-scan is kept anyway: it still refuses the case the guard cannot - see — a lone interface with no same-named class, which would otherwise start - binding — and that is a behavior change #47 deliberately did not make. - Whether to lift the refusal now that the nodes exist is a separate decision. + These names once drove a REFUSAL: the member-call resolver would not bind a + receiver typed with one of them. Laravel's conventions make the collision + that motivated it realistic — an `App\\Contracts\\Notifier` interface beside + an unrelated `App\\Support\\Notifier` class, or an `App\\Enums\\Status` enum + beside an Eloquent `App\\Models\\Status` — because while the three minted no + definition node, exactly ONE definition existed under the short name, which + satisfied the single-definition guard and bound the receiver to a stranger. + + #47 gave all three kinds definition nodes, so the collision now presents TWO + and the guard refuses on its own; #53 lifted the blanket refusal on that + basis, and nothing consumes these names for resolution any more (see + `_php_context_interface_entry` in extract.py). The pre-scan and its + `_php_non_class_types` marker are kept because they are persisted in + graph.json: they are the only record of declaration KIND that survives into + a graph, and older graphs carry them. """ out: set[str] = set() stack = [root_node] @@ -190,8 +191,22 @@ def _php_pre_scan_non_class_declarations(root_node, source: bytes) -> set[str]: return out +# The declaration kinds `_php_pre_scan_class_namespaces` reads a declared FQN +# off. `class_declaration` alone until #53: once an interface/enum/trait-typed +# receiver became bindable, a declaration WITHOUT a declared FQN was one the +# name guard could only judge by its PSR-4 path — strictly weaker, and weakest +# on exactly the incremental path where paths arrive relativized. All four kinds +# declare a namespaced type, so all four are read. +_PHP_FQN_DECLARATIONS = frozenset({ + "class_declaration", + "interface_declaration", + "enum_declaration", + "trait_declaration", +}) + + def _php_pre_scan_class_namespaces(root_node, source: bytes) -> dict[str, str]: - """Map every namespaced class in this PHP file to its fully qualified name (#14). + """Map every namespaced type in this PHP file to its fully qualified name (#14). PHP class NODES carry no namespace, so the inline-`new` corroboration in ``_php_qualified_corroborates`` had only the file's path to compare a @@ -201,9 +216,23 @@ def _php_pre_scan_class_namespaces(root_node, source: bytes) -> dict[str, str]: files, generated code), and the written name then corroborates a class that exists nowhere. The declaration is right there in the source — read it. + ``interface``, ``enum`` and ``trait`` declarations are read alongside + ``class`` (#53). They were left out while #5/#12 refused every receiver + typed with such a name, which made their FQNs unused; once the refusal was + lifted and those declarations became bindable, the omission left + ``PhpNameResolver`` judging them by PSR-4 path alone — and + ``_php_fqn_names_another_class`` treats a path with fewer segments than the + written name as NO EVIDENCE and keeps the edge. A rebuild replays context + nodes with RELATIVIZED paths, so ``use Illuminate\\Contracts\\…\\Notifier;`` + (4 segments) against a replayed ``app/Contracts/Notifier.php`` (3) bound the + in-corpus interface that the vendor import provably does not name — a wrong + edge of the #16 class, and only on the incremental path. A declared FQN + makes that comparison whole-name and decisive on both paths, and lets + ``fqn_def_nid`` (#22) pick the imported one of several namesakes. + Both namespace forms are handled: ``namespace X;`` applies to the declarations that follow it (a file may switch namespaces mid-way), and - ``namespace X { … }`` applies to its block. A class declared in NO namespace + ``namespace X { … }`` applies to its block. A type declared in NO namespace is deliberately absent from the map: the file states nothing, so the resolver falls back to the path check rather than refusing. A short name declared twice under different namespaces in one file is dropped — the map @@ -213,9 +242,9 @@ def _php_pre_scan_class_namespaces(root_node, source: bytes) -> dict[str, str]: conflicting: set[str] = set() # Each entry carries the namespace in force where it was queued, so the - # scopes stay right without walking siblings in order. A class body is never - # descended into: PHP has no nested class declarations, and an anonymous - # class inside a method names nothing. + # scopes stay right without walking siblings in order. A declaration body is + # never descended into: PHP has no nested type declarations, and an + # anonymous class inside a method names nothing. stack = [(root_node, "")] while stack: node, namespace = stack.pop() @@ -232,7 +261,7 @@ def _php_pre_scan_class_namespaces(root_node, source: bytes) -> dict[str, str]: else: current = declared # applies to the declarations that follow continue - if child.type == "class_declaration": + if child.type in _PHP_FQN_DECLARATIONS: name_node = child.child_by_field_name("name") name = _read_text(name_node, source) if name_node is not None else "" if name and current: @@ -4903,6 +4932,13 @@ def walk_calls( # reference to the method, not an invocation — re-tagged as # `indirect_call` below and by the cross-file PHP resolver. php_fcc: bool = False + # PHP `function_call_expression`, i.e. a bare `name(...)` (#52). It + # can only invoke a global/namespaced FUNCTION — a method needs + # `$obj->`, `Class::` or callable syntax — so the cross-file pass + # refuses method candidates for it. Recorded here because the + # raw-call facts otherwise cannot tell it from a `scoped_call` + # (which is also not a member call and names its scope as callee). + php_function_call: bool = False # Special handling per language if config.ts_module == "tree_sitter_swift": @@ -5015,6 +5051,7 @@ def walk_calls( elif config.ts_module == "tree_sitter_php": # PHP: distinguish call expression subtypes if node.type == "function_call_expression": + php_function_call = True func_node = node.child_by_field_name("function") if func_node: callee_name = _read_text(func_node, source) @@ -5363,6 +5400,10 @@ def walk_calls( # stamp the receiver type resolved above (#1682). if config.ts_module == "tree_sitter_php": rc_entry["lang"] = "php" + if php_function_call: + # Marker read by the shared cross-file pass, which + # drops METHOD candidates for this call site (#52). + rc_entry["php_function_call"] = True if php_fcc: # Marker read by _resolve_php_member_calls, which # emits `indirect_call` for it under the SAME @@ -5739,32 +5780,36 @@ def _scan_js_module_dispatch(n) -> None: if swift_extensions: result["swift_extensions"] = swift_extensions if php_non_class_type_names: - # Interfaces, enums and traits mint no definition node, so the resolver - # cannot tell one from a same-named class without this (#1682). Sorted - # for a stable AST-cache payload. + # Which names this file declares as `interface`/`enum`/`trait` rather + # than `class` (#1682). Sorted for a stable AST-cache payload. The + # member-call resolver consumed this to refuse such receivers until #53 + # lifted the refusal; what is left is a persisted record of declaration + # KIND, which nothing else in a graph carries. result["php_non_class_types"] = sorted(php_non_class_type_names) # The per-file payload above only reaches the resolver for files # dispatched THIS run, so on an incremental rebuild an unchanged - # declaring file stopped refusing and the receiver bound to a stranger - # class sharing the short name (#11). Also stamp the names on the FILE - # node — the marker rides the node dict into graph.json and back in as - # resolution context, the same channel `_callable` uses (#2438). The - # file node is the host because none of these declarations mints a node - # of its own; the names are listed explicitly rather than read off the - # node's `.php` label, which would only hold under - # one-declaration-per-file PSR-4 convention. `_php_interfaces` is the - # pre-#12 spelling, carrying interfaces alone; readers still accept it, - # so a graph.json written before enums and traits joined the set keeps - # refusing what it does name. + # declaring file used to drop out of the refusal entirely and the + # receiver bound to a stranger class sharing the short name (#11). Also + # stamp the names on the FILE node — the marker rides the node dict into + # graph.json and back in as resolution context, the same channel + # `_callable` uses (#2438). The file node is the host because the marker + # describes the file's declarations as a set; the names are listed + # explicitly rather than read off the node's `.php` label, which + # would only hold under one-declaration-per-file PSR-4 convention. + # `_php_interfaces` is the pre-#12 spelling, carrying interfaces alone; + # readers still accept it, so a graph.json written before enums and + # traits joined the set still hands back what it does name. for n in nodes: if n["id"] == file_nid: n["_php_non_class_types"] = list(result["php_non_class_types"]) break if php_class_fqns: - # The `namespace` this file declares for each class it defines, so the - # inline-`new` corroboration can compare a written FQN against the real - # one instead of guessing from the path (#14). Same `{"path": …}` shape - # as the type tables, which the cache re-anchors on load. + # The `namespace` this file declares for each type it defines — classes + # plus, since #53, interfaces/enums/traits — so the inline-`new` + # corroboration and the `use`-claim guard can compare a written FQN + # against the real one instead of guessing from the path (#14). Same + # `{"path": …}` shape as the type tables, which the cache re-anchors on + # load. result["php_class_fqns"] = {"path": str_path, "classes": php_class_fqns} # Like `_php_non_class_types` above, the payload only reaches the # resolver for files dispatched THIS run — but the declared-FQN diff --git a/graphify/extractors/php.py b/graphify/extractors/php.py index 5fcc1e8ec..74281ea52 100644 --- a/graphify/extractors/php.py +++ b/graphify/extractors/php.py @@ -101,6 +101,13 @@ def _php_fqn_names_another_class( from a different class. Refusing there would delete true edges on incremental rebuilds only, where the declaration is what is missing; persisting it for unchanged files is #23. + + That second branch is why every bindable declaration KIND has to reach the + declared-FQN map: an `interface`/`enum`/`trait` left out of it was judged by + path alone, and a replayed context node's relativized path has fewer + segments than a 4-segment vendor FQN — so `use Illuminate\\Contracts\\…\\ + Notifier;` read as "no evidence" and kept an edge onto the in-corpus + `App\\Contracts\\Notifier` it provably does not name (#53). """ if type_node is None: return False @@ -138,14 +145,17 @@ class PhpNameResolver: The refusal (#21) is strictly subtractive; the declared-FQN index layered on top (#22) is its additive counterpart. When the claimed FQN matches the name - some in-corpus class's own file DECLARES (#14), that match outranks the + some in-corpus type's own file DECLARES (#14), that match outranks the short-name census: it picks the imported one of several same-short-named - classes, and follows a renaming alias (``use App\\X as Y;``) to a class the - written short name never would have found. The index only knows classes - whose declared FQNs are available — for a file left undispatched by an - incremental rebuild that is the persisted ``_php_class_fqns`` marker (#23); - a graph written before that marker simply yields no match and the verdict - falls back to the #21 rules, adding no edge rather than a wrong one. + types, and follows a renaming alias (``use App\\X as Y;``) to a type the + written short name never would have found. The index only knows types whose + declared FQNs are available — for a file left undispatched by an incremental + rebuild that is the persisted ``_php_class_fqns`` marker (#23); a graph + written before that marker simply yields no match and the verdict falls back + to the #21 rules, adding no edge rather than a wrong one. ``interface``, + ``enum`` and ``trait`` declarations are in the index since #53 made them + bindable receiver types; without their FQNs the #21 rules judged them by + PSR-4 path alone, which is weaker and, on replayed context nodes, wrong. """ def __init__( diff --git a/graphify/serve.py b/graphify/serve.py index 4726a523e..0b1f019ef 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -425,6 +425,21 @@ class _QueryScores(NamedTuple): best_seed_by_term: dict[str, str] +def _is_sourceless(data: dict) -> bool: + """True when a node carries no `source_file` — extractor placeholder, not a + declaration. + + A sourceless node is a stub the extractor minted for a reference it could not + resolve (an unresolved base type, a dangling import target); serve also + materializes attributeless nodes for dangling edge endpoints, which have no + `source_file` key at all. Either way, when a real sourced declaration carries + the same label the stub is a broken duplicate of it and never the better + answer — the same presence test (never a count threshold) `_find_node_tiers` + applies to its exact tier (#49). + """ + return not str(data.get("source_file") or "") + + def _score_nodes(G: nx.Graph, terms: list[str]) -> list[tuple[float, str]]: """Combined query scorer returning the existing ranked `(score, node_id)` list. @@ -442,9 +457,11 @@ def _score_query( """Single-pass combined scorer that optionally also records the best seed for each normalized query token. - The combined ranking is byte-identical to what `_score_nodes` produced - before the refactor; `_score_nodes` is now a thin wrapper that asks for - `collect_per_term_seeds=False` and returns only `.ranked`. + The combined ranking is the one `_score_nodes` has always returned — the + single-pass refactor left it byte-identical, and the only deliberate change + since is #54's sourced-preference tie-break in the sort below, which reorders + exact score ties and nothing else. `_score_nodes` is now a thin wrapper that + asks for `collect_per_term_seeds=False` and returns only `.ranked`. When `collect_per_term_seeds=True`, the per-token singleton winner is computed alongside the combined score in the *same* per-node visit (it @@ -453,14 +470,17 @@ def _score_query( straight into `_pick_seeds` and skip the T additional whole-graph rescoring passes the old per-token `_score_nodes([token])` loop ran. - Singleton-winner semantics match the legacy per-token path exactly. The - score itself mirrors `_score_nodes([token])` with `n_terms == 1` (so the - coverage term is 1 and the per-token tier is unscaled) plus the broader - joined-singlet tier (which also checks `label_tokens` and `nid_lower`). - Tie-break order is (1) highest singleton score, (2) highest graph degree, - (3) shortest displayed label, (4) lexicographically smallest node id — - exactly what `max(tied, key=degree)` over a sort by `(-score, label_len, - nid)` produced in the legacy `_pick_seeds` per-token loop. The combined + Singleton-winner semantics match the legacy per-token path, with #54's + sourced preference layered on top of it. The score itself mirrors + `_score_nodes([token])` with `n_terms == 1` (so the coverage term is 1 and + the per-token tier is unscaled) plus the broader joined-singlet tier (which + also checks `label_tokens` and `nid_lower`). Tie-break order is (1) highest + singleton score, (2) a sourced node over a sourceless stub, (3) highest + graph degree, (4) shortest displayed label, (5) lexicographically smallest + node id. Everything but (2) is exactly what `max(tied, key=degree)` over a + sort by `(-score, label_len, nid)` produced in the legacy `_pick_seeds` + per-token loop; (2) is #54's, and sits where the combined sort below puts + it — directly under the score, above degree. The combined trigram candidate set (needles `norm_terms + [joined]`) is a superset of each per-token `[t]` candidate set, so iterating combined candidates discovers every non-zero singleton-score node for every term. @@ -588,8 +608,14 @@ def _score_query( # Tie-break key mirrors the legacy sort+max(degree): # (-singleton, -degree, label_len, nid) — the minimum # tuple wins, exactly matching max(tied, key=degree) - # over (label_len asc, nid asc)-sorted ties. - key = (-singleton, -G.degree(nid), len(data.get("label") or nid), nid) + # over (label_len asc, nid asc)-sorted ties — with the + # sourced preference (#54) inserted directly under the + # score, exactly where the combined sort below puts it, so + # a term's per-token winner cannot be a stub that a sourced + # rival ties with. Same carve-out: an all-sourceless field + # still yields a winner. + key = (-singleton, _is_sourceless(data), -G.degree(nid), + len(data.get("label") or nid), nid) cur = best_by_term.get(t) if cur is None or key < cur[0]: best_by_term[t] = (key, nid) @@ -597,9 +623,26 @@ def _score_query( score += tiered * (matched / n_terms) ** 2 if score > 0: scored.append((score, nid)) - # Sort by score desc; break ties toward the shorter label so a concise exact - # match beats a longer superset that happens to share the same score. - scored.sort(key=lambda s: (-s[0], len(G.nodes[s[1]].get("label") or s[1]), s[1])) + # Sort by score desc; among equal scores prefer a sourced declaration over a + # sourceless stub (#54), then break remaining ties toward the shorter label so + # a concise exact match beats a longer superset that happens to share the same + # score. + # + # The sourced preference sits directly under the score because a stub can + # never *out*-score an otherwise-identical sourced node — the source-file tier + # above only ever adds — so a shadowing stub always arrives here as an exact + # tie, and both remaining keys (label length, node id) are arbitrary with + # respect to which node is real: on the #54 repro two `FooRepository` nodes + # score 5619.08 apiece and the id sort answered with the stub, while + # `_find_node`/`resolve_seed` answered with the declaration. This is #49's rule + # applied to the tie the scored path actually produces; ordering it above the + # label-length key mirrors `_find_node_tiers`, which drops stubs from the tier + # outright rather than ranking them within it. Ties with no sourced candidate + # at all are untouched, so a lone stub still wins its query (#49's carve-out). + scored.sort(key=lambda s: ( + -s[0], _is_sourceless(G.nodes[s[1]]), + len(G.nodes[s[1]].get("label") or s[1]), s[1], + )) best_seed_by_term: dict[str, str] = {} if collect_per_term_seeds and best_by_term: best_seed_by_term = {t: nid for t, (_key, nid) in best_by_term.items()} diff --git a/graphify/watch.py b/graphify/watch.py index 2a257c75e..fbd82fc55 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -1286,14 +1286,16 @@ def _add_deleted_source(path: Path) -> None: # thing that lets an unchanged target pass the # indirect_call guard — never re-derived from the label. # `_php_non_class_types` (#11, #12) rides the same channel: - # it is the only way an unchanged PHP file declaring an - # interface, enum or trait keeps refusing such a receiver on - # an incremental rebuild. `_php_interfaces` is that marker's - # pre-#12 spelling, carried so a graph.json written before + # it is the only record of which of an unchanged PHP file's + # declarations are interfaces, enums or traits rather than + # classes. It drove a receiver refusal until #53 lifted it; + # it is still carried, and `_php_interfaces` — that marker's + # pre-#12 spelling — with it, so a graph.json written before # enums and traits joined the set still round-trips. - # `_php_class_fqns` (#23) is the positive counterpart: the - # declared FQNs that let a claimed `use` import keep binding - # into an unchanged defining file (#22). + # `_php_class_fqns` (#23) is what resolution reads today: + # the declared FQNs that let a claimed `use` import bind + # into an unchanged defining file (#22) and let the guard + # refuse a vendor import that merely shares a short name. for marker in ("_callable", "_callable_class", "_php_non_class_types", "_php_interfaces", "_php_class_fqns"): diff --git a/pyproject.toml b/pyproject.toml index bfb646227..0693926dd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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" diff --git a/tests/test_php_ctor_body_receiver.py b/tests/test_php_ctor_body_receiver.py index d2802c234..b6416bcb5 100644 --- a/tests/test_php_ctor_body_receiver.py +++ b/tests/test_php_ctor_body_receiver.py @@ -622,9 +622,16 @@ def test_assignment_in_a_non_constructor_method_emits_no_edge(tmp_path: Path): assert _no_search_edge(calls, index) -def test_interface_typed_param_assigned_to_a_property_emits_no_edge(tmp_path: Path): - """The interface/enum/trait refusal (#5, #12) applies to a ctor-body-bound - receiver too: an interface names no implementation.""" +def test_interface_typed_param_assigned_to_a_property_binds_the_interface(tmp_path: Path): + """A ctor-body-bound receiver typed with an INTERFACE binds to the + interface's own method (#53), not to a same-short-named implementation. + + #5/#12 refused this outright, back when an interface minted no definition + node and the only thing a receiver could land on was a stranger class. Post + #47/#53 the contract has its own `search()` node and the `use` names it + unambiguously — `App\\Services\\LeadHunterService` is a DIFFERENT type that + the controller never imported, and it stays unbound, which is the whole + point the original refusal was protecting.""" calls, r = _calls(tmp_path, { "app/Contracts/LeadHunterService.php": ( "m()``, +``Class::m()`` or first-class-callable syntax. The shared cross-file pass in +``extract()`` matched by normalized label, and the label index strips the +member marker (``.event()`` -> ``event``), so Laravel's ``event(...)`` helper +bound to whatever class happened to declare an ``event()`` METHOD — 848 +incoming ``calls`` edges on a single test method in the measured corpus. + +Refusal is language-semantic, not a heuristic: methods are simply not +candidates at a function-call site. Real global functions still resolve, and +no other language's candidate filtering changes (Ruby/Python bare calls keep +today's behavior — implicit-self dispatch makes a method a plausible target +there). +""" +from __future__ import annotations + +from pathlib import Path + +from graphify.extract import extract + +CALLER = ( + " source) and return (result, {(src, tgt): edge}).""" + paths = [] + for name, body in files.items(): + path = tmp_path / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + paths.append(path) + result = extract(paths, cache_root=tmp_path / "graphify-out") + calls = { + (edge["source"], edge["target"]): edge + for edge in result["edges"] + if edge.get("relation") == "calls" + } + return result, calls + + +def _find(result: dict, label: str, id_contains: str) -> str: + nid = next( + ( + node["id"] + for node in result["nodes"] + if node.get("label") == label and id_contains in node["id"] + ), + None, + ) + assert nid is not None, f"no node {label}/{id_contains}" + return nid + + +def test_function_call_does_not_bind_to_method(tmp_path): + """`event(...)` + a class method `event()` elsewhere -> no `calls` edge.""" + result, calls = _extract(tmp_path, { + "app/Caller.php": CALLER, + "app/Decoy.php": DECOY_METHOD, + }) + caller = _find(result, ".handle()", "caller") + method = _find(result, ".event()", "decoy") + assert (caller, method) not in calls + + +def test_function_call_still_binds_to_global_function(tmp_path): + """The legitimate global-function resolution must not regress.""" + result, calls = _extract(tmp_path, { + "app/Caller.php": CALLER, + "app/helpers.php": GLOBAL_FUNCTION, + }) + caller = _find(result, ".handle()", "caller") + function = _find(result, "event()", "helpers") + assert (caller, function) in calls + + +def test_function_wins_when_method_and_function_both_exist(tmp_path): + """With both candidates present the call binds to the function, never the method.""" + result, calls = _extract(tmp_path, { + "app/Caller.php": CALLER, + "app/Decoy.php": DECOY_METHOD, + "app/helpers.php": GLOBAL_FUNCTION, + }) + caller = _find(result, ".handle()", "caller") + method = _find(result, ".event()", "decoy") + function = _find(result, "event()", "helpers") + assert (caller, method) not in calls + assert (caller, function) in calls + + +def test_case_insensitive_path_does_not_bind_to_method(tmp_path): + """`EVENT(...)` matches `.event()` only via the folded index — still refused.""" + result, calls = _extract(tmp_path, { + "app/Caller.php": CALLER_FOLDED, + "app/Decoy.php": DECOY_METHOD, + }) + caller = _find(result, ".handle()", "caller") + method = _find(result, ".event()", "decoy") + assert (caller, method) not in calls + + +def test_case_insensitive_path_still_binds_to_global_function(tmp_path): + """PHP is case-insensitive: `EVENT(...)` still resolves to `function event()`.""" + result, calls = _extract(tmp_path, { + "app/Caller.php": CALLER_FOLDED, + "app/helpers.php": GLOBAL_FUNCTION, + }) + caller = _find(result, ".handle()", "caller") + function = _find(result, "event()", "helpers") + assert (caller, function) in calls + + +def test_case_insensitive_function_wins_over_method(tmp_path): + """Folded path, both candidates: the function is picked, the method refused.""" + result, calls = _extract(tmp_path, { + "app/Caller.php": CALLER_FOLDED, + "app/Decoy.php": DECOY_METHOD, + "app/helpers.php": GLOBAL_FUNCTION, + }) + caller = _find(result, ".handle()", "caller") + method = _find(result, ".event()", "decoy") + function = _find(result, "event()", "helpers") + assert (caller, method) not in calls + assert (caller, function) in calls + + +def test_case_mismatched_function_resolves_when_method_shadows_exact_key(tmp_path): + """The folded index is retried after the refusal, not skipped. + + `event(...)` finds the METHOD `.event()` on the exact-case key, so the + folded fallback never fires on its own; refusing that sole candidate must + not lose `function Event()`, which PHP's case-insensitive function lookup + makes the real target and which only exists under the folded key. + """ + result, calls = _extract(tmp_path, { + "app/Caller.php": CALLER, + "app/Decoy.php": DECOY_METHOD, + "app/helpers.php": GLOBAL_FUNCTION_FOLDED, + }) + caller = _find(result, ".handle()", "caller") + method = _find(result, ".event()", "decoy") + function = _find(result, "Event()", "helpers") + assert (caller, method) not in calls + assert (caller, function) in calls + + +def test_function_call_does_not_bind_to_class_through_folded_retry(tmp_path): + """A CLASS is no more invocable by `name(...)` than a method is. + + `report($e)` with a `.report()` method holding the exact-case key: the + refusal empties that list, and the folded retry then reaches the + capitalized `class Report` — a candidate the exact-case path never offered. + """ + result, calls = _extract(tmp_path, { + "app/Caller.php": CALLER_REPORT, + "app/Decoy.php": DECOY_REPORT_METHOD, + "app/Report.php": CLASS_REPORT, + }) + caller = _find(result, ".handle()", "caller") + method = _find(result, ".report()", "decoy") + klass = _find(result, "Report", "report") + assert (caller, klass) not in calls + assert (caller, method) not in calls + + +def test_function_call_does_not_bind_to_class_unshadowed(tmp_path): + """`foo(...)` + only `class Foo` cross-file -> no edge (no method involved).""" + result, calls = _extract(tmp_path, { + "app/Caller.php": ( + " bool: return any(src == caller and "notify" in tgt.lower() for src, tgt in calls) +def test_unique_interface_typed_property_binds_to_the_interface_method(tmp_path: Path): + """#53 criterion 1: `private Notifier $n` + `$this->n->send()` where the + corpus holds exactly ONE `Notifier` — the interface — binds to the + interface's own `send()` declaration node (post-#47 it has one).""" + calls, r = _calls(tmp_path, { + **_UNIQUE_IFACE_CORPUS, + "app/Http/Dispatcher.php": ( + "notifier->send('x'); }\n" + "}\n" + ), + }) + + go = _find(r, ".go()", "dispatcher") + contract_send = _find(r, ".send()", "contracts_notifier") + assert (go, contract_send) in calls, \ + "an unambiguous interface-typed receiver names the interface's method" + assert (go, _find(r, ".send()", "auditlog")) not in calls, \ + "the same method name on an unrelated class is not the receiver's type" + edge = calls[(go, contract_send)] + assert edge["confidence"] == "INFERRED" + assert edge["confidence_score"] == 0.8 + assert edge["context"] == "call" + + +def test_unique_interface_binds_through_the_single_definition_guard(tmp_path: Path): + """The same binding with NO `use` import to claim the name, so + `PhpNameResolver` abstains and the corpus-wide short-name census decides — + the plain single-definition guard path. Both files sit in the global + namespace, which is what makes the unqualified annotation name the + interface.""" + calls, r = _calls(tmp_path, { + "app/Contracts/Notifier.php": ( + "notifier->send('x'); }\n" + "}\n" + ), + }) + + go = _find(r, ".go()", "dispatcher") + assert (go, _find(r, ".send()", "contracts_notifier")) in calls + assert (go, _find(r, ".send()", "auditlog")) not in calls + + +def test_interface_typed_param_binds_to_the_interface_method(tmp_path: Path): + """The typed-parameter receiver path (#4) reaches the interface too.""" + calls, r = _calls(tmp_path, { + **_UNIQUE_IFACE_CORPUS, + "app/Http/Dispatcher.php": ( + "send('x'); }\n" + "}\n" + ), + }) + + go = _find(r, ".go()", "dispatcher") + assert (go, _find(r, ".send()", "contracts_notifier")) in calls + assert (go, _find(r, ".send()", "auditlog")) not in calls + + +def test_colliding_interface_and_class_short_name_emits_no_edge(tmp_path: Path): + """#53 criterion 3, on the guard the lifted refusal hands the job to: with + `App\\Contracts\\Notifier` (interface) and `App\\Support\\Notifier` (class) + both in the corpus the short name censuses TWO definitions, so the + single-definition guard refuses on its own. No `use` import here — the + caller shares the interface's namespace, which is what makes the bare + annotation name it — so `PhpNameResolver` abstains and the guard is the only + thing standing between the receiver and the stranger. PHP itself would bind + the interface; refusing is a recall gap, never a wrong edge.""" + calls, r = _calls(tmp_path, { + **_IFACE_CORPUS, + "app/Contracts/Dispatcher.php": ( + "notifier->notify('x'); }\n" + "}\n" + ), + }) + + go = _find(r, ".go()", "dispatcher") + assert (go, _find(r, ".notify()", "support_notifier")) not in calls, \ + "the same-short-named class is not the interface the receiver declares" + assert not _notified(calls, go) + + def test_interface_typed_property_does_not_guess_implementation(tmp_path: Path): + """The contract is the receiver's type; its implementations are not. + + `MailNotifier implements Notifier` is the only class that could satisfy the + annotation at runtime, and it still gets NOTHING — an interface names a + contract, and picking one implementation out of the corpus is the guess #5 + forbade and #53 does not reintroduce. What the receiver does bind is the + interface's OWN `notify()` node (#47), which is where the fan-in belongs.""" calls, r = _calls(tmp_path, { **_IFACE_CORPUS, "app/Http/Dispatcher.php": ( @@ -1193,13 +1329,20 @@ def test_interface_typed_property_does_not_guess_implementation(tmp_path: Path): go = _find(r, ".go()", "dispatcher") assert (go, _find(r, ".notify()", "mailnotifier")) not in calls, \ "an interface names a contract, not an implementation — never guess" - assert not _notified(calls, go) + assert (go, _find(r, ".notify()", "contracts_notifier")) in calls -def test_interface_short_name_collision_emits_no_edge(tmp_path: Path): +def test_interface_short_name_collision_binds_the_imported_one(tmp_path: Path): """`App\\Contracts\\Notifier` (interface) and `App\\Support\\Notifier` - (unrelated class): exactly one DEFINITION exists, so the ambiguity guard - alone would happily bind the call to the stranger.""" + (unrelated class) share a short name, and the calling file's `use` says + WHICH one it means. + + #5 could only refuse here: the interface minted no node, so the short-name + census saw the stranger alone and would have bound it. Post-#47/#53 both + are definitions, and the declared-FQN index (#22, extended to non-class + declarations for #53) matches the imported FQN against the name the + interface's own file declares — decisively, without ever consulting the + census. The stranger is still what must not be bound.""" calls, r = _calls(tmp_path, { **_IFACE_CORPUS, "app/Http/Dispatcher.php": ( @@ -1216,11 +1359,13 @@ def test_interface_short_name_collision_emits_no_edge(tmp_path: Path): go = _find(r, ".go()", "dispatcher") assert (go, _find(r, ".notify()", "support_notifier")) not in calls, \ "the same-short-named class is not the interface the receiver declares" - assert not _notified(calls, go) + assert (go, _find(r, ".notify()", "contracts_notifier")) in calls -def test_interface_refusal_is_case_insensitive(tmp_path: Path): - """PHP type names are case-insensitive: `notifier` IS `Notifier`.""" +def test_interface_binding_is_case_insensitive(tmp_path: Path): + """PHP type names are case-insensitive: `notifier` IS `Notifier`, so the + lowercase annotation reaches the same interface and still never the + same-short-named class.""" calls, r = _calls(tmp_path, { **_IFACE_CORPUS, "app/Http/Dispatcher.php": ( @@ -1235,11 +1380,12 @@ def test_interface_refusal_is_case_insensitive(tmp_path: Path): }) go = _find(r, ".go()", "dispatcher") - assert not _notified(calls, go) + assert (go, _find(r, ".notify()", "contracts_notifier")) in calls + assert (go, _find(r, ".notify()", "support_notifier")) not in calls -def test_interface_typed_param_emits_no_edge(tmp_path: Path): - """The typed-parameter receiver path (#4) refuses interfaces too.""" +def test_interface_typed_param_binds_the_imported_interface(tmp_path: Path): + """The typed-parameter receiver path (#4) reaches the interface too.""" calls, r = _calls(tmp_path, { **_IFACE_CORPUS, "app/Http/Dispatcher.php": ( @@ -1253,13 +1399,19 @@ def test_interface_typed_param_emits_no_edge(tmp_path: Path): }) go = _find(r, ".go()", "dispatcher") + assert (go, _find(r, ".notify()", "contracts_notifier")) in calls assert (go, _find(r, ".notify()", "support_notifier")) not in calls - assert not _notified(calls, go) + assert (go, _find(r, ".notify()", "mailnotifier")) not in calls def test_interface_inline_new_emits_no_edge(tmp_path: Path): - """The inline-new receiver path (#3) refuses interfaces too — an interface - cannot be instantiated, so such a receiver must never bind a stranger.""" + """The inline-new receiver path (#3) emits nothing here, and for the reason + that survives the lift: this file imports nothing, and an inline `new` + carries its written namespace on `receiver_qualified` — which only feeds the + EXTRACTED promotion, not `resolve_type_name`. So the resolver abstains, the + short name censuses TWO `Notifier` definitions, and the single-definition + guard refuses. `new` on an interface is invalid PHP anyway; the invariant + worth pinning is that the same-short-named stranger is never what it binds.""" calls, r = _calls(tmp_path, { **_IFACE_CORPUS, "app/Http/Dispatcher.php": ( @@ -1278,8 +1430,11 @@ def test_interface_inline_new_emits_no_edge(tmp_path: Path): assert not _notified(calls, go) -def test_interface_typed_local_new_emits_no_edge(tmp_path: Path): - """The typed-local receiver path (#4) refuses interfaces too.""" +def test_interface_typed_local_new_binds_the_imported_interface(tmp_path: Path): + """The typed-local receiver path (#4): `$n = new Notifier()` is broken PHP + for an interface, but the local's declared type is still the name the file + imported — so it binds the interface it NAMES, never the stranger the short + name would otherwise census.""" calls, r = _calls(tmp_path, { **_IFACE_CORPUS, "app/Http/Dispatcher.php": ( @@ -1297,12 +1452,12 @@ def test_interface_typed_local_new_emits_no_edge(tmp_path: Path): go = _find(r, ".go()", "dispatcher") assert (go, _find(r, ".notify()", "support_notifier")) not in calls - assert not _notified(calls, go) + assert (go, _find(r, ".notify()", "contracts_notifier")) in calls def test_class_receiver_still_resolves_when_an_interface_exists(tmp_path: Path): - """The refusal is name-scoped: a CLASS-typed receiver still resolves, and - the same-named interface elsewhere in the corpus changes nothing.""" + """Name scoping: a CLASS-typed receiver resolves to its class, and the + same-named interface elsewhere in the corpus changes nothing.""" calls, r = _calls(tmp_path, { **_IFACE_CORPUS, "app/Audit/AuditTrail.php": ( @@ -1326,15 +1481,15 @@ def test_class_receiver_still_resolves_when_an_interface_exists(tmp_path: Path): assert (go, _find(r, ".notify()", "support_notifier")) not in calls -# ── Enum- and trait-typed receivers are refused (#12) ──────────────────────── +# ── Enum- and trait-typed receivers (#12, #53) ─────────────────────────────── # -# `enum_declaration` and `trait_declaration` mint no definition node either, so -# they leak exactly like interfaces did before #5: `App\Enums\Status` (enum) -# beside an unrelated `App\Legacy\Status` (class) leaves ONE definition under -# that short name, and the single-definition guard binds the stranger. The -# Laravel shape is an enum mirroring a model. Enums and traits are added to the -# refusal pre-scan only — they still mint no nodes, so an enum's own methods -# stay unresolvable as call targets (a deliberate recall gap, not a wrong edge). +# Pre-#47 `enum_declaration` and `trait_declaration` minted no definition node +# either, so they leaked exactly like interfaces did before #5: `App\Enums\Status` +# (enum) beside an unrelated `App\Legacy\Status` (class) left ONE definition under +# that short name, and the single-definition guard bound the stranger. The +# Laravel shape is an enum mirroring a model. Post-#47/#53 the collision censuses +# two definitions and is refused on that basis, while an enum's own methods ARE +# call targets — the recall gap #12 documented is closed below. _ENUM_CORPUS = { "app/Enums/Status.php": ( @@ -1365,9 +1520,11 @@ def _runner(body: str) -> str: ) -def test_enum_typed_property_emits_no_edge(tmp_path: Path): - """`private Status $status;` where Status is an enum: the same-short-named - `App\\Legacy\\Status` class is a total stranger, never the receiver.""" +def test_enum_typed_property_binds_the_imported_enum(tmp_path: Path): + """`private Status $status;` where Status is the imported enum: the + same-short-named `App\\Legacy\\Status` class is a total stranger and never + the receiver, while the enum's own `label()` (#47) is exactly what the + annotation names.""" calls, r = _calls(tmp_path, { **_ENUM_CORPUS, "app/Runner.php": _runner( @@ -1378,10 +1535,10 @@ def test_enum_typed_property_emits_no_edge(tmp_path: Path): go = _find(r, ".go()", "runner") assert (go, _find(r, ".label()", "legacy_status")) not in calls - assert not _labelled(calls, go) + assert (go, _find(r, ".label()", "enums_status")) in calls -def test_enum_promoted_ctor_param_emits_no_edge(tmp_path: Path): +def test_enum_promoted_ctor_param_binds_the_imported_enum(tmp_path: Path): calls, r = _calls(tmp_path, { **_ENUM_CORPUS, "app/Runner.php": _runner( @@ -1392,10 +1549,10 @@ def test_enum_promoted_ctor_param_emits_no_edge(tmp_path: Path): go = _find(r, ".go()", "runner") assert (go, _find(r, ".label()", "legacy_status")) not in calls - assert not _labelled(calls, go) + assert (go, _find(r, ".label()", "enums_status")) in calls -def test_enum_typed_param_emits_no_edge(tmp_path: Path): +def test_enum_typed_param_binds_the_imported_enum(tmp_path: Path): calls, r = _calls(tmp_path, { **_ENUM_CORPUS, "app/Runner.php": _runner( @@ -1405,12 +1562,13 @@ def test_enum_typed_param_emits_no_edge(tmp_path: Path): go = _find(r, ".go()", "runner") assert (go, _find(r, ".label()", "legacy_status")) not in calls - assert not _labelled(calls, go) + assert (go, _find(r, ".label()", "enums_status")) in calls -def test_enum_fqn_typed_property_emits_no_edge(tmp_path: Path): +def test_enum_fqn_typed_property_binds_the_written_enum(tmp_path: Path): """The sharpest form: the source names `\\App\\Enums\\Status` outright, so - binding `App\\Legacy\\Status` contradicts the written type.""" + binding `App\\Legacy\\Status` would contradict the written type — and the + written type is precisely what the declared-FQN index now matches.""" calls, r = _calls(tmp_path, { **_ENUM_CORPUS, "app/Runner.php": _runner( @@ -1421,11 +1579,13 @@ def test_enum_fqn_typed_property_emits_no_edge(tmp_path: Path): go = _find(r, ".go()", "runner") assert (go, _find(r, ".label()", "legacy_status")) not in calls - assert not _labelled(calls, go) + assert (go, _find(r, ".label()", "enums_status")) in calls -def test_enum_typed_local_new_emits_no_edge(tmp_path: Path): - """The typed-local receiver path (#4) refuses enums too.""" +def test_enum_typed_local_new_binds_the_imported_enum(tmp_path: Path): + """The typed-local receiver path (#4): `new Status()` is broken PHP for an + enum, but the local's declared type still names the imported enum and never + the stranger.""" calls, r = _calls(tmp_path, { **_ENUM_CORPUS, "app/Runner.php": _runner( @@ -1438,12 +1598,17 @@ def test_enum_typed_local_new_emits_no_edge(tmp_path: Path): go = _find(r, ".go()", "runner") assert (go, _find(r, ".label()", "legacy_status")) not in calls - assert not _labelled(calls, go) + assert (go, _find(r, ".label()", "enums_status")) in calls -def test_enum_inline_new_emits_no_edge(tmp_path: Path): - """The inline-new receiver path (#3) refuses enums too — an enum cannot be - instantiated, so such a receiver must never bind a stranger.""" +def test_enum_inline_new_binds_the_written_enum(tmp_path: Path): + """The inline-new receiver path (#3) contrasted with the interface one + above: `new` on an enum is equally invalid PHP, but here the runner DOES + `use App\\Enums\\Status`, so the claim is decided by the declared-FQN index + rather than left to the two-candidate census. The written + `\\App\\Enums\\Status` then corroborates the enum's declared name, which is + what promotes the edge to EXTRACTED. The stranger stays unbound either + way — that is the invariant #12 was protecting.""" calls, r = _calls(tmp_path, { **_ENUM_CORPUS, "app/Runner.php": _runner( @@ -1454,11 +1619,13 @@ def test_enum_inline_new_emits_no_edge(tmp_path: Path): }) go = _find(r, ".go()", "runner") + enum_label = _find(r, ".label()", "enums_status") assert (go, _find(r, ".label()", "legacy_status")) not in calls - assert not _labelled(calls, go) + assert (go, enum_label) in calls + assert calls[(go, enum_label)]["confidence"] == "EXTRACTED" -def test_enum_refusal_is_case_insensitive(tmp_path: Path): +def test_enum_binding_is_case_insensitive(tmp_path: Path): """PHP type names are case-insensitive: `status` IS `Status`.""" calls, r = _calls(tmp_path, { **_ENUM_CORPUS, @@ -1469,15 +1636,22 @@ def test_enum_refusal_is_case_insensitive(tmp_path: Path): }) go = _find(r, ".go()", "runner") - assert not _labelled(calls, go) + assert (go, _find(r, ".label()", "enums_status")) in calls + assert (go, _find(r, ".label()", "legacy_status")) not in calls -def test_enum_without_a_colliding_class_emits_no_edge(tmp_path: Path): - """Control: an enum mints no definition node, so its methods are not call - targets at all. The collision above supplies the only candidate — this - documents the (deliberate) recall gap that leaves.""" +def test_enum_without_a_colliding_class_binds_to_the_enum_method(tmp_path: Path): + """#53 criterion 2: drop the colliding class and `Status` names exactly one + type — the enum — whose `label()` hangs off its own declaration node + (#47). The receiver binds there. This is the recall gap #12 documented, + now closed; the decoy class proves it is the declared type doing the work + and not a bare method-name match.""" calls, r = _calls(tmp_path, { "app/Enums/Status.php": _ENUM_CORPUS["app/Enums/Status.php"], + "app/Models/Lead.php": ( + "status->label(); }" @@ -1485,12 +1659,21 @@ def test_enum_without_a_colliding_class_emits_no_edge(tmp_path: Path): }) go = _find(r, ".go()", "runner") - assert not _labelled(calls, go) + enum_label = _find(r, ".label()", "enums_status") + assert (go, enum_label) in calls, \ + "an unambiguous enum-typed receiver names the enum's own method" + assert (go, _find(r, ".label()", "models_lead")) not in calls + edge = calls[(go, enum_label)] + assert edge["confidence"] == "INFERRED" + assert edge["confidence_score"] == 0.8 -def test_trait_typed_receiver_emits_no_edge(tmp_path: Path): - """A trait is not a type, so a trait-typed receiver is already broken PHP — - but it must still refuse rather than bind the same-short-named class.""" +def test_trait_typed_receiver_binds_the_imported_trait(tmp_path: Path): + """A trait is not a type, so a trait-typed receiver is already broken PHP. + The graph follows the name the source actually writes — the imported + `App\\Support\\Cache` trait, whose `flush()` is a real node post-#47 — and + never the same-short-named `App\\Legacy\\Cache` class, which is what the + #12 refusal existed to prevent.""" calls, r = _calls(tmp_path, { "app/Support/Cache.php": ( " bool: + return any(src == caller and "send" in tgt.lower() for src, tgt in calls) + + +def test_vendor_import_shadowing_an_interface_refuses_on_both_builds(tmp_path: Path): + """A `use` of a same-short-named type from OUTSIDE the corpus must refuse — + on the incremental path as well as the full one (#16, #53). + + `use Illuminate\\Contracts\\Notifications\\Notifier;` CLAIMS the short name + for a vendor interface the corpus does not contain, so the in-corpus + `App\\Contracts\\Notifier` is a different type and must get no edge. That + verdict is `PhpNameResolver`'s (#21), and it is only decisive when the + declaration carries a declared FQN: without one the guard falls back to + comparing the node's PSR-4 PATH, and a replayed context node's path is + RELATIVIZED — fewer segments than the vendor FQN has — which trips the + "not enough evidence" bail-out and binds. The full build never sees that + (its paths are still absolute at resolver time), so full and incremental + must be asserted TOGETHER or the hole hides in the mode `graphify update` + actually runs in.""" + (full_calls, full), (inc_calls, inc) = _full_then_incremental(tmp_path, { + "app/Contracts/Notifier.php": ( + "notifier->send('x'); }\n" + "}\n" + ), + }, changed=_INCR_DISPATCHER) + + go = _find(inc, ".go()", "dispatcher") + assert not _sent(full_calls, go), "full-build baseline must refuse the claim" + assert not _sent(inc_calls, go), \ + "a vendor `use` must not bind the in-corpus interface on a rebuild" + + +def test_vendor_import_shadowing_an_enum_refuses_on_both_builds(tmp_path: Path): + """The enum shape of the test above, same channel and same verdict.""" + (full_calls, full), (inc_calls, inc) = _full_then_incremental(tmp_path, { + "app/Enums/Status.php": _ENUM_CORPUS["app/Enums/Status.php"], + _INCR_RUNNER: ( + "status->label(); }\n" + "}\n" + ), + }, changed=_INCR_RUNNER) + + go = _find(inc, ".go()", "runner") + assert not _labelled(full_calls, go), "full-build baseline must refuse the claim" + assert not _labelled(inc_calls, go), \ + "a vendor `use` must not bind the in-corpus enum on a rebuild" + + +def test_unique_interface_binding_survives_incremental_rebuild(tmp_path: Path): + """#53 criterion 4: the interface's file is unchanged and therefore NOT + dispatched — its declaration node and its `method` edge reach the resolver + only through the #2437 replay channel. The binding must be the same one the + full build makes, or every interface edge would evaporate on the first + `graphify update`.""" + (full_calls, full), (inc_calls, inc) = _full_then_incremental(tmp_path, { + **_UNIQUE_IFACE_CORPUS, + _INCR_DISPATCHER: ( + "notifier->send('x'); }\n" + "}\n" + ), + }, changed=_INCR_DISPATCHER) + + go = _find(inc, ".go()", "dispatcher") + contract_send = _find(full, ".send()", "contracts_notifier") + assert (go, contract_send) in full_calls, "full-build baseline must bind" + assert (go, contract_send) in inc_calls, \ + "an undispatched interface file must keep its replayed binding" + assert (go, _find(full, ".send()", "auditlog")) not in inc_calls + + +# ENUM and TRAIT declarations replay over the same channel and get the same +# parity treatment (#12). Before #47 an unchanged `App\Enums\Status` file left +# `App\Legacy\Status` as the one visible definition on a rebuild — the wrong edge +# #12 had closed on the full-build path, coming straight back on the incremental +# one. Now it is the declared FQN that has to survive the replay for the two +# builds to agree; the assertions below pin both ends of that. _INCR_RUNNER = "app/Http/Runner.php" @@ -1748,9 +2032,11 @@ def _incr_enum_corpus(body: str) -> dict[str, str]: } -def test_enum_refusal_survives_incremental_rebuild(tmp_path: Path): - """The enum's file is unchanged and therefore NOT dispatched: its name must - still reach the resolver, so the rebuild agrees with the full build.""" +def test_enum_binding_agrees_across_an_incremental_rebuild(tmp_path: Path): + """The enum's file is unchanged and therefore NOT dispatched: its node, its + `method` edge and its declared FQN must all still reach the resolver, so the + rebuild agrees with the full build — and the same-short-named + `App\\Legacy\\Status` gets nothing on either.""" (full_calls, full), (inc_calls, inc) = _full_then_incremental( tmp_path, _incr_enum_corpus( @@ -1762,27 +2048,34 @@ def test_enum_refusal_survives_incremental_rebuild(tmp_path: Path): go = _find(inc, ".go()", "runner") stranger = _find(full, ".label()", "legacy_status") - assert not _labelled(full_calls, go), "full-build baseline must refuse" + enum_label = _find(full, ".label()", "enums_status") + assert (go, enum_label) in full_calls, "full-build baseline binds the enum" + assert (go, enum_label) in inc_calls, \ + "an undispatched enum file must keep its replayed binding" assert (go, stranger) not in inc_calls, \ - "an undispatched enum file must not hand the edge to App\\Legacy\\Status" - assert not _labelled(inc_calls, go) + "a rebuild must not hand the edge to App\\Legacy\\Status" -def test_enum_typed_param_refusal_survives_incremental_rebuild(tmp_path: Path): - """The typed-parameter entry point refuses across a rebuild too.""" - (_, full), (inc_calls, inc) = _full_then_incremental( +def test_enum_typed_param_binding_agrees_across_an_incremental_rebuild(tmp_path: Path): + """The typed-parameter entry point keeps full/incremental parity too.""" + (full_calls, full), (inc_calls, inc) = _full_then_incremental( tmp_path, _incr_enum_corpus(" public function go(Status $s): void { $s->label(); }"), changed=_INCR_RUNNER, ) go = _find(inc, ".go()", "runner") + enum_label = _find(full, ".label()", "enums_status") + assert (go, enum_label) in full_calls + assert (go, enum_label) in inc_calls assert (go, _find(full, ".label()", "legacy_status")) not in inc_calls - assert not _labelled(inc_calls, go) -def test_trait_refusal_survives_incremental_rebuild(tmp_path: Path): - (_, full), (inc_calls, inc) = _full_then_incremental(tmp_path, { +def test_trait_binding_agrees_across_an_incremental_rebuild(tmp_path: Path): + """A trait-typed receiver is broken PHP, but its verdict must not depend on + which files a rebuild happened to dispatch: the imported trait on both + builds, the same-short-named `App\\Legacy\\Cache` class on neither.""" + (full_calls, full), (inc_calls, inc) = _full_then_incremental(tmp_path, { "app/Support/Cache.php": ( "status->label(); }\n" + "}\n" + ), + }, changed=_INCR_RUNNER) + + go = _find(inc, ".go()", "runner") + enum_label = _find(full, ".label()", "enums_status") + assert (go, enum_label) in full_calls, "full-build baseline must bind" + assert (go, enum_label) in inc_calls, \ + "an undispatched enum file must keep its replayed binding" + assert (go, _find(full, ".label()", "models_lead")) not in inc_calls + + def test_legacy_php_interfaces_marker_spelling_is_still_read(tmp_path: Path): """Cache compatibility: a graph.json written before #12 carries the names - under `_php_interfaces`. Interfaces it names keep refusing — the rename must - not silently drop a channel that older graphs are still using.""" + under `_php_interfaces`, and both spellings must keep being read back off + the resolution context — the rename must not silently drop a channel that + older graphs are still using. + + #53 lifted the RECEIVER REFUSAL these names used to feed, so what is pinned + here is the channel itself: `_php_context_interface_entry` recovering the + unchanged corpus's declared names under either spelling. Resolution now runs + off a different marker on the same nodes — `_php_class_fqns` (#23) — which + is why the downgraded context still binds the imported contract and still + leaves `App\\Support\\Notifier` alone.""" (_, full), _ = _full_then_incremental(tmp_path, { **_IFACE_CORPUS, _INCR_DISPATCHER: ( @@ -1865,6 +2197,9 @@ def test_legacy_php_interfaces_marker_spelling_is_still_read(tmp_path: Path): node["_php_interfaces"] = names # the pre-#12 spelling downgraded += 1 assert downgraded == 1, "exactly the interface's file node carries the names" + assert _php_context_interface_entry(ctx_nodes) == { + "php_non_class_types": ["Notifier"] + }, "the pre-#12 spelling must still be recovered off the context nodes" caller = tmp_path / "corpus" / _INCR_DISPATCHER inc = extract([caller], cache_root=tmp_path / "corpus", @@ -1877,7 +2212,7 @@ def test_legacy_php_interfaces_marker_spelling_is_still_read(tmp_path: Path): go = _find(inc, ".go()", "dispatcher") assert (go, _find(full, ".notify()", "support_notifier")) not in inc_calls - assert not _notified(inc_calls, go) + assert (go, _find(full, ".notify()", "contracts_notifier")) in inc_calls # ── Same-file union / intersection receivers (user story 11, #9) ────────────── diff --git a/tests/test_scored_endpoint_sourced_preference.py b/tests/test_scored_endpoint_sourced_preference.py new file mode 100644 index 000000000..d596ac6ff --- /dev/null +++ b/tests/test_scored_endpoint_sourced_preference.py @@ -0,0 +1,236 @@ +"""The scored-endpoint path must prefer a sourced node over a sourceless stub +(#54, extending #49's rule from `_find_node` to `_score_nodes`). + +`_find_node_tiers` drops sourceless nodes from a mixed exact tier (#49), so +`explain` and `affected` resolve `FooRepository` to the real declaration. The +scored path `graphify query` / `shortest_path` run on — `_score_nodes` ranking, +then `_pick_scored_endpoint` / `_pick_seeds` — never learned that rule: a stub +and a sourced declaration sharing a label score *identically* (5619.08 on the +two-node repro below), so the score sort fell through to its node-id tie-break +and answered with whichever id sorted first. `graphify query` then traversed a +stub's neighborhood and `shortest_path` anchored on a disconnected placeholder +and reported "No path found" — while `explain` confidently named the other node. +The comment at serve.py:515-520 states the design intent this violated: `path` +and `query` resolve the same node `explain` does. + +The carve-out is #49's: the rule demotes a stub against a real declaration, it +does not delete it. When every candidate is sourceless the stub still wins. +""" +from __future__ import annotations + +import pytest +from networkx.readwrite import json_graph + +from graphify.affected import resolve_seed +from graphify.serve import ( + _find_node, + _pick_scored_endpoint, + _query_graph_text, + _score_nodes, + _score_query, + _shortest_path_text, +) + + +LABEL = "FooRepository" +SOURCED_ID = "zzz_src" +SOURCE_FILE = "app/bindings.php" + + +def _stub(index: int = 0, *, omit_source_key: bool = False) -> dict: + """A sourceless stub, id-sorted *ahead* of the sourced node (the repro shape). + + `omit_source_key` reproduces the attributeless node serve materializes for a + dangling edge endpoint (no `source_file` key at all, not an empty one). + """ + node = {"id": f"aaa_stub{index}", "label": LABEL, "community": 0} + if not omit_source_key: + node["source_file"] = "" + return node + + +def _sourced() -> dict: + return {"id": SOURCED_ID, "label": LABEL, "source_file": SOURCE_FILE, + "source_location": "L10", "community": 0} + + +def _load(nodes: list[dict], links: list[dict] | None = None): + return json_graph.node_link_graph( + {"directed": True, "multigraph": False, "graph": {}, + "nodes": nodes, "links": links or []}, + edges="links", + ) + + +def _endpoint(G, query: str = LABEL) -> str: + """Resolve `query` exactly as `shortest_path` resolves each of its endpoints.""" + scored = _score_nodes(G, [t.lower() for t in query.split()]) + assert scored, "precondition: the query must match something" + return _pick_scored_endpoint(G, scored, query) + + +# --- criterion 1: the reviewer's two-node repro ------------------------------ + + +def test_scored_endpoint_prefers_the_sourced_node(): + """Both nodes score 5619.08; the id sort used to hand back the stub.""" + G = _load([_stub(), _sourced()]) + scored = _score_nodes(G, ["foorepository"]) + assert {nid for _s, nid in scored} == {"aaa_stub0", SOURCED_ID} + assert scored[0][0] == pytest.approx(scored[1][0]), "the repro is a score tie" + assert _pick_scored_endpoint(G, scored, LABEL) == SOURCED_ID + + +@pytest.mark.parametrize("stub_count", [1, 2, 3, 18]) +def test_scored_endpoint_prefers_the_sourced_node_at_any_stub_count(stub_count): + """The rule is a presence test on `source_file`, never a count threshold.""" + G = _load([_stub(i) for i in range(stub_count)] + [_sourced()]) + assert _endpoint(G) == SOURCED_ID + + +def test_scored_endpoint_answer_does_not_depend_on_node_order(): + stubs = [_stub(i) for i in range(3)] + forward = _load(stubs + [_sourced()]) + reverse = _load([_sourced()] + list(reversed(stubs))) + assert _endpoint(forward) == _endpoint(reverse) == SOURCED_ID + + +def test_attributeless_dangling_endpoints_are_also_stubs(): + """A dangling edge endpoint has no `source_file` key at all — same rule.""" + G = _load([_stub(i, omit_source_key=True) for i in range(3)] + [_sourced()]) + assert _endpoint(G) == SOURCED_ID + + +# --- criterion 2: a stub with no sourced rival is still an answer ------------ + + +@pytest.mark.parametrize("omit_source_key", [False, True]) +def test_lone_stub_with_no_sourced_rival_still_resolves(omit_source_key): + """#49's carve-out: demoted against a real declaration, never deleted.""" + stub = _stub(0, omit_source_key=omit_source_key) + G = _load([stub]) + assert _endpoint(G) == stub["id"] + + +@pytest.mark.parametrize("stub_count", [2, 18]) +def test_all_sourceless_candidates_still_return_one(stub_count): + """Never return nothing where today something returns.""" + G = _load([_stub(i) for i in range(stub_count)]) + assert _endpoint(G) in {f"aaa_stub{i}" for i in range(stub_count)} + + +def test_query_over_only_stubs_still_traverses_them(): + G = _load( + [_stub(0), {"id": "beta", "label": "BetaService", "source_file": "app/Beta.php", + "community": 0}], + [{"source": "aaa_stub0", "target": "beta", "context": "call"}], + ) + out = _query_graph_text(G, LABEL) + assert "No matching nodes found." not in out + assert "BetaService" in out + + +# --- criterion 3: query / shortest_path agree with explain's _find_node ------ + + +@pytest.mark.parametrize("stub_count", [1, 2, 18]) +def test_scored_path_agrees_with_find_node_and_resolve_seed(stub_count): + G = _load([_stub(i) for i in range(stub_count)] + [_sourced()]) + assert _endpoint(G) == _find_node(G, LABEL)[0] == resolve_seed(G, LABEL) == SOURCED_ID + + +def _repro_graph_with_neighbors(): + """The repro pair, each node in its own component. + + `AlphaService` hangs off the sourced declaration, `BetaService` off the stub, + so which endpoint got picked is visible in the traversal output and in + whether `shortest_path` finds a path at all. Both edges point away from the + `FooRepository` node so the directed defaults of `query` and `shortest_path` + reach the neighbor. The two rivals necessarily share a label, so the endpoint + is identified by which neighbor the answer reaches, not by name. + """ + return _load( + [ + _stub(), + _sourced(), + {"id": "alpha", "label": "AlphaService", "source_file": "app/Alpha.php", + "source_location": "L4", "community": 0}, + {"id": "beta", "label": "BetaService", "source_file": "app/Beta.php", + "source_location": "L4", "community": 0}, + ], + [ + {"source": SOURCED_ID, "target": "alpha", "context": "call"}, + {"source": "aaa_stub0", "target": "beta", "context": "call"}, + ], + ) + + +def test_shortest_path_anchors_on_the_sourced_node(): + """Anchored on the stub, this query was a false "No path found".""" + G = _repro_graph_with_neighbors() + out = _shortest_path_text(G, {"source": LABEL, "target": "AlphaService"}) + assert "No path" not in out and "No directed path" not in out + assert "FooRepository --related--> AlphaService" in out + + +def test_query_traverses_the_sourced_nodes_neighborhood(): + G = _repro_graph_with_neighbors() + out = _query_graph_text(G, LABEL) + assert "AlphaService" in out + assert "BetaService" not in out + + +# --- the per-term seed winner needs the rule too ---------------------------- + + +def _decorated_pair(): + """A stub whose label normalizes *differently* from its sourced rival's. + + `_pick_seeds` dedupes seeds by normalized label, so on the same-label repro + above the stub's per-term entry is dropped before it can be seeded and the + combined-ranking half of the fix carries the whole outcome. A decorated + declaration breaks that cover: `handle` and `handle()` are distinct dedup + keys, so the stub keeps its own seat and `best_seed_by_term`'s own tie-break + is the only thing deciding which node fills it (the #49 suite pins the same + pair for `resolve_seed`'s bare-name pass). + """ + return _load([ + {"id": "aaa_stub_handle", "label": "handle", "source_file": "", "community": 0}, + {"id": "zzz_app_svc_handle", "label": "handle()", + "source_file": "app/Svc.php", "source_location": "L9", "community": 0}, + ]) + + +def test_per_term_seed_winner_prefers_the_sourced_node(): + """Pins the `best_seed_by_term` half of the fix, which the same-label repro + cannot reach — goes red if the singleton tie-break key drops its sourced + preference.""" + G = _decorated_pair() + qs = _score_query(G, ["handle"], collect_per_term_seeds=True) + assert qs.best_seed_by_term == {"handle": "zzz_app_svc_handle"} + + +def test_query_ranks_the_sourced_node_first_for_a_decorated_declaration(): + """The same pair through the query pipeline. Both labels survive the seed + dedupe here, so both get seeded — what the rule decides is the order, and the + top seed is what `_pick_seeds`' coverage check and the seed-first rendering + both key off. + """ + G = _decorated_pair() + out = _query_graph_text(G, "handle") + assert "Start: ['handle()', 'handle']" in out + assert out.index("NODE handle() [src=app/Svc.php") < out.index("NODE handle [src=") + + +# --- the pre-existing sourced-vs-sourced tie is untouched ------------------- + + +def test_two_sourced_rivals_keep_their_existing_tie_break(): + """Sourced-vs-sourced (#2032's symbol case) still resolves by the id sort.""" + G = _load([ + {"id": "chat_port", "label": "MetricsPort", + "source_file": "services/chat/ports/metrics.port.ts", "community": 0}, + {"id": "scrape_port", "label": "MetricsPort", + "source_file": "services/scraping/ports/metrics.port.ts", "community": 0}, + ]) + assert _endpoint(G, "MetricsPort") == "chat_port" diff --git a/tests/test_watch.py b/tests/test_watch.py index a8c6b37f2..b2a462d47 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -3384,14 +3384,19 @@ def test_incremental_indirect_call_parity_and_idempotency(tmp_path): assert sorted(_2438_indirects(_2406_graph(fresh))) == sorted(incremental) -# --- #11: PHP interface refusal survives an incremental rebuild -------------- -# A PHP `interface` mints no definition node, so the resolver learns the names -# from the extractor (#1682). On a rebuild the interface's own file is usually -# unchanged and therefore never dispatched, so the names must come back through -# the persisted graph — the `_php_interfaces` marker on the file node, the same -# channel `_callable` uses (#2438). Without it the refusal stopped applying and -# an `App\Contracts\Notifier`-typed receiver bound to the unrelated -# `App\Support\Notifier` class: a WRONG edge, not just a missing one. +# --- #11: an interface-typed receiver survives an incremental rebuild -------- +# The wrong edge #11 closed: an `App\Contracts\Notifier`-typed receiver binding +# to the unrelated `App\Support\Notifier` class because the interface's own file +# was unchanged and therefore never re-dispatched, leaving the stranger as the +# one visible definition. The facts the resolver needs about an undispatched +# file come back through the persisted graph — the markers on its file node, the +# same channel `_callable` uses (#2438). +# +# Post-#47/#53 the interface has a definition node and its own `notify()`, so +# the receiver BINDS THE CONTRACT rather than refusing; the stranger is still +# what must never be bound, and the rebuild must still agree with the full build +# (now via the `_php_class_fqns` marker, #23, which carries the declared FQN the +# `use`-claim guard compares against). _11_CALLER = ( "