Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/pipeline/pass_calls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -651,6 +652,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, tsjs_drop_plain_call);
return SKIP_ONE;
Expand Down Expand Up @@ -810,8 +820,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++;
Expand Down
8 changes: 8 additions & 0 deletions src/pipeline/pass_parallel.c
Original file line number Diff line number Diff line change
Expand Up @@ -2540,6 +2540,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
Expand Down
11 changes: 11 additions & 0 deletions src/pipeline/pipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,17 @@ bool cbm_perl_suppress_generic_match(bool is_perl, bool is_method, const char *c
* Pure; unit-tested in test_registry.c. */
bool cbm_tsjs_suppress_weak_method_match(bool is_tsjs, bool is_method, 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
* and is left alone; same_module / import_map / lsp_* are kept. JS/TS/TSX
Expand Down
102 changes: 102 additions & 0 deletions src/pipeline/registry.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ enum { REG_MAX_CANDIDATES = 256 };
#include "foundation/dyn_array.h"
#include "foundation/platform.h"

#include <ctype.h>
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -456,6 +457,107 @@ bool cbm_tsjs_suppress_weak_method_match(bool is_tsjs, bool is_method, const cha
strcmp(strategy, "field_type_hint") == 0 || strcmp(strategy, "fuzzy") == 0;
}

/* 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;
}

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;
}
Expand Down
137 changes: 137 additions & 0 deletions tests/test_pipeline.c
Original file line number Diff line number Diff line change
Expand Up @@ -4649,6 +4649,142 @@ 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();
}

TEST(pipeline_tsjs_receiver_parallel_keeps_service_edges) {
char tmp[256];
snprintf(tmp, sizeof(tmp), "/tmp/cbm_tsjs_par_XXXXXX");
Expand Down Expand Up @@ -12123,6 +12259,7 @@ SUITE(pipeline) {
#endif
RUN_TEST(pipeline_tsjs_receiver_suppresses_weak_method_edge);
RUN_TEST(pipeline_tsjs_receiver_parallel_keeps_service_edges);
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_native_fetch_classified_as_http_calls);
Expand Down
Loading
Loading