Skip to content

PHP: give use A\B\C as D the imported class's identity (#3421) - #3453

Open
shivamtiwari3 wants to merge 1 commit into
Graphify-Labs:v8from
shivamtiwari3:fix/3421-php-use-alias
Open

PHP: give use A\B\C as D the imported class's identity (#3421)#3453
shivamtiwari3 wants to merge 1 commit into
Graphify-Labs:v8from
shivamtiwari3:fix/3421-php-use-alias

Conversation

@shivamtiwari3

@shivamtiwari3 shivamtiwari3 commented Sep 9, 2026

Copy link
Copy Markdown

Fixes #3421.

Root cause

_import_php built the import edge target from the last segment of the imported name, discarding both the namespace and the alias:

raw = _read_text(child, source)          # "GuzzleHttp\Client"
module_name = raw.split("\\")[-1]        # "Client"
tgt_nid = _make_id(module_name)          # "client"

That target is not meant to be final — _resolve_php_type_references canonicalizes it. But that pass keys its per-file uses map by the local binding:

key = (alias or fqn.rsplit("\\", 1)[-1]).strip().lower()
uses.setdefault(key, fqn)

So for use GuzzleHttp\Client as HttpClient the map is {"httpclient": "GuzzleHttp\Client"} while the edge target was client — nothing matched, and the canonicalization silently did nothing. The pass even carries a fallback that only makes sense if the target is the alias, and which could therefore never fire:

if not label and relation == "imports":
    label = next((alias for alias in uses if _make_id(alias) == tgt), "")

A plain use worked only because there the binding and the last segment happen to be the same string. The alias is the trigger, exactly as reported.

Both reported symptoms, verified

Baseline on the issue's repro:

nodes: client, guzzlehttp_client, src_a, src_b, ...
src_a --imports--> guzzlehttp_client
src_b --imports--> client              ← one class, two identities

Baseline on the ambiguous-name shape (the Laravel symptom):

use App\Models\Session as LocalSession;
use Shopify\Auth\Session as ShopifySession;
use GuzzleHttp\{Client, Psr7\Request as Req};
src_http_controller --imports--> session   <-- DANGLING
src_http_controller --imports--> session   <-- DANGLING   (both aliases collapsed)
src_http_controller --imports--> request   <-- DANGLING

That is the reporter's session (44 edges) / client (21) / connection (5) breakdown: aliasing exists precisely to disambiguate two classes with one simple name, and the bare name is the one thing that cannot represent either.

Fix

Mint the edge on the local binding — the alias when the clause has one, else the imported name's last segment. That is the key the resolver already looks up, so both forms now canonicalize onto the imported class: to its own definition node when internal, to an FQN-labeled stub when external.

After the fix, the same two fixtures:

nodes: guzzlehttp_client, src_a, src_b, ...      (no stray `client`)
src_a --imports--> guzzlehttp_client
src_b --imports--> guzzlehttp_client

src_http_controller --imports--> src_models_session_session
src_http_controller --imports--> shopify_auth_session
src_http_controller --imports--> guzzlehttp_client
src_http_controller --imports--> guzzlehttp_psr7_request

Group-use aliases (use Ns\{A, Sub\B as C}) travel the same path, since the resolver already composes the group prefix with each clause.

Deliberately out of scope

