diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c255520..0b4b8696 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ All notable changes to EigenScript are documented here. ## [Unreleased] +### Changed + +- **import resolution is project-first, and a stdlib collision warns + (#821).** `import name` now tries `name.eigs` (script-relative, plus + the chain's other locations and the `eigs_modules` walk) **before** + the stdlib's `lib/name.eigs` — previously stdlib-first, so a project + file sharing a stdlib module's name was silently shadowed and every + member access on the intended module read `null` (dynamics' + `physics.eigs`, F-DYN-8; the stdlib namespace grows, so any consumer + was one new stdlib module away from silent capture). A name matching + both now prints a one-line stderr warning (once per name per process) + naming the file used and the file shadowed. Sweep of the repo and all + 15 consumer repos found zero imports whose resolution flips. + ### Fixed - **ui: dispatch no longer swallows the rapid second click on widgets diff --git a/docs/SPEC.md b/docs/SPEC.md index 87026bb2..3c5b2718 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -961,10 +961,18 @@ with code 1: ## Modules `import name` loads a module into a **namespace**: it executes -`lib/name.eigs` (the standard library) or, failing that, `name.eigs` -resolved relative to the script — and binds the module's top-level -definitions as a dict named `name`. Nothing leaks into the global -scope; names starting with `_` stay private to the module. +`name.eigs` resolved relative to the script (the project) or, failing +that, `lib/name.eigs` (the standard library) — and binds the module's +top-level definitions as a dict named `name`. Nothing leaks into the +global scope; names starting with `_` stay private to the module. + +Resolution is **project-first** (#821): a `name.eigs` beside the +importing file wins over a stdlib module of the same name. The stdlib +namespace grows over release to release, so the other order would let a +new stdlib module silently capture an existing project's import. When a +name matches **both**, the runtime prints a one-line warning to stderr +(once per name per process) naming the file used and the file shadowed +— rename the project file if the stdlib module is the one you want. ```eigenscript import math diff --git a/src/vm.c b/src/vm.c index 58f6ff39..e927a8ab 100644 --- a/src/vm.c +++ b/src/vm.c @@ -775,6 +775,26 @@ static inline uint32_t read_u32(uint8_t *ip) { /* is_truthy declared in eigenscript.h */ +#if !EIGENSCRIPT_FREESTANDING +/* #821: import-collision diagnostic dedup — one warning per module name + * per process (an import statement re-resolves on every execution, and a + * collided name imported from several files would otherwise repeat the + * same line). Process-lifetime by design: still-reachable at exit, which + * LeakSanitizer does not report. Main-thread only, like the module cache. */ +static int import_collision_first_report(const char *name) { + static char **warned = NULL; + static size_t warned_n = 0, warned_cap = 0; + for (size_t i = 0; i < warned_n; i++) + if (strcmp(warned[i], name) == 0) return 0; + if (warned_n == warned_cap) { + warned_cap = warned_cap ? warned_cap * 2 : 8; + warned = xrealloc_array(warned, warned_cap, sizeof(char *)); + } + warned[warned_n++] = xstrdup(name); + return 1; +} +#endif + /* Iterator state: stored as a list [iterable, index] */ static Value *make_iter_state(Value *iterable) { Value *state = make_list(3); @@ -5215,7 +5235,6 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) { if (!source) { char request[4096]; char path_buf[8192]; - snprintf(request, sizeof(request), "lib/%.1024s.eigs", name); extern int resolve_eigenscript_file_from(const char *base, const char *name, char *out, size_t outlen); @@ -5229,20 +5248,48 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) { ? g_import_resolve_dir : g_script_dir; - if (!resolve_eigenscript_file_from(resolve_base, request, - path_buf, sizeof(path_buf))) { - /* Not a stdlib module — fall back to a user module: - * .eigs resolved against the per-file resolve base - * (and the other standard locations the chain tries). */ - snprintf(request, sizeof(request), "%.1024s.eigs", name); - if (!resolve_eigenscript_file_from(resolve_base, request, - path_buf, sizeof(path_buf))) { - rt_error(EK_IO, current_line, "import: module '%s' not found " - "(tried lib/%s.eigs and %s.eigs)", name, name, name); - vm_push(make_null()); - DISPATCH(); - } + /* #821: PROJECT-FIRST resolution. The user module `.eigs` + * (script-relative, plus the chain's other locations and the + * eigs_modules walk) is tried BEFORE the stdlib's + * `lib/.eigs`. The stdlib namespace grows over time, so + * under stdlib-first a new stdlib module could silently capture + * an existing project's import (dynamics' physics.eigs, + * F-DYN-8). Both requests are always probed: a name matching + * both is a collision worth a diagnostic whichever way + * resolution goes. */ + char stdlib_buf[8192]; + snprintf(request, sizeof(request), "%.1024s.eigs", name); + int user_hit = resolve_eigenscript_file_from(resolve_base, request, + path_buf, sizeof(path_buf)); + snprintf(request, sizeof(request), "lib/%.1024s.eigs", name); + int stdlib_hit = resolve_eigenscript_file_from(resolve_base, request, + stdlib_buf, sizeof(stdlib_buf)); + + if (!user_hit && !stdlib_hit) { + rt_error(EK_IO, current_line, "import: module '%s' not found " + "(tried %s.eigs and lib/%s.eigs)", name, name, name); + vm_push(make_null()); + DISPATCH(); + } + if (user_hit && stdlib_hit && + import_collision_first_report(name)) { + /* Same-file double hit is possible (e.g. a chain step that + * resolves both request shapes to one path after symlinks) — + * only a genuinely forked resolution is a collision. */ + char ureal[8192], sreal[8192]; + if (!realpath(path_buf, ureal)) + snprintf(ureal, sizeof(ureal), "%s", path_buf); + if (!realpath(stdlib_buf, sreal)) + snprintf(sreal, sizeof(sreal), "%s", stdlib_buf); + if (strcmp(ureal, sreal) != 0) + fprintf(stderr, "Warning: import '%s' matches both a " + "project file and a stdlib module — using '%s', " + "shadowing '%s' (project-first; rename the file " + "to use the stdlib module)\n", + name, ureal, sreal); } + if (!user_hit) + memcpy(path_buf, stdlib_buf, sizeof(path_buf)); /* Module cache: canonicalize to absolute path so two different * importers (different cwds, different relative paths) hash to diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index 3ec7563a..f6946585 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -1441,6 +1441,30 @@ else echo " FAIL: import tests" echo "$IM_OUTPUT" | grep -i "FAIL\|assert\|error" | head -5 fi + +# #821: stdlib-shadowing collision diagnostic. Resolution is project-first +# (asserted inside test_import.eigs); the warning is stderr-only, so it is +# asserted here: exactly ONE line for a collided name however many import +# statements execute (warn-once dedup), and NO line when only the stdlib +# matches. Runs in a temp dir so the probe cannot touch tree state. +SH821_DIR=$(mktemp -d) +SH821_BIN="$PWD/eigenscript" +printf 'MARKER is 42\n' > "$SH821_DIR/physics.eigs" +printf 'import physics\nimport physics\nprint of physics.MARKER\n' > "$SH821_DIR/shadow.eigs" +printf 'import math\nprint of (math.abs of -5)\n' > "$SH821_DIR/clean.eigs" +SH821_ERR=$(cd "$SH821_DIR" && "$SH821_BIN" shadow.eigs 2>&1 >/dev/null) +SH821_WARNS=$(printf '%s\n' "$SH821_ERR" | grep -c "Warning: import 'physics'") +SH821_CLEAN=$(cd "$SH821_DIR" && "$SH821_BIN" clean.eigs 2>&1 >/dev/null | grep -c "Warning: import") +TOTAL=$((TOTAL + 2)) +if [ "$SH821_WARNS" = "1" ] && [ "$SH821_CLEAN" = "0" ]; then + PASS=$((PASS + 2)) + echo " PASS: import collision warning (#821: once on shadow, none clean)" +else + FAIL=$((FAIL + 2)) + echo " FAIL: import collision warning (#821) — shadow warnings=$SH821_WARNS (want 1), clean warnings=$SH821_CLEAN (want 0)" + printf '%s\n' "$SH821_ERR" | head -3 +fi +rm -rf "$SH821_DIR" echo "" # [38] Pattern matching diff --git a/tests/test_import.eigs b/tests/test_import.eigs index 5e10c3bc..8b8abbce 100644 --- a/tests/test_import.eigs +++ b/tests/test_import.eigs @@ -41,8 +41,8 @@ import format result is format.fmt_num of [3.14159, 2] assert_eq of [result, "3.14", "import format.fmt_num"] -# --- User modules: when lib/.eigs doesn't exist, import falls -# back to .eigs resolved relative to the script. Same namespacing. +# --- User modules: .eigs resolved relative to the script is tried +# FIRST, before the stdlib's lib/.eigs (#821). Same namespacing. # (The suite runs from src/, so the script dir is ../tests.) write_text of ["../tests/tmp_user_module.eigs", "FACTOR is 7\ndefine scaled(x) as:\n return x * FACTOR\n"] import tmp_user_module @@ -58,6 +58,17 @@ assert_eq of [has_key of [tmp_priv_module, "visible"], 1, "public name exported" assert_eq of [has_key of [tmp_priv_module, "_secret"], 0, "_name stays private"] rm of "../tests/tmp_priv_module.eigs" +# --- #821: a project file with a stdlib module's name wins (project-first). +# Before #821, `import physics` here silently bound the stdlib's +# lib/physics.eigs and every member access on the intended module read +# null (dynamics F-DYN-8). The collision warning itself is asserted at +# the shell level in run_all_tests.sh (it goes to stderr). +write_text of ["../tests/physics.eigs", "SHADOW_MARKER is 821\n"] +import physics +assert_eq of [has_key of [physics, "SHADOW_MARKER"], 1, "project file shadows stdlib (#821)"] +assert_eq of [physics.SHADOW_MARKER, 821, "shadowing module's own binding read"] +rm of "../tests/physics.eigs" + # Missing module raises a catchable error naming both tried paths. import_err is "" try: