From 39847957699ee56900cf62f6e9336c06989e49e3 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 26 Sep 2026 02:23:23 +0200 Subject: [PATCH] fix(ts-lsp): type imported class receivers for cross-file method calls (#1354) A method call on an instance of a class defined in another file produced no CALLS edge, while the identical code in the class's own file resolved via lsp_ts_method. The type-aware tier typed the receiver wrongly in two places, so the method lookup in the shared TS registry missed and the call fell to method_not_in_registry: 1. `new C()` always qualified a bare constructor name against the CURRENT module. `const t = new ImportedClass(); t.m()` and `new ImportedClass().m()` were typed as a phantom `.ImportedClass`. The constructor type now follows TS scoping: a class declared in this module, else the class an unambiguous import binding names, else the old spelling. 2. ts_import_symbol_qn (and the copy of its heuristic in resolve_type_with_imports) treated an import value ending in "." as an already-qualified symbol QN. For the dominant TS convention of a file named after its class (`Foo.ts` exports `class Foo`, module `x.Foo`, class `x.Foo.Foo`), the receiver was typed as the MODULE, for `new`, annotations and parameters alike. The registry now decides the ambiguous case: a registered `module.name` wins, else the value is the symbol QN as before. Both are resolved through the existing shared cross-file registry; no name-based fallback is reintroduced. Cost is one or two hashed lookups on the finalized overlay / sealed shared registry, per `new` expression or per ambiguous import spelling during the walk; nothing is looked up during registration and nothing is written to the shared registry. typeorm (3,593 .ts files, shallow f279fd13): lsp_ts_method 2,551 -> 3,130, all 579 new edges cross-file (cross-file lsp_ts_method 696 -> 1,275), 0 edges lost, other strategies unchanged, index time unchanged. 20 sampled new edges checked against source: 20 true positives. Signed-off-by: Martin Vogel --- internal/cbm/lsp/ts_lsp.c | 53 +++++++++---- tests/test_pipeline.c | 88 +++++++++++++++++++++ tests/test_ts_lsp.c | 160 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 285 insertions(+), 16 deletions(-) diff --git a/internal/cbm/lsp/ts_lsp.c b/internal/cbm/lsp/ts_lsp.c index e8c3bfe4af..7fe5055fd2 100644 --- a/internal/cbm/lsp/ts_lsp.c +++ b/internal/cbm/lsp/ts_lsp.c @@ -1406,17 +1406,30 @@ static bool ts_scope_has_binding(const CBMScope *scope, const char *name) { return ts_scope_binding_owner(scope, name) != NULL; } +/* Full QN of an imported symbol. An import value is normally the module QN + * (symbol = module + "." + name), but may already be the symbol QN. When the + * module QN ends in ".name" the spelling is ambiguous: `Foo.ts` exporting + * `Foo` (module `x.Foo`, symbol `x.Foo.Foo`) looks exactly like an + * already-qualified value. Treating it as the symbol typed every imported + * same-name class as its module (#1354), so the registry decides: a + * registered `module.name` wins, else the value is the symbol. Two hashed + * lookups on a finalized/sealed registry, only in the ambiguous case. */ static const char *ts_import_symbol_qn(TSLSPContext *ctx, const char *module_qn, const char *name) { if (!ctx || !module_qn || !name) { return NULL; } + const char *appended = cbm_arena_sprintf(ctx->arena, "%s.%s", module_qn, name); size_t module_len = strlen(module_qn); size_t name_len = strlen(name); if (module_len > name_len + 1 && module_qn[module_len - name_len - 1] == '.' && strcmp(module_qn + module_len - name_len, name) == 0) { + if (appended && (cbm_registry_lookup_type(ctx->registry, appended) || + cbm_registry_lookup_func(ctx->registry, appended))) { + return appended; + } return module_qn; } - return cbm_arena_sprintf(ctx->arena, "%s.%s", module_qn, name); + return appended; } /* Same-file function values use only the exact current-module QN. No short-name, @@ -1965,6 +1978,26 @@ static const char *ts_import_module_for_local(const TSLSPContext *ctx, const cha return matched; } +/* Type of `new C()` for a bare class name, in TS scoping order: a class + * declared in this module, else the class an unambiguous import binding + * names, else the historical current-module spelling. Without the import + * step (#1354) `new ImportedClass()` was typed as a phantom current-module + * class, so every method call on the instance missed the registry and the + * cross-file edge was lost while the same code in the class's own file + * resolved. One hashed type lookup; the registry is only read. */ +static const CBMType *ts_new_bare_class_type(TSLSPContext *ctx, const char *cname) { + if (!ctx->module_qn) + return cbm_type_named(ctx->arena, cname); + const char *local_qn = cbm_arena_sprintf(ctx->arena, "%s.%s", ctx->module_qn, cname); + if (cbm_registry_lookup_type(ctx->registry, local_qn)) + return cbm_type_named(ctx->arena, local_qn); + bool import_matched = false; + const char *import_module = ts_import_module_for_local(ctx, cname, &import_matched); + if (import_module) + return cbm_type_named(ctx->arena, ts_import_symbol_qn(ctx, import_module, cname)); + return cbm_type_named(ctx->arena, local_qn); +} + static const CBMRegisteredFunc *ts_lookup_namespace_call(TSLSPContext *ctx, const char *object_name, const char *method_name, TSNode args, bool *out_is_import) { @@ -2195,13 +2228,8 @@ const CBMType *ts_eval_expr_type(TSLSPContext *ctx, TSNode node) { if (!ts_node_is_null(ctor)) { char *cname = node_text(ctx, ctor); if (cname) { - // Bare class name → qualify against module. - if (strchr(cname, '.') == NULL && ctx->module_qn) { - const char *qn = cbm_arena_sprintf(ctx->arena, "%s.%s", ctx->module_qn, cname); - result = cbm_type_named(ctx->arena, qn); - } else { - result = cbm_type_named(ctx->arena, cname); - } + result = strchr(cname, '.') == NULL ? ts_new_bare_class_type(ctx, cname) + : cbm_type_named(ctx->arena, cname); } } } else if (strcmp(kind, "call_expression") == 0) { @@ -2435,14 +2463,7 @@ static const CBMType *resolve_type_with_imports(TSLSPContext *ctx, const CBMType continue; if (strcmp(lname, bare) != 0) continue; - // Heuristic: if mqn already ends in ".bare" use as-is; otherwise append. - size_t mqn_len = strlen(mqn); - size_t bare_len = strlen(bare); - if (mqn_len > bare_len + 1 && mqn[mqn_len - bare_len - 1] == '.' && - strcmp(mqn + mqn_len - bare_len, bare) == 0) { - return cbm_type_named(ctx->arena, mqn); - } - return cbm_type_named(ctx->arena, cbm_arena_sprintf(ctx->arena, "%s.%s", mqn, bare)); + return cbm_type_named(ctx->arena, ts_import_symbol_qn(ctx, mqn, bare)); } return t; } diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 03a5cad70a..56be6b88e3 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -6118,6 +6118,93 @@ TEST(pipeline_axios_wrapper_baseurl_composes_http_calls_issue1916) { PASS(); } +/* Issue #1354: a method call on an instance built with `new ImportedClass()` + * produced no CALLS edge (only the constructor edge), while the identical + * shape in the class's own file resolved via lsp_ts_method. End to end: + * both cross-file shapes must now carry the type-aware lsp_ts_method edge, + * and the same-file control must keep it. */ +TEST(pipeline_ts_crossfile_new_instance_method_call_issue1354) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_ts_1354_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + + /* write_temp_file creates one directory level only. */ + write_temp_file(tmp, "lib/toast.service.ts", + "export class ToastService {\n" + " openSuccessUniqueXyz(msg: string): void {\n" + " console.log(msg);\n" + " }\n" + "}\n" + "\n" + "export function sameFileControl(): void {\n" + " const t = new ToastService();\n" + " t.openSuccessUniqueXyz('same-file control');\n" + "}\n"); + write_temp_file(tmp, "app/variants.ts", + "import { ToastService } from '../lib/toast.service';\n" + "\n" + "export function crossNewLocal(): void {\n" + " const t = new ToastService();\n" + " t.openSuccessUniqueXyz('cross-file');\n" + "}\n" + "\n" + "export function crossNewChain(): void {\n" + " new ToastService().openSuccessUniqueXyz('chained');\n" + "}\n"); + /* File named after its class (`NotifierService.ts` exports + * `NotifierService`): the imported module QN already ends in the class + * name, which must not be mistaken for the class QN. */ + write_temp_file(tmp, "lib/NotifierService.ts", + "export class NotifierService {\n" + " notifyUniqueAbc(): void {}\n" + "}\n"); + write_temp_file(tmp, "app/notify.ts", + "import { NotifierService } from '../lib/NotifierService';\n" + "\n" + "export function crossSameNameNew(): void {\n" + " const n = new NotifierService();\n" + " n.notifyUniqueAbc();\n" + "}\n" + "\n" + "export function crossSameNameTyped(n: NotifierService): void {\n" + " n.notifyUniqueAbc();\n" + "}\n"); + + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/ts_1354.db", tmp); + cbm_pipeline_t *p = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + const char *project = cbm_pipeline_project_name(p); + + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + + bool same_file = cross_file_call_has_strategy(s, project, "sameFileControl", + "openSuccessUniqueXyz", "lsp_ts_method"); + bool cross_local = cross_file_call_has_strategy(s, project, "crossNewLocal", + "openSuccessUniqueXyz", "lsp_ts_method"); + bool cross_chain = cross_file_call_has_strategy(s, project, "crossNewChain", + "openSuccessUniqueXyz", "lsp_ts_method"); + bool same_name_new = cross_file_call_has_strategy(s, project, "crossSameNameNew", + "notifyUniqueAbc", "lsp_ts_method"); + bool same_name_typed = cross_file_call_has_strategy(s, project, "crossSameNameTyped", + "notifyUniqueAbc", "lsp_ts_method"); + + cbm_store_close(s); + cbm_pipeline_free(p); + th_rmtree(tmp); + + ASSERT_TRUE(same_file); + ASSERT_TRUE(cross_local); + ASSERT_TRUE(cross_chain); + ASSERT_TRUE(same_name_new); + ASSERT_TRUE(same_name_typed); + PASS(); +} + TEST(pipeline_tsjs_receiver_suppresses_weak_method_edge) { char tmp[256]; snprintf(tmp, sizeof(tmp), "/tmp/cbm_tsjs_recv_XXXXXX"); @@ -16255,6 +16342,7 @@ SUITE(pipeline) { #endif RUN_TEST(pipeline_tsjs_receiver_suppresses_weak_method_edge); RUN_TEST(pipeline_axios_wrapper_baseurl_composes_http_calls_issue1916); + RUN_TEST(pipeline_ts_crossfile_new_instance_method_call_issue1354); RUN_TEST(pipeline_python_receiver_suppresses_weak_method_edge); RUN_TEST(pipeline_python_receiver_keeps_specific_unique_name_member_call); RUN_TEST(pipeline_html_embedded_member_call_stays_unbound); diff --git a/tests/test_ts_lsp.c b/tests/test_ts_lsp.c index 4058c15668..c75397975b 100644 --- a/tests/test_ts_lsp.c +++ b/tests/test_ts_lsp.c @@ -2088,6 +2088,164 @@ TEST(tslsp_crossfile_method_dispatch) { PASS(); } +/* Exact resolved-call probe: caller QN, callee QN and strategy all match. */ +static bool has_exact_resolved(const CBMResolvedCallArray *arr, const char *caller_qn, + const char *callee_qn, const char *strategy) { + for (int i = 0; i < arr->count; i++) { + const CBMResolvedCall *rc = &arr->items[i]; + if (rc->confidence > 0 && rc->caller_qn && rc->callee_qn && rc->strategy && + strcmp(rc->caller_qn, caller_qn) == 0 && strcmp(rc->callee_qn, callee_qn) == 0 && + strcmp(rc->strategy, strategy) == 0) + return true; + } + return false; +} + +/* Issue #1354: `new ImportedClass()` typed the instance as a phantom + * CURRENT-module class (`test.main.Conn`), so every method call on it missed + * the shared registry and fell to method_not_in_registry. Driven through the + * production Tier-2 path (shared sealed registry + per-file overlay). A local + * class with the imported name must still shadow the import. */ +TEST(tslsp_crossfile_new_expression_receiver_issue1354) { + CBMLSPDef defs[] = { + {.qualified_name = "test.conn.Conn", + .short_name = "Conn", + .label = "Class", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.conn", + .method_names_str = "ping"}, + {.qualified_name = "test.conn.Conn.ping", + .short_name = "ping", + .label = "Method", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.conn", + .receiver_type = "test.conn.Conn"}, + {.qualified_name = "test.main.viaLocal", + .short_name = "viaLocal", + .label = "Function", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.main"}, + {.qualified_name = "test.main.viaChain", + .short_name = "viaChain", + .label = "Function", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.main"}, + {.qualified_name = "test.shadow.Conn", + .short_name = "Conn", + .label = "Class", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.shadow", + .method_names_str = "ping"}, + {.qualified_name = "test.shadow.Conn.ping", + .short_name = "ping", + .label = "Method", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.shadow", + .receiver_type = "test.shadow.Conn"}, + {.qualified_name = "test.shadow.viaShadow", + .short_name = "viaShadow", + .label = "Function", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.shadow"}, + }; + enum { NDEFS = (int)(sizeof(defs) / sizeof(defs[0])) }; + const char *imp_names[] = {"Conn"}; + const char *imp_qns[] = {"test.conn"}; + + CBMArena arena; + cbm_arena_init(&arena); + CBMTypeRegistry *reg = cbm_ts_build_cross_registry(&arena, defs, NDEFS); + ASSERT_NOT_NULL(reg); + + const char *main_src = "import { Conn } from './conn';\n" + "export function viaLocal() { const c = new Conn(); c.ping(); }\n" + "export function viaChain() { new Conn().ping(); }\n"; + CBMResolvedCallArray out = {0}; + cbm_run_ts_lsp_cross_with_registry(&arena, main_src, (int)strlen(main_src), "test.main", false, + false, false, reg, defs, NDEFS, imp_names, imp_qns, 1, NULL, + &out); + ASSERT_TRUE( + has_exact_resolved(&out, "test.main.viaLocal", "test.conn.Conn.ping", "lsp_ts_method")); + ASSERT_TRUE( + has_exact_resolved(&out, "test.main.viaChain", "test.conn.Conn.ping", "lsp_ts_method")); + + /* Shadow control: the file declares its own Conn and imports nothing. */ + const char *shadow_src = "export class Conn { ping() {} }\n" + "export function viaShadow() { const c = new Conn(); c.ping(); }\n"; + CBMResolvedCallArray out2 = {0}; + cbm_run_ts_lsp_cross_with_registry(&arena, shadow_src, (int)strlen(shadow_src), "test.shadow", + false, false, false, reg, defs, NDEFS, NULL, NULL, 0, NULL, + &out2); + ASSERT_TRUE(has_exact_resolved(&out2, "test.shadow.viaShadow", "test.shadow.Conn.ping", + "lsp_ts_method")); + ASSERT_FALSE( + has_exact_resolved(&out2, "test.shadow.viaShadow", "test.conn.Conn.ping", "lsp_ts_method")); + + cbm_arena_destroy(&arena); + PASS(); +} + +/* Issue #1354, file named after its class: `Conn.ts` exports `class Conn`, so + * the imported module QN `test.Conn` already ends in `.Conn`. The spelling was + * read as an already-qualified symbol QN and the receiver typed as the MODULE, + * losing every method call on imported same-name classes (the dominant TS file + * convention). The registry must decide; an import value that really is the + * symbol QN (`test.conn.Conn`, no `test.conn.Conn.Conn`) keeps resolving. */ +TEST(tslsp_crossfile_same_name_file_receiver_issue1354) { + CBMLSPDef defs[] = { + {.qualified_name = "test.Conn.Conn", + .short_name = "Conn", + .label = "Class", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.Conn", + .method_names_str = "ping"}, + {.qualified_name = "test.Conn.Conn.ping", + .short_name = "ping", + .label = "Method", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.Conn", + .receiver_type = "test.Conn.Conn"}, + {.qualified_name = "test.conn.Link", + .short_name = "Link", + .label = "Class", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.conn", + .method_names_str = "open"}, + {.qualified_name = "test.conn.Link.open", + .short_name = "open", + .label = "Method", + .lang = CBM_LANG_TYPESCRIPT, + .def_module_qn = "test.conn", + .receiver_type = "test.conn.Link"}, + }; + enum { NDEFS = (int)(sizeof(defs) / sizeof(defs[0])) }; + const char *imp_names[] = {"Conn", "Link"}; + const char *imp_qns[] = {"test.Conn", "test.conn.Link"}; + + CBMArena arena; + cbm_arena_init(&arena); + CBMTypeRegistry *reg = cbm_ts_build_cross_registry(&arena, defs, NDEFS); + ASSERT_NOT_NULL(reg); + + const char *src = "import { Conn } from './Conn';\n" + "import { Link } from './conn';\n" + "export function viaNew() { const c = new Conn(); c.ping(); }\n" + "export function viaTyped(c: Conn) { c.ping(); }\n" + "export function viaQualified(l: Link) { l.open(); }\n"; + CBMResolvedCallArray out = {0}; + cbm_run_ts_lsp_cross_with_registry(&arena, src, (int)strlen(src), "test.main", false, false, + false, reg, defs, NDEFS, imp_names, imp_qns, 2, NULL, &out); + ASSERT_TRUE( + has_exact_resolved(&out, "test.main.viaNew", "test.Conn.Conn.ping", "lsp_ts_method")); + ASSERT_TRUE( + has_exact_resolved(&out, "test.main.viaTyped", "test.Conn.Conn.ping", "lsp_ts_method")); + ASSERT_TRUE( + has_exact_resolved(&out, "test.main.viaQualified", "test.conn.Link.open", "lsp_ts_method")); + + cbm_arena_destroy(&arena); + PASS(); +} + /* Issue #344 / #340: SIGSEGV in the TS cross-file LSP pass when the collected * def array exceeded ~1189 entries — scale-dependent, not file-specific. Drive * the resolver with a def array far past that threshold; the v0.7.0 rewrite @@ -4519,6 +4677,8 @@ SUITE(ts_lsp) { /* Cross-file resolution */ RUN_TEST(tslsp_crossfile_method_dispatch); + RUN_TEST(tslsp_crossfile_new_expression_receiver_issue1354); + RUN_TEST(tslsp_crossfile_same_name_file_receiver_issue1354); RUN_TEST(tslsp_scale_many_defs_no_crash_issue344); RUN_TEST(tslsp_crossfile_function_call); RUN_TEST(tslsp_crossfile_chain_through_return);