From eaf59046e5371eda24bfafc60743291b429e79d8 Mon Sep 17 00:00:00 2001 From: Yyunozor Date: Thu, 20 Aug 2026 01:31:29 +0200 Subject: [PATCH 1/5] 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/5] 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/5] 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); } From 138a62253d28b66650cf2d4ea25ba7c3b4f3a111 Mon Sep 17 00:00:00 2001 From: Yyunozor Date: Sat, 19 Sep 2026 23:46:59 +0200 Subject: [PATCH 4/5] fix(registry): resolve package-path imports against the project's own packages JVM package imports were read as external. A Kotlin or Java specifier is a dotted package path whether the package belongs to the indexed tree or to a dependency -- `import org.jetbrains.exposed.v1.tests.shared.assertEquals` and `import kotlin.test.assertEquals` have the same shape, and neither is "relative". The package map cannot separate them either: the manifests it reads for the JVM (pom.xml, build.gradle) name build artifacts, not packages. So every package-path specifier fell through to "external" and the guard cut calls into the project's own code. Measured on JetBrains/Exposed at 0e4d81a5, full index: the guard removed 426 CALLS edges, all of them suffix_match, all of them into the repository's own exposed-tests/.../shared/Assert.kt. Every callee was assertEquals, assertTrue or assertFalse -- names that file declares and that the test files import twice, once from the project package and once from kotlin.test. The double import is what leaves the import map without a key and hands the call to the project-wide guess. The evidence that separates the two is the tree's own `package` declarations, and the pipeline already collects them: the import passes build a namespace map keyed by the dot-normalized `package` / `namespace` / `use` of every indexed file, to resolve namespace imports. Publish that map for the duration of the run instead of freeing it at the end of the import pass, and consult it before calling a package-path specifier external. The trailing member segment is walked off one at a time, the same way the namespace-map lookup in cbm_pipeline_resolve_import_node does it. This keeps the effect the census called valuable. `java.net.URL` walks to "java.net" and then "java", neither of which any project file declares, so a bare `URL` bound to an in-tree URL class is still cut. Only specifiers the tree itself claims are kept. Passing NULL restores the previous contract, which is what the unit tests exercise. After the change Exposed carries 31195 CALLS edges, the same as the merge-base, with none removed and none added -- stable across 28 indexations of the repository (8 with this branch, 20 single-threaded). Total edge counts are not a usable invariant here: roughly one indexation in ten materializes a `.properties` resource file as a File node under Folder nodes instead of a Module with its Variables, on the merge-base binary as well, and the file it picks varies. That flip never touches a CALLS edge and disappears entirely under CBM_INDEX_SINGLE_THREAD=1. The walk that strips the trailing member segment has no floor, on purpose: a third-party package published under a prefix the tree declares reads as in-tree and the guard stands down, leaving exactly the edge the merge-base emits. A call-resolution suppressor has to fail on that side. Signed-off-by: Yyunozor --- src/pipeline/pass_calls.c | 3 +- src/pipeline/pass_definitions.c | 4 +- src/pipeline/pass_parallel.c | 9 +- src/pipeline/pipeline.c | 14 ++ src/pipeline/pipeline.h | 7 +- src/pipeline/pipeline_incremental.c | 6 + src/pipeline/pipeline_internal.h | 10 ++ src/pipeline/registry.c | 93 ++++++++++++- tests/test_pipeline.c | 127 +++++++++++++++++ tests/test_registry.c | 204 ++++++++++++++++++++++------ 10 files changed, 429 insertions(+), 48 deletions(-) diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index 7fc6bc423..f0f9b499f 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -675,7 +675,8 @@ 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, cbm_pipeline_get_pkgmap())) { + imp_count, cbm_pipeline_get_pkgmap(), + cbm_pipeline_get_nsmap())) { return 0; } emit_classified_edge(ctx, call, source_node, target_node, &res, module_qn, imp_keys, imp_vals, diff --git a/src/pipeline/pass_definitions.c b/src/pipeline/pass_definitions.c index 7ac98e9cd..eac9d0a96 100644 --- a/src/pipeline/pass_definitions.c +++ b/src/pipeline/pass_definitions.c @@ -844,7 +844,9 @@ int cbm_pipeline_pass_definitions(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t create_channel_edges_for_file(ctx, result, files[i].rel_path); cbm_pipeline_create_env_configures_for_file(ctx, result, files[i].rel_path); } - cbm_pipeline_namespace_map_free(namespace_map); + /* Publish instead of free — see the twin in pass_parallel.c (#1355). */ + cbm_pipeline_namespace_map_free(cbm_pipeline_get_nsmap()); + cbm_pipeline_set_nsmap(namespace_map); if (owns_local_cache) { for (int i = 0; i < file_count; i++) { if (local_cache[i]) { diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index a57d594cf..d7aa7bba9 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -1343,7 +1343,11 @@ int cbm_build_registry_from_cache(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t cbm_pipeline_create_env_configures_for_file(ctx, result, rel); } - cbm_pipeline_namespace_map_free(namespace_map); + /* Publish instead of free: the call pass needs the same declared-package + * set to tell an in-tree package path from a third-party one (#1355). + * The pipeline owns it from here and frees it in its cleanup. */ + cbm_pipeline_namespace_map_free(cbm_pipeline_get_nsmap()); + cbm_pipeline_set_nsmap(namespace_map); cbm_log_info("parallel.registry.done", "entries", itoa_log(reg_entries), "defines", itoa_log(defines_edges), "imports", itoa_log(imports_edges)); @@ -2571,7 +2575,8 @@ 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, cbm_pipeline_get_pkgmap())) { + imp_keys, imp_count, cbm_pipeline_get_pkgmap(), + cbm_pipeline_get_nsmap())) { /* #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.c b/src/pipeline/pipeline.c index 938437549..1f15263a4 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -229,6 +229,18 @@ void cbm_pipeline_set_pkgmap(CBMHashTable *map) { g_pkgmap = map; } +/* ── Global declared-package map (one active pipeline at a time) ── */ + +static CBMHashTable *g_nsmap = NULL; + +CBMHashTable *cbm_pipeline_get_nsmap(void) { + return g_nsmap; +} + +void cbm_pipeline_set_nsmap(CBMHashTable *map) { + g_nsmap = map; +} + bool cbm_pipeline_had_format_migration(const cbm_pipeline_t *p) { return p && p->format_migration; } @@ -2349,6 +2361,8 @@ static int cbm_pipeline_run_staged(cbm_pipeline_t *p) { cleanup: cbm_pkgmap_free(cbm_pipeline_get_pkgmap()); cbm_pipeline_set_pkgmap(NULL); + cbm_pipeline_namespace_map_free(cbm_pipeline_get_nsmap()); + cbm_pipeline_set_nsmap(NULL); cbm_discover_free(files, file_count); cbm_pipeline_free_semantic_manifest(baseline_manifest, baseline_count); cbm_gbuf_free(p->gbuf); diff --git a/src/pipeline/pipeline.h b/src/pipeline/pipeline.h index 45ef74f84..81afe6b9a 100644 --- a/src/pipeline/pipeline.h +++ b/src/pipeline/pipeline.h @@ -301,11 +301,16 @@ bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_b * 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. + * `declared_packages` is the pipeline namespace map and does the same job for + * package-path specifiers, which have no in-tree/external shape of their own: + * `import org.example.util.assertThing` is kept when some indexed file + * declares `package org.example.util`; 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 CBMHashTable *indexed_packages); + const CBMHashTable *indexed_packages, + const CBMHashTable *declared_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/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index bf71a1f84..6f147b25a 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -1612,6 +1612,8 @@ static int closure_probe_surfaces(cbm_pipeline_t *p, const char *project, * leaked one map per probed run (LSan, macOS CI). */ cbm_pkgmap_free(cbm_pipeline_get_pkgmap()); cbm_pipeline_set_pkgmap(NULL); + cbm_pipeline_namespace_map_free(cbm_pipeline_get_nsmap()); + cbm_pipeline_set_nsmap(NULL); cbm_path_alias_collection_free(aliases); cbm_gbuf_free(probe_gbuf); return rc; @@ -2183,6 +2185,8 @@ static int run_closure_delta(cbm_pipeline_t *p, const char *db_path, const char } cbm_pkgmap_free(cbm_pipeline_get_pkgmap()); cbm_pipeline_set_pkgmap(NULL); + cbm_pipeline_namespace_map_free(cbm_pipeline_get_nsmap()); + cbm_pipeline_set_nsmap(NULL); if (phase_rc != 0) { cbm_log_error("delta.err", "phase", "extract_resolve", "rc", itoa_buf(phase_rc)); goto out; @@ -2762,6 +2766,8 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil * full pipeline's ownership boundary on both success and failure. */ cbm_pkgmap_free(cbm_pipeline_get_pkgmap()); cbm_pipeline_set_pkgmap(NULL); + cbm_pipeline_namespace_map_free(cbm_pipeline_get_nsmap()); + cbm_pipeline_set_nsmap(NULL); if (phase_rc != 0) { cbm_log_error("incremental.err", "phase", "extract_resolve", "rc", itoa_buf(phase_rc)); diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 45b0baf6b..a8e0d5616 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -172,6 +172,16 @@ static inline int cbm_pipeline_relpath_is_excluded(const char *rel_path, char *c CBMHashTable *cbm_pipeline_get_pkgmap(void); void cbm_pipeline_set_pkgmap(CBMHashTable *map); +/* Get the current pipeline's declared-package map (NULL if none): the + * namespace/package → File-QN map built by cbm_pipeline_namespace_map_build + * from every indexed file's own `package`/`namespace`/`using` declaration. + * The import passes build it to resolve namespace imports; it is published + * here so the call passes can ask whether a package-path specifier names a + * package the tree itself declares (#1355). Set before resolve starts, read + * only afterwards — same single-writer contract as the pkgmap above. */ +CBMHashTable *cbm_pipeline_get_nsmap(void); +void cbm_pipeline_set_nsmap(CBMHashTable *map); + /* Unified module resolver: relative → pkgmap → fqn_module fallback. * Handles bare specifiers via pkgmap lookup with prefix matching. * Caller must free() the returned string. */ diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index adbe2c713..f16c0ddb8 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -565,6 +565,84 @@ static bool specifier_names_indexed_package(const CBMHashTable *indexed_packages return false; } +/* True when the specifier is a package PATH the indexed tree itself DECLARES. + * `declared_packages` is the pipeline's namespace map, keyed by the + * dot-normalized `package` / `namespace` / `use` declaration of every indexed + * file (cbm_pipeline_namespace_map_build), so this is the JVM/CLR/PHP + * counterpart of the manifest-name test above: those languages have no + * specifier shape that separates in-tree from third-party, and their manifests + * (pom.xml, build.gradle) name artifacts, not packages. + * + * A package-path specifier names a member inside a package, so the trailing + * segment is walked off one at a time, exactly like the namespace-map lookup in + * cbm_pipeline_resolve_import_node: `org.jetbrains.exposed.v1.tests.shared + * .assertEquals` -> `org.jetbrains.exposed.v1.tests.shared`, which Exposed's + * own Assert.kt declares, so the call keeps its edge. `java.net.URL` walks to + * `java.net` then `java`, which no project file declares, so it stays external + * and the guard still cuts its bind to an in-tree `URL` class. + * + * Separator normalization mirrors the map builder's: PHP writes `\`, some + * grammars `::` or `/`. NULL disables the check. */ +static bool specifier_names_declared_package(const CBMHashTable *declared_packages, + const char *module_path) { + if (!declared_packages || !module_path || !module_path[0]) { + return false; + } + char buf[CBM_SZ_512]; + if (strlen(module_path) >= sizeof(buf)) { + return false; + } + snprintf(buf, sizeof(buf), "%s", module_path); + char *brace = strchr(buf, '{'); + if (brace) { + *brace = '\0'; /* grouped import — the prefix is the package */ + } + char *alias = strstr(buf, " as "); + if (alias) { + *alias = '\0'; + } + for (char *p = buf; *p; p++) { + if (*p == '\\' || *p == ':' || *p == '/') { + *p = '.'; + } + } + /* Collapse the empty segments "::" just produced, then strip glob/ + * separator noise off the tail. */ + for (;;) { + char *dd = strstr(buf, ".."); + if (!dd) { + break; + } + memmove(dd, dd + 1, strlen(dd + 1) + 1); + } + size_t len = strlen(buf); + while (len > 0 && (buf[len - 1] == '*' || buf[len - 1] == '.' || buf[len - 1] == ' ')) { + buf[--len] = '\0'; + } + if (buf[0] == '\0') { + return false; + } + if (cbm_ht_has(declared_packages, buf)) { + return true; + } + /* The walk has no floor, and that is a deliberate false-NEGATIVE: once the + * tree declares `com.example`, EVERY specifier under `com.example.**` reads + * as in-tree, including a third-party library that happens to publish under + * a declared prefix. Such a call keeps the same-name fallback it has on + * main, so the cost is a fabricated edge this guard could have removed — + * never a real edge lost. Stopping earlier would need a rule for where a + * package path ends and a type begins, and getting that wrong fails the + * other way. Pinned by external_import_shadow_declared_prefix_is_permissive + * in test_registry.c. */ + for (char *dot = strrchr(buf, '.'); dot != NULL; dot = strrchr(buf, '.')) { + *dot = '\0'; + if (cbm_ht_has(declared_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) { @@ -612,11 +690,21 @@ static bool import_map_binds(const char **import_map_keys, int import_map_count, * 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. * + * `declared_packages` closes the same hole for the package-path languages, + * where NO specifier shape says in-tree: `import + * org.jetbrains.exposed.v1.tests.shared.assertEquals` is spelled exactly like + * `import kotlin.test.assertEquals`, and only the tree's own `package` + * declarations tell them apart. It is permissive by construction — any + * specifier UNDER a declared package prefix counts as in-tree, see the note on + * the walk — so it can leave a fabricated edge in place, never remove a real + * one. NULL disables the check. + * * 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 CBMHashTable *indexed_packages) { + const CBMHashTable *indexed_packages, + const CBMHashTable *declared_packages) { if (!callee_name || !callee_name[0] || !strategy || !strategy[0]) { return false; } @@ -640,7 +728,8 @@ bool cbm_suppress_external_import_shadow(const char *callee_name, const char *st continue; } if (specifier_is_relative(imp->module_path) || - specifier_names_indexed_package(indexed_packages, imp->module_path)) { + specifier_names_indexed_package(indexed_packages, imp->module_path) || + specifier_names_declared_package(declared_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 f772acc69..36bb243a3 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -5222,6 +5222,132 @@ TEST(pipeline_external_import_shadow_keeps_workspace_sibling_issue1355) { PASS(); } +/* #1355: the same tree once more, in a package-path language. A Kotlin/Java + * specifier is a dotted package path whether the package is the project's own + * or a dependency, so specifier shape decides nothing and the package map only + * knows manifest artifact names. The tree's own `package` declarations are the + * evidence, and the pipeline already collects them (the namespace map the + * import passes build). `assertEquals` is imported twice — from the project's + * package and from kotlin.test — exactly as JetBrains/Exposed writes it, which + * is what leaves the import map without a key and hands the call to the + * project-wide guess. */ +static void write_declared_package_shadow_fixture(const char *dir, int pad_files) { + /* One directory level only — write_temp_file's mkdir does not recurse, and + * a deeper path fails silently. The package path lives in the `package` + * declaration, not in the directory layout, which is the whole point. */ + write_temp_file(dir, "util/Assert.kt", + "package com.example.util\n" + "\n" + "fun pkgEq(a: String, b: String): Boolean {\n" + " return a == b\n" + "}\n"); + /* The lone project symbol named pkgSpec, in a third package so the guess + * that reaches it is a project-wide one and not a same-module hit. */ + write_temp_file(dir, "spec/SpecHelpers.kt", + "package com.example.spec\n" + "\n" + "fun pkgSpec(label: String): Boolean {\n" + " return label.isNotEmpty()\n" + "}\n"); + write_temp_file(dir, "app/Query.kt", + "package com.example.app\n" + "\n" + "import com.example.util.pkgEq\n" + "import kotlin.test.pkgEq\n" + "import kotlin.test.pkgSpec\n" + "import org.junit.pkgSpec\n" + "\n" + "fun pkgBuild(id: String): Boolean {\n" + " return pkgEq(id, id) && pkgSpec(id)\n" + "}\n"); + for (int i = 0; i < pad_files; i++) { + char name[64]; + char body[128]; + snprintf(name, sizeof(name), "app/Pad%02d.kt", i); + snprintf(body, sizeof(body), + "package com.example.app\n\nfun pkgPad%02d(): Int {\n return %d\n}\n", i, i); + write_temp_file(dir, name, body); + } +} + +static int assert_declared_package_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; + /* The package the tree declares: `com.example.util` is in-project, so the + * call keeps its edge. RED without the declared-package check — this is the + * 426-edge class measured on JetBrains/Exposed. + * + * The matching negative — an undeclared package is still cut — is asserted + * where it can be reached on BOTH resolvers: directly on the predicate in + * test_registry.c (external_import_shadow_declared_package_kept, including + * `java.net.URL`), and end-to-end in the route fixture below. The + * sequential resolver builds its own import map straight from the + * extraction records, without the ambiguity check its parallel twin + * applies, so a doubly-imported Kotlin name resolves by `import_map` there + * and never reaches this guard at all — asserting the negative here would + * assert that quirk, not this contract. */ + if (!cross_file_call_exists(s, project, "pkgBuild", "pkgEq")) { + rc = 4; + } + cbm_store_close(s); + cbm_pipeline_free(p); + return rc; +} + +TEST(pipeline_external_import_shadow_keeps_declared_package_issue1355) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_pkg_import_shadow_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + write_declared_package_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_declared_package_shadow_contract(tmp, "pkg-sequential.db"); + + cbm_unsetenv("CBM_INDEX_SINGLE_THREAD"); + cbm_setenv("CBM_WORKERS", "4", 1); + int parallel = assert_declared_package_shadow_contract(tmp, "pkg-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"); @@ -13572,6 +13698,7 @@ SUITE(pipeline) { 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_external_import_shadow_keeps_declared_package_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 d8494ce82..d12048d83 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -986,14 +986,19 @@ 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, 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)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("eq", "unique_name", &arr, keys, 1, NULL, NULL)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("sql", "unique_name", &arr, keys, 1, NULL, NULL)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("eq", "suffix_match", &arr, keys, 1, NULL, NULL)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("eq", "field_type_hint", &arr, keys, 1, NULL, NULL)); + ASSERT_TRUE(cbm_suppress_external_import_shadow("eq", "fuzzy", &arr, keys, 1, NULL, 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, NULL)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, NULL, NULL)); PASS(); } @@ -1011,42 +1016,47 @@ 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, 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)); + cbm_suppress_external_import_shadow("eq", "import_map", &arr, keys, nkeys, NULL, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "import_map_suffix", &arr, keys, nkeys, + NULL, NULL)); ASSERT_FALSE( - cbm_suppress_external_import_shadow("eq", "qualified_suffix", &arr, keys, nkeys, NULL)); + cbm_suppress_external_import_shadow("eq", "same_module", &arr, keys, nkeys, NULL, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "qualified_suffix", &arr, keys, nkeys, + NULL, NULL)); ASSERT_FALSE( - cbm_suppress_external_import_shadow("eq", "lsp_ts_import", &arr, keys, nkeys, NULL)); + cbm_suppress_external_import_shadow("eq", "lsp_ts_import", &arr, keys, nkeys, NULL, 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)); + cbm_suppress_external_import_shadow("eq", "lsp_ts_method", &arr, keys, nkeys, NULL, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "service_pattern", &arr, keys, nkeys, + NULL, 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, + NULL, NULL)); ASSERT_FALSE( - cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, keys, nkeys, NULL)); - ASSERT_FALSE( - cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0, NULL)); + cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0, NULL, 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)); + nkeys, NULL, 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, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("helper", "unique_name", &arr, keys, nkeys, + NULL, 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, + NULL, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq::apply", "unique_name", &arr, keys, nkeys, + NULL, NULL)); + /* Empty / absent inputs. */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", NULL, &arr, keys, nkeys, NULL, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "", &arr, keys, nkeys, NULL, NULL)); ASSERT_FALSE( - cbm_suppress_external_import_shadow("eq.apply", "unique_name", &arr, keys, nkeys, NULL)); + cbm_suppress_external_import_shadow(NULL, "unique_name", &arr, keys, nkeys, NULL, 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, 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)); + cbm_suppress_external_import_shadow("", "unique_name", &arr, keys, nkeys, NULL, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("eq", "unique_name", NULL, keys, nkeys, NULL, NULL)); PASS(); } @@ -1065,8 +1075,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, NULL)); - ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &b, NULL, 0, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &a, NULL, 0, NULL, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &b, NULL, 0, NULL, NULL)); PASS(); } @@ -1089,10 +1099,12 @@ 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, 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)); + cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, NULL, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("sql", "unique_name", &arr, NULL, 0, NULL, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("normalize", "unique_name", &arr, NULL, 0, NULL, 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". */ @@ -1100,7 +1112,8 @@ 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, NULL)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("eq", "unique_name", &parr, NULL, 0, NULL, NULL)); PASS(); } @@ -1127,33 +1140,140 @@ TEST(external_import_shadow_workspace_sibling_kept) { /* 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)); + cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, pkgs, NULL)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("pgTable", "suffix_match", &arr, NULL, 0, pkgs, NULL)); ASSERT_FALSE( - cbm_suppress_external_import_shadow("hoisted", "unique_name", &arr, NULL, 0, pkgs)); + cbm_suppress_external_import_shadow("hoisted", "unique_name", &arr, NULL, 0, pkgs, NULL)); /* 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)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("test", "suffix_match", &arr, NULL, 0, pkgs, NULL)); /* 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)); + cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, NULL, NULL)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("pgTable", "suffix_match", &arr, NULL, 0, NULL, 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)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("eq", "unique_name", &arr, NULL, 0, other, NULL)); cbm_ht_free(other); cbm_ht_free(pkgs); PASS(); } +TEST(external_import_shadow_declared_package_kept) { + /* #1355: package-path specifiers (Java, Kotlin, C#, PHP) carry NO shape + * that separates in-tree from third-party — `import + * org.jetbrains.exposed.v1.tests.shared.assertEquals` is spelled exactly + * like `import kotlin.test.assertEquals`, and neither is "relative". Only + * the tree's own `package` declarations tell them apart; the manifests the + * package map reads (pom.xml, build.gradle) name artifacts, not packages. + * Measured on JetBrains/Exposed at 0e4d81a4: without this check the guard + * removed 426 CALLS edges, every one of them a real in-project call into + * the repository's own Assert.kt. */ + CBMImport imports[] = { + {.local_name = "assertEquals", .module_path = "org.example.tests.shared.assertEquals"}, + {.local_name = "assertEquals", .module_path = "kotlin.test.assertEquals"}, + {.local_name = "Helper", .module_path = "org.example.util.Helper"}, + {.local_name = "URL", .module_path = "java.net.URL"}, + {.local_name = "Logger", .module_path = "App\\Support\\Logger"}, + }; + CBMImportArray arr = {.items = imports, .count = 5, .cap = 5}; + + CBMHashTable *nspkgs = cbm_ht_create(8); + ASSERT_NOT_NULL(nspkgs); + cbm_ht_set(nspkgs, "org.example.tests.shared", (void *)"qn"); + cbm_ht_set(nspkgs, "org.example.util", (void *)"qn"); + cbm_ht_set(nspkgs, "App.Support", (void *)"qn"); + + /* The specifier names a member of a package the tree declares, so the walk + * strips the member segment and finds it — the call keeps its edge. */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("assertEquals", "suffix_match", &arr, NULL, 0, + NULL, nspkgs)); + ASSERT_FALSE( + cbm_suppress_external_import_shadow("Helper", "unique_name", &arr, NULL, 0, NULL, nspkgs)); + /* PHP writes its separator as a backslash; the key normalization has to + * match the namespace map's, which stores dots. */ + ASSERT_FALSE( + cbm_suppress_external_import_shadow("Logger", "unique_name", &arr, NULL, 0, NULL, nspkgs)); + /* Negative space, and the effect the census called valuable: `java.net.URL` + * walks to "java.net" then "java", neither declared by any project file, so + * a bare `URL` bound to an in-tree URL class is still cut. */ + ASSERT_TRUE( + cbm_suppress_external_import_shadow("URL", "suffix_match", &arr, NULL, 0, NULL, nspkgs)); + /* NULL restores the previous contract: without the declared-package set + * every one of these reads as external again. */ + ASSERT_TRUE(cbm_suppress_external_import_shadow("assertEquals", "suffix_match", &arr, NULL, 0, + NULL, NULL)); + ASSERT_TRUE( + cbm_suppress_external_import_shadow("Helper", "unique_name", &arr, NULL, 0, NULL, NULL)); + + /* The walk is by dotted segment, not by substring: a tree that declares + * "org.example.tests" must not claim "org.example.testsuite.helper". */ + CBMImport near[] = { + {.local_name = "near", .module_path = "org.example.testsuite.helper.near"}, + }; + CBMImportArray narr = {.items = near, .count = 1, .cap = 1}; + CBMHashTable *prefix_only = cbm_ht_create(8); + ASSERT_NOT_NULL(prefix_only); + cbm_ht_set(prefix_only, "org.example.tests", (void *)"qn"); + ASSERT_TRUE(cbm_suppress_external_import_shadow("near", "unique_name", &narr, NULL, 0, NULL, + prefix_only)); + + cbm_ht_free(prefix_only); + cbm_ht_free(nspkgs); + PASS(); +} + +TEST(external_import_shadow_declared_prefix_is_permissive) { + /* #1355, known limitation pinned deliberately. The declared-package walk + * has no floor: once the tree declares `com.example`, a third-party + * library publishing under `com.example.vendor` reads as in-tree and the + * guard stands down. The call then keeps exactly the edge it has on main, + * so this direction costs a fabricated edge the guard could have removed — + * it never removes a real one, which is the side a call-resolution + * suppressor has to fail on. + * + * This test exists so the behaviour cannot change silently: tightening the + * walk (stopping at the first type-shaped segment, say) must flip these + * two assertions, not slip through unnoticed. */ + CBMImport imports[] = { + {.local_name = "vendorCall", .module_path = "com.example.vendor.pkg.vendorCall"}, + }; + CBMImportArray arr = {.items = imports, .count = 1, .cap = 1}; + + CBMHashTable *declares_prefix = cbm_ht_create(8); + ASSERT_NOT_NULL(declares_prefix); + cbm_ht_set(declares_prefix, "com.example", (void *)"qn"); + /* The tree declares only the prefix, yet the whole subtree is treated as + * in-tree — the permissive direction. */ + ASSERT_FALSE(cbm_suppress_external_import_shadow("vendorCall", "unique_name", &arr, NULL, 0, + NULL, declares_prefix)); + + /* Change nothing but the declared package and the guard fires again, which + * is what proves the previous assertion came from the prefix walk and not + * from some other clause standing down. */ + CBMHashTable *declares_other = cbm_ht_create(8); + ASSERT_NOT_NULL(declares_other); + cbm_ht_set(declares_other, "com.other", (void *)"qn"); + ASSERT_TRUE(cbm_suppress_external_import_shadow("vendorCall", "unique_name", &arr, NULL, 0, + NULL, declares_other)); + + cbm_ht_free(declares_other); + cbm_ht_free(declares_prefix); + PASS(); +} + /* ── Suite ─────────────────────────────────────────────────────── */ /* Method call THROUGH an imported symbol that is itself an indexed node @@ -1258,4 +1378,6 @@ SUITE(registry) { 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); + RUN_TEST(external_import_shadow_declared_package_kept); + RUN_TEST(external_import_shadow_declared_prefix_is_permissive); } From 81820262e61f6e7f11920a5c4596b4297e793cf8 Mon Sep 17 00:00:00 2001 From: Yyunozor Date: Sat, 19 Sep 2026 23:46:59 +0200 Subject: [PATCH 5/5] fix(pipeline): run the external-import guard after route classification The #1355 guard dropped the call before the emitters ran, so it did not only remove the fabricated CALLS edge it is meant to remove: it also skipped the route, HTTP, CONFIG and URL classification that happens inside emit_classified_edge / emit_service_edge. A PR that claims to touch nothing but weak same-name CALLS edges was silently costing Route nodes, and with them every HANDLES edge that pass_route_nodes.c later bridges onto those routes. The shape is not hypothetical. A route registration whose router is a dependency is exactly a bare call bound by a package specifier: the resolved QN carries the router library name, emit_classified_edge reads it as CBM_SVC_ROUTE_REG, and the first argument is path-shaped. On a 66-file TypeScript fixture main mints __route__ANY__/orders and the guard removed it, on both resolvers. The fix is the seam the #592/#606 member guard already uses, for the same reason and with the same comment: join the predicate to drop_plain_call and let the emitter decide. Only the plain-CALLS fall-through is suppressed, so the fabricated edge still goes and every service edge stays main-identical. The two resolvers keep identical gates, as the notes in both files require. The new pipeline test asserts both halves on both resolvers: the Route node survives, and a second bare call from the same external package in the same file is still cut, so the first assertion cannot pass by the guard having been disarmed. Signed-off-by: Yyunozor --- src/pipeline/pass_calls.c | 24 ++++---- src/pipeline/pass_parallel.c | 19 +++--- tests/test_pipeline.c | 109 +++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 21 deletions(-) diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index f0f9b499f..508db7310 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -634,10 +634,22 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, * flag is set only for Python — this gate MUST match pass_parallel.c's * exactly, for the same divergence reason noted above. */ bool suppress_weak_local_binding = lang == CBM_LANG_PYTHON; + /* #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`. Joined to + * drop_plain_call for the reason spelled out above: dropping the call here + * would also skip route/HTTP/CONFIG classification, and a route + * registration reached through a static import (`import static + * spark.Spark.get` + `get("/x", handler)`) is exactly a bare call bound by + * a package specifier. Suppressing only the plain-CALLS fall-through keeps + * every Route node and service edge main-identical. */ bool drop_plain_call = cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy) || cbm_suppress_weak_local_binding_call(suppress_weak_local_binding, - call->callee_is_locally_bound, res.strategy); + call->callee_is_locally_bound, res.strategy) || + cbm_suppress_external_import_shadow(call->callee_name, res.strategy, file_imports, imp_keys, + imp_count, cbm_pipeline_get_pkgmap(), + cbm_pipeline_get_nsmap()); /* Service-pattern HTTP/ASYNC calls to an EXTERNAL client library (e.g. * `requests.get("/api/orders/{id}")`) resolve to a QN containing the library @@ -669,16 +681,6 @@ 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, cbm_pipeline_get_pkgmap(), - cbm_pipeline_get_nsmap())) { - 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; diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index d7aa7bba9..7af3f5c7a 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -2499,10 +2499,18 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB /* Bare-call local-binding suppression — see the note in pass_calls.c. * This gate MUST stay identical to the one there. */ bool suppress_weak_local_binding = lang == CBM_LANG_PYTHON; + /* #1355: same guard as pass_calls.c, and joined to drop_plain_call for + * the same reason — a `continue` here would also skip route/HTTP/CONFIG + * classification, and a static-import route registration is a bare call + * bound by a package specifier. Suppress only the plain-CALLS + * fall-through so every Route node stays main-identical. */ bool drop_plain_call = cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy) || cbm_suppress_weak_local_binding_call(suppress_weak_local_binding, - call->callee_is_locally_bound, res.strategy); + call->callee_is_locally_bound, res.strategy) || + cbm_suppress_external_import_shadow(call->callee_name, res.strategy, &result->imports, + imp_keys, imp_count, cbm_pipeline_get_pkgmap(), + cbm_pipeline_get_nsmap()); /* Service-pattern HTTP/ASYNC client call (`requests.get(url)`): the * service signal lives in the callee_name. The registry can mis-resolve @@ -2573,15 +2581,6 @@ 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, cbm_pipeline_get_pkgmap(), - cbm_pipeline_get_nsmap())) { - /* #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/tests/test_pipeline.c b/tests/test_pipeline.c index 36bb243a3..fa6e16fb3 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -5348,6 +5348,114 @@ TEST(pipeline_external_import_shadow_keeps_declared_package_issue1355) { PASS(); } +/* #1355: the guard must suppress ONLY the plain-CALLS fall-through, never the + * route/HTTP/CONFIG classification that runs inside the emitters. A bare call + * bound by a package import is exactly the shape a route registration takes + * when the router is a dependency, and dropping it before classification costs + * the Route node and every HANDLES edge downstream of it. Same reasoning, and + * the same drop_plain_call seam, as the #592/#606 member guard. */ +static void write_route_shadow_fixture(const char *dir, int pad_files) { + /* The project symbols the guess would bind to. The file name puts "express" + * in their qualified names, which is what makes the resolved QN classify as + * a route registration — the same substring match main uses on real trees. */ + write_temp_file(dir, "app/express-routes.ts", + "export function regGet(path: string, handler: unknown): string {\n" + " return path;\n" + "}\n" + "export function regSpec(label: string): string {\n" + " return label;\n" + "}\n"); + /* Both names come from a package outside the tree. `regGet` carries a + * path-shaped first argument, so it is a route registration; `regSpec` is + * an ordinary fabricated edge and must still go. */ + write_temp_file(dir, "app/server.ts", + "import { regGet, regSpec } from 'vendor-router';\n" + "export function boot(): string {\n" + " return regGet('/orders', () => 'ok') + regSpec('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 routePad%02d(): number { return %d; }\n", i, + i); + write_temp_file(dir, name, body); + } +} + +static int assert_route_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 Route node survives the guard. RED when the guard returns before + * the emitter: main mints __route__ANY__/orders here and the published + * guard lost it, together with every HANDLES edge that hangs off it. */ + if (count_nodes_named(s, project, "/orders") < 1) { + rc = 4; + } + /* (2) the fabricated plain-CALLS edge in the same file is still removed, so + * (1) cannot pass by the guard having been disarmed. */ + if (rc == 0 && cross_file_call_exists(s, project, "boot", "regSpec")) { + rc = 5; + } + cbm_store_close(s); + cbm_pipeline_free(p); + return rc; +} + +TEST(pipeline_external_import_shadow_keeps_route_registration_issue1355) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_route_import_shadow_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + write_route_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_route_shadow_contract(tmp, "route-sequential.db"); + + cbm_unsetenv("CBM_INDEX_SINGLE_THREAD"); + cbm_setenv("CBM_WORKERS", "4", 1); + int parallel = assert_route_shadow_contract(tmp, "route-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"); @@ -13699,6 +13807,7 @@ SUITE(pipeline) { 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_external_import_shadow_keeps_declared_package_issue1355); + RUN_TEST(pipeline_external_import_shadow_keeps_route_registration_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);