Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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\<Area>\<Name>` 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.
Expand Down
18 changes: 10 additions & 8 deletions graphify/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand Down
Loading