From eaf59046e5371eda24bfafc60743291b429e79d8 Mon Sep 17 00:00:00 2001 From: Yyunozor Date: Thu, 20 Aug 2026 01:31:29 +0200 Subject: [PATCH 1/3] fix(registry): stop external package imports binding to project homonyms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bare call whose name the calling file imports from a package outside the indexed tree was bound to whatever project symbol shared the simple name. An `import { eq, sql } from "drizzle-orm"` plus an unrelated local module exporting `eq`/`sql` produced two CALLS edges from the caller into that module, strategy unique_name — the same defect on the sequential and the fused-parallel resolver. The TS-LSP is not involved: it declines the external names, and the textual registry fallback fires anyway. The package materializes no node, so resolve_import_node returns NULL, no IMPORTS edge is written, and the per-file import map carries no key for the name. Strategies 1-2 miss and resolve_name_lookup binds the call by simple name. Confidence does not separate the cases: the fabricated edge lands at 0.75 whenever the file also imports anything from the target's module. Add cbm_suppress_external_import_shadow(), a pure predicate beside the existing perl/tsjs/cross-language guards and called at the same two emit sites. It drops the edge only when the callee is a bare identifier, the strategy is a project-wide guess (suffix_match / unique_name / field_type_hint / fuzzy), the file imports that exact local name from a non-relative specifier, and no import-map key binds it. Relative specifiers are excluded on purpose: they name a path inside the tree, so a missing IMPORTS edge there is an in-project resolution gap and the fallback can still be right. A name imported both relatively and from a package keeps its edge, independently of extraction order. Addresses the external-import sub-case of #1355. Signed-off-by: Yyunozor --- src/pipeline/pass_calls.c | 16 ++++- src/pipeline/pass_parallel.c | 8 +++ src/pipeline/pipeline.h | 10 +++ src/pipeline/registry.c | 82 +++++++++++++++++++++++ tests/test_pipeline.c | 124 +++++++++++++++++++++++++++++++++++ tests/test_registry.c | 89 +++++++++++++++++++++++++ 6 files changed, 326 insertions(+), 3 deletions(-) diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index c9744dda2..734e9b956 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -476,7 +476,8 @@ static const cbm_gbuf_node_t *calls_find_source(cbm_pipeline_ctx_t *ctx, const c /* Resolve one call and emit the appropriate edge. Returns 1 if resolved, 0 if not. */ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, - const CBMResolvedCallArray *lsp_calls, const char *rel, + const CBMResolvedCallArray *lsp_calls, + const CBMImportArray *file_imports, const char *rel, const char *module_qn, const char **imp_keys, const char **imp_vals, int imp_count, CBMLanguage lang) { const cbm_gbuf_node_t *source_node = calls_find_source(ctx, rel, call->enclosing_func_qn); @@ -668,6 +669,15 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, if (cbm_suppress_cross_language_suffix_match(lang, target_node->file_path, res.strategy)) { return 0; } + /* #1355: `import { eq } from "drizzle-orm"` binds `eq` to a package that is + * not in the indexed tree, so a project-wide same-name guess must not turn + * `eq(...)` into a CALLS edge to an unrelated project `eq`. Placed with the + * #725 guard, after the service-pattern bypasses above, so no HTTP/route + * edge can be lost to it. */ + if (cbm_suppress_external_import_shadow(call->callee_name, res.strategy, file_imports, imp_keys, + imp_count)) { + return 0; + } emit_classified_edge(ctx, call, source_node, target_node, &res, module_qn, imp_keys, imp_vals, imp_count, drop_plain_call); return SKIP_ONE; @@ -827,8 +837,8 @@ int cbm_pipeline_pass_calls(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t *file continue; } total_calls++; - if (resolve_single_call(ctx, call, &result->resolved_calls, rel, module_qn, imp_keys, - imp_vals, imp_count, files[i].language)) { + if (resolve_single_call(ctx, call, &result->resolved_calls, &result->imports, rel, + module_qn, imp_keys, imp_vals, imp_count, files[i].language)) { resolved++; } else { unresolved++; diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index cdeafe9d8..76ed4e9a4 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -2569,6 +2569,14 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB * CALLS edge across a language boundary. */ continue; } + if (target_node && source_node->id != target_node->id && + cbm_suppress_external_import_shadow(call->callee_name, res.strategy, &result->imports, + imp_keys, imp_count)) { + /* #1355: same guard as pass_calls.c — a bare call bound by an + * external package import must not become a CALLS edge to an + * unrelated project symbol of the same name. */ + continue; + } if (!target_node || source_node->id == target_node->id) { /* HTTP/ASYNC calls to an EXTERNAL client library (`requests.get(url)`) * resolve to an unindexed QN (target_node == NULL), but their edge diff --git a/src/pipeline/pipeline.h b/src/pipeline/pipeline.h index dfe81f726..f24cc3045 100644 --- a/src/pipeline/pipeline.h +++ b/src/pipeline/pipeline.h @@ -291,6 +291,16 @@ bool cbm_suppress_weak_member_match(bool enabled, bool is_method, const char *st * Pure; unit-tested in test_registry.c. */ bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_bound, const char *strategy); +/* #1355: drop a project-wide same-name guess (suffix_match / unique_name / + * field_type_hint / fuzzy) for a BARE call whose name the calling file binds to + * a NON-RELATIVE (package) import that resolved to nothing in the graph — + * `import { eq } from "drizzle-orm"` must not make `eq(...)` a CALLS edge to an + * unrelated project `eq`. A name the import map does bind, a relative + * specifier, a member/qualified callee, and every import-/receiver-aware + * strategy are all kept. Pure; unit-tested in test_registry.c. */ +bool cbm_suppress_external_import_shadow(const char *callee_name, const char *strategy, + const CBMImportArray *file_imports, + const char **import_map_keys, int import_map_count); /* #725: drop a suffix_match CALLS edge when the caller language and the * target file's language disagree. unique_name (candidates == 1) is #1572 diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index 97c843355..ff78c4354 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -501,6 +501,88 @@ bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_b return weak_short_name_strategy(strategy); } +/* A module specifier that names a path inside the indexed tree ("./x", "../x", + * "/abs/x") rather than an external package. Package specifiers are everything + * else — "drizzle-orm", "rxjs/operators", "@scope/pkg". */ +static bool specifier_is_relative(const char *module_path) { + return module_path && (module_path[0] == '.' || module_path[0] == '/'); +} + +static bool import_map_binds(const char **import_map_keys, int import_map_count, + const char *local_name) { + if (!import_map_keys || import_map_count <= 0) { + return false; + } + for (int i = 0; i < import_map_count; i++) { + if (import_map_keys[i] && strcmp(import_map_keys[i], local_name) == 0) { + return true; + } + } + return false; +} + +/* #1355: a bare call whose name the calling file binds to an EXTERNAL package + * import must not fall back to a project-wide same-name guess. + * + * `import { eq } from "drizzle-orm"` binds `eq` in this file's scope. When the + * package is not part of the indexed tree it materializes no IMPORTS edge, so + * the import map has no entry, strategies 1-2 miss, and strategy 3/4 attach + * `eq(users.id, id)` to whatever project symbol happens to share the simple + * name — a CALLS edge into an unrelated local helper. The explicit import + * statement is positive evidence, taken from the caller's own source, that the + * identifier does NOT denote that symbol. + * + * Fires only when ALL of: + * - the callee is a BARE identifier: member (`x.foo()`) and package/namespace + * qualified callees are the receiver-aware guards' business, not this one; + * - the match came from a project-wide guess (suffix_match / unique_name; + * field_type_hint / fuzzy listed for the same defensive reason as the TS/JS + * guard) — every import-, receiver- or module-aware strategy is KEPT; + * - the file imports that exact local name from a NON-RELATIVE specifier; + * - no import-map key binds the name, i.e. that import resolved to nothing in + * the graph. A name the map does bind already had its chance at strategy 1 + * and is import-aware by construction. + * + * Relative specifiers are deliberately excluded: they name a path inside the + * indexed tree, so a missing IMPORTS edge is an in-project resolution gap and + * the same-name fallback can still be right. A name imported from both a + * relative and a package specifier keeps the edge — the relative binding wins + * the tie explicitly, so the outcome does not depend on import order. + * + * Pure + side-effect-free so the contract is unit-testable without a pipeline. */ +bool cbm_suppress_external_import_shadow(const char *callee_name, const char *strategy, + const CBMImportArray *file_imports, + const char **import_map_keys, int import_map_count) { + if (!callee_name || !callee_name[0] || !strategy || !strategy[0]) { + return false; + } + if (strcmp(strategy, "suffix_match") != 0 && strcmp(strategy, "unique_name") != 0 && + strcmp(strategy, "field_type_hint") != 0 && strcmp(strategy, "fuzzy") != 0) { + return false; + } + if (strchr(callee_name, '.') != NULL || strstr(callee_name, "::") != NULL) { + return false; + } + if (!file_imports || file_imports->count <= 0) { + return false; + } + if (import_map_binds(import_map_keys, import_map_count, callee_name)) { + return false; + } + bool external_binding = false; + for (int i = 0; i < file_imports->count; i++) { + const CBMImport *imp = &file_imports->items[i]; + if (!imp->local_name || !imp->module_path || strcmp(imp->local_name, callee_name) != 0) { + continue; + } + if (specifier_is_relative(imp->module_path)) { + return false; /* an in-tree binding for this name — never suppress */ + } + external_binding = true; + } + return external_binding; +} + static bool js_ts_family(CBMLanguage lang) { return lang == CBM_LANG_JAVASCRIPT || lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX || lang == CBM_LANG_ARKTS; diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index c5d3a9539..fd6d51855 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -4976,6 +4976,129 @@ static int count_nodes_named(cbm_store_t *s, const char *project, const char *na * (axios.get, api.patch on a renamed-axios instance, supertest request(app).get). * The regex false edge must stay suppressed in parallel too. CBM_WORKERS forces * >1 worker so the parallel path is taken regardless of the host core count. */ +/* #1355: write the shared external-import-shadow fixture into `dir`. + * `pad_files` filler modules push the run over MIN_FILES_FOR_PARALLEL so the + * same tree can be indexed by both resolvers. */ +static void write_external_import_shadow_fixture(const char *dir, int pad_files) { + /* The lone project symbols named eq/sql — ordinary local helpers. */ + write_temp_file(dir, "src/text-utils.ts", + "export function eq(a: string, b: string): boolean {\n" + " return a.trim() === b.trim();\n" + "}\n" + "export function sql(chunk: string): string {\n" + " return chunk.replace(/\\s+/g, ' ');\n" + "}\n" + "export function normalize(s: string): string {\n" + " return s.trim().toLowerCase();\n" + "}\n"); + /* `eq` and `sql` come from an external package that is not in the tree, so + * the registry falls through to a project-wide same-name guess. These are + * the fabricated edges. `normalize` is imported relatively from the SAME + * file the guess would have picked — it must survive. */ + write_temp_file(dir, "src/queries.ts", + "import { eq, sql } from 'drizzle-orm';\n" + "import { normalize } from './text-utils';\n" + "export function buildQuery(id: string): unknown {\n" + " return [eq({ id }, id), sql('select 1'), normalize(id)];\n" + "}\n"); + /* No import of `eq` at all: nothing in the caller's source contradicts the + * guess, so the pre-existing fallback keeps its edge. */ + write_temp_file(dir, "src/no-import.ts", + "export function callsWithoutImport(a: string, b: string): boolean {\n" + " return eq(a, b);\n" + "}\n"); + for (int i = 0; i < pad_files; i++) { + char name[64]; + char body[128]; + snprintf(name, sizeof(name), "src/pad_%02d.ts", i); + snprintf(body, sizeof(body), "export function shadowPad%02d(): number { return %d; }\n", i, + i); + write_temp_file(dir, name, body); + } +} + +/* #1355: assert the guard's whole contract against one indexed store. */ +static int assert_external_import_shadow_contract(const char *dir, const char *db_name) { + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/%s", dir, db_name); + cbm_pipeline_t *p = cbm_pipeline_new(dir, db_path, CBM_MODE_FULL); + if (!p) { + return 1; + } + if (cbm_pipeline_run(p) != 0) { + cbm_pipeline_free(p); + return 2; + } + const char *project = cbm_pipeline_project_name(p); + cbm_store_t *s = cbm_store_open_path(db_path); + if (!s) { + cbm_pipeline_free(p); + return 3; + } + int rc = 0; + /* (1) the reported bug: an externally-imported name must not bind to the + * local homonym (RED before the fix, on both resolvers). */ + if (cross_file_call_exists(s, project, "buildQuery", "eq")) { + rc = 4; + } + if (rc == 0 && cross_file_call_exists(s, project, "buildQuery", "sql")) { + rc = 5; + } + /* (2) the relative import to the very same file still resolves. */ + if (rc == 0 && !cross_file_call_exists(s, project, "buildQuery", "normalize")) { + rc = 6; + } + /* (3) a bare call the caller never imports keeps its pre-existing edge. */ + if (rc == 0 && !cross_file_call_exists(s, project, "callsWithoutImport", "eq")) { + rc = 7; + } + cbm_store_close(s); + cbm_pipeline_free(p); + return rc; +} + +TEST(pipeline_external_import_shadow_not_bound_to_local_homonym_issue1355) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_ext_import_shadow_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + /* Enough files that CBM_WORKERS can take the fused-parallel path; the same + * tree is then indexed by each resolver in turn, because the guard lives at + * two independent emit sites (pass_calls.c and pass_parallel.c). */ + write_external_import_shadow_fixture(tmp, 50); + + char *old_workers = getenv("CBM_WORKERS"); + char *saved_workers = old_workers ? strdup(old_workers) : NULL; + char *old_single = getenv("CBM_INDEX_SINGLE_THREAD"); + char *saved_single = old_single ? strdup(old_single) : NULL; + + cbm_setenv("CBM_INDEX_SINGLE_THREAD", "1", 1); + int sequential = assert_external_import_shadow_contract(tmp, "shadow-sequential.db"); + + cbm_unsetenv("CBM_INDEX_SINGLE_THREAD"); + cbm_setenv("CBM_WORKERS", "4", 1); + int parallel = assert_external_import_shadow_contract(tmp, "shadow-parallel.db"); + + if (saved_workers) { + cbm_setenv("CBM_WORKERS", saved_workers, 1); + free(saved_workers); + } else { + cbm_unsetenv("CBM_WORKERS"); + } + if (saved_single) { + cbm_setenv("CBM_INDEX_SINGLE_THREAD", saved_single, 1); + free(saved_single); + } else { + cbm_unsetenv("CBM_INDEX_SINGLE_THREAD"); + } + th_rmtree(tmp); + + ASSERT_EQ(sequential, 0); + ASSERT_EQ(parallel, 0); + PASS(); +} + TEST(pipeline_tsjs_receiver_parallel_keeps_service_edges) { char tmp[256]; snprintf(tmp, sizeof(tmp), "/tmp/cbm_tsjs_par_XXXXXX"); @@ -13324,6 +13447,7 @@ SUITE(pipeline) { RUN_TEST(pipeline_python_receiver_parallel_suppresses_weak_method_edges); RUN_TEST(pipeline_python_bare_local_binding_suppresses_weak_edge); RUN_TEST(pipeline_python_bare_local_binding_parallel_suppresses_weak_edge); + RUN_TEST(pipeline_external_import_shadow_not_bound_to_local_homonym_issue1355); RUN_TEST(pipeline_parallel_python_cross_only_dunder_gets_synthetic_carrier); RUN_TEST(pipeline_parallel_rust_cross_only_macro_hidden_gets_synthetic_carrier); RUN_TEST(pipeline_arg_url_rejects_non_http_slash_arguments); diff --git a/tests/test_registry.c b/tests/test_registry.c index 247ca61ad..eb47679f3 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -970,6 +970,92 @@ TEST(weak_call_guards_share_one_drop_list) { } ASSERT_EQ(member, binding); } +TEST(external_import_shadow_drops_package_bound_bare_call) { + /* #1355: `import { eq, sql } from "drizzle-orm"` binds both names to a + * package outside the indexed tree, so no import-map key exists for them and + * a project-wide same-name guess is a fabricated edge. */ + CBMImport imports[] = { + {.local_name = "eq", .module_path = "drizzle-orm"}, + {.local_name = "sql", .module_path = "drizzle-orm"}, + {.local_name = "users", .module_path = "./schema"}, + }; + CBMImportArray arr = {.items = imports, .count = 3, .cap = 3}; + /* Only the relative import materialized an IMPORTS edge. */ + const char *keys[] = {"users"}; + + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, keys, 1)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("sql", "unique_name", &arr, keys, 1)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "suffix_match", &arr, keys, 1)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "field_type_hint", &arr, keys, 1)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "fuzzy", &arr, keys, 1)); + /* A file whose imports all failed to materialize is still covered: the + * evidence is the import statement, not the map. */ + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0)); + PASS(); +} + +TEST(external_import_shadow_keeps_everything_else) { + /* Negative space: every input that is NOT "bare call bound to a package + * import the graph could not resolve" must keep its edge. */ + CBMImport imports[] = { + {.local_name = "eq", .module_path = "drizzle-orm"}, + {.local_name = "normalize", .module_path = "./text-utils"}, + {.local_name = "getRootContainer", .module_path = "@repo/shared"}, + }; + CBMImportArray arr = {.items = imports, .count = 3, .cap = 3}; + /* The relative import and the workspace package both materialized. */ + const char *keys[] = {"normalize", "getRootContainer"}; + const int nkeys = 2; + + /* Import-/receiver-/module-aware strategies are never this guard's business. */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "import_map", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "import_map_suffix", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "same_module", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "qualified_suffix", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "lsp_ts_import", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "lsp_ts_method", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "service_pattern", &arr, keys, nkeys)); + /* A relative specifier names a path inside the tree — a missing IMPORTS edge + * there is an in-project gap, not an external binding. */ + ASSERT_FALSE( + cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0)); + /* A package the import map DOES bind resolved in-graph (workspace package). */ + ASSERT_FALSE( + cbm_suppress_external_import_shadow("getRootContainer", "unique_name", &arr, keys, nkeys)); + /* A name the file never imports (called with no import at all). */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("helper", "unique_name", &arr, keys, nkeys)); + /* Member and package/namespace-qualified callees belong to the + * receiver-aware guards, not to this one. */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq.apply", "unique_name", &arr, keys, nkeys)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("eq::apply", "unique_name", &arr, keys, nkeys)); + /* Empty / absent inputs. */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", NULL, &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow(NULL, "unique_name", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("", "unique_name", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", NULL, keys, nkeys)); + PASS(); +} + +TEST(external_import_shadow_relative_binding_wins_the_tie) { + /* A name imported from BOTH a package and a relative path keeps its edge, + * and does so independently of the order the imports were extracted in — + * the relative binding is an explicit tie-break, not an emergent property + * of iteration order. */ + CBMImport pkg_first[] = { + {.local_name = "eq", .module_path = "drizzle-orm"}, + {.local_name = "eq", .module_path = "./text-utils"}, + }; + CBMImport rel_first[] = { + {.local_name = "eq", .module_path = "./text-utils"}, + {.local_name = "eq", .module_path = "drizzle-orm"}, + }; + CBMImportArray a = {.items = pkg_first, .count = 2, .cap = 2}; + CBMImportArray b = {.items = rel_first, .count = 2, .cap = 2}; + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &a, NULL, 0)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &b, NULL, 0)); PASS(); } @@ -1072,4 +1158,7 @@ SUITE(registry) { RUN_TEST(local_binding_suppress_drops_weak_shadowed_bare_calls); RUN_TEST(local_binding_suppress_keeps_unshadowed_and_strong_strategies); RUN_TEST(weak_call_guards_share_one_drop_list); + RUN_TEST(external_import_shadow_drops_package_bound_bare_call); + RUN_TEST(external_import_shadow_keeps_everything_else); + RUN_TEST(external_import_shadow_relative_binding_wins_the_tie); } From a8293c498ca60c97c4116e40f58dffb751fe22ab Mon Sep 17 00:00:00 2001 From: Yyunozor Date: Thu, 20 Aug 2026 20:41:26 +0200 Subject: [PATCH 2/3] fix: address review feedback on external-import-shadow guard - specifier_is_relative() now also recognizes Windows drive-letter (C:\ / C:/) and UNC (\\server\share) specifiers as in-tree, not external packages. pr-smoke runs this pipeline on Windows, and the previous POSIX-only check ('.' / '/') classified those specifiers as external, which could suppress a real edge on that platform only. Added external_import_shadow_windows_relative_specifier_kept (tests/test_registry.c): red before this fix, green after. - old_workers / old_single in the #1355 pipeline test are now const char*, matching getenv()'s read-only contract. - The pad_files literal (50) is now a named EXTERNAL_IMPORT_SHADOW_PARALLEL_PAD constant with a comment tying it to MIN_FILES_FOR_PARALLEL (a private #define in pipeline.c, not reachable for a static_assert from tests), so a future bump to that threshold can't silently drop the fixture back onto the sequential-only path. Signed-off-by: Yyunozor --- src/pipeline/registry.c | 26 +++++++++++++++++++++++--- tests/test_pipeline.c | 19 ++++++++++++++++--- tests/test_registry.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index ff78c4354..6c0cdb42a 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -31,6 +31,7 @@ enum { REG_MAX_CANDIDATES = 256 }; #include "foundation/dyn_array.h" #include "foundation/platform.h" +#include #include #include #include @@ -502,10 +503,29 @@ bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_b } /* A module specifier that names a path inside the indexed tree ("./x", "../x", - * "/abs/x") rather than an external package. Package specifiers are everything - * else — "drizzle-orm", "rxjs/operators", "@scope/pkg". */ + * "/abs/x", and on Windows "C:\x", "C:/x", "\\server\share\x") rather than an + * external package. Package specifiers are everything else — "drizzle-orm", + * "rxjs/operators", "@scope/pkg". Windows paths must count as in-tree here: + * pr-smoke runs this pipeline on Windows, and misclassifying a drive-letter or + * UNC specifier as "external" is exactly the false-external edge this guard + * exists to prevent (#1355). */ static bool specifier_is_relative(const char *module_path) { - return module_path && (module_path[0] == '.' || module_path[0] == '/'); + if (!module_path || !module_path[0]) { + return false; + } + if (module_path[0] == '.' || module_path[0] == '/') { + return true; + } + /* UNC share: \\server\share\... */ + if (module_path[0] == '\\' && module_path[1] == '\\') { + return true; + } + /* Drive-letter absolute path: C:\repo\... or C:/repo/... */ + if (isalpha((unsigned char)module_path[0]) && module_path[1] == ':' && + (module_path[2] == '\\' || module_path[2] == '/')) { + return true; + } + return false; } static bool import_map_binds(const char **import_map_keys, int import_map_count, diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index fd6d51855..863254fda 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -4976,6 +4976,17 @@ static int count_nodes_named(cbm_store_t *s, const char *project, const char *na * (axios.get, api.patch on a renamed-axios instance, supertest request(app).get). * The regex false edge must stay suppressed in parallel too. CBM_WORKERS forces * >1 worker so the parallel path is taken regardless of the host core count. */ +/* #1355: padding count for write_external_import_shadow_fixture, deliberately + * well past MIN_FILES_FOR_PARALLEL (=50, a private #define in + * src/pipeline/pipeline.c — not exposed to tests, so this cannot be a + * static_assert against it) rather than sitting right at the threshold. The + * margin, not the exact value, is what the test depends on: a modest bump to + * the real threshold must not silently drop this fixture back onto the + * sequential-only path and leave the parallel resolver unexercised. Same + * pattern as ET_PARALLEL_PAD / CP_PARALLEL_PAD in test_edge_types_probe.c / + * test_convergence_probe.c. */ +enum { EXTERNAL_IMPORT_SHADOW_PARALLEL_PAD = 64 }; + /* #1355: write the shared external-import-shadow fixture into `dir`. * `pad_files` filler modules push the run over MIN_FILES_FOR_PARALLEL so the * same tree can be indexed by both resolvers. */ @@ -5066,11 +5077,13 @@ TEST(pipeline_external_import_shadow_not_bound_to_local_homonym_issue1355) { /* Enough files that CBM_WORKERS can take the fused-parallel path; the same * tree is then indexed by each resolver in turn, because the guard lives at * two independent emit sites (pass_calls.c and pass_parallel.c). */ - write_external_import_shadow_fixture(tmp, 50); + write_external_import_shadow_fixture(tmp, EXTERNAL_IMPORT_SHADOW_PARALLEL_PAD); - char *old_workers = getenv("CBM_WORKERS"); + /* getenv() returns a pointer into the process environment that must be + * treated as read-only; strdup() below only ever reads through it. */ + const char *old_workers = getenv("CBM_WORKERS"); char *saved_workers = old_workers ? strdup(old_workers) : NULL; - char *old_single = getenv("CBM_INDEX_SINGLE_THREAD"); + const char *old_single = getenv("CBM_INDEX_SINGLE_THREAD"); char *saved_single = old_single ? strdup(old_single) : NULL; cbm_setenv("CBM_INDEX_SINGLE_THREAD", "1", 1); diff --git a/tests/test_registry.c b/tests/test_registry.c index eb47679f3..d4d7545aa 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -1059,6 +1059,39 @@ TEST(external_import_shadow_relative_binding_wins_the_tie) { PASS(); } +TEST(external_import_shadow_windows_relative_specifier_kept) { + /* #1355 follow-up: a Windows-style specifier — + * drive-letter absolute (`C:\...` / `C:/...`) or UNC share (`\\server\...`) + * — names a path inside the indexed tree exactly like a POSIX "./x" or + * "/x". pr-smoke runs this pipeline on Windows; before this fix these + * specifiers fell through to "external package" and the same-name guess + * they'd otherwise validate got suppressed on that platform only. */ + CBMImport imports[] = { + {.local_name = "eq", .module_path = "C:\\repo\\src\\text-utils.ts"}, + {.local_name = "sql", .module_path = "C:/repo/src/text-utils.ts"}, + {.local_name = "normalize", .module_path = "\\\\server\\share\\text-utils.ts"}, + }; + CBMImportArray arr = {.items = imports, .count = 3, .cap = 3}; + + /* None of these materialized an import-map key (as if resolution missed + * them, mirroring the reported scenario) — the specifier alone must still + * keep the edge because it is in-tree, not external. RED before the fix + * (specifier_is_relative saw '.' / '/' only, so all three were classified + * external and suppressed); GREEN after. */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("sql", "unique_name", &arr, NULL, 0)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0)); + /* A bare package specifier without any drive letter or leading separator + * (e.g. "drizzle-orm") must still be external — this predicate must not + * become "anything with a colon or backslash". */ + CBMImport pkg_only[] = { + {.local_name = "eq", .module_path = "drizzle-orm"}, + }; + CBMImportArray parr = {.items = pkg_only, .count = 1, .cap = 1}; + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &parr, NULL, 0)); + PASS(); +} + /* ── Suite ─────────────────────────────────────────────────────── */ /* Method call THROUGH an imported symbol that is itself an indexed node @@ -1161,4 +1194,5 @@ SUITE(registry) { RUN_TEST(external_import_shadow_drops_package_bound_bare_call); RUN_TEST(external_import_shadow_keeps_everything_else); RUN_TEST(external_import_shadow_relative_binding_wins_the_tie); + RUN_TEST(external_import_shadow_windows_relative_specifier_kept); } From 797a11a15eff6154428aeb268c0b55edb5647a7a Mon Sep 17 00:00:00 2001 From: Yyunozor Date: Fri, 4 Sep 2026 00:32:18 +0200 Subject: [PATCH 3/3] fix(registry): keep workspace-sibling imports out of the external-import guard A per-language blast-radius census on twelve public repositories found one regression class in the #1355 guard, and it is not per-language: it is per-topology. In a workspace monorepo a BARE specifier can still name code inside the indexed tree. `import { eq } from "drizzle-orm"` written inside the drizzle-orm repository is a sibling package, not a dependency. Measured on drizzle-team/drizzle-orm at b7862528, full index, both resolvers: the guard removed 2609 CALLS edges. Resolving each removed edge's specifier against the repository's own package manifests puts 1377 of them (52.8%) on the file the specifier actually names. 690 of those are strict: the specifier names a subpath, that subpath maps to one directory, and the target file is inside it. The other 687 come from the bare package root, where the check can only confirm the target is somewhere in the package, so read 690 as the defensible floor. Only 373 removed edges pointed at a third-party package, which is the defect #1355 reports. The same census on eleven other repositories (flask, scrapy, express, zustand, got, cobra, gin, chi, ripgrep, gson, jq) removed 12 CALLS edges in total: nine fabricated, one lost (a documentation example importing the repository's own published name), two weak links between a shell command and a manifest key. The regression is specific to trees that ship the package they import. Consult the pipeline package map before calling a specifier external. It is keyed by the `name` of every manifest found in the tree, so a workspace registers each of its own packages there, and a subpath specifier is walked back one slash at a time ("drizzle-orm/pg-core" -> "drizzle-orm"). A specifier the tree itself claims is now treated exactly like a relative one -- kept. The check is deliberately a NAME test rather than a resolution test. A workspace package usually points `main` at a build artifact ("./index.cjs") that is not checked in, so asking whether the entry file exists in the graph answers "no" for exactly the monorepos this has to protect. Whether the tree claims the name is decidable from the manifest alone. Passing NULL restores the previous specifier-shape-only contract, which is what the unit tests exercise. After the change the same census removes 391 edges on drizzle-orm, 375 of them CALLS, and every one resolves to a third-party package; of the other eleven repositories only got changes, recovering its self-referencing edge. Signed-off-by: Yyunozor --- src/pipeline/pass_calls.c | 2 +- src/pipeline/pass_parallel.c | 2 +- src/pipeline/pipeline.h | 13 ++-- src/pipeline/registry.c | 49 +++++++++++++- tests/test_pipeline.c | 111 +++++++++++++++++++++++++++++++ tests/test_registry.c | 125 ++++++++++++++++++++++++++--------- 6 files changed, 263 insertions(+), 39 deletions(-) diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index 734e9b956..7fc6bc423 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -675,7 +675,7 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, * #725 guard, after the service-pattern bypasses above, so no HTTP/route * edge can be lost to it. */ if (cbm_suppress_external_import_shadow(call->callee_name, res.strategy, file_imports, imp_keys, - imp_count)) { + imp_count, cbm_pipeline_get_pkgmap())) { return 0; } emit_classified_edge(ctx, call, source_node, target_node, &res, module_qn, imp_keys, imp_vals, diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index 76ed4e9a4..a57d594cf 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -2571,7 +2571,7 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB } if (target_node && source_node->id != target_node->id && cbm_suppress_external_import_shadow(call->callee_name, res.strategy, &result->imports, - imp_keys, imp_count)) { + imp_keys, imp_count, cbm_pipeline_get_pkgmap())) { /* #1355: same guard as pass_calls.c — a bare call bound by an * external package import must not become a CALLS edge to an * unrelated project symbol of the same name. */ diff --git a/src/pipeline/pipeline.h b/src/pipeline/pipeline.h index f24cc3045..45ef74f84 100644 --- a/src/pipeline/pipeline.h +++ b/src/pipeline/pipeline.h @@ -19,8 +19,9 @@ #include #include -#include "discover/discover.h" /* cbm_ignored_file_t (#963) */ -#include "foundation/constants.h" /* CBM_SZ_512 */ +#include "discover/discover.h" /* cbm_ignored_file_t (#963) */ +#include "foundation/constants.h" /* CBM_SZ_512 */ +#include "foundation/hash_table.h" /* CBMHashTable (#1355 pkgmap lookup) */ /* Forward declarations */ typedef struct cbm_store cbm_store_t; @@ -297,10 +298,14 @@ bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_b * `import { eq } from "drizzle-orm"` must not make `eq(...)` a CALLS edge to an * unrelated project `eq`. A name the import map does bind, a relative * specifier, a member/qualified callee, and every import-/receiver-aware - * strategy are all kept. Pure; unit-tested in test_registry.c. */ + * strategy are all kept. `indexed_packages` is the pipeline package map: a + * specifier naming a package the tree itself declares (a workspace sibling) + * counts as in-tree and is kept too; NULL disables that check. + * Pure; unit-tested in test_registry.c. */ bool cbm_suppress_external_import_shadow(const char *callee_name, const char *strategy, const CBMImportArray *file_imports, - const char **import_map_keys, int import_map_count); + const char **import_map_keys, int import_map_count, + const CBMHashTable *indexed_packages); /* #725: drop a suffix_match CALLS edge when the caller language and the * target file's language disagree. unique_name (candidates == 1) is #1572 diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index 6c0cdb42a..adbe2c713 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -528,6 +528,43 @@ static bool specifier_is_relative(const char *module_path) { return false; } +/* True when the specifier names a package that the indexed tree DECLARES — + * a workspace sibling, not a third-party dependency. `indexed_packages` is the + * pipeline's package map, keyed by the `name` of every manifest found in the + * tree (package.json, go.mod, Cargo.toml, ...), so a pnpm/npm/cargo workspace + * registers each of its own packages here. + * + * The key is the bare package name, so a subpath specifier is walked back one + * slash at a time: "drizzle-orm/pg-core" -> "drizzle-orm". Scoped names keep + * their leading segment ("@scope/pkg/sub" -> "@scope/pkg" -> "@scope"). + * + * Deliberately a NAME test, not a resolution test. A workspace package usually + * points `main`/`module` at a build artifact ("./index.cjs") that is not + * checked in, so asking whether the entry file exists in the graph answers + * "no" for exactly the monorepos this has to protect. Whether the tree claims + * the name is decidable from the manifest alone. */ +static bool specifier_names_indexed_package(const CBMHashTable *indexed_packages, + const char *module_path) { + if (!indexed_packages || !module_path || !module_path[0]) { + return false; + } + if (cbm_ht_has(indexed_packages, module_path)) { + return true; + } + char buf[CBM_SZ_512]; + if (strlen(module_path) >= sizeof(buf)) { + return false; + } + snprintf(buf, sizeof(buf), "%s", module_path); + for (char *slash = strrchr(buf, '/'); slash != NULL; slash = strrchr(buf, '/')) { + *slash = '\0'; + if (cbm_ht_has(indexed_packages, buf)) { + return true; + } + } + return false; +} + static bool import_map_binds(const char **import_map_keys, int import_map_count, const char *local_name) { if (!import_map_keys || import_map_count <= 0) { @@ -569,10 +606,17 @@ static bool import_map_binds(const char **import_map_keys, int import_map_count, * relative and a package specifier keeps the edge — the relative binding wins * the tie explicitly, so the outcome does not depend on import order. * + * `indexed_packages` extends that same reasoning to workspace monorepos, where + * a BARE specifier also names in-tree code: `import { eq } from "drizzle-orm"` + * inside the drizzle-orm repository is a sibling package, not a dependency. A + * specifier the tree's own manifests claim is treated exactly like a relative + * one. NULL disables the check and restores the specifier-shape-only contract. + * * Pure + side-effect-free so the contract is unit-testable without a pipeline. */ bool cbm_suppress_external_import_shadow(const char *callee_name, const char *strategy, const CBMImportArray *file_imports, - const char **import_map_keys, int import_map_count) { + const char **import_map_keys, int import_map_count, + const CBMHashTable *indexed_packages) { if (!callee_name || !callee_name[0] || !strategy || !strategy[0]) { return false; } @@ -595,7 +639,8 @@ bool cbm_suppress_external_import_shadow(const char *callee_name, const char *st if (!imp->local_name || !imp->module_path || strcmp(imp->local_name, callee_name) != 0) { continue; } - if (specifier_is_relative(imp->module_path)) { + if (specifier_is_relative(imp->module_path) || + specifier_names_indexed_package(indexed_packages, imp->module_path)) { return false; /* an in-tree binding for this name — never suppress */ } external_binding = true; diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 863254fda..f772acc69 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -5112,6 +5112,116 @@ TEST(pipeline_external_import_shadow_not_bound_to_local_homonym_issue1355) { PASS(); } +/* #1355 / #1732: the same tree, but the bare specifier now names a WORKSPACE + * package that the repository itself ships. `main` deliberately points at a + * build artifact that is not checked in — the shape every real monorepo has, + * and the reason an entry-file lookup cannot answer this question. */ +static void write_workspace_shadow_fixture(const char *dir, int pad_files) { + /* One directory level only: write_temp_file's mkdir is documented as + * "simple version, one level", and a deeper path fails silently — which + * makes the fixture look healthy while the file never lands. */ + write_temp_file(dir, "lib/package.json", + "{\n \"name\": \"@fixture/lib\",\n \"main\": \"./dist/index.js\"\n}\n"); + write_temp_file(dir, "lib/index.ts", + "export function wsEq(a: string, b: string): boolean {\n" + " return a === b;\n" + "}\n"); + write_temp_file(dir, "app/package.json", "{\n \"name\": \"@fixture/app\"\n}\n"); + /* The lone project symbol named wsSpec — an ordinary local helper that a + * project-wide guess would happily bind an external `wsSpec` to. */ + write_temp_file(dir, "app/spec-helpers.ts", + "export function wsSpec(label: string): string {\n" + " return label;\n" + "}\n"); + write_temp_file(dir, "app/query.ts", + "import { wsEq } from '@fixture/lib';\n" + "import { wsSpec } from 'vitest';\n" + "export function wsBuild(id: string): unknown {\n" + " return [wsEq(id, id), wsSpec('case')];\n" + "}\n"); + for (int i = 0; i < pad_files; i++) { + char name[64]; + char body[128]; + snprintf(name, sizeof(name), "app/pad_%02d.ts", i); + snprintf(body, sizeof(body), "export function wsPad%02d(): number { return %d; }\n", i, i); + write_temp_file(dir, name, body); + } +} + +static int assert_workspace_shadow_contract(const char *dir, const char *db_name) { + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/%s", dir, db_name); + cbm_pipeline_t *p = cbm_pipeline_new(dir, db_path, CBM_MODE_FULL); + if (!p) { + return 1; + } + if (cbm_pipeline_run(p) != 0) { + cbm_pipeline_free(p); + return 2; + } + const char *project = cbm_pipeline_project_name(p); + cbm_store_t *s = cbm_store_open_path(db_path); + if (!s) { + cbm_pipeline_free(p); + return 3; + } + int rc = 0; + /* (1) the workspace sibling: the tree declares @fixture/lib, so the bare + * specifier is in-project and the fallback keeps its chance. RED without + * the package-map check — this is the 1377-edge class measured on + * drizzle-team/drizzle-orm. */ + if (!cross_file_call_exists(s, project, "wsBuild", "wsEq")) { + rc = 4; + } + /* (2) a genuine third-party package in the very same file is still cut: + * the check must not disarm the guard wholesale. */ + if (rc == 0 && cross_file_call_exists(s, project, "wsBuild", "wsSpec")) { + rc = 5; + } + cbm_store_close(s); + cbm_pipeline_free(p); + return rc; +} + +TEST(pipeline_external_import_shadow_keeps_workspace_sibling_issue1355) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_ws_import_shadow_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + write_workspace_shadow_fixture(tmp, EXTERNAL_IMPORT_SHADOW_PARALLEL_PAD); + + const char *old_workers = getenv("CBM_WORKERS"); + char *saved_workers = old_workers ? strdup(old_workers) : NULL; + const char *old_single = getenv("CBM_INDEX_SINGLE_THREAD"); + char *saved_single = old_single ? strdup(old_single) : NULL; + + cbm_setenv("CBM_INDEX_SINGLE_THREAD", "1", 1); + int sequential = assert_workspace_shadow_contract(tmp, "ws-sequential.db"); + + cbm_unsetenv("CBM_INDEX_SINGLE_THREAD"); + cbm_setenv("CBM_WORKERS", "4", 1); + int parallel = assert_workspace_shadow_contract(tmp, "ws-parallel.db"); + + if (saved_workers) { + cbm_setenv("CBM_WORKERS", saved_workers, 1); + free(saved_workers); + } else { + cbm_unsetenv("CBM_WORKERS"); + } + if (saved_single) { + cbm_setenv("CBM_INDEX_SINGLE_THREAD", saved_single, 1); + free(saved_single); + } else { + cbm_unsetenv("CBM_INDEX_SINGLE_THREAD"); + } + th_rmtree(tmp); + + ASSERT_EQ(sequential, 0); + ASSERT_EQ(parallel, 0); + PASS(); +} + TEST(pipeline_tsjs_receiver_parallel_keeps_service_edges) { char tmp[256]; snprintf(tmp, sizeof(tmp), "/tmp/cbm_tsjs_par_XXXXXX"); @@ -13461,6 +13571,7 @@ SUITE(pipeline) { RUN_TEST(pipeline_python_bare_local_binding_suppresses_weak_edge); RUN_TEST(pipeline_python_bare_local_binding_parallel_suppresses_weak_edge); RUN_TEST(pipeline_external_import_shadow_not_bound_to_local_homonym_issue1355); + RUN_TEST(pipeline_external_import_shadow_keeps_workspace_sibling_issue1355); RUN_TEST(pipeline_parallel_python_cross_only_dunder_gets_synthetic_carrier); RUN_TEST(pipeline_parallel_rust_cross_only_macro_hidden_gets_synthetic_carrier); RUN_TEST(pipeline_arg_url_rejects_non_http_slash_arguments); diff --git a/tests/test_registry.c b/tests/test_registry.c index d4d7545aa..d8494ce82 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -970,6 +970,9 @@ TEST(weak_call_guards_share_one_drop_list) { } ASSERT_EQ(member, binding); } + PASS(); +} + TEST(external_import_shadow_drops_package_bound_bare_call) { /* #1355: `import { eq, sql } from "drizzle-orm"` binds both names to a * package outside the indexed tree, so no import-map key exists for them and @@ -983,14 +986,14 @@ TEST(external_import_shadow_drops_package_bound_bare_call) { /* Only the relative import materialized an IMPORTS edge. */ const char *keys[] = {"users"}; - ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, keys, 1)); - ASSERT_TRUE(cbm_suppress_external_import_shadow("sql", "unique_name", &arr, keys, 1)); - ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "suffix_match", &arr, keys, 1)); - ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "field_type_hint", &arr, keys, 1)); - ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "fuzzy", &arr, keys, 1)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, keys, 1, NULL)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("sql", "unique_name", &arr, keys, 1, NULL)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "suffix_match", &arr, keys, 1, NULL)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "field_type_hint", &arr, keys, 1, NULL)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "fuzzy", &arr, keys, 1, NULL)); /* A file whose imports all failed to materialize is still covered: the * evidence is the import statement, not the map. */ - ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, NULL)); PASS(); } @@ -1008,34 +1011,42 @@ TEST(external_import_shadow_keeps_everything_else) { const int nkeys = 2; /* Import-/receiver-/module-aware strategies are never this guard's business. */ - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "import_map", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "import_map_suffix", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "same_module", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "qualified_suffix", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "lsp_ts_import", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "lsp_ts_method", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "service_pattern", &arr, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "import_map", &arr, keys, nkeys, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("eq", "import_map_suffix", &arr, keys, nkeys, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "same_module", &arr, keys, nkeys, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("eq", "qualified_suffix", &arr, keys, nkeys, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("eq", "lsp_ts_import", &arr, keys, nkeys, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("eq", "lsp_ts_method", &arr, keys, nkeys, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("eq", "service_pattern", &arr, keys, nkeys, NULL)); /* A relative specifier names a path inside the tree — a missing IMPORTS edge * there is an in-project gap, not an external binding. */ ASSERT_FALSE( - cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0)); - /* A package the import map DOES bind resolved in-graph (workspace package). */ + cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, keys, nkeys, NULL)); ASSERT_FALSE( - cbm_suppress_external_import_shadow("getRootContainer", "unique_name", &arr, keys, nkeys)); + cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0, NULL)); + /* A package the import map DOES bind resolved in-graph (workspace package). */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("getRootContainer", "unique_name", &arr, keys, + nkeys, NULL)); /* A name the file never imports (called with no import at all). */ - ASSERT_FALSE(cbm_suppress_external_import_shadow("helper", "unique_name", &arr, keys, nkeys)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("helper", "unique_name", &arr, keys, nkeys, NULL)); /* Member and package/namespace-qualified callees belong to the * receiver-aware guards, not to this one. */ - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq.apply", "unique_name", &arr, keys, nkeys)); ASSERT_FALSE( - cbm_suppress_external_import_shadow("eq::apply", "unique_name", &arr, keys, nkeys)); + cbm_suppress_external_import_shadow("eq.apply", "unique_name", &arr, keys, nkeys, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("eq::apply", "unique_name", &arr, keys, nkeys, NULL)); /* Empty / absent inputs. */ - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", NULL, &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow(NULL, "unique_name", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("", "unique_name", &arr, keys, nkeys)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", NULL, keys, nkeys)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", NULL, &arr, keys, nkeys, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "", &arr, keys, nkeys, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow(NULL, "unique_name", &arr, keys, nkeys, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("", "unique_name", &arr, keys, nkeys, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", NULL, keys, nkeys, NULL)); PASS(); } @@ -1054,8 +1065,8 @@ TEST(external_import_shadow_relative_binding_wins_the_tie) { }; CBMImportArray a = {.items = pkg_first, .count = 2, .cap = 2}; CBMImportArray b = {.items = rel_first, .count = 2, .cap = 2}; - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &a, NULL, 0)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &b, NULL, 0)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &a, NULL, 0, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &b, NULL, 0, NULL)); PASS(); } @@ -1078,9 +1089,10 @@ TEST(external_import_shadow_windows_relative_specifier_kept) { * keep the edge because it is in-tree, not external. RED before the fix * (specifier_is_relative saw '.' / '/' only, so all three were classified * external and suppressed); GREEN after. */ - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("sql", "unique_name", &arr, NULL, 0)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("sql", "unique_name", &arr, NULL, 0, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0, NULL)); /* A bare package specifier without any drive letter or leading separator * (e.g. "drizzle-orm") must still be external — this predicate must not * become "anything with a colon or backslash". */ @@ -1088,7 +1100,57 @@ TEST(external_import_shadow_windows_relative_specifier_kept) { {.local_name = "eq", .module_path = "drizzle-orm"}, }; CBMImportArray parr = {.items = pkg_only, .count = 1, .cap = 1}; - ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &parr, NULL, 0)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &parr, NULL, 0, NULL)); + PASS(); +} + +TEST(external_import_shadow_workspace_sibling_kept) { + /* #1355 / #1732: inside a workspace monorepo a BARE specifier can still name + * in-tree code — `import { eq } from "drizzle-orm"` written inside the + * drizzle-orm repository is a sibling package, not a dependency. Measured on + * drizzle-team/drizzle-orm at b7862528: without this check the guard removed + * 2609 CALLS edges, 1377 of which pointed at exactly the file the specifier + * names. The package map is keyed by each in-tree manifest's `name`, so the + * tree's own claim decides. */ + CBMImport imports[] = { + {.local_name = "eq", .module_path = "drizzle-orm"}, + {.local_name = "pgTable", .module_path = "drizzle-orm/pg-core"}, + {.local_name = "hoisted", .module_path = "@scope/kit/sub"}, + {.local_name = "test", .module_path = "vitest"}, + }; + CBMImportArray arr = {.items = imports, .count = 4, .cap = 4}; + + CBMHashTable *pkgs = cbm_ht_create(8); + ASSERT_NOT_NULL(pkgs); + cbm_ht_set(pkgs, "drizzle-orm", (void *)"qn"); + cbm_ht_set(pkgs, "@scope/kit", (void *)"qn"); + + /* Exact name, subpath, and scoped subpath all resolve to an in-tree + * package — the same-name fallback keeps its chance. */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, pkgs)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("pgTable", "suffix_match", &arr, NULL, 0, pkgs)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("hoisted", "unique_name", &arr, NULL, 0, pkgs)); + /* A genuine third-party dependency is untouched by the new check: the tree + * declares no package called "vitest", so the #1355 removal still happens. */ + ASSERT_TRUE(cbm_suppress_external_import_shadow("test", "suffix_match", &arr, NULL, 0, pkgs)); + + /* A tree with no manifests at all (pkgmap NULL) keeps the pre-existing + * specifier-shape-only contract — every one of these is external again. */ + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, NULL)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("pgTable", "suffix_match", &arr, NULL, 0, NULL)); + + /* An unrelated package map must not accidentally match by prefix: "drizzle" + * is not "drizzle-orm", and the walk is by path segment, not by substring. */ + CBMHashTable *other = cbm_ht_create(8); + ASSERT_NOT_NULL(other); + cbm_ht_set(other, "drizzle", (void *)"qn"); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, other)); + + cbm_ht_free(other); + cbm_ht_free(pkgs); PASS(); } @@ -1195,4 +1257,5 @@ SUITE(registry) { RUN_TEST(external_import_shadow_keeps_everything_else); RUN_TEST(external_import_shadow_relative_binding_wins_the_tie); RUN_TEST(external_import_shadow_windows_relative_specifier_kept); + RUN_TEST(external_import_shadow_workspace_sibling_kept); }