mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 07:29:33 +00:00
fix(ts): pick a same-named receiver type by the caller's import, never bind to a #private method
_resolve_typescript_member_calls required a receiver's type name to be unique in the
corpus before applying the #2553 origin gate, so hono's `c: Context` (a class plus two
interfaces and a type named Context) never resolved, though the caller's
`import type { Context } from '../../context'` names the exact one. Apply the origin
gate to the candidates first and emit only when exactly one is visible; one definition
behaves as before, an undecided import still emits nothing.
_key() also folds `#name` into `name`, so `c.newResponse()` could bind to a private
`#newResponse()`. Skip a match whose leading `#` disagrees with the call.
Against the TypeScript compiler, 0.9.80 vs this change: hono obj.method() 20.3% -> 47.3%
(+23 edges), NestJS +5, TypeORM +3, all 31 confirmed at the call site, 0 lost.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8gwEpjajiWHVW3c9smuKf
(cherry picked from commit 7935593eca)
This commit is contained in:
1 parent
18682e8dce
commit
27df0f9a2c
2 files changed
+75
-9
No files matched your search
+22
-9
@@ -4499,28 +4499,41 @@ def _resolve_typescript_member_calls(
|
||||
if type_name in _LANGUAGE_BUILTIN_GLOBALS:
|
||||
continue
|
||||
type_defs = type_def_nids.get(_key(type_name), [])
|
||||
if len(type_defs) != 1:
|
||||
continue
|
||||
type_nid = type_defs[0]
|
||||
# Origin gate (#2553): the caller's file must actually see the matched
|
||||
# type — same file, a named import of the type, or a module import of
|
||||
# the type's file. Otherwise a third-party type name that happens to
|
||||
# collide with a local class fabricates an edge; emit nothing.
|
||||
# The same gate also picks among same-named definitions: `Context` can be
|
||||
# a class in one file and an interface in two others, and the caller's
|
||||
# import says which one its `c: Context` means. Exactly one visible
|
||||
# definition is required; none or several still emit nothing.
|
||||
caller_file = file_of_node.get(caller)
|
||||
type_file = file_of_node.get(type_nid)
|
||||
imported = imported_by_filenode.get(caller_file, set())
|
||||
if not (
|
||||
(caller_file is not None and caller_file == type_file)
|
||||
or type_nid in imported
|
||||
or (type_file is not None and type_file in imported)
|
||||
):
|
||||
|
||||
def _visible(nid: str) -> bool:
|
||||
type_file = file_of_node.get(nid)
|
||||
return (
|
||||
(caller_file is not None and caller_file == type_file)
|
||||
or nid in imported
|
||||
or (type_file is not None and type_file in imported)
|
||||
)
|
||||
|
||||
visible = [nid for nid in type_defs if _visible(nid)]
|
||||
if len(visible) != 1:
|
||||
continue
|
||||
type_nid = visible[0]
|
||||
method_nid = method_index.get((type_nid, _key(callee)))
|
||||
if not method_nid:
|
||||
# Receiver typed, but the type has no such method. The old fallback
|
||||
# (a `references` edge to the type node) was another fabrication
|
||||
# vector; skip instead, matching the C# resolver.
|
||||
continue
|
||||
# _key() drops the `#`, so `c.newResponse()` also keys onto a private
|
||||
# `#newResponse()`. A `#name` is only reachable as `#name`; never bind
|
||||
# across that difference.
|
||||
method_name = str(node_by_id.get(method_nid, {}).get("label", "")).lstrip(".")
|
||||
if method_name.startswith("#") != str(callee).startswith("#"):
|
||||
continue
|
||||
if method_nid == caller or (caller, method_nid) in existing_pairs:
|
||||
continue
|
||||
existing_pairs.add((caller, method_nid))
|
||||
|
||||
@@ -214,3 +214,56 @@ def test_class_field_of_an_unimported_type_emits_no_edge(tmp_path):
|
||||
"export class K {\n repo: Svc;\n a(): number { return this.repo.doThing(); }\n}\n"),
|
||||
})
|
||||
assert not _caller_hits(calls, "a")
|
||||
# ── Same-named types: `Context` can be a class in one file and an interface or
|
||||
# type in others. The caller's own import says which one `c: Context` means.
|
||||
|
||||
_CTX_CLASS = "export class Context {\n header(): number { return 1; }\n}\n"
|
||||
_CTX_IFACE = "export interface Context {\n header(): number;\n}\n"
|
||||
|
||||
|
||||
def _hits(calls, caller: str, callee: str = "header") -> bool:
|
||||
return any(caller in s and callee in t for s, t in calls)
|
||||
|
||||
|
||||
def test_same_named_type_is_picked_by_the_callers_import(tmp_path):
|
||||
calls, r = _calls(tmp_path, {
|
||||
"context.ts": _CTX_CLASS,
|
||||
"jsx/context.ts": _CTX_IFACE,
|
||||
"router/node.ts": "export type Context = { header(): number };\n",
|
||||
"cookie.ts": ('import type { Context } from "./context";\n'
|
||||
"export const setCookie = (c: Context): number => c.header();\n"),
|
||||
})
|
||||
nodes = {n["id"]: n for n in r["nodes"]}
|
||||
targets = [e["target"] for e in r["edges"]
|
||||
if e["relation"] == "calls" and "setCookie" in nodes[e["source"]]["label"]]
|
||||
# the class in ./context.ts, not the jsx interface or the router type alias
|
||||
assert len(targets) == 1, targets
|
||||
assert targets[0].endswith("context_context_header"), targets
|
||||
assert "jsx" not in targets[0] and "router" not in targets[0], targets
|
||||
|
||||
|
||||
def test_same_named_type_the_import_cannot_decide_emits_no_edge(tmp_path):
|
||||
calls, _ = _calls(tmp_path, {
|
||||
"a/context.ts": _CTX_CLASS,
|
||||
"b/context.ts": _CTX_CLASS,
|
||||
"both.ts": ('import * as a from "./a/context";\nimport * as b from "./b/context";\n'
|
||||
"type Context = a.Context;\n"
|
||||
"export function both(c: Context): number { return c.header(); }\n"),
|
||||
"neither.ts": "export function neither(c: Context): number { return c.header(); }\n",
|
||||
})
|
||||
assert not _hits(calls, "both")
|
||||
assert not _hits(calls, "neither")
|
||||
|
||||
|
||||
def test_a_public_call_never_binds_to_a_private_hash_method(tmp_path):
|
||||
# `_key()` folds `#newResponse` and `newResponse` together; a `#name` is only
|
||||
# reachable as `#name`, so `c.newResponse()` must not land on it.
|
||||
calls, _ = _calls(tmp_path, {
|
||||
"ctx.ts": ("export class Ctx {\n"
|
||||
" #newResponse(): number { return 1; }\n"
|
||||
" build(): number { return this.#newResponse(); }\n"
|
||||
"}\n"),
|
||||
"use.ts": ('import { Ctx } from "./ctx";\n'
|
||||
"export function make(c: Ctx): number { return c.newResponse(); }\n"),
|
||||
})
|
||||
assert not _hits(calls, "make", "newResponse")
|
||||
Reference in new issue
Block a user