From 8c89cc486c8882bac25865586714578ff32eb626 Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Sun, 12 Jul 2026 20:40:15 -0500 Subject: [PATCH] fix(tooling): W013 line attribution, W019 discarded interrogatives, phantom LSP input, quiet load_file (#556 #559 #560 #583) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - parser: AST_FUNC nodes carry the define token's line, not the line of the first statement after the body — W013 now lands on the define, same-line allow-pragmas match, consecutive shadows no longer chain-suppress (#556). Stepper scope fixture step index updated (the define's tape record moved to its own line). - lint: new W019 — statement-level interrogatives (question words, prev of) evaluate-and-discard; warn (#583). DIAGNOSTICS.md row added; lib/tests/examples scan clean. - eigenlsp: remove phantom 'input' completion (#559). - builtins: load_file banner gated behind EIGS_VERBOSE_LOAD, silent default (#560). Suite [115b] pins the quiet default. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 30 ++++++++++++++ docs/DIAGNOSTICS.md | 6 ++- src/builtins.c | 6 ++- src/eigenlsp.c | 1 - src/lint.c | 77 ++++++++++++++++++++++++++++++++++++ src/parser.c | 6 ++- tests/run_all_tests.sh | 25 ++++++++++++ tests/test_lint.sh | 88 ++++++++++++++++++++++++++++++++++++++++++ tests/test_lsp.py | 5 +++ tests/test_step.sh | 5 ++- 10 files changed, 243 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 521e9749..83069ab7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,36 @@ All notable changes to EigenScript are documented here. ## [Unreleased] +### Added +- **Lint W019 — statement-level interrogative discards its result + (#583).** `why is "..."` where `why` is a question word parses as the + interrogative *expression* form, not an assignment — as a bare + statement it is a silent no-op (the Tidepool hit: a catch handler + "reassigning" a `local why` silently kept the stale value). Every + statement-level interrogative (question words and `prev of`) is dead + code and now warns; interrogatives inside expressions + (`print of (why is x)`) are untouched. docs/DIAGNOSTICS.md documents + the code. + +### Fixed +- **Lint W013 attributed to the shadowing `define` line (#556).** The + parser stamped `AST_FUNC` nodes with the line of the first statement + *after* the body, so W013 pointed at innocent code, the documented + same-line `# lint: allow W013` never matched, and consecutive + shadowing defines chain-suppressed each other (N shadows → only the + last warned). The node now carries the `define` token's line; + same-line pragmas work and each shadow warns on its own line. +- **LSP no longer advertises a phantom `input` builtin (#559).** + `eigenlsp` completion offered `input of prompt`, which was never + registered in the runtime — accepting the completion produced + `undefined variable: input`. Entry removed (#558 tracks a real + stdin-read builtin). +- **`load_file` is silent by default (#560).** Every successful load + printed an unconditional `[load_file] Loading ...` banner to stderr — + a consumer CLI loading 17 fragments opened with 17 lines of runtime + chatter no flag could turn off. No successful builtin announces + itself; the banner is now opt-in via `EIGS_VERBOSE_LOAD=1`. + ### Added - **lib/ui input-event trio (#567, #568, #569)** — first-consumer findings from the DeslanStudio arrangement timeline: diff --git a/docs/DIAGNOSTICS.md b/docs/DIAGNOSTICS.md index 3c7db6dc..b2f39856 100644 --- a/docs/DIAGNOSTICS.md +++ b/docs/DIAGNOSTICS.md @@ -198,7 +198,10 @@ load_file: cannot read 'missing.eigs' ## Informational Messages -Diagnostic messages use bracketed prefixes and are not errors: +Diagnostic messages use bracketed prefixes and are not errors. They are +**off by default** (#560 — a shipped CLI must be able to keep stderr clean; +no successful builtin announces itself). Set `EIGS_VERBOSE_LOAD=1` to enable +the `load_file` banner during development: ``` [load_file] Loading lib/math.eigs (1301 bytes) @@ -235,6 +238,7 @@ a code's meaning never changes, and retired codes are not reused. | `W016` | warning | Bare trajectory predicate **outside a loop condition** (`if stable:`, `ok is converged`, `return diverging`) reads the last-observed binding — an invisible alias (#247/#262) — write ` of `. Loop conditions are exempt: the single-assign `loop while not converged` form is the documented idiom, and the ambiguous multi-assign case is `W014`. Any explicit subject counts as named, including `stable of (x + 0.0)`; deliberate bare reads carry `# lint: allow W016`. | | `W017` | warning | Bare 1-element literal arg list: `f of [x]` passes **one argument** — the element, not the list (#405; the pre-#405 rule meant the opposite, so the form reads ambiguously). Write `f of x` for one argument, or `f of ([x])` (#355) to pass a 1-element list. Doubles as the #405 migration audit: `--lint` over a consumer repo surfaces every behavior-changed call site. | | `W018` | warning | A `catch`-bound error's `.kind` is compared (`==`/`!=`) against a string that is a **near-miss** of a real kind — a case variant (`"IO"`) or a single-character typo (`"index_rage"`), or a kind renamed out from under the handler — so the branch is dead code that silently never fires (#469). Kinds are a closed set (below). Zero-false-positive by construction: only near-misses of a closed kind fire, and only off a catch-bound variable — an exactly-valid kind, and a genuinely custom `throw {kind: "..."}` value many edits from every builtin, both stay silent. | +| `W019` | warning | An interrogative used as a **bare statement** — `why is "..."`, `what is x`, `prev of y` at statement level — evaluates and **discards** its result: a silent no-op (#583). Question words (`what/who/when/where/why/how`) cannot be assigned with `is` — the "assignment" is the interrogative expression form — so when a same-named binding exists in scope the statement is almost certainly a mistaken assignment (the real hit: a catch handler "reassigning" a `local why` that silently kept its stale value). An interrogative inside an expression (`print of (why is x)`, `r is prev of y`) is never flagged. | The human linter output carries the code inline: diff --git a/src/builtins.c b/src/builtins.c index d863ed57..ef897a81 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -2706,7 +2706,11 @@ Value* builtin_load_file(Value *arg) { return make_null(); } - fprintf(stderr, "[load_file] Loading %s (%ld bytes)\n", path, size); + /* #560: silent by default — no other successful builtin announces + * itself, and shipped CLI tools built on load_file must be able to keep + * stderr clean. Set EIGS_VERBOSE_LOAD=1 for the development banner. */ + if (getenv("EIGS_VERBOSE_LOAD")) + fprintf(stderr, "[load_file] Loading %s (%ld bytes)\n", path, size); /* A parse error in the loaded file must surface, not be silently run as a * partial/incorrect AST. Direct execution aborts on g_parse_errors; mirror diff --git a/src/eigenlsp.c b/src/eigenlsp.c index 3f7f16af..078bb4e1 100644 --- a/src/eigenlsp.c +++ b/src/eigenlsp.c @@ -89,7 +89,6 @@ static const char *builtin_docs[][2] = { {"time", "time of null -- seconds since epoch"}, {"sleep", "sleep of seconds -- pause execution"}, {"random", "random of null -- random float in [0,1)"}, - {"input", "input of prompt -- read line from stdin"}, {"eval", "eval of string -- evaluate EigenScript code string"}, {NULL, NULL} }; diff --git a/src/lint.c b/src/lint.c index 9fab1b00..6ffb92c4 100644 --- a/src/lint.c +++ b/src/lint.c @@ -612,6 +612,82 @@ static void check_builtin_shadow(ASTNode *node, LintContext *ctx) { } } +/* ---- Check: statement-level interrogative (result discarded) ---- */ + +/* #583: `why is "..."` where `why` is a question word parses as the + * INTERROGATIVE form (an expression), not an assignment — as a bare + * statement its result is always discarded, a silent no-op. When a + * same-named binding exists in scope this is almost certainly a mistaken + * assignment (the real downstream hit: Tidepool's catch handler). The + * discarded form is dead code either way, so every statement-level + * AST_INTERROGATE is flagged. `check_disc_interrog` is called only on + * nodes sitting directly in a statement list, so an interrogative used + * inside an expression (`print of (why is x)`) is never reached. */ + +static const char *interrog_word(int kind) { + static const char *words[] = {"what", "who", "when", "where", "why", "how"}; + return (kind >= 0 && kind <= 5) ? words[kind] : "prev"; +} + +static void check_disc_interrog(ASTNode *node, LintContext *ctx) { + if (!node) return; + if (node->type == AST_INTERROGATE) { + int k = node->data.interrogate.kind; + if (k >= 0 && k <= 5) + lint_warn(ctx, node->line, "W019", + "'%s is ...' is an interrogative (question words cannot be " + "assigned with 'is'); as a statement its result is discarded — " + "rename the variable, or use the interrogative inside an expression", + interrog_word(k)); + else + lint_warn(ctx, node->line, "W019", + "interrogative 'prev of ...' as a bare statement discards its result"); + return; + } + switch (node->type) { + case AST_IF: + for (int i = 0; i < node->data.cond.if_count; i++) + check_disc_interrog(node->data.cond.if_body[i], ctx); + for (int i = 0; i < node->data.cond.else_count; i++) + check_disc_interrog(node->data.cond.else_body[i], ctx); + break; + case AST_LOOP: + for (int i = 0; i < node->data.loop.body_count; i++) + check_disc_interrog(node->data.loop.body[i], ctx); + break; + case AST_FOR: + for (int i = 0; i < node->data.forloop.body_count; i++) + check_disc_interrog(node->data.forloop.body[i], ctx); + break; + case AST_FUNC: + for (int i = 0; i < node->data.func.body_count; i++) + check_disc_interrog(node->data.func.body[i], ctx); + break; + case AST_TRY: + for (int i = 0; i < node->data.trycatch.try_count; i++) + check_disc_interrog(node->data.trycatch.try_body[i], ctx); + for (int i = 0; i < node->data.trycatch.catch_count; i++) + check_disc_interrog(node->data.trycatch.catch_body[i], ctx); + break; + case AST_MATCH: + for (int c = 0; c < node->data.match.case_count; c++) + for (int k2 = 0; k2 < node->data.match.body_counts[c]; k2++) + check_disc_interrog(node->data.match.bodies[c][k2], ctx); + break; + case AST_BLOCK: + case AST_UNOBSERVED: + for (int i = 0; i < node->data.block.count; i++) + check_disc_interrog(node->data.block.stmts[i], ctx); + break; + case AST_PROGRAM: + for (int i = 0; i < node->data.program.count; i++) + check_disc_interrog(node->data.program.stmts[i], ctx); + break; + default: + break; + } +} + /* ---- Check: unreachable code in function bodies ---- */ static void check_func_unreachable(ASTNode *node, LintContext *ctx) { @@ -1923,6 +1999,7 @@ static void lint_run_checks(ASTNode *ast, const char *path, check_empty_blocks(ast, ctx); check_dup_keys(ast, ctx); check_builtin_shadow(ast, ctx); + check_disc_interrog(ast, ctx); check_func_unreachable(ast, ctx); check_is_conditions(ast, ctx); check_unused_params(ast, ctx); diff --git a/src/parser.c b/src/parser.c index 76061ab4..2e5ccbe7 100644 --- a/src/parser.c +++ b/src/parser.c @@ -1373,7 +1373,11 @@ static ASTNode* parse_statement_inner(Parser *p) { p_skip_newlines(p); int body_count; ASTNode **body = parse_block(p, &body_count); - ASTNode *n = make_node(AST_FUNC, p_cur(p)->line); + /* #556: the node's line is the `define` line (t), NOT p_cur(p) — + * after parse_block the cursor sits on the first statement AFTER + * the body, so diagnostics (lint W013) were attributed to innocent + * following code and same-line `# lint: allow` pragmas never matched. */ + ASTNode *n = make_node(AST_FUNC, t->line); n->data.func.name = xstrdup((name_tok && name_tok->str_val) ? name_tok->str_val : ""); set_name_hash(n, n->data.func.name); n->data.func.params = params; diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index 1a6ad0e8..2f905819 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -2827,6 +2827,31 @@ else fi echo "" +echo "[115b] load_file quiet by default (#560, 2 checks)" +# A successful load_file used to print an unconditional "[load_file] +# Loading ..." banner to stderr per call — 17 lines of runtime chatter +# before a consumer CLI's own output. Default is silent now (no other +# successful builtin announces itself); EIGS_VERBOSE_LOAD=1 re-enables +# the development banner. +QL_DIR=$(mktemp -d /tmp/eigs_quietload_XXXX) +printf 'print of "frag"\n' > "$QL_DIR/frag.eigs" +printf 'load_file of "frag.eigs"\n' > "$QL_DIR/main.eigs" +QL_ERR=$( cd "$QL_DIR" && "$BIN_ABS" main.eigs &1 >/dev/null ) +QL_VERB=$( cd "$QL_DIR" && EIGS_VERBOSE_LOAD=1 "$BIN_ABS" main.eigs &1 >/dev/null ) +rm -rf "$QL_DIR" +TOTAL=$((TOTAL + 2)) +if [ -z "$QL_ERR" ]; then + echo " PASS: successful load_file emits nothing on stderr"; PASS=$((PASS + 1)) +else + echo " FAIL: load_file stderr not empty: '$QL_ERR'"; FAIL=$((FAIL + 1)) +fi +if echo "$QL_VERB" | grep -q '^\[load_file\] Loading'; then + echo " PASS: EIGS_VERBOSE_LOAD=1 re-enables the banner"; PASS=$((PASS + 1)) +else + echo " FAIL: EIGS_VERBOSE_LOAD banner missing: '$QL_VERB'"; FAIL=$((FAIL + 1)) +fi +echo "" + echo "[92] Module Resolve Base (1 check)" # Phase 0b: an `import` inside a module resolves relative to *that # module's* directory, not the main script's. Shell-driven because diff --git a/tests/test_lint.sh b/tests/test_lint.sh index 04006d81..677b00bc 100644 --- a/tests/test_lint.sh +++ b/tests/test_lint.sh @@ -859,6 +859,94 @@ OUT_CWD=$( (cd / && "$EIGS" --lint "$LINTPKG/lib/gen.eigs" 2>&1) || true) check_not_contains "#455 allow-list resolves from project root, not cwd" "$OUT_CWD" "W017" rm -rf "$LINTPKG" +# --- #556: W013 is attributed to the define line itself --- +# The warning used to land on the first statement AFTER the shadowing define +# (p_cur had advanced past the body), so a same-line `# lint: allow W013` on +# the define never matched and consecutive shadows chain-suppressed each other. +TMPFILE=$(mktemp /tmp/lint_test_XXXXXX.eigs) +cat > "$TMPFILE" << 'EIGS' +_real is remove_file +define remove_file(path) as: + return _real of path +r is remove_file of "/nonexistent" +print of r +EIGS +OUTPUT=$($EIGS --lint "$TMPFILE" 2>&1 || true) +check_contains "#556 W013 reported at the define line (2)" "$OUTPUT" ":2: warning\[W013\]" +check_not_contains "#556 W013 not attributed to the next statement (4)" "$OUTPUT" ":4: warning\[W013\]" +rm -f "$TMPFILE" + +TMPFILE=$(mktemp /tmp/lint_test_XXXXXX.eigs) +cat > "$TMPFILE" << 'EIGS' +_real is remove_file +define remove_file(path) as: # lint: allow W013 + return _real of path +r is remove_file of "/nonexistent" +print of r +EIGS +OUTPUT=$($EIGS --lint "$TMPFILE" 2>&1 || true) +check_not_contains "#556 same-line allow pragma on the define suppresses W013" "$OUTPUT" "W013" +rm -f "$TMPFILE" + +# Consecutive shadowing defines: each warns on its OWN define line (the old +# attribution made each define's warning land on the NEXT define, where that +# define's pragma chain-suppressed it). +TMPFILE=$(mktemp /tmp/lint_test_XXXXXX.eigs) +cat > "$TMPFILE" << 'EIGS' +_r1 is remove_file +_r2 is rename +define remove_file(path) as: + return _r1 of path +define rename(args) as: + return _r2 of args +x is remove_file of "/nonexistent" +y is rename of ["/a", "/b"] +print of x +print of y +EIGS +OUTPUT=$($EIGS --lint "$TMPFILE" 2>&1 || true) +check_contains "#556 first of two consecutive shadows warns (line 3)" "$OUTPUT" ":3: warning\[W013\]" +check_contains "#556 second of two consecutive shadows warns (line 5)" "$OUTPUT" ":5: warning\[W013\]" +rm -f "$TMPFILE" + +# --- #583 (W019): statement-level interrogative discards its result --- +TMPFILE=$(mktemp /tmp/lint_test_XXXXXX.eigs) +cat > "$TMPFILE" << 'EIGS' +define f(e) as: + local why is "init failed" + if e == 1: + why is "no builtins" + return why +print of (f of 1) +EIGS +OUTPUT=$($EIGS --lint "$TMPFILE" 2>&1 || true) +check_contains "#583 W019 fires on statement-level 'why is ...'" "$OUTPUT" "W019" +check_contains "#583 W019 reported at the interrogative's line (4)" "$OUTPUT" ":4: warning\[W019\]" +rm -f "$TMPFILE" + +TMPFILE=$(mktemp /tmp/lint_test_XXXXXX.eigs) +cat > "$TMPFILE" << 'EIGS' +x is 5 +x is 7 +prev of x +print of x +EIGS +OUTPUT=$($EIGS --lint "$TMPFILE" 2>&1 || true) +check_contains "#583 W019 fires on statement-level 'prev of'" "$OUTPUT" "W019" +rm -f "$TMPFILE" + +TMPFILE=$(mktemp /tmp/lint_test_XXXXXX.eigs) +cat > "$TMPFILE" << 'EIGS' +x is 5 +x is 7 +print of (what is x) +p is prev of x +print of p +EIGS +OUTPUT=$($EIGS --lint "$TMPFILE" 2>&1 || true) +check_not_contains "#583 interrogatives inside expressions are not flagged" "$OUTPUT" "W019" +rm -f "$TMPFILE" + echo "" echo "Results: $PASS passed, $FAIL failed, $TOTAL total" exit $FAIL diff --git a/tests/test_lsp.py b/tests/test_lsp.py index 8b1548de..1e65ff14 100755 --- a/tests/test_lsp.py +++ b/tests/test_lsp.py @@ -197,6 +197,11 @@ def main(): isinstance(items, list) and all("label" in it for it in items[:5])) check("completion surfaces a user symbol", isinstance(items, list) and any(it.get("label") == "greeting" for it in items)) + # #559: the list once advertised a phantom `input` builtin that was never + # registered in the runtime — accepting the completion produced + # `undefined variable: input` at runtime. + check("completion does not advertise a phantom 'input' builtin", + isinstance(items, list) and not any(it.get("label") == "input" for it in items)) # --- hover over a defined symbol returns contents --- hover = {"jsonrpc": "2.0", "id": 3, "method": "textDocument/hover", diff --git a/tests/test_step.sh b/tests/test_step.sh index 7952dc21..f08425fd 100644 --- a/tests/test_step.sh +++ b/tests/test_step.sh @@ -204,9 +204,10 @@ echo "$OUT" | grep -q "^i = 3 .*(3 assigns)" \ || fail "module-level i folds only its own stream" \ "$(echo "$OUT" | grep '^i =' | head -1)" -# inside the second work frame (step 11 = its 'i is i + 1' line): the +# inside the second work frame (step 12 = its 'i is i + 1' line; #556 +# moved the define statement's tape record to the define's own line): the # frame-local i (31, 2 assigns, {in work}) shadows the module i -OUT=$(printf 's 10\np\nt i\nq\n' | "$EIGS" --step "$SCOPE_TAPE" "$SCOPE_FIX" 2>&1) +OUT=$(printf 's 11\np\nt i\nq\n' | "$EIGS" --step "$SCOPE_TAPE" "$SCOPE_FIX" 2>&1) echo "$OUT" | grep -q "^i = 31 .*{in work}" \ && ok "frame-local i shadows module i inside the frame" \ || fail "frame-local i shadows module i inside the frame" \