Skip to content

fix(php): preserve use-statement facts and stop group-form use function/const from claiming class names - #2502

Open
filipechagas wants to merge 2 commits into
Graphify-Labs:v8from
lawnstarter:upstream-feat/php-use-metadata
Open

fix(php): preserve use-statement facts and stop group-form use function/const from claiming class names#2502
filipechagas wants to merge 2 commits into
Graphify-Labs:v8from
lawnstarter:upstream-feat/php-use-metadata

Conversation

@filipechagas

Copy link
Copy Markdown

Two coupled PHP use-statement fixes, backported from our fork where they landed with full red-first evidence (lawnstarter#29, lawnstarter#30). This PR is independent of #2492 (it touches only pre-existing code); a heads-up was posted there for context.

Commit 1 — capture use FQN/alias/kind metadata on imports edges (metadata-only)

_import_php threw away everything but the trailing short name (raw.split("\\")[-1]), unlike _import_csharp, which preserves the written FQN and alias. The already-correct use-parser inside _resolve_php_type_references (group use, aliases, function/const rejection) is extracted into shared helpers consumed by both paths, and PHP imports edges now carry use_kind / alias / target_fqn metadata.

No behavior change: metadata-stripped extract() output over a corpus covering plain / aliased / group / aliased-group / function / const / group-function / group-const / leading-backslash use, trait use, inheritance, and interfaces is sha256-identical before and after.

Commit 2 — group-form use function / use const no longer claim class names (strictly subtractive)

The grammar puts the function/const keyword on the clause for use function A\f; but on the declaration for use function A\{f, g}; (verified on tree-sitter-php 0.24.1). _resolve_php_type_references only ever honored the clause-level keyword, so group-form function/const names wrongly entered the class-name map — e.g. use function Vendor\Sdk\{render}; plus class I extends Render mis-repoints the inheritance. Now the declaration-level keyword is always honored; one code path, no compatibility flag.

Test evidence

  • Baseline established on this tree (v8 @ 07b9143): 3976 passed / 36 skipped.
  • Commit 1: red 7/8 (the passing one is a targets-unchanged guard, green by construction) → full suite 3984/36.
  • Commit 2: red 3/4 (over-subtraction guard green by design: keyword-less use A\{B}; still claims the class name, decoy gets no edge) → full suite 3988/36.
  • 12 new tests, all through the public extract() seam.

🤖 Generated with Claude Code

filipechagas and others added 2 commits August 6, 2026 08:17
PHP `imports` edges now carry `use_kind` / `alias` / `target_fqn` metadata,
mirroring `_import_csharp`. The already-correct `use`-parser inside
`_resolve_php_type_references` was extracted into shared helpers
(`_php_use_clause_fact`, `_php_use_clause_context`,
`_php_use_declaration_facts`) consumed by both the resolution pass and the
capture path, replacing `_import_php`'s lossy `raw.split("\\")[-1]`. Group use
`use A\{B, C as X};`, aliases, leading-backslash absolutes and
`use function` / `use const` are all handled in one place, so a clause
dispatched on its own (as `_import_php` is) can still spell its own FQN by
reading the group prefix and keyword off the parent declaration.

Strictly metadata-only: no resolver behavior change, `_PHP_CONFIG.import_types`
untouched, edge targets still keyed on the imported short name. Full
`extract()` output with metadata stripped, before vs after, over a corpus
covering plain / aliased / group / aliased-group / `function` / `const` /
group-function / group-const / leading-backslash `use`, trait `use`,
inheritance, interfaces and a typed member call: 16 nodes / 27 edges,
byte-identical (sha256 f6c6168f).

Group-form `use function A\{f, g};` and `use const A\{K};` put the keyword on
the declaration rather than the clause, so those names enter the class-name map
today; that pre-existing bug is deliberately preserved bit-for-bit here via
`apply_declaration_kind=False` and fixed in the follow-up commit. The new
metadata already reports the correct kind.

Note for consumers: the `use_kind` vocabulary is `class`/`function`/`const`
with `alias` as a separate key (unlike C#'s `using_kind == "alias"`), and
`_resolve_php_type_references` re-points `imports` edges without touching
metadata, so `metadata.target_fqn` is the reliable read rather than the target
node's label.

Tests: 8 new, all through the public `extract()` seam; 7 failed against
unfixed code (the 8th is the targets-unchanged guard, green by construction).
Full suite 3984 passed / 36 skipped (baseline 3976/36 + 8).

Adapted from fork PR #29

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Group-form `use function A\{f, g};` and `use const A\{K};` put the keyword on
the *declaration* node, not the clause, so those names wrongly entered
`_resolve_php_type_references`'s class-name map. A group-imported function or
constant whose short name was also used in a class position in the same file
therefore re-pointed that `inherits`/`implements`/`mixes_in`/`imports`/
`references` edge onto an external stub labeled with an FQN that names a
function or a constant, not a class.

The shared parser added in the previous commit already computed the correct
kind behind an `apply_declaration_kind=False` compatibility flag, which existed
only to keep that commit metadata-only. This removes the flag and its call
site, leaving one code path that always honors the declaration-level keyword,
so both spellings agree. Strictly subtractive: it can only remove a class-name
claim, never add one. The reference then falls back to the namespace-relative
FQN or to the legacy unique-label rewire, exactly as the unbraced form always
did.

Pre-existing, and rare in practice because it needs the same short name used
both as a group-imported function/constant and in a class position within one
file.

Tests: 4 new, all through the public `extract()` seam, each braced form paired
with its semantically equivalent unbraced control; 3 failed against unfixed
code, and the over-subtraction guard (`use App\Cms\{Page};` still claims the
class name, decoy `App\Models\Page` gets no edge) passes on both sides by
design. Full suite 3988 passed / 36 skipped (3984/36 + 4).
`grep -rn apply_declaration_kind` across the repo now returns nothing.

Adapted from fork PR #30

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).


Graphify review — findings

This PR reworks how PHP use statements are parsed during graph extraction. It introduces a shared use-statement parser (_php_use_clause_fact, _php_use_clause_context, _php_use_declaration_facts) in resolution.py and updates both _import_php (the imports edge capture) and _resolve_php_type_references to use it, so group-use syntax, aliases, absolute names, and function/const keyword placement are handled consistently. It also adds use_kind, alias, and target_fqn metadata to PHP imports edges, and adds a new test file plus CHANGELOG entries. The surface area spans graphify/extract.py, graphify/extractors/resolution.py, a new tests/test_php_group_use_kind.py, and CHANGELOG updates.

No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1673 functions depend on the 511 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: extract() — 379 callers, 39 callees

Verification — 1673 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 1543 function(s) in the blast radius were not formally verified this run

· 1 more finding(s) on lines outside this diff (see the check run).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant