mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-14 16:09:26 +00:00
Prefer the alias for a PHP use import edge's target
An aliased use clause (use GuzzleHttp\Client as HttpClient) always targeted the bare imported name, ignoring the alias entirely. Two files importing the same external class under different local aliases, a real pattern for disambiguating two same named classes from different namespaces, produced two different stub targets for one class, splitting its identity. The cross file resolver repoints a stub through the file's own alias to FQN map, keyed by the alias when the import has one, so a bare name derived stub could never be found under that key and stayed stuck, unresolved. The alias, when present, is also what the rest of the file actually references, so preferring it here keeps this edge's target consistent with those reference edges too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
This commit is contained in:
1 parent
67f99bd005
commit
73f9f1667b
2 files changed
+106
-15
No files matched your search
+38
-15
@@ -728,23 +728,46 @@ def _import_scala(node, source: bytes, file_nid: str, stem: str, edges: list, st
|
||||
|
||||
|
||||
def _import_php(node, source: bytes, file_nid: str, stem: str, edges: list, str_path: str, scope_stack: list[str] | None = None) -> None:
|
||||
# `node` is a namespace_use_clause: `Client` or `Client as HttpClient`,
|
||||
# whose children are the qualified/bare name, then (if present) `as`
|
||||
# and the alias name, in that order. An aliased clause used to always
|
||||
# target the bare imported name ("Client"), ignoring the alias
|
||||
# entirely -- so two files importing the SAME external class under
|
||||
# DIFFERENT local aliases (a real pattern: disambiguating two
|
||||
# same-named classes from different namespaces) produced two
|
||||
# DIFFERENT stub targets for one class, and _resolve_php_type_references
|
||||
# (which resolves a stub via the file's own alias -> FQN map, keyed by
|
||||
# the alias when the import has one) could never find this one under
|
||||
# its bare-name-derived key, leaving the edge stuck on an unresolved,
|
||||
# never-repointed stub (#3421). The alias, when present, is also what
|
||||
# the rest of the file actually references (a parameter typed
|
||||
# `HttpClient`, not `Client`), so preferring it here keeps this edge's
|
||||
# target consistent with those reference edges too.
|
||||
saw_as = False
|
||||
module_name = None
|
||||
for child in node.children:
|
||||
if child.type == "as":
|
||||
saw_as = True
|
||||
continue
|
||||
if child.type in ("qualified_name", "name", "identifier"):
|
||||
raw = _read_text(child, source)
|
||||
module_name = raw.split("\\")[-1].strip()
|
||||
if module_name:
|
||||
tgt_nid = _make_id(module_name)
|
||||
edges.append({
|
||||
"source": file_nid,
|
||||
"target": tgt_nid,
|
||||
"relation": "imports",
|
||||
"context": "import",
|
||||
"confidence": "EXTRACTED",
|
||||
"source_file": str_path,
|
||||
"source_location": f"L{node.start_point[0] + 1}",
|
||||
"weight": 1.0,
|
||||
})
|
||||
break
|
||||
bare = _read_text(child, source).split("\\")[-1].strip()
|
||||
if saw_as:
|
||||
module_name = bare
|
||||
break # the alias name always comes last; nothing more to see
|
||||
if module_name is None:
|
||||
module_name = bare
|
||||
if module_name:
|
||||
tgt_nid = _make_id(module_name)
|
||||
edges.append({
|
||||
"source": file_nid,
|
||||
"target": tgt_nid,
|
||||
"relation": "imports",
|
||||
"context": "import",
|
||||
"confidence": "EXTRACTED",
|
||||
"source_file": str_path,
|
||||
"source_location": f"L{node.start_point[0] + 1}",
|
||||
"weight": 1.0,
|
||||
})
|
||||
|
||||
|
||||
# ── C/C++ function name helpers ───────────────────────────────────────────────
|
||||
|
||||
@@ -98,6 +98,74 @@ def test_php_ambiguous_base_disambiguated_by_use(tmp_path: Path):
|
||||
assert "Cms" in tgt["source_file"] and "Models" not in tgt["source_file"]
|
||||
|
||||
|
||||
def test_php_aliased_import_edge_shares_target_with_plain_import(tmp_path: Path):
|
||||
"""#3421: an `imports` edge for an aliased `use` clause always targeted
|
||||
the bare imported class name (the last segment of the qualified name),
|
||||
completely ignoring the alias. A file with a PLAIN `use GuzzleHttp\\Client;`
|
||||
and a file with an ALIASED `use GuzzleHttp\\Client as HttpClient;` both
|
||||
minted a stub keyed by "Client", so the resolver's alias -> FQN lookup
|
||||
(keyed by "httpclient" for the aliased file) missed and the aliased
|
||||
file's edge stayed stuck, unresolved, on the bare stub -- while the
|
||||
plain file's edge correctly resolved to the external FQN stub. Two
|
||||
files importing the SAME external class ended up with TWO different
|
||||
import targets, splitting the class identity (the exact real-world
|
||||
case an alias exists for: disambiguating two same-named classes from
|
||||
different namespaces)."""
|
||||
a = _write(
|
||||
tmp_path / "src/A.php",
|
||||
"<?php\nnamespace App;\n"
|
||||
"use GuzzleHttp\\Client;\n"
|
||||
"class A { public function __construct(private Client $client) {} }\n",
|
||||
)
|
||||
b = _write(
|
||||
tmp_path / "src/B.php",
|
||||
"<?php\nnamespace App;\n"
|
||||
"use GuzzleHttp\\Client as HttpClient;\n"
|
||||
"class B { public function __construct(private HttpClient $client) {} }\n",
|
||||
)
|
||||
result = extract([a, b], cache_root=tmp_path)
|
||||
|
||||
imports = {
|
||||
e["source"]: e["target"]
|
||||
for e in result["edges"]
|
||||
if e["relation"] == "imports"
|
||||
}
|
||||
src_a_id = next(n["id"] for n in result["nodes"] if n.get("label") == "A.php")
|
||||
src_b_id = next(n["id"] for n in result["nodes"] if n.get("label") == "B.php")
|
||||
assert src_a_id in imports and src_b_id in imports
|
||||
assert imports[src_a_id] == imports[src_b_id], (
|
||||
f"plain and aliased imports of the same class split into different "
|
||||
f"targets: {imports}"
|
||||
)
|
||||
tgt = _node_by_id(result, imports[src_a_id])
|
||||
assert tgt is not None and tgt.get("label") == "GuzzleHttp\\Client"
|
||||
|
||||
|
||||
def test_php_distinct_aliases_for_same_named_classes_do_not_collapse(tmp_path: Path):
|
||||
"""The reason an alias exists at all: disambiguating two DIFFERENT
|
||||
classes that share a bare name (App\\Models\\Session vs
|
||||
Shopify\\Auth\\Session). Fixing #3421 by preferring the alias for an
|
||||
import edge's target must not go too far the other way and collapse
|
||||
genuinely different classes onto one node just because they were both
|
||||
imported under an alias."""
|
||||
a = _write(
|
||||
tmp_path / "src/A.php",
|
||||
"<?php\nnamespace App;\n"
|
||||
"use App\\Models\\Session as ModelSession;\n"
|
||||
"use Shopify\\Auth\\Session as ShopifySession;\n"
|
||||
"class A {\n"
|
||||
" public function __construct(private ModelSession $m, private ShopifySession $s) {}\n"
|
||||
"}\n",
|
||||
)
|
||||
result = extract([a], cache_root=tmp_path)
|
||||
|
||||
imports = {e["target"] for e in result["edges"] if e["relation"] == "imports"}
|
||||
labels = {_node_by_id(result, t).get("label") for t in imports}
|
||||
assert "App\\Models\\Session" in labels
|
||||
assert "Shopify\\Auth\\Session" in labels
|
||||
assert len(imports) == 2, f"distinct aliased classes collapsed onto one target: {imports}"
|
||||
|
||||
|
||||
def test_php_use_alias_resolves(tmp_path: Path):
|
||||
_write(
|
||||
tmp_path / "src/Foo/Bar.php",
|
||||
|
||||
Reference in new issue
Block a user