Skip to content

Commit 8d7d389

Browse files
authored
Merge pull request DeusData#1938 from ilyabrykau-orca/fix/go-import-symbol-fallback
fix(pipeline): never bind a Go import to a symbol
2 parents a6468ed + 7d76b88 commit 8d7d389

3 files changed

Lines changed: 85 additions & 4 deletions

File tree

‎src/pipeline/pass_pkgmap.c‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,6 +1539,21 @@ static bool import_targetable_label(const char *label) {
15391539
return false;
15401540
}
15411541

1542+
/* #1934: whether the name-guess import fallbacks — Strategy 1b (sibling file,
1543+
* whose label filter admits symbols) and Strategy 3 (symbol name) — may run
1544+
* for imports from this language. A Go import path names a package — never a
1545+
* function, method
1546+
* or field — and every correct Go import resolves in Strategy 1 (module path
1547+
* → the package's Folder node); when that misses the import is external and
1548+
* the correct result is NO edge. The fallback instead bound the last path
1549+
* segment to an arbitrary same-named project symbol (`import "os/exec"` → a
1550+
* test harness's exec() method, two imports → a Makefile target). Languages
1551+
* whose import genuinely can name a member (Python `from m import f`, Java
1552+
* `import com.example.Foo`, Rust `use crate::ops::helper`) keep it. */
1553+
bool cbm_import_symbol_fallback_allowed(CBMLanguage lang) {
1554+
return lang != CBM_LANG_GO;
1555+
}
1556+
15421557
static const char *path_leaf(const char *path) {
15431558
const char *leaf = path;
15441559
for (const char *p = path; p && *p; p++) {
@@ -1817,10 +1832,24 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t
18171832
return target;
18181833
}
18191834

1835+
/* Name-guess fallbacks below (Strategy 1b sibling-file, Strategy 3
1836+
* symbol-name) are gated per importing-file language — see
1837+
* cbm_import_symbol_fallback_allowed (#1934). */
1838+
const char *src_base = source_rel ? source_rel : "";
1839+
for (const char *pb = src_base; *pb; pb++) {
1840+
if (*pb == '/' || *pb == '\\') {
1841+
src_base = pb + SKIP_ONE;
1842+
}
1843+
}
1844+
const bool symbol_fallback_allowed =
1845+
cbm_import_symbol_fallback_allowed(cbm_language_for_filename(src_base));
1846+
18201847
/* Strategy 1b: sibling-file resolution for build/markup grammars whose
18211848
* import string is a sibling filename or directory (SCSS partials, Just/
1822-
* BitBake/func includes, Meson subdir, Pony use). */
1823-
{
1849+
* BitBake/func includes, Meson subdir, Pony use). Its label filter admits
1850+
* symbols too, so for Go it re-creates the Strategy-3 bug one directory
1851+
* closer (`os/exec` → a same-package exec() method) — gated the same. */
1852+
if (symbol_fallback_allowed) {
18241853
const cbm_gbuf_node_t *sib =
18251854
resolve_sibling_file(ctx, source_rel, source_file_qn, imp->module_path);
18261855
if (sib) {
@@ -1898,7 +1927,8 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t
18981927
/* Strategy 3: symbol-name fallback. Derive a representative imported
18991928
* symbol (handling alias / glob / grouped forms) and match it against an
19001929
* in-graph definition of the same simple name in another file
1901-
* (Rust `helper`, Java `Util`, Kotlin grouped, ...). */
1930+
* (Rust `helper`, Java `Util`, Kotlin grouped, ...).
1931+
* Gated per importing-file language, like Strategy 1b above (#1934). */
19021932
char symbuf[256];
19031933
/* Prefer the clean candidate from the module path; the local_name may be an
19041934
* alias (Rust `as h`, Kotlin `as U`) that names no real symbol. */
@@ -1962,7 +1992,7 @@ const cbm_gbuf_node_t *cbm_pipeline_resolve_import_node(const cbm_pipeline_ctx_t
19621992
*dot = '\0';
19631993
end = dot;
19641994
}
1965-
for (int ci = 0; ci < ncands; ci++) {
1995+
for (int ci = 0; symbol_fallback_allowed && ci < ncands; ci++) {
19661996
const cbm_gbuf_node_t **hits = NULL;
19671997
int n = 0;
19681998
if (cbm_gbuf_find_by_name(ctx->gbuf, cands[ci], &hits, &n) == 0 && hits) {

‎src/pipeline/pipeline_internal.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,14 @@ static inline int cbm_pipeline_check_cancel(const cbm_pipeline_ctx_t *ctx) {
234234

235235
/* ── Testable helpers ────────────────────────────────────────────── */
236236

237+
/* #1934: whether the import resolver's name-guess fallbacks — Strategy 1b
238+
* (sibling file; its label filter admits symbols) and Strategy 3 (symbol
239+
* name) — may run for imports from this language. False for Go: an import
240+
* path names a package, never a symbol, so a Strategy-1 miss means the import
241+
* is external and the correct result is no edge. Pure; exercised through
242+
* ei_go_import_never_binds_symbol. */
243+
bool cbm_import_symbol_fallback_allowed(CBMLanguage lang);
244+
237245
/* Check if a file path is worth tracking for git history analysis. */
238246
bool cbm_is_trackable_file(const char *path);
239247

‎tests/test_edge_imports.c‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,20 @@ static void ei_cleanup(EILangProj *lp, cbm_store_t *store) {
241241

242242
/* Index `files`, check IMPORTS count >= `floor`. Dumps a diagnostic on
243243
* failure so failures are self-diagnosable without re-running manually. */
244+
/* Exact-count variant of ei_edge_present: a fabricated EXTRA edge must fail
245+
* the probe, so a floor is not enough (#1932's negative-assertion gap). */
246+
static int ei_edge_count_is(const EILangFile *files, int nfiles, const char *edge_type,
247+
int expected) {
248+
EILangProj lp;
249+
cbm_store_t *store = ei_index_files(&lp, files, nfiles);
250+
int got = store ? cbm_store_count_edges_by_type(store, lp.project, edge_type) : -1;
251+
if (got != expected) {
252+
fprintf(stderr, " [%s] FAIL count=%d expected==%d\n", edge_type, got, expected);
253+
}
254+
ei_cleanup(&lp, store);
255+
return got == expected;
256+
}
257+
244258
static int ei_edge_present(const EILangFile *files, int nfiles, const char *edge_type, int floor) {
245259
EILangProj lp;
246260
cbm_store_t *store = ei_index_files(&lp, files, nfiles);
@@ -513,6 +527,34 @@ TEST(ei_go_two_consumers_same_package) {
513527
PASS();
514528
}
515529

530+
TEST(ei_go_import_never_binds_symbol) {
531+
/* #1934: a Go import path names a package, never a symbol. `os/exec` is
532+
* external (not in the graph), so the ONLY correct outcome is no edge —
533+
* but Strategy 3's symbol-name fallback matched the path's last segment
534+
* against any project definition named `exec` and bound the import to a
535+
* test harness's method. Exact count: the internal `util` import (edge 1,
536+
* via Strategy 1 → the package Folder) must be the whole IMPORTS
537+
* relation; the fallback edge onto harness.exec (reproduce-first RED:
538+
* count 2) must not exist. */
539+
static const EILangFile f[] = {
540+
{"go.mod", "module example.com/fxi\n\ngo 1.22\n"},
541+
{"util/util.go", "package util\n\nfunc Tag() string { return \"t\" }\n"},
542+
{"helper/harness.go", "package helper\n\ntype harness struct{ n int }\n\n"
543+
"func (h *harness) exec(cmd string) error { return nil }\n"},
544+
/* Same-package decoy: the field-measured survivor bound os/exec to a
545+
* method in the IMPORTER'S OWN package (Strategy 1b's sibling-file
546+
* resolution accepts symbol labels too), so the fixture needs the
547+
* collision both cross-package and same-package. */
548+
{"app/aux.go", "package app\n\ntype runner struct{ n int }\n\n"
549+
"func (r *runner) exec(cmd string) error { return nil }\n"},
550+
{"app/run.go", "package app\n\nimport (\n\t\"os/exec\"\n\n"
551+
"\t\"example.com/fxi/util\"\n)\n\n"
552+
"func Run() error {\n\t_ = util.Tag()\n"
553+
"\treturn exec.Command(\"true\").Run()\n}\n"}};
554+
ASSERT_TRUE(ei_edge_count_is(f, 5, "IMPORTS", 1));
555+
PASS();
556+
}
557+
516558
/* C++: header include should resolve to the header file node, not the same-stem
517559
* source node. Also exercises angle-bracket include resolution. */
518560
TEST(ei_cpp_header_include_targets_header_file) {
@@ -1037,6 +1079,7 @@ SUITE(edge_imports) {
10371079
RUN_TEST(ei_go_subpackage_import);
10381080
RUN_TEST(ei_go_blank_import);
10391081
RUN_TEST(ei_go_two_consumers_same_package);
1082+
RUN_TEST(ei_go_import_never_binds_symbol);
10401083
RUN_TEST(ei_cpp_header_include_targets_header_file);
10411084

10421085
/* ── RED REPRODUCTIONS — Rust (expected to FAIL until pipeline fixed) ── */

0 commit comments

Comments
 (0)