mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 23:49:14 +00:00
Park TS/JS member calls whose receiver type is declared in another repo (#3152)
`_resolve_typescript_member_calls` binds `this.repo.save()` only when the
receiver's type is declared in the same build. With the type in another
repository the resolver held the receiver type and dropped the call, so
`graph.json` — the only artifact `merge-graphs` and `global add` read — recorded
nothing and no merge-time pass could recover it.
Those calls are now parked on the caller as `metadata.unresolved_calls`, and
`link_cross_repo_member_calls` finishes them after the merge. One
`_LANG_SUFFIXES` key covers all eight TS/JS suffixes: a TS class legitimately
answers a JS call site, and two keys would drop every TS<->JS cross-repo call.
Only a table-typed receiver parks. The uppercase-spelling arm also matches
namespace aliases and default imports (`React.createElement()`), and
`node_modules` is never scanned, so parking those would send npm names to the
merge to be matched by bare name. The park sits strictly upstream of the #2553
origin gate, which only ever vets a name that did match locally.
(cherry picked from commit 5b593c6d00)
This commit is contained in:
1 parent
b4527cc8a6
commit
60c2f16ea3
4 files changed
+101
-4
No files matched your search
@@ -45,6 +45,9 @@ _LANG_SUFFIXES: dict[str, frozenset[str]] = {
|
||||
"java": frozenset({".java"}),
|
||||
"swift": frozenset({".swift"}),
|
||||
"php": frozenset({".php", ".phtml", ".php3", ".php4", ".php5", ".php7", ".phps"}),
|
||||
# TS and JS share one key: a TS class legitimately answers a JS call site, so
|
||||
# splitting them would drop every TS<->JS cross-repo call.
|
||||
"typescript": frozenset({".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", ".mjs", ".cjs"}),
|
||||
}
|
||||
|
||||
# A declaration owns its members through a `method` edge, except in C++, where an
|
||||
|
||||
@@ -5004,6 +5004,10 @@ def _resolve_typescript_member_calls(
|
||||
defined in the caller's own file, a named import of the caller's file, or
|
||||
contained in a module the caller's file imports. Otherwise EMIT NOTHING —
|
||||
a false call edge is worse than a missing one (the C++ resolver's bar).
|
||||
|
||||
A receiver typed to a class this corpus declares nowhere is parked on the caller
|
||||
for a merged graph to finish (#3152); the gate above never sees it, since it only
|
||||
vets a name that did match locally.
|
||||
"""
|
||||
type_table_by_file: dict[str, dict[str, str]] = {}
|
||||
for result in per_file:
|
||||
@@ -5086,6 +5090,15 @@ def _resolve_typescript_member_calls(
|
||||
if type_name in _LANGUAGE_BUILTIN_GLOBALS:
|
||||
continue
|
||||
type_defs = type_def_nids.get(_key(type_name), [])
|
||||
if not type_defs:
|
||||
# Table-typed only: an uppercase receiver also matches namespace aliases and
|
||||
# default imports, and node_modules is unscanned, so those are npm names.
|
||||
# The suffix stands in for the `lang` tag TS raw_calls do not carry.
|
||||
if not type_qualified and str(rc.get("source_file", "")).endswith(_JS_TS_SUFFIXES):
|
||||
_park_unresolved_member_call(
|
||||
node_by_id.get(caller), callee, type_name, "typescript", rc,
|
||||
)
|
||||
continue
|
||||
# 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
|
||||
|
||||
@@ -6,10 +6,10 @@ type already in hand and nothing about it reached `graph.json` — the only arti
|
||||
`merge-graphs` and `global add` read. The two-repo graph was missing precisely the
|
||||
edges that make it a call graph.
|
||||
|
||||
The Java, C++, C#, Swift and Go resolvers now park those calls on the caller node
|
||||
and this pass finishes them after the merge. The cases below pin what it must NOT do
|
||||
as much as what it must: the single-definition guard, the cross-repo-only scope,
|
||||
and the language guard are what keep it from fabricating an edge from a name
|
||||
The Java, C++, C#, Swift, Go, PHP and TS/JS resolvers now park those calls on the
|
||||
caller node and this pass finishes them after the merge. The cases below pin what it
|
||||
must NOT do as much as what it must: the single-definition guard, the cross-repo-only
|
||||
scope, and the language guard are what keep it from fabricating an edge from a name
|
||||
collision.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
@@ -46,6 +46,7 @@ needs_cpp = _needs("tree_sitter_cpp")
|
||||
needs_csharp = _needs("tree_sitter_c_sharp")
|
||||
needs_swift = _needs("tree_sitter_swift")
|
||||
needs_go = _needs("tree_sitter_go")
|
||||
needs_typescript = _needs("tree_sitter_typescript")
|
||||
|
||||
|
||||
def _caller(repo: str, parked: list[dict], node_id: str = "app_run",
|
||||
@@ -201,6 +202,33 @@ def test_a_defines_member_does_not_answer_a_java_call():
|
||||
assert link_cross_repo_member_calls(G) == 0
|
||||
|
||||
|
||||
PARKED_TS = [{"callee": "greet", "receiver_type": "Greeter", "lang": "typescript",
|
||||
"line": "L4"}]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("declaring_file", ["src/greeter.ts", "src/greeter.js",
|
||||
"src/greeter.mjs", "src/greeter.tsx"])
|
||||
def test_a_typescript_call_binds_across_the_whole_js_family(declaring_file: str):
|
||||
# One key for TS and JS: a TS class legitimately answers a JS call site, so
|
||||
# splitting the two would drop every TS<->JS cross-repo call.
|
||||
G = _graph(
|
||||
caller=_caller("a", PARKED_TS, source_file="src/app.ts"),
|
||||
declarations=[(_declaration("b", "Greeter", declaring_file),
|
||||
_method("b", ".greet()", source_file=declaring_file))],
|
||||
)
|
||||
assert link_cross_repo_member_calls(G) == 1, declaring_file
|
||||
assert _added_calls(G) == {("a::app_run", "b::greeter_greet")}
|
||||
|
||||
|
||||
def test_a_typescript_call_does_not_bind_to_a_python_declaration():
|
||||
G = _graph(
|
||||
caller=_caller("a", PARKED_TS, source_file="src/app.ts"),
|
||||
declarations=[(_declaration("b", "Greeter", "greeter.py"),
|
||||
_method("b", ".greet()", source_file="greeter.py"))],
|
||||
)
|
||||
assert link_cross_repo_member_calls(G) == 0
|
||||
|
||||
|
||||
def test_the_definition_answers_before_a_same_named_declaration():
|
||||
# A C++ class declares `void greet();` in its header (`defines`) and defines
|
||||
# it out of line in the `.cpp` (`method`). Both hang off the one folded class
|
||||
@@ -412,6 +440,16 @@ def test_a_java_build_parks_the_call_and_the_merge_finishes_it(tmp_path: Path):
|
||||
("src/Greeter.cs", "class Greeter { public void Greet() {} }\n"),
|
||||
marks=needs_csharp, id="csharp-field-receiver",
|
||||
),
|
||||
pytest.param(
|
||||
"typescript", "greet",
|
||||
("src/app.ts", 'import { Greeter } from "greeter-pkg";\n'
|
||||
"export class App {\n"
|
||||
" constructor(private greeter: Greeter) {}\n"
|
||||
" run(): void { this.greeter.greet(); }\n"
|
||||
"}\n"),
|
||||
("src/greeter.ts", "export class Greeter {\n greet(): void {}\n}\n"),
|
||||
marks=needs_typescript, id="typescript-parameter-property",
|
||||
),
|
||||
pytest.param(
|
||||
"swift", "greet",
|
||||
("src/App.swift", "class App {\n"
|
||||
|
||||
@@ -307,3 +307,46 @@ def test_a_pure_esm_corpus_still_activates_the_resolver(tmp_path):
|
||||
"export function usesDirect() { return s.doThing(); }\n"),
|
||||
})
|
||||
assert any("usesDirect" in s and "doThing" in t for s, t in calls)
|
||||
|
||||
|
||||
def _parked(r) -> list[dict]:
|
||||
return [entry for n in r["nodes"]
|
||||
if isinstance(n.get("metadata"), dict)
|
||||
for entry in n["metadata"].get("unresolved_calls", [])]
|
||||
|
||||
|
||||
def test_an_annotated_type_declared_nowhere_is_parked_for_the_merge(tmp_path):
|
||||
_, r = _calls(tmp_path, {
|
||||
"app.ts": ("import { Greeter } from 'greeter-pkg';\n"
|
||||
"export class App {\n"
|
||||
" constructor(private greeter: Greeter) {}\n"
|
||||
" run(): void { this.greeter.greet(); }\n"
|
||||
"}\n"),
|
||||
})
|
||||
assert _parked(r) == [{"callee": "greet", "receiver_type": "Greeter",
|
||||
"lang": "typescript", "line": "L4"}]
|
||||
|
||||
|
||||
def test_a_namespace_style_receiver_is_not_parked(tmp_path):
|
||||
# An uppercase receiver types the type by spelling alone, which a namespace
|
||||
# alias, a default import or a plain const object satisfies just as well.
|
||||
_, r = _calls(tmp_path, {
|
||||
"app.ts": ("import * as React from 'react';\n"
|
||||
"const s = new Svc();\n"
|
||||
"export function render(): void { React.createElement(); }\n"),
|
||||
})
|
||||
assert _parked(r) == []
|
||||
|
||||
|
||||
def test_a_type_the_origin_gate_rejected_is_not_parked(tmp_path):
|
||||
# A local declaration exists and the gate refused it, so parking would ask the
|
||||
# merge to accept a remote match on evidence the gate just judged weaker.
|
||||
_, r = _calls(tmp_path, {
|
||||
"fileb.ts": _LOCAL_REPO,
|
||||
"filea.ts": ("import type { Repo } from 'external-pkg';\n"
|
||||
"export class ReportService {\n"
|
||||
" constructor(private repo: Repo) {}\n"
|
||||
" run(): void { this.repo.save(); }\n"
|
||||
"}\n"),
|
||||
})
|
||||
assert _parked(r) == []
|
||||
Reference in new issue
Block a user