From 9953e317b7e0c7ed04a37dad81cb02f63fbc3fd3 Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Thu, 9 Jul 2026 00:50:48 -0500 Subject: [PATCH] change: raise on invalid input in remaining builtins (silent-tolerance batch-2b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continues the silent-tolerance audit (#490–#512). The remaining builtins in the "bad input folds to a silent sentinel" class now raise or (where the bug is an inconsistency) align with the [] operator: - len (#508): a value with no length (number, fn, null, ...) raises type_mismatch instead of folding to 0. Guarded callers use `x != null and len of x`, and `and` short-circuits, so the guarded len is never reached. - append (#506): a non-list target — including the `append of xs` #405 arg-vector footgun — raises type_mismatch instead of a silent no-op. - regex_match/regex_find/regex_replace (#500): an invalid pattern raises value (was [] / input-unchanged, indistinguishable from a clean no-match); a non-string operand raises type_mismatch. - substr (#504): a negative start counts from the end, matching char_at and [] (was a flat clamp-to-0 — an inconsistency, not a raise). - list_truncate (#503): a negative length raises value instead of silently emptying the list. - json_path (#507): an empty path segment (leading/trailing dot or ..) raises value instead of being silently skipped by strtok; a genuine lookup miss still returns "". Load-bearing check: the only reliance was test_list_truncate T4 (asserted the negative-clamps-to-0 behavior) — updated to assert the raise. len, append, regex, substr and json_path broke zero existing tests. Regression: suite section [116] extended (test_raise_on_bad_input.eigs, now 35 checks). Release + ASan/leak suites green (2660/2660, leak tally 0). Closes #500 Closes #503 Closes #504 Closes #506 Closes #507 Closes #508 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 21 ++++++ src/builtins.c | 108 ++++++++++++++++++++++++----- tests/run_all_tests.sh | 9 ++- tests/test_list_truncate.eigs | 10 ++- tests/test_raise_on_bad_input.eigs | 84 ++++++++++++++++++++++ 5 files changed, 211 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00f7a27..1493412 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,27 @@ All notable changes to EigenScript are documented here. index raises `type_mismatch`, matching the `xs[i]` operator (was a silent fold-to-0 / no-op) (#499, #502). + Batch 2b — the remaining builtins in the same class (#500, #503, #504, + #506, #507, #508): + - `len` — a value with no length (number, fn, `null`, …) raises + `type_mismatch` instead of folding to `0`. Callers meaning "empty if + absent" already guard with `x != null and len of x`, and `and` + short-circuits, so the guarded `len` is never reached (#508). + - `append` — a non-list target (including the `append of xs` #405 + arg-vector footgun, where a 2-element `xs` arrives as `[target, item]`) + raises `type_mismatch` instead of a silent no-op. Pass a literal list + whole with the paren form: `append of ([ys, item])` (#506). + - `regex_match` / `regex_find` / `regex_replace` — an invalid pattern + raises `value` (was `[]` / the input unchanged — indistinguishable from + a clean no-match) and a non-string operand raises `type_mismatch` (#500). + - `substr` — a negative start counts from the end, matching `char_at` and + the `[]` operator (was a flat clamp-to-0 — an inconsistency) (#504). + - `list_truncate` — a negative length raises `value` instead of silently + emptying the list (an undocumented soft clamp) (#503). + - `json_path` — an empty path segment (a leading/trailing dot or `..`) + raises `value` instead of being silently skipped by `strtok`, which + masked a malformed path; a genuine lookup miss still returns `""` (#507). + ### Fixed - **Circular `import` / `load_file` no longer crashes (#496).** A mutual or self-referential `import` (a→b→a) or `load_file` used to recurse through diff --git a/src/builtins.c b/src/builtins.c index 0bdba12..523ed3b 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -451,6 +451,12 @@ Value* builtin_len(Value *arg) { return make_num(arg->data.buffer.count); if (arg->type == VAL_TEXT_BUILDER) return make_num((double)arg->data.text_builder.len); + /* #508: a type with no length (number, fn, null, …) used to fold to 0, + * silently masking a wrong value. Raise. Callers that legitimately mean + * "empty if absent" already guard with `x != null and len of x` — and + * `and` short-circuits, so the guarded len is never reached. */ + rt_error(EK_TYPE, 0, "len: %s has no length", + arg ? val_type_name(arg->type) : "null"); return make_num(0); } @@ -495,12 +501,24 @@ Value* builtin_num(Value *arg) { } Value* builtin_append(Value *arg) { - if (arg->type != VAL_LIST || arg->data.list.count < 2) return make_null(); + /* #506: append requires exactly a [list, item] pair. Fewer than two + * elements (or a non-list arg-vector) was a silent no-op returning the + * target unchanged. NB a bare list is the builtin's arg-vector (#405), + * so `append of xs` with xs=[a,b] arrives here as target=a, item=b — if + * a isn't a list this now raises instead of silently doing nothing. Pass + * a literal list whole with the paren form: `append of ([ys, item])`. */ + if (arg->type != VAL_LIST || arg->data.list.count < 2) { + rt_error(EK_TYPE, 0, "append requires [list, item]"); + return make_null(); + } Value *target = arg->data.list.items[0]; Value *item = arg->data.list.items[1]; - if (target->type == VAL_LIST) { - list_append(target, item); + if (target->type != VAL_LIST) { + rt_error(EK_TYPE, 0, "append: target must be a list (got %s)", + val_type_name(target->type)); + return make_null(); } + list_append(target, item); return target; } @@ -1433,14 +1451,24 @@ Value* builtin_str_replace(Value *arg) { /* match of [string, pattern] -> [full_match, group1, ...] or [] */ #if !EIGENSCRIPT_FREESTANDING Value* builtin_match(Value *arg) { - if (!arg || arg->type != VAL_LIST || arg->data.list.count < 2) return make_list(0); - if (arg->data.list.items[0]->type != VAL_STR || arg->data.list.items[1]->type != VAL_STR) + if (!arg || arg->type != VAL_LIST || arg->data.list.count < 2) { + rt_error(EK_TYPE, 0, "regex_match requires [string, pattern]"); return make_list(0); + } + if (arg->data.list.items[0]->type != VAL_STR || arg->data.list.items[1]->type != VAL_STR) { + rt_error(EK_TYPE, 0, "regex_match: string and pattern must be strings"); + return make_list(0); + } const char *str = arg->data.list.items[0]->data.str; const char *pattern = arg->data.list.items[1]->data.str; regex_t re; - if (regcomp(&re, pattern, REG_EXTENDED) != 0) return make_list(0); + /* #500: an invalid pattern used to return [] — indistinguishable from a + * clean no-match. Raise so a broken pattern is visible. */ + if (regcomp(&re, pattern, REG_EXTENDED) != 0) { + rt_error(EK_VALUE, 0, "regex_match: invalid pattern '%s'", pattern); + return make_list(0); + } regmatch_t matches[16]; if (regexec(&re, str, 16, matches, 0) != 0) { @@ -1466,14 +1494,22 @@ Value* builtin_match(Value *arg) { /* match_all of [string, pattern] -> [match1, match2, ...] */ #if !EIGENSCRIPT_FREESTANDING Value* builtin_match_all(Value *arg) { - if (!arg || arg->type != VAL_LIST || arg->data.list.count < 2) return make_list(0); - if (arg->data.list.items[0]->type != VAL_STR || arg->data.list.items[1]->type != VAL_STR) + if (!arg || arg->type != VAL_LIST || arg->data.list.count < 2) { + rt_error(EK_TYPE, 0, "regex_find requires [string, pattern]"); + return make_list(0); + } + if (arg->data.list.items[0]->type != VAL_STR || arg->data.list.items[1]->type != VAL_STR) { + rt_error(EK_TYPE, 0, "regex_find: string and pattern must be strings"); return make_list(0); + } const char *str = arg->data.list.items[0]->data.str; const char *pattern = arg->data.list.items[1]->data.str; regex_t re; - if (regcomp(&re, pattern, REG_EXTENDED) != 0) return make_list(0); + if (regcomp(&re, pattern, REG_EXTENDED) != 0) { /* #500 */ + rt_error(EK_VALUE, 0, "regex_find: invalid pattern '%s'", pattern); + return make_list(0); + } Value *result = make_list(16); regmatch_t m[1]; @@ -1498,17 +1534,27 @@ Value* builtin_match_all(Value *arg) { /* regex_replace of [string, pattern, replacement] -> string */ #if !EIGENSCRIPT_FREESTANDING Value* builtin_regex_replace(Value *arg) { - if (!arg || arg->type != VAL_LIST || arg->data.list.count < 3) return make_str(""); + if (!arg || arg->type != VAL_LIST || arg->data.list.count < 3) { + rt_error(EK_TYPE, 0, "regex_replace requires [string, pattern, replacement]"); + return make_str(""); + } if (arg->data.list.items[0]->type != VAL_STR || arg->data.list.items[1]->type != VAL_STR || - arg->data.list.items[2]->type != VAL_STR) + arg->data.list.items[2]->type != VAL_STR) { + rt_error(EK_TYPE, 0, "regex_replace: string, pattern and replacement must be strings"); return make_str(""); + } const char *str = arg->data.list.items[0]->data.str; const char *pattern = arg->data.list.items[1]->data.str; const char *replacement = arg->data.list.items[2]->data.str; regex_t re; - if (regcomp(&re, pattern, REG_EXTENDED) != 0) return make_str(str); + /* #500: an invalid pattern used to return the input unchanged — a silent + * no-op that hides the broken pattern. Raise. */ + if (regcomp(&re, pattern, REG_EXTENDED) != 0) { + rt_error(EK_VALUE, 0, "regex_replace: invalid pattern '%s'", pattern); + return make_str(""); + } strbuf out; strbuf_init(&out); @@ -1585,6 +1631,10 @@ Value* builtin_substr(Value *arg) { int slen = strlen(str_val->data.str); int start = (int)start_val->data.num; int rlen = (int)len_val->data.num; + /* #504: a negative start counts from the end, matching char_at and the + * [] operator (was a flat clamp-to-0 — inconsistent). A start still + * negative after the adjustment (before the string) clamps to 0. */ + if (start < 0) start += slen; if (start < 0) start = 0; if (start >= slen) return make_str(""); if (rlen < 0) rlen = 0; @@ -2300,6 +2350,18 @@ Value* builtin_json_path(Value *arg) { if (arg->data.list.items[0]->type == VAL_STR) json_str = arg->data.list.items[0]->data.str; if (arg->data.list.items[1]->type == VAL_STR) path = arg->data.list.items[1]->data.str; + /* #507: reject empty path segments (a leading/trailing dot or a `..`). + * strtok silently skips them, so "a..b", ".a" and "a." used to resolve + * as if the empty piece weren't there — masking a malformed path. An + * entirely empty path ("") still means the root document. */ + if (path[0] != '\0') { + size_t plen = strlen(path); + if (path[0] == '.' || path[plen - 1] == '.' || strstr(path, "..")) { + rt_error(EK_VALUE, 0, "json_path: empty path segment in '%s'", path); + return make_str(""); + } + } + int pos = 0; Value *root = eigs_json_parse_value(json_str, &pos); if (!root) return make_str(""); @@ -5402,13 +5464,27 @@ Value* builtin_sign_extend(Value *arg) { /* list_truncate of [list, new_len] — shrink list in-place to new_len items. * If new_len >= current length, no-op. Returns the list. */ Value* builtin_list_truncate(Value *arg) { - if (!arg || arg->type != VAL_LIST || arg->data.list.count < 2) return make_null(); + if (!arg || arg->type != VAL_LIST || arg->data.list.count < 2) { + rt_error(EK_TYPE, 0, "list_truncate requires [list, new_len]"); + return make_null(); + } Value *list = arg->data.list.items[0]; Value *len_val = arg->data.list.items[1]; - if (!list || list->type != VAL_LIST) return make_null(); - if (!len_val || len_val->type != VAL_NUM) return list; + if (!list || list->type != VAL_LIST) { + rt_error(EK_TYPE, 0, "list_truncate: first argument must be a list"); + return make_null(); + } + if (!len_val || len_val->type != VAL_NUM) { + rt_error(EK_TYPE, 0, "list_truncate: new_len must be a number"); + return make_null(); + } int new_len = (int)len_val->data.num; - if (new_len < 0) new_len = 0; + /* #503: a negative new_len used to silently empty the list (an + * undocumented soft clamp to 0). Raise instead. */ + if (new_len < 0) { + rt_error(EK_VALUE, 0, "list_truncate: new_len must be >= 0 (got %d)", new_len); + return make_null(); + } if (new_len >= list->data.list.count) return list; for (int i = new_len; i < list->data.list.count; i++) { val_decref(list->data.list.items[i]); diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index bdb8502..e6f3969 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -1027,9 +1027,14 @@ rm -f "$CPG_FILE" # 1M cap), sort_by (non-numeric key), get_at/set_at + buf_get/buf_set (OOB # → index_range, matching the [] operator). Each case asserts it raises (and # the kind where it matters) plus a happy-path sanity check. -echo "[116] Silent-Tolerance Batch-2: bad input raises (7 issues)" +# Batch-2b (#500/#503/#504/#506/#507/#508) extends the same file: len (no- +# length type), append (non-list target), regex_match/find/replace (invalid +# pattern → value, non-string → type_mismatch), substr (negative start counts +# from the end), list_truncate (negative len → value), json_path (empty path +# segment → value). +echo "[116] Silent-Tolerance Batch-2: bad input raises (13 issues)" check_eigs_suite "invalid input raises instead of silent null/0/empty" \ - test_raise_on_bad_input.eigs "ALL_RAISE_TESTS_DONE" 21 + test_raise_on_bad_input.eigs "ALL_RAISE_TESTS_DONE" 35 # [23] Named parameters echo "[23/27] Named Parameters (9 checks)" diff --git a/tests/test_list_truncate.eigs b/tests/test_list_truncate.eigs index 3e20433..c3b0118 100644 --- a/tests/test_list_truncate.eigs +++ b/tests/test_list_truncate.eigs @@ -29,11 +29,15 @@ checks += 1 if (len of c) == 3: passed += 1 -# T4: negative clamps to 0 +# T4: negative new_len raises (#503, was a silent clamp-to-0) d is [1, 2, 3] -list_truncate of [d, -1] checks += 1 -if (len of d) == 0: +truncate_raised is 0 +try: + list_truncate of [d, -1] +catch e: + truncate_raised is 1 +if truncate_raised == 1 and (len of d) == 3: passed += 1 # T5: truncate to 1 diff --git a/tests/test_raise_on_bad_input.eigs b/tests/test_raise_on_bad_input.eigs index 8dff37d..acc5b38 100644 --- a/tests/test_raise_on_bad_input.eigs +++ b/tests/test_raise_on_bad_input.eigs @@ -149,5 +149,89 @@ assert_true of [bs_oob == 1, "buf_set OOB raises"] assert_eq of [buf_get of [b, 1], 2, "buf_get in range still works"] +# ================================================================ +# batch-2b (#500/#503/#504/#506/#507/#508) +# ================================================================ + +# ---- #508 len: a type with no length raises (was a silent 0) ---- +len_num is 0 +len_num_k is "" +try: + ig is len of 42 +catch e: + len_num is 1 + len_num_k is e.kind +assert_true of [len_num == 1 and len_num_k == "type_mismatch", "len of a number raises type_mismatch"] + +len_null is 0 +try: + ig is len of null +catch e: + len_null is 1 +assert_true of [len_null == 1, "len of null raises"] + +assert_eq of [len of [1, 2, 3], 3, "len of a list still works"] +assert_eq of [len of {"a": 1, "b": 2}, 2, "len of a dict still works"] + +# ---- #506 append: non-list target raises (was a silent no-op) ---- +app_bad is 0 +app_bad_k is "" +try: + ig is append of [5, 6] +catch e: + app_bad is 1 + app_bad_k is e.kind +assert_true of [app_bad == 1 and app_bad_k == "type_mismatch", "append onto a non-list raises type_mismatch"] + +app_xs is [1, 2] +append of [app_xs, 9] +assert_true of [app_xs[2] == 9, "append onto a list still works"] + +# ---- #500 regex: an invalid pattern raises (was [] / no-op) ---- +rx_bad is 0 +rx_bad_k is "" +try: + ig is regex_match of ["abc", "["] +catch e: + rx_bad is 1 + rx_bad_k is e.kind +assert_true of [rx_bad == 1 and rx_bad_k == "value", "regex_match invalid pattern raises value"] + +rxr_bad is 0 +try: + ig is regex_replace of ["abc", "(", "X"] +catch e: + rxr_bad is 1 +assert_true of [rxr_bad == 1, "regex_replace invalid pattern raises"] + +rx_ok is regex_match of ["abc123", "[0-9]+"] +assert_eq of [rx_ok[0], "123", "regex_match with a valid pattern still works"] + +# ---- #504 substr: negative start counts from the end (not clamp-to-0) ---- +assert_eq of [substr of ["hello", 0 - 2, 2], "lo", "substr negative start counts from the end"] +assert_eq of [substr of ["hello", 1, 3], "ell", "substr with a positive start still works"] + +# ---- #503 list_truncate: a negative length raises (was empty-the-list) ---- +lt_neg is 0 +lt_neg_k is "" +try: + ig is list_truncate of [[1, 2, 3], 0 - 1] +catch e: + lt_neg is 1 + lt_neg_k is e.kind +assert_true of [lt_neg == 1 and lt_neg_k == "value", "list_truncate negative length raises value"] + +# ---- #507 json_path: an empty path segment raises (was silently skipped) ---- +jp_empty is 0 +jp_empty_k is "" +try: + ig is json_path of ["{\"a\": 1}", "a..b"] +catch e: + jp_empty is 1 + jp_empty_k is e.kind +assert_true of [jp_empty == 1 and jp_empty_k == "value", "json_path empty segment raises value"] + +assert_eq of [json_path of ["{\"a\": {\"b\": 5}}", "a.b"], "5", "json_path with a valid path still works"] + print of "ALL_RAISE_TESTS_DONE" test_summary of null