use function ns\f as g and use const ns\C as D keep the old behavior: _resolve_php_type_references skips symbol imports by design (if c.type in ("function", "const"): return # not a class import), so their alias is absent from uses and switching to it would strand the edge on a one-letter id. The imported name's last segment stays the right bare name for the unique-label rewire. A regression test pins this.

Those two still dangle on both sides of the change (slug, version) — resolving symbol imports needs a function/const index rather than the class index this pass builds, which is a separate change.

Tests

Five tests in tests/test_php_type_resolution.py; the first four fail on unpatched extract.py and pass with the fix, the fifth is the function/const regression guard.

  • test_php_aliased_external_import_shares_target_with_plain_import — the issue's repro; also asserts the stray client node is gone.
  • test_php_aliased_import_of_internal_class_resolves_to_definition
  • test_php_two_aliased_imports_of_same_bare_name_stay_distinct — two targets, neither dangling, internal vs external correctly split.
  • test_php_aliased_group_use_resolves
  • test_php_aliased_function_and_const_imports_keep_bare_name

Full suite: 5498 passed, 3 pre-existing environment-dependent failures unchanged (test_extract_code_only_cli, two test_ollama backend-detection tests — all fail on a clean tree here too).

Note on #3346

The linked TypeScript issue looks like the same shape (identity taken from a name that is not the one the resolver keys on) but a different pass; not addressed here.

🤖 Generated with Claude Code

graphify-labs[bot]
graphify-labs Bot reviewed Sep 9, 2026
View reviewed changes

@graphify-labs graphify-labs Bot left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.

Graphify review — findings

Fixes PHP aliased imports so use A\B\C as D mints its import edge on the alias binding D rather than the imported name's last segment, giving it the same target as a plain use A\B\C so _resolve_php_type_references can canonicalize both onto one class node instead of splitting it or leaving the edge dangling. Aliased group-uses (use Ns\{A, Sub\B as C}) now resolve the same way. use function/use const imports are treated as symbol imports and deliberately keep the imported name's last bare segment, since their alias never lands in the resolver's uses map.

Worth a look

  • Aliased group-use only mints one import edge for the last clause — graphify/extract.py:731 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Group-use with aliased clauses handled by unclear child iteration — graphify/extract.py:731 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • saw_as set globally can attach an alias from one clause to a different clause's name — graphify/extract.py:738 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • PHP group use clauses are not decomposed into their imported members — graphify/extract.py:738 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Symbol-import (use function/const) may still consume its alias when structured as a group-use — graphify/extract.py:745 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2070 functions depend on the 460 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 568 callers, 43 callees
  • new: _rebuild_code() — 115 callers, 51 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: dispatch_command() — 2 callers, 124 callees
  • new: _get_extractor() — 26 callers, 6 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: collect_files() — 17 callers, 6 callees
  • …and 28 more — each is listed as a finding

Verification — 2070 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: 1905 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

262 of 262 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

  • tests/test_affected_cli.py — full-run-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_agents_platform.py — full-run-safety
  • tests/test_analyze.py — full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — impact, full-run-safety
  • tests/test_astro_import_ids.py — impact, full-run-safety
  • tests/test_atomic_canvas_export.py — full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_build.py — impact, full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — impact, full-run-safety
  • tests/test_cache.py — full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — full-run-safety
  • tests/test_case_sensitive_resolution.py — impact, full-run-safety
  • tests/test_charmap_encoding.py — full-run-safety
  • tests/test_chunking.py — full-run-safety
  • tests/test_cjs_module_extension.py — impact, full-run-safety
  • tests/test_claude_cli_backend.py — full-run-safety
  • tests/test_claude_md.py — full-run-safety
  • tests/test_cli_broken_pipe.py — full-run-safety
  • tests/test_cli_export.py — full-run-safety
  • tests/test_cli_help.py — full-run-safety
  • tests/test_cluster.py — full-run-safety
  • tests/test_codebuddy.py — full-run-safety
  • tests/test_community_hub_labels.py — full-run-safety
  • tests/test_community_labels_skill.py — full-run-safety
  • tests/test_confidence.py — full-run-safety
  • tests/test_corrupt_graph_json.py — full-run-safety
  • tests/test_cpp_nested_and_cli.py — impact, full-run-safety
  • tests/test_cpp_objc_cross_file_calls.py — impact, full-run-safety
  • tests/test_cpp_preprocess.py — full-run-safety
  • tests/test_cross_extension_reexport_self_cycle.py — impact, full-run-safety
  • tests/test_cross_language_call_resolution.py — impact, full-run-safety
  • tests/test_cross_repo_member_calls.py — impact, full-run-safety
  • tests/test_cross_repo_shared_types.py — full-run-safety
  • tests/test_csharp_call_site_generic_args.py — impact, full-run-safety
  • tests/test_csharp_enum_members.py — impact, full-run-safety
  • tests/test_csharp_field_generic_args.py — impact, full-run-safety
  • tests/test_csharp_generic_callsites.py — impact, full-run-safety
  • … and 212 more

non-code file(s) changed (CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

Could not verify: Could not verify \_import\_php.

The verifier did not have enough to check \_import\_php, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

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

@shivamtiwari3
Give PHP aliased imports the imported class's identity (Graphify-Labs…
4f7419c 
…#3421)

`_import_php` derived the import edge target from the last segment of the
*imported* name, discarding both the namespace and the alias. But
`_resolve_php_type_references` files each file's imported FQNs in a `uses`
map keyed by the **local binding** — the alias when there is one — and
canonicalizes an import edge by looking its target up in that map. An
aliased `use` therefore minted a target no `uses` entry could match, so the
canonicalization silently did nothing.

The pass even has a fallback that only makes sense if the target is the
alias (`next(alias for alias in uses if _make_id(alias) == tgt)`), which
could never fire.

Two symptoms follow, both reported:

- `use GuzzleHttp\Client` and `use GuzzleHttp\Client as HttpClient` in one
  project gave `guzzlehttp_client` and `client` — one class, two nodes,
  neither file reachable from the other's traversal.
- Where no bare-name node exists the edge dangles instead: 103 of 145
  `dangling_endpoint_edges`, concentrated on exactly the names a project
  aliases to disambiguate (`session` 44, `client` 21, `connection` 5).

The edge is now minted on the local binding, so both forms canonicalize
onto the imported class — internal (to its definition node) or external (to
an FQN-labeled stub) — and two aliased imports of the same simple name stay
distinct instead of collapsing. Aliases inside a group use
(`use Ns\{A, Sub\B as C}`) work through the same path.

`use function`/`use const` are deliberately excluded: the resolver skips
symbol imports, so their alias is absent from `uses` and the imported
name's last segment stays the right bare name for the unique-label rewire.
graphify-labs[bot]
graphify-labs Bot reviewed Sep 10, 2026
View reviewed changes

@graphify-labs graphify-labs Bot left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.

Graphify review — findings

Fixes PHP use A\B\C as D so its import edge targets the same node as a plain use A\B\C: _import_php now mints the edge on the aliased binding (the name this file's references and _resolve_php_type_references's uses map key on) instead of the imported name's last segment, which had either split the class into a second identity or left the edge dangling (103 of 145 dangling endpoints on the reported repo). Aliased group-uses like use Ns\{A, Sub\B as C} resolve the same way. use function/use const imports are detected as symbol imports and deliberately keep the imported name's bare last segment rather than the alias, since the resolution pass skips them.

Worth a look

  • Group-use imports only emit a single edge, dropping all but the first clause — graphify/extract.py:731 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Symbol import (use function ... as g) still emits a target derived from the last segment, but binding computation reads only top-level children — graphify/extract.py:733 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2070 functions depend on the 460 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 568 callers, 43 callees
  • new: _rebuild_code() — 115 callers, 51 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: dispatch_command() — 2 callers, 124 callees
  • new: _get_extractor() — 26 callers, 6 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: collect_files() — 17 callers, 6 callees
  • …and 28 more — each is listed as a finding

Verification — 2070 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: 1905 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

262 of 262 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

  • tests/test_affected_cli.py — full-run-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_agents_platform.py — full-run-safety
  • tests/test_analyze.py — full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — impact, full-run-safety
  • tests/test_astro_import_ids.py — impact, full-run-safety
  • tests/test_atomic_canvas_export.py — full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_build.py — impact, full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — impact, full-run-safety
  • tests/test_cache.py — full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — full-run-safety
  • tests/test_case_sensitive_resolution.py — impact, full-run-safety
  • tests/test_charmap_encoding.py — full-run-safety
  • tests/test_chunking.py — full-run-safety
  • tests/test_cjs_module_extension.py — impact, full-run-safety
  • tests/test_claude_cli_backend.py — full-run-safety
  • tests/test_claude_md.py — full-run-safety
  • tests/test_cli_broken_pipe.py — full-run-safety
  • tests/test_cli_export.py — full-run-safety
  • tests/test_cli_help.py — full-run-safety
  • tests/test_cluster.py — full-run-safety
  • tests/test_codebuddy.py — full-run-safety
  • tests/test_community_hub_labels.py — full-run-safety
  • tests/test_community_labels_skill.py — full-run-safety
  • tests/test_confidence.py — full-run-safety
  • tests/test_corrupt_graph_json.py — full-run-safety
  • tests/test_cpp_nested_and_cli.py — impact, full-run-safety
  • tests/test_cpp_objc_cross_file_calls.py — impact, full-run-safety
  • tests/test_cpp_preprocess.py — full-run-safety
  • tests/test_cross_extension_reexport_self_cycle.py — impact, full-run-safety
  • tests/test_cross_language_call_resolution.py — impact, full-run-safety
  • tests/test_cross_repo_member_calls.py — impact, full-run-safety
  • tests/test_cross_repo_shared_types.py — full-run-safety
  • tests/test_csharp_call_site_generic_args.py — impact, full-run-safety
  • tests/test_csharp_enum_members.py — impact, full-run-safety
  • tests/test_csharp_field_generic_args.py — impact, full-run-safety
  • tests/test_csharp_generic_callsites.py — impact, full-run-safety
  • … and 212 more

non-code file(s) changed (CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

Could not verify: Could not verify \_import\_php.

The verifier did not have enough to check \_import\_php, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

· 36 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

Reviewers

@graphify-labs graphify-labs[bot] graphify-labs[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PHP: use A\B\C as D gives the import edge a target derived from the last segment, splitting the class identity

1 participant

@shivamtiwari3