Skip to content

Commit bedbc0a

Browse files
authored
Merge pull request #2345 from DeusData/fix/issue-2127
fix(pipeline): never bind an external Python import to a same-named project symbol (#2127)
2 parents 7d4a12c + 492ae13 commit bedbc0a

8 files changed

Lines changed: 456 additions & 6 deletions

File tree

‎src/pipeline/pass_calls.c‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,8 @@ static const cbm_gbuf_node_t *calls_find_source(cbm_pipeline_ctx_t *ctx, const c
645645
/* Resolve one call and emit the appropriate edge. Returns 1 if resolved, 0 if not. */
646646
static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, const CBMFileResult *result,
647647
const char *rel, const char *module_qn, const char **imp_keys,
648-
const char **imp_vals, int imp_count, CBMLanguage lang) {
648+
const char **imp_vals, int imp_count, CBMLanguage lang,
649+
const CBMImportArray *imports) {
649650
const CBMResolvedCallArray *lsp_calls = &result->resolved_calls;
650651
const cbm_gbuf_node_t *source_node = calls_find_source(ctx, rel, call->enclosing_func_qn);
651652
if (!source_node) {
@@ -847,7 +848,14 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, const CBM
847848
call->receiver_is_self_attribute, call->callee_name,
848849
res.strategy)) ||
849850
cbm_suppress_weak_local_binding_call(suppress_weak_local_binding,
850-
call->callee_is_locally_bound, res.strategy);
851+
call->callee_is_locally_bound, res.strategy) ||
852+
/* Import-binding suppression (#2127): an external `from m import f`
853+
* binds `f` for the whole module, so a weak short-name match onto a
854+
* project `X.f` outside m is fabricated. Python-only (the import
855+
* paths are module chains) — MUST match pass_parallel.c exactly. */
856+
(lang == CBM_LANG_PYTHON && cbm_suppress_weak_import_bound_call(true, true, res.strategy) &&
857+
cbm_python_import_binding_contradicts(imports, call->callee_name, res.qualified_name,
858+
ctx->gbuf, ctx->project_name, rel));
851859

852860
/* Service-pattern HTTP/ASYNC calls to an EXTERNAL client library (e.g.
853861
* `requests.get("/api/orders/{id}")`) resolve to a QN containing the library
@@ -1039,7 +1047,7 @@ int cbm_pipeline_pass_calls(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t *file
10391047
}
10401048
total_calls++;
10411049
if (resolve_single_call(ctx, call, result, rel, module_qn, imp_keys, imp_vals,
1042-
imp_count, files[i].language)) {
1050+
imp_count, files[i].language, &result->imports)) {
10431051
resolved++;
10441052
} else {
10451053
unresolved++;

‎src/pipeline/pass_parallel.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3052,7 +3052,14 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB
30523052
call->receiver_is_self_attribute,
30533053
call->callee_name, res.strategy)) ||
30543054
cbm_suppress_weak_local_binding_call(suppress_weak_local_binding,
3055-
call->callee_is_locally_bound, res.strategy);
3055+
call->callee_is_locally_bound, res.strategy) ||
3056+
/* Import-binding suppression (#2127) — see pass_calls.c; this gate
3057+
* MUST stay identical to the one there. */
3058+
(lang == CBM_LANG_PYTHON &&
3059+
cbm_suppress_weak_import_bound_call(true, true, res.strategy) &&
3060+
cbm_python_import_binding_contradicts(&result->imports, call->callee_name,
3061+
res.qualified_name, rc->main_gbuf,
3062+
rc->project_name, rel));
30563063

30573064
/* Service-pattern HTTP/ASYNC client call (`requests.get(url)`): the
30583065
* service signal lives in the callee_name. The registry can mis-resolve

‎src/pipeline/pass_pkgmap.c‎

Lines changed: 217 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1543,6 +1543,213 @@ static bool import_targetable_label(const char *label) {
15431543
return false;
15441544
}
15451545

1546+
/* #2127: a Python import path spells the module chain that owns the imported
1547+
* name (`from unittest.mock import patch` -> unittest, mock). Strategy 3 matches
1548+
* the leaf name alone, so an EXTERNAL import (stdlib / third party: no project
1549+
* module, Strategy 1 misses) bound to any same-named project definition -- a
1550+
* REST view's `patch` handler became the target of every mock.patch call at
1551+
* import_map confidence. For such an external import, accept a symbol hit only
1552+
* when the module segments preceding `name` in the import path occur, in
1553+
* order, among the hit's enclosing QN segments -- a src/ layout or sys.path
1554+
* root the module resolver missed (`from template_tests.utils import setup` ->
1555+
* proj.tests.template_tests.utils.setup) still qualifies. Relative prefixes
1556+
* (leading dots) carry no segment and are skipped. Imports of project modules
1557+
* are never judged by it (they may re-export from anywhere). Python only. */
1558+
enum { PY_IMPORT_MAX_SEGS = 64 };
1559+
bool cbm_python_import_path_matches_qn(const char *module_path, const char *name,
1560+
const char *hit_qn) {
1561+
if (!module_path || !name || !hit_qn) {
1562+
return true;
1563+
}
1564+
char path[1024];
1565+
snprintf(path, sizeof(path), "%s", module_path);
1566+
char *as = strstr(path, " as ");
1567+
if (as) {
1568+
*as = '\0';
1569+
}
1570+
const char *p = path;
1571+
while (*p == '.') {
1572+
p++;
1573+
}
1574+
/* Segments of the import path; name_at = index of the LAST `name`. */
1575+
const char *segs[PY_IMPORT_MAX_SEGS];
1576+
size_t lens[PY_IMPORT_MAX_SEGS];
1577+
int nseg = 0;
1578+
int name_at = -1;
1579+
size_t name_len = strlen(name);
1580+
while (*p && nseg < PY_IMPORT_MAX_SEGS) {
1581+
const char *dot = strchr(p, '.');
1582+
size_t len = dot ? (size_t)(dot - p) : strlen(p);
1583+
if (len == name_len && strncmp(p, name, len) == 0) {
1584+
name_at = nseg;
1585+
}
1586+
segs[nseg] = p;
1587+
lens[nseg] = len;
1588+
nseg++;
1589+
if (!dot) {
1590+
break;
1591+
}
1592+
p = dot + SKIP_ONE;
1593+
}
1594+
if (name_at <= 0) {
1595+
return true; /* no module chain before the name: nothing to check */
1596+
}
1597+
/* Ordered-subsequence match against the hit's enclosing segments (every
1598+
* segment before the hit's own leaf). */
1599+
const char *leaf = strrchr(hit_qn, '.');
1600+
const char *q = hit_qn;
1601+
int want = 0;
1602+
while (want < name_at && leaf && q < leaf) {
1603+
const char *dot = strchr(q, '.');
1604+
size_t len = (size_t)(dot - q);
1605+
if (len == lens[want] && strncmp(q, segs[want], len) == 0) {
1606+
want++;
1607+
}
1608+
q = dot + SKIP_ONE;
1609+
}
1610+
return want == name_at;
1611+
}
1612+
1613+
/* #2127, call side: an imported name binds the file's identifier to that
1614+
* import for the whole module, exactly like a parameter binds it for a body.
1615+
* When the import could not be materialized (external module) the resolver
1616+
* falls through to project-wide short-name strategies and binds `patch(...)`
1617+
* (or `mock.patch(...)`) to whatever project definition shares the leaf. True
1618+
* when the callee's root identifier is bound by this file's Python imports and
1619+
* EVERY such binding's module chain (`unittest.mock` for `from unittest.mock
1620+
* import patch`; the callee's own dotted path for a root `import a.b`) is
1621+
* absent from `resolved_qn`'s enclosing segments AND the binding is external
1622+
* (no IMPORTS edge materialized it: the module is not in the project). An
1623+
* internal binding is never judged here -- a project module can re-export a
1624+
* name from anywhere -- and a chain consistent with the target (src/ layout)
1625+
* keeps the edge. `gbuf` NULL means "no IMPORTS edges" (all external). */
1626+
static bool python_import_local_materialized(const cbm_gbuf_t *gbuf, const char *project_name,
1627+
const char *rel_path, const char *local_name) {
1628+
if (!gbuf || !rel_path) {
1629+
return false;
1630+
}
1631+
char *file_qn = cbm_pipeline_fqn_compute(project_name, rel_path, "__file__");
1632+
const cbm_gbuf_node_t *file_node = file_qn ? cbm_gbuf_find_by_qn(gbuf, file_qn) : NULL;
1633+
safe_free(file_qn);
1634+
const cbm_gbuf_edge_t **edges = NULL;
1635+
int edge_count = 0;
1636+
if (!file_node || cbm_gbuf_find_edges_by_source_type(gbuf, file_node->id, "IMPORTS", &edges,
1637+
&edge_count) != 0) {
1638+
return false;
1639+
}
1640+
char needle[CBM_SZ_256];
1641+
snprintf(needle, sizeof(needle), "\"local_name\":\"%s\"", local_name);
1642+
for (int i = 0; i < edge_count; i++) {
1643+
if (edges[i]->properties_json && strstr(edges[i]->properties_json, needle)) {
1644+
return true;
1645+
}
1646+
}
1647+
return false;
1648+
}
1649+
1650+
bool cbm_python_import_binding_contradicts(const CBMImportArray *imports, const char *callee_name,
1651+
const char *resolved_qn, const cbm_gbuf_t *gbuf,
1652+
const char *project_name, const char *rel_path) {
1653+
if (!imports || imports->count <= 0 || !callee_name || !callee_name[0] || !resolved_qn ||
1654+
!resolved_qn[0]) {
1655+
return false;
1656+
}
1657+
const char *root_end = strchr(callee_name, '.');
1658+
size_t root_len = root_end ? (size_t)(root_end - callee_name) : strlen(callee_name);
1659+
bool bound = false;
1660+
for (int i = 0; i < imports->count; i++) {
1661+
const CBMImport *imp = &imports->items[i];
1662+
if (!imp->local_name || !imp->module_path || strlen(imp->local_name) != root_len ||
1663+
strncmp(imp->local_name, callee_name, root_len) != 0) {
1664+
continue;
1665+
}
1666+
char full[1024];
1667+
snprintf(full, sizeof(full), "%s", imp->module_path);
1668+
char *as = strstr(full, " as ");
1669+
if (as) {
1670+
*as = '\0';
1671+
}
1672+
const char *path_leaf_seg = strrchr(full, '.');
1673+
bool from_import = path_leaf_seg && strcmp(path_leaf_seg + SKIP_ONE, imp->local_name) == 0;
1674+
if (!as && !from_import && strncmp(full, callee_name, root_len) == 0 &&
1675+
(full[root_len] == '.' || full[root_len] == '\0')) {
1676+
/* `import a.b` binds the ROOT package `a`: the callee already
1677+
* spells its own full dotted path (`a.b.f()`). */
1678+
snprintf(full, sizeof(full), "%s", callee_name);
1679+
}
1680+
/* Otherwise (`from m import x [as y]`) only the import's own module
1681+
* chain is evidence: members reached THROUGH x (`x.objects.create`)
1682+
* may live on any type, so they are not compared. */
1683+
const char *leaf = strrchr(full, '.');
1684+
leaf = leaf ? leaf + SKIP_ONE : full;
1685+
if (cbm_python_import_path_matches_qn(full, leaf, resolved_qn) ||
1686+
python_import_local_materialized(gbuf, project_name, rel_path, imp->local_name)) {
1687+
return false; /* consistent with the target, or a project import */
1688+
}
1689+
bound = true;
1690+
}
1691+
return bound;
1692+
}
1693+
1694+
/* #2127: whether a Python import's module lives in this project. Relative
1695+
* imports always do; otherwise the module part (the path minus the imported
1696+
* name) must resolve to a graph node. Strategy 1 has already failed on the
1697+
* full path when this is asked. */
1698+
static bool python_import_module_in_project(const cbm_pipeline_ctx_t *ctx, const char *source_rel,
1699+
const char *module_path) {
1700+
if (!module_path || module_path[0] == '.') {
1701+
return true;
1702+
}
1703+
char part[1024];
1704+
snprintf(part, sizeof(part), "%s", module_path);
1705+
char *as = strstr(part, " as ");
1706+
if (as) {
1707+
*as = '\0';
1708+
}
1709+
char *dot = strrchr(part, '.');
1710+
if (!dot) {
1711+
return false; /* `import x`: Strategy 1 already missed x itself */
1712+
}
1713+
*dot = '\0';
1714+
char *qn = cbm_pipeline_resolve_module(ctx, source_rel, part);
1715+
bool found = qn && cbm_gbuf_find_by_qn(ctx->gbuf, qn) != NULL;
1716+
safe_free(qn);
1717+
return found;
1718+
}
1719+
1720+
/* `import a.b` / `import a` (as opposed to `from a import b`): every segment
1721+
* names a module. The extractor binds the ROOT (`a`) for a dotted plain
1722+
* import and the leaf for a from-import; a dot-less path is always a plain
1723+
* import. An aliased form is ambiguous and reads as a from-import. */
1724+
static bool python_is_plain_module_import(const CBMImport *imp) {
1725+
const char *p = imp->module_path;
1726+
if (!p || p[0] == '.' || strstr(p, " as ")) {
1727+
return false;
1728+
}
1729+
const char *first_dot = strchr(p, '.');
1730+
if (!first_dot) {
1731+
return true;
1732+
}
1733+
const char *leaf = strrchr(p, '.') + SKIP_ONE;
1734+
size_t root_len = (size_t)(first_dot - p);
1735+
return imp->local_name && strlen(imp->local_name) == root_len &&
1736+
strncmp(imp->local_name, p, root_len) == 0 && strcmp(leaf, imp->local_name) != 0;
1737+
}
1738+
1739+
/* #2127: Strategy 3 hit filter for an EXTERNAL Python import. The hit must
1740+
* spell the import's module chain (a src/ layout the module resolver missed
1741+
* still does), and whatever names a module -- an enclosing path segment, or
1742+
* any segment of a plain `import x` -- can only be a Module/File node (`import
1743+
* copy` must not fall back to a project method called `copy`). */
1744+
static bool python_external_hit_rejected(const CBMImport *imp, const char *name,
1745+
bool enclosing_segment, const cbm_gbuf_node_t *hit) {
1746+
if (!cbm_python_import_path_matches_qn(imp->module_path, name, hit->qualified_name)) {
1747+
return true;
1748+
}
1749+
bool names_module = enclosing_segment || python_is_plain_module_import(imp);
1750+
return names_module && strcmp(hit->label, "Module") != 0 && strcmp(hit->label, "File") != 0;
1751+
}
1752+
15461753
/* #1934: whether the name-guess import fallbacks — Strategy 1b (sibling file,
15471754
* whose label filter admits symbols) and Strategy 3 (symbol name) — may run
15481755
* for imports from this language. A Go import path names a package — never a
@@ -1882,8 +2089,8 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t
18822089
src_base = pb + SKIP_ONE;
18832090
}
18842091
}
1885-
const bool symbol_fallback_allowed =
1886-
cbm_import_symbol_fallback_allowed(cbm_language_for_filename(src_base));
2092+
const CBMLanguage src_lang = cbm_language_for_filename(src_base);
2093+
const bool symbol_fallback_allowed = cbm_import_symbol_fallback_allowed(src_lang);
18872094

18882095
/* Strategy 1b: sibling-file resolution for build/markup grammars whose
18892096
* import string is a sibling filename or directory (SCSS partials, Just/
@@ -2033,6 +2240,11 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t
20332240
*dot = '\0';
20342241
end = dot;
20352242
}
2243+
/* #2127: a Python import whose module is not in the project (stdlib /
2244+
* third party) may only fall back to a hit that spells its chain. */
2245+
const bool py_external =
2246+
symbol_fallback_allowed && src_lang == CBM_LANG_PYTHON &&
2247+
!python_import_module_in_project(ctx, source_rel, imp->module_path);
20362248
for (int ci = 0; symbol_fallback_allowed && ci < ncands; ci++) {
20372249
const cbm_gbuf_node_t **hits = NULL;
20382250
int n = 0;
@@ -2054,6 +2266,9 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t
20542266
strcmp(cand->qualified_name, source_file_qn) == 0) {
20552267
continue; /* self */
20562268
}
2269+
if (py_external && python_external_hit_rejected(imp, cands[ci], ci > 0, cand)) {
2270+
continue; /* #2127: not the external module's symbol */
2271+
}
20572272
if (!best || (cand->qualified_name && best->qualified_name &&
20582273
strcmp(cand->qualified_name, best->qualified_name) < 0)) {
20592274
best = cand;

‎src/pipeline/pipeline.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,16 @@ bool cbm_weak_member_unique_name_exempt(bool is_python, bool receiver_is_self_at
334334
bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_bound,
335335
const char *strategy);
336336

337+
/* Import-binding counterpart (#2127). True when the callee's root identifier
338+
* is bound by an import of this file whose module chain contradicts the
339+
* resolved target (`from unittest.mock import patch; patch()` must not bind a
340+
* project `PkgConfigView.patch`), and the match came from a weak short-name
341+
* strategy. Same drop-list as the guards above; the language set lives at the
342+
* call sites and must be identical in pass_calls.c and pass_parallel.c.
343+
* Pure; unit-tested in test_registry.c. */
344+
bool cbm_suppress_weak_import_bound_call(bool enabled, bool import_binding_contradicts,
345+
const char *strategy);
346+
337347
/* #725: drop a suffix_match CALLS edge when the caller language and the
338348
* target file's language disagree. unique_name (candidates == 1) is #1572
339349
* and is left alone; same_module / import_map / lsp_* are kept. JS/TS/TSX

‎src/pipeline/pipeline_internal.h‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,23 @@ static inline int cbm_pipeline_check_cancel(const cbm_pipeline_ctx_t *ctx) {
300300
* ei_go_import_never_binds_symbol. */
301301
bool cbm_import_symbol_fallback_allowed(CBMLanguage lang);
302302

303+
/* #2127: true when the module segments preceding `name` in a Python import
304+
* path (`unittest.mock.patch` -> unittest, mock) occur, in order, among the
305+
* enclosing segments of `hit_qn`. Leading relative dots and ` as alias` are
306+
* ignored; a path with no module chain before `name` always matches. Gates
307+
* the import resolver's symbol-name fallback for Python. */
308+
bool cbm_python_import_path_matches_qn(const char *module_path, const char *name,
309+
const char *hit_qn);
310+
311+
/* #2127: true when the callee's root identifier is bound by the file's Python
312+
* imports and every such binding is EXTERNAL (no IMPORTS edge from `rel_path`
313+
* in `gbuf` carries its local name; NULL gbuf = none) with a module chain that
314+
* contradicts `resolved_qn`. Feeds cbm_suppress_weak_import_bound_call at both
315+
* resolver call sites. */
316+
bool cbm_python_import_binding_contradicts(const CBMImportArray *imports, const char *callee_name,
317+
const char *resolved_qn, const cbm_gbuf_t *gbuf,
318+
const char *project_name, const char *rel_path);
319+
303320
/* Check if a file path is worth tracking for git history analysis. */
304321
bool cbm_is_trackable_file(const char *path);
305322

‎src/pipeline/registry.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,22 @@ bool cbm_suppress_weak_local_binding_call(bool enabled, bool callee_is_locally_b
695695
return weak_short_name_strategy(strategy);
696696
}
697697

698+
/* Import-binding counterpart of the parameter guard above (#2127). An import
699+
* binds the identifier for the whole module; when that import is external it
700+
* never materializes, and the chain falls through to project-wide short-name
701+
* strategies (`from unittest.mock import patch; patch()` -> a REST view's
702+
* `PkgConfigView.patch`). The caller decides whether the binding contradicts
703+
* the target (cbm_python_import_binding_contradicts: a SCOPE FACT about this
704+
* file, not a spelling list); only weak strategies are dropped, so import_map /
705+
* same_module / lsp_* edges are untouched. Pure; unit-tested in test_registry.c. */
706+
bool cbm_suppress_weak_import_bound_call(bool enabled, bool import_binding_contradicts,
707+
const char *strategy) {
708+
if (!enabled || !import_binding_contradicts) {
709+
return false;
710+
}
711+
return weak_short_name_strategy(strategy);
712+
}
713+
698714
static bool js_ts_family(CBMLanguage lang) {
699715
return lang == CBM_LANG_JAVASCRIPT || lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX ||
700716
lang == CBM_LANG_ARKTS;

0 commit comments

Comments
 (0)