diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index c9744dda2..508db7310 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); @@ -633,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 @@ -827,8 +840,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_definitions.c b/src/pipeline/pass_definitions.c index 7c7ed6884..e71843db6 100644 --- a/src/pipeline/pass_definitions.c +++ b/src/pipeline/pass_definitions.c @@ -911,7 +911,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 dbf459814..82216bb04 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -1753,7 +1753,11 @@ int cbm_build_registry_from_cache(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t } } - 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)); @@ -2935,10 +2939,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 diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index 7a69be998..2b0e01149 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -271,6 +271,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; } @@ -2835,6 +2847,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 954686849..3657f06b4 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; @@ -296,6 +297,25 @@ 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. `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 *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 28ec2f282..031f33b2d 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -1620,6 +1620,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; @@ -2191,6 +2193,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; @@ -2770,6 +2774,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 aa675603d..19b0a04c5 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -205,6 +205,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 f052f80c4..287ede4cb 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 @@ -585,6 +586,241 @@ 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", 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) { + 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; +} + +/* 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; +} + +/* 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) { + 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. + * + * `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. + * + * `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 *declared_packages) { + 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) || + 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; + } + 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 86032b99e..ec49ac352 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -5927,6 +5927,486 @@ 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. */ +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, EXTERNAL_IMPORT_SHADOW_PARALLEL_PAD); + + /* 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; + 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_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(); +} + +/* #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(); +} + +/* #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(); +} + +/* #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"); @@ -14781,6 +15261,10 @@ 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_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); diff --git a/tests/test_registry.c b/tests/test_registry.c index 2674e7b1d..8475653a7 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -1010,6 +1010,307 @@ TEST(weak_call_guards_share_one_drop_list) { 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 + * 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, 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, NULL)); + 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, 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", "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, NULL)); + ASSERT_FALSE( + 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, 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, 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, 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(NULL, "unique_name", &arr, keys, nkeys, NULL, NULL)); + ASSERT_FALSE( + 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(); +} + +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, NULL, NULL)); + ASSERT_FALSE(cbm_suppress_external_import_shadow("eq", "unique_name", &b, NULL, 0, NULL, NULL)); + 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, 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". */ + 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, NULL, 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, 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, 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, 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, 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, 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 @@ -1110,4 +1411,11 @@ 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); + 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); }