From 694f139119453bc4aac497da5eb298a4a6df6bf2 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:31:23 -0400 Subject: [PATCH 01/15] docs: $(( )) reads bases and does checked arithmetic The docs come first on this pass and the code will be built to them. $(( )) was the one place kaish could read a number in another base and it read none: $((0xff)) was an error, fromjson refused hex as invalid JSON, and printf '%d' 0xff printed 0. Users reached for sed or bc. The decision was to make $(( )) the home for bases rather than add a cast builtin or a --base flag: fromjson's contract is JSON, which has one number grammar, and bash already spells explicit bases as 0xff and base#digits. Leading-zero octal stays refused; the error now names 8#10 and 10#$x as the fixes, since a 5-model panel showed 4 of 5 writing base#$var and 2 of 5 tripping on a month from date +%m. The panel also settled two spellings: no model wrote 0b or 0o, so bash spellings are the only ones taught; two models wrote bare (( expr )) as a loop condition, so it is documented as a command. Divergences from bash are stated where they apply: overflow is an error, an unset variable is an error, an empty $(( )) is an error, a string is a value and never an expression, and a $(cmd) on the skipped side of && || ?: does not run. Co-Authored-By: Claude Fable 5 --- crates/kaish-help/content/en/syntax.md | 17 ++- crates/kaish-help/src/fragments.rs | 19 +++- docs/LANGUAGE.md | 148 +++++++++++++++++++------ 3 files changed, 143 insertions(+), 41 deletions(-) diff --git a/crates/kaish-help/content/en/syntax.md b/crates/kaish-help/content/en/syntax.md index 24484401..f9786625 100644 --- a/crates/kaish-help/content/en/syntax.md +++ b/crates/kaish-help/content/en/syntax.md @@ -339,12 +339,21 @@ DATA=$(kaish-last) # capture for later use ## Arithmetic ```sh -X=$((5 + 3)) # 8 -Y=$((X * 2)) # 16 -# Operators: + - * / % > < >= <= == != -# Comparisons return 1/0 +echo $((5 + 3 * 2)) # 11 checked 64-bit integers; overflow is an error +echo $((0xff)) # 255 hex; also $((16#ff)) — base#digits, base 2 to 36 +echo $((8#17)) # 15 octal; $((010)) is an error — write 8#10 or 10 +m=$(date +%m); echo $((10#$m + 1)) # text with a leading zero, read as decimal +echo $((2 ** 10)) # 1024 +echo $((5 > 3)) # 1 comparisons return 1 or 0 +echo $((a > b ? a : b)) # ternary; && || ? : skip the side they do not need +echo $(( $(wc -l < f) * 2 )) # $(cmd) is an operand +x=$((x + 1)) # assignment stays outside; x++ is an error +while (( i <= 5 )); do ...; done # (( )) alone is a condition ``` +Operators, highest first: `+ - ! ~` · `**` · `* / %` · `+ -` · `<< >>` · `< <= > >=` · `== !=` · `&` · `^` · `|` · `&&` · `||` · `? :`. +An unset variable, a float, or `$RANDOM` is an error that names the fix. + ## Functions ```sh diff --git a/crates/kaish-help/src/fragments.rs b/crates/kaish-help/src/fragments.rs index 293d4203..d8ea60b0 100644 --- a/crates/kaish-help/src/fragments.rs +++ b/crates/kaish-help/src/fragments.rs @@ -645,11 +645,20 @@ DATA=$(kaish-last) # capture for later use "arithmetic", "Arithmetic", r#"```sh -X=$((5 + 3)) # 8 -Y=$((X * 2)) # 16 -# Operators: + - * / % > < >= <= == != -# Comparisons return 1/0 -```"#, +echo $((5 + 3 * 2)) # 11 checked 64-bit integers; overflow is an error +echo $((0xff)) # 255 hex; also $((16#ff)) — base#digits, base 2 to 36 +echo $((8#17)) # 15 octal; $((010)) is an error — write 8#10 or 10 +m=$(date +%m); echo $((10#$m + 1)) # text with a leading zero, read as decimal +echo $((2 ** 10)) # 1024 +echo $((5 > 3)) # 1 comparisons return 1 or 0 +echo $((a > b ? a : b)) # ternary; && || ? : skip the side they do not need +echo $(( $(wc -l < f) * 2 )) # $(cmd) is an operand +x=$((x + 1)) # assignment stays outside; x++ is an error +while (( i <= 5 )); do ...; done # (( )) alone is a condition +``` + +Operators, highest first: `+ - ! ~` · `**` · `* / %` · `+ -` · `<< >>` · `< <= > >=` · `== !=` · `&` · `^` · `|` · `&&` · `||` · `? :`. +An unset variable, a float, or `$RANDOM` is an error that names the fix."#, ), syntax_section( "functions", diff --git a/docs/LANGUAGE.md b/docs/LANGUAGE.md index 0a7f325e..0f1eaffc 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -59,9 +59,10 @@ echo ${xs[007]} # error — a list is indexed by number; write ${xs[7]} `$((010 + 1))` is the case worth stating plainly: bash reads `010` as octal and answers 9, and reading it as decimal would answer 11. kaish reads no -octal, so it refuses rather than answering a third number. Convert a base -deliberately instead — `printf "%o"` and `printf "%x"` format one, and -`xxd` dumps bytes. +octal, so it refuses rather than answering a third number. Name the base +instead: `$((8#10))` is 8 and `$((10#$x))` reads text with a leading zero as +decimal (see "Arithmetic"); `printf "%o"` and `printf "%x"` format the other +way. A bare integer must also fit in 64 bits (`-9223372036854775808` to `9223372036854775807`); a longer numeral is an error naming the limit, and @@ -73,7 +74,7 @@ text is refused the same way the literal is: ```sh x=010 -echo $((x)) # error — write `10` +echo $((x)) # error — write `10#$x` for decimal or `8#$x` for octal ``` A record key is text, so a leading zero is a key like any other and reads @@ -994,41 +995,124 @@ for line in $(cat file); do echo $line; done ## Arithmetic -Arithmetic expansion is **integer-only**. Floats exist as a data type (for JSON interop) but `$(( ))` operates on integers. +`$(( ))` has two jobs: it reads a number written in another base, and it does +checked 64-bit integer arithmetic. ```sh -# Arithmetic expansion with $((expression)) -X=$((5 + 3)) # X = 8 -Y=$((X * 2)) # Y = 16 -Z=$((10 / 3)) # Z = 3 (integer division) +echo $((5 + 3 * 2)) # 11 +echo $((10 / 3)) # 3 division drops the fraction, toward zero +echo $((-7 % 3)) # -1 remainder takes the sign of the left side +echo $((2 ** 10)) # 1024 +x=$((x + 1)) # the counter idiom; assignment stays outside +``` + +A result outside `-9223372036854775808..9223372036854775807` is an error, +never a wrapped number. Division by zero is an error. + +### Read another base -# Supported operators (by precedence, highest first) -result=$((-5)) # unary minus: -5 -result=$((10 % 3)) # modulo: 1 -result=$((5 * 4 / 2)) # multiply, divide: 10 -result=$((10 - 3 + 2)) # add, subtract: 9 -result=$(((2 + 3) * 4)) # parentheses: 20 +```sh +echo $((0xff)) # 255 hex, bash spelling; 0XFF works too +echo $((16#ff)) # 255 base#digits, base 2 to 36 +echo $((8#17)) # 15 octal +echo $((2#1011)) # 11 binary +echo $((36#z)) # 35 digits a-z are case-insensitive +echo $((-0xff)) # -255 the sign is an operator, before the number +``` -# Comparison operators (return 1 for true, 0 for false) -echo $((5 > 3)) # 1 -echo $((3 >= 3)) # 1 -echo $((5 == 5)) # 1 -echo $((5 != 3)) # 1 -echo $((3 < 5)) # 1 -echo $((3 <= 3)) # 1 +A leading zero is not octal. `$((010))` is an error that names both fixes: +write `8#10` for octal (8) or `10` for decimal. `0b101` and `0o17` are not +kaish spellings; the error names `2#101` and `8#17`. `printf '%x'` and +`printf '%o'` format a number the other way. -# Comparisons have lowest precedence (arithmetic first) -echo $(( (2 + 3) > 4 )) # 1 (5 > 4) -echo $((10 / 2 == 5)) # 1 (5 == 5) +The digits after `base#` may come from a variable or a command. This is the +fix for text with a leading zero, such as a month from `date`: -# Variables in arithmetic -A=10 -B=3 -echo $((A + B)) # 13 -echo $((A * B + 1)) # 31 -echo $((A > B)) # 1 +```sh +m=$(date +%m) # "08" — text, a leading zero +echo $((10#$m % 12 + 1)) # 9 read as decimal; next month +echo $((8#$mode)) # a mode like 755 read as octal +echo $((2#$bits)) # a string of binary digits ``` +### Variables and expansions + +A variable name may omit the `$`. An integer works directly; `true` is 1 and +`false` is 0; a string holding one number in any spelling above is read as +that number. + +```sh +count=4 +echo $((count + 1)) # 5 +echo $(($count + 1)) # 5 +mask="0xff" +echo $((mask & 16#0f)) # 15 +echo $(( $(wc -l < f) * 2 )) # a command that prints one integer is an operand +echo $(( ${limit:-0} + 1 )) # a parameter default works inside +``` + +An unset or null variable is an error naming the variable — write +`${name:-0}` when zero is the right default. A float (`2.7`), a list, a +record, or text that is not one number is an error naming the variable. +`$RANDOM` and `$SECONDS` have no value in kaish; the error names the +replacement. A string is a value, never an expression: `x="1 + 2"; $((x))` +is an error — write the expression inside `$(( ))`. + +### Operators + +Highest precedence first. Shared operators follow C and bash; `**` follows +bash: unary operators bind before it, and it groups to the right. + +| operators | meaning | +|---|---| +| `+` `-` `!` `~` (unary) | sign, logical not, bitwise not | +| `**` | power; the exponent must be 0 or greater | +| `*` `/` `%` | multiply, divide, remainder | +| `+` `-` | add, subtract | +| `<<` `>>` | shift; count 0 to 63; `>>` keeps the sign | +| `<` `<=` `>` `>=` | compare | +| `==` `!=` | compare for equality | +| `&` | bitwise and | +| `^` | bitwise xor | +| `\|` | bitwise or | +| `&&` | logical and, short-circuit | +| `\|\|` | logical or, short-circuit | +| `? :` | choose one value | + +```sh +echo $((2 ** 3 ** 2)) # 512 (2 ** (3 ** 2)) +echo $((-2 ** 2)) # 4 ((-2) ** 2), as in bash +echo $((1 << 2 + 1)) # 8 + binds tighter than <<, as in C +echo $((5 & 3 == 3)) # 1 == binds tighter than &, as in C +echo $((5 > 3)) # 1 comparisons return 1 or 0 +echo $((a > b ? a : b)) # the larger of a and b +echo $(( (flags & 8) != 0 )) # 1 if bit 3 is set +``` + +Zero is false and every other value is true. `&&`, `||`, and `? :` evaluate +only the side they need; a `$(cmd)` on the skipped side does not run. + +### Arithmetic as a condition + +`(( expr ))` on its own is a command: it succeeds when the value is nonzero. + +```sh +i=1 +while (( i <= 5 )); do echo $i; i=$((i + 1)); done +if (( n % 2 == 0 )); then echo even; else echo odd; fi +``` + +### Not supported, and what to write + +| written | error names | +|---|---| +| `$((x++))`, `$((x += 1))`, `$((x = 5))` | `x=$((x + 1))`, `x=5` — assignment stays outside | +| `$((a, b))` | one expression per `$(( ))` | +| `$((1.5))`, `$((1e3))` | arithmetic is integer-only; `jq` and `awk` do float math | +| `$((1 <<< 2))` | `<<` — `<<<` is a here-string | +| `$(( ))` | a number or an expression | +| `$((0x))`, `$((16#))` | digits after the prefix | + ## Shell Options ```sh @@ -1493,7 +1577,7 @@ The table below records which lint shaped which design decision. | **Floats** | Integer only | Native `3.14` | JSON interop | | **Booleans** | Exit codes | Native `true`/`false` | JSON interop, clearer conditions | | **Typed params** | None | `name:string` | Tool definitions with validation | -| **Arithmetic** | `$(( ))` | `$((expr))` with comparisons | Integer arithmetic + `>`, `<`, `==` returning 1/0 | +| **Arithmetic** | `$(( ))` | `$((expr))`, `(( expr ))` | Checked 64-bit integers, bases via `0x`/`base#`, C precedence, no assignment inside | | **Scatter/gather** | None | `散/集` | Built-in parallelism *(experimental)* | | **VFS** | None | `/tmp/`, `/v/` | Unified resource access | | **Pre-validation** | None | `kaish-validate` builtin | Catch errors before execution | From dd4f3a01c671795e89df525b5175cdcd575f3225 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 09:23:04 -0400 Subject: [PATCH 02/15] arithmetic: replace the hand-rolled parser with a tokenizer + precedence-climbing parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old $(( )) evaluator was a single recursive-descent parser mixing lexing, precedence, and evaluation in one pass over the source text with no bitwise, power, ternary, or base-reading support, and no notion of a token span for error text. The new documentation for $(( )) promises another-base reading (0x, base#digits, base#$var), the full C precedence table through ?:, checked overflow on every operator, and specific error text naming the exact fix. Retrofitting all of that onto the old character-at-a-time parser would have meant threading base and span state through every recursive call. Split into three stages instead: tokenize() turns the raw text into spanned tokens (numbers already validated in their base, $-expansions already carrying pre-parsed statements or nested expressions, `0b`/`0o`/`<<<`/`++`/`,` and the rest of the non-operators diagnosed at the token boundary where the offending text is still in view), a precedence-climbing parse() builds the ArithExpr tree with a 256-form depth cap, and eval_sync() walks it with the checked i64 arithmetic shared between binary and unary operators. Command substitution inside $(( )) parses eagerly (Expansion::CommandSubst carries a Vec already) but evaluates lazily — eval_sync errors loudly if it actually reaches one, which only happens when a caller runs arithmetic with no async evaluator available. The async path that actually runs $(...) lands in a follow-up commit; this one keeps the sync callers (interpreter/eval.rs, scheduler/pipeline.rs) working exactly as before, since parse()+eval_sync() together are a drop-in replacement for the old eval_arithmetic() signature. Three pre-existing integration tests pinned the old error wording ("division by zero", "(leading zero)" on a bare literal, "overflow") that the new spec text deliberately changed to "divides by zero", the parenthesized form reserved for a variable's held value, and "does not fit in a 64-bit integer" naming the actual limit. A fourth pinned old permissive whitespace handling ($ COUNT expanding like $COUNT) that the new tokenizer refuses on purpose, matching the same "no whitespace inside a token" rule that already refused `16 # ff`. find_cmd_subst_close in parser.rs is now pub(crate): the arithmetic tokenizer reuses it verbatim to find where a $(...) operand closes, rather than writing a second paren/quote-aware scanner. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 2117 +++++++++++++---- crates/kaish-kernel/src/parser.rs | 2 +- .../tests/correctness_oneoffs_tests.rs | 2 +- .../tests/heredoc_fragment_tests.rs | 31 +- .../kaish-kernel/tests/kernel_error_tests.rs | 4 +- .../kaish-kernel/tests/leading_zero_tests.rs | 18 +- 6 files changed, 1632 insertions(+), 542 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 7f8b1b3d..e535b0c5 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -1,41 +1,54 @@ -//! Arithmetic expression evaluation for shell-style `$(( ))` expressions. +//! `$(( ))` — checked 64-bit integer arithmetic and another-base number +//! reading. //! -//! Supports: -//! - Integer arithmetic: `+`, `-`, `*`, `/`, `%` -//! - Comparison operators: `>`, `<`, `>=`, `<=`, `==`, `!=` (return 1 or 0) -//! - Parentheses for grouping: `(expr)` -//! - Variable references: `$VAR` or bare `VAR` -//! - Integer literals +//! Three stages, kept separate so each can be tested on its own: +//! [`tokenize`] (text → [`Tok`]), [`parse`] ([`Tok`] → [`ArithExpr`]), and +//! evaluation ([`eval_sync`] for a scope with no `$(...)` reachable, and +//! `Kernel::eval_arith_async` in `kernel.rs` for the general case). //! -//! Does NOT support: -//! - Floating point (pipe to `jq` for float math) -//! - Bitwise operations (shell-ism we're skipping) -//! - Assignment within expressions (confusing) +//! Supports: decimal/hex/`base#digits` literals (base 2..=36), the full C +//! precedence table down through `?:`, `$name`/`${...}`/`$(...)`/nested +//! `$((...))` as operands, and bare `(( expr ))` as a condition (see +//! `Stmt::Arith`/`Expr::Arith` in `ast/types.rs`). +//! +//! Diverges from bash on purpose: overflow is an error, never a wrap; a +//! leading-zero numeral is refused, never read as octal; an unset or empty +//! operand is an error, never 0; `$(...)` on the unselected side of +//! `&&`/`||`/`?:` never runs. + +use crate::ast::{Stmt, Value, VarPath}; +use crate::interpreter::{value_to_string, Scope}; +use std::ops::Range; + +/// An error from tokenizing, parsing, or evaluating `$(( ))`. `message` is +/// the full, final text shown to the caller; `span` is the byte range in the +/// arithmetic source the error concerns, when one exists (evaluation errors +/// over already-resolved values carry `0..0` — the message already names +/// the values). +#[derive(Debug, Clone, PartialEq)] +pub struct ArithError { + pub message: String, + pub span: Range, +} -use crate::interpreter::Scope; -use crate::ast::{Value, VarPath, VarSegment}; -use anyhow::{bail, Context, Result}; +impl ArithError { + fn new(message: impl Into, span: Range) -> Self { + Self { message: message.into(), span } + } +} -/// Evaluate an arithmetic expression string. -/// -/// The expression should be the content between `$((` and `))`. -/// -/// # Example -/// ```ignore -/// let scope = Scope::new(); -/// scope.set("X", Value::Int(5)); -/// let result = eval_arithmetic("X + 3", &scope)?; -/// assert_eq!(result, 8); -/// ``` -pub fn eval_arithmetic(expr: &str, scope: &Scope) -> Result { - let mut parser = ArithParser::new(expr, scope); - let result = parser.parse_comparison()?; - parser.expect_end()?; - Ok(result) -} - -/// The decimal a leading-zero numeral was probably meant to be — `010` becomes -/// `10`, `-007` becomes `-7`. `None` when the text is not one. +impl std::fmt::Display for ArithError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +impl std::error::Error for ArithError {} + +const MAX_DEPTH: usize = 256; + +/// The decimal a leading-zero numeral was probably meant to be — `010` +/// becomes `10`, `-007` becomes `-7`. `None` when the text is not one. /// /// The suggestion keeps the sign: `-007` is not fixed by writing `7`. pub(crate) fn leading_zero_decimal(text: &str) -> Option { @@ -47,685 +60,1757 @@ pub(crate) fn leading_zero_decimal(text: &str) -> Option { Some(format!("{sign}{}", if digits.is_empty() { "0" } else { digits })) } -/// Simple recursive descent parser for arithmetic expressions. -struct ArithParser<'a> { - input: &'a str, - pos: usize, - scope: &'a Scope, +// ═══════════════════════════════════════════════════════════════════ +// Tokens +// ═══════════════════════════════════════════════════════════════════ + +#[derive(Debug, Clone, Copy, PartialEq)] +pub(crate) enum BinOp { + Add, Sub, Mul, Div, Rem, Pow, Shl, Shr, + Lt, Le, Gt, Ge, Eq, Ne, + BitAnd, BitXor, BitOr, And, Or, } -impl<'a> ArithParser<'a> { - fn new(input: &'a str, scope: &'a Scope) -> Self { - Self { input, pos: 0, scope } +impl BinOp { + fn symbol(self) -> &'static str { + match self { + BinOp::Add => "+", BinOp::Sub => "-", BinOp::Mul => "*", + BinOp::Div => "/", BinOp::Rem => "%", BinOp::Pow => "**", + BinOp::Shl => "<<", BinOp::Shr => ">>", + BinOp::Lt => "<", BinOp::Le => "<=", BinOp::Gt => ">", BinOp::Ge => ">=", + BinOp::Eq => "==", BinOp::Ne => "!=", + BinOp::BitAnd => "&", BinOp::BitXor => "^", BinOp::BitOr => "|", + BinOp::And => "&&", BinOp::Or => "||", + } } +} - fn skip_whitespace(&mut self) { - while self.pos < self.input.len() { - let ch = self.input.as_bytes()[self.pos]; - if ch == b' ' || ch == b'\t' { - self.pos += 1; - } else { - break; +#[derive(Debug, Clone, Copy, PartialEq)] +pub(crate) enum UnOp { + Neg, + Not, + BitNot, +} + +/// A `$(...)`/`${...}`/`$name`/`$?`/`$$`/`$((...))` operand, still +/// unresolved. Evaluating one is the only place `$(( ))` needs the async +/// evaluator — everything else in [`ArithExpr`] is pure. +#[derive(Debug, Clone, PartialEq)] +pub(crate) enum Expansion { + /// Bare `x` or `$x` — the whole value. + Var(String), + /// `${root[...]...}` — a literal-key subscript path (the interpolation + /// reading: brackets hold a KEY, not an expression). + BracedPath { root: String, brackets: String }, + /// `${root[...]:-default}` — `default` is itself arithmetic source, + /// evaluated only when `root` is unset or null. + BracedDefault { root: String, brackets: String, default: String }, + /// `$?` + LastExitCode, + /// `$$` + CurrentPid, + /// `$(...)` — pre-parsed; running it needs the async evaluator. + CommandSubst(Vec), + /// `$((...))` — a nested arithmetic form, evaluated recursively. + Nested(Box), +} + +#[derive(Debug, Clone, PartialEq)] +pub(crate) enum ArithExpr { + Int(i64), + Expansion(Expansion), + /// `xs[i]`, `xs[i][j]` — Decision B: each bracket's contents is a + /// numeric expression (the opposite of `${xs[i]}`'s literal key). + Subscript { root: String, indices: Vec }, + /// `base#` — the expansion's rendered text is read as digits + /// in `base` (`2#$BITS`, `10#$(date +%m)`). + BasedExpansion { base: u32, expansion: Box }, + Unary { op: UnOp, operand: Box }, + Binary { op: BinOp, left: Box, right: Box }, + Ternary { cond: Box, then_branch: Box, else_branch: Box }, +} + +impl ArithExpr { + /// True when some reachable node is a `$(...)` — used by callers that + /// want the sync fast path when it is safe. + pub(crate) fn contains_command_subst(&self) -> bool { + fn expansion_has(e: &Expansion) -> bool { + match e { + Expansion::CommandSubst(_) => true, + Expansion::Nested(inner) => inner.contains_command_subst(), + Expansion::BracedDefault { .. } + | Expansion::Var(_) + | Expansion::BracedPath { .. } + | Expansion::LastExitCode + | Expansion::CurrentPid => false, + } + } + match self { + ArithExpr::Int(_) => false, + ArithExpr::Expansion(e) => expansion_has(e), + ArithExpr::Subscript { indices, .. } => { + indices.iter().any(ArithExpr::contains_command_subst) + } + ArithExpr::BasedExpansion { expansion, .. } => expansion_has(expansion), + ArithExpr::Unary { operand, .. } => operand.contains_command_subst(), + ArithExpr::Binary { left, right, .. } => { + left.contains_command_subst() || right.contains_command_subst() + } + ArithExpr::Ternary { cond, then_branch, else_branch } => { + cond.contains_command_subst() + || then_branch.contains_command_subst() + || else_branch.contains_command_subst() } } } +} + +#[derive(Debug, Clone, PartialEq)] +enum TokKind { + Number(u64), + BasedExpansion { base: u32, expansion: Box }, + Ident(String), + Expansion(Expansion), + LParen, + RParen, + LBracket, + RBracket, + Question, + Colon, + Op(BinOp), + Bang, + Tilde, +} - fn peek(&mut self) -> Option { - self.skip_whitespace(); - self.input[self.pos..].chars().next() +impl std::fmt::Display for TokKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + TokKind::Number(n) => write!(f, "{n}"), + TokKind::BasedExpansion { base, .. } => write!(f, "{base}#..."), + TokKind::Ident(name) => write!(f, "{name}"), + TokKind::Expansion(_) => write!(f, "$..."), + TokKind::LParen => write!(f, "("), + TokKind::RParen => write!(f, ")"), + TokKind::LBracket => write!(f, "["), + TokKind::RBracket => write!(f, "]"), + TokKind::Question => write!(f, "?"), + TokKind::Colon => write!(f, ":"), + TokKind::Op(op) => write!(f, "{}", op.symbol()), + TokKind::Bang => write!(f, "!"), + TokKind::Tilde => write!(f, "~"), + } } +} - fn advance(&mut self) -> Option { - self.skip_whitespace(); - let ch = self.input[self.pos..].chars().next()?; - self.pos += ch.len_utf8(); - Some(ch) +#[derive(Debug, Clone, PartialEq)] +struct Tok { + kind: TokKind, + span: Range, +} + +// ═══════════════════════════════════════════════════════════════════ +// Tokenizer +// ═══════════════════════════════════════════════════════════════════ + +struct Tokenizer<'a> { + text: &'a str, + chars: Vec<(usize, char)>, + pos: usize, +} + +impl<'a> Tokenizer<'a> { + fn new(text: &'a str) -> Self { + Self { text, chars: text.char_indices().collect(), pos: 0 } } - /// Peek at the character n positions ahead (0 = current after whitespace skip). - fn peek_ahead(&mut self, n: usize) -> Option { - self.skip_whitespace(); - self.input[self.pos..].chars().nth(n) + fn byte_pos(&self) -> usize { + self.chars.get(self.pos).map(|(b, _)| *b).unwrap_or(self.text.len()) } - fn expect_end(&mut self) -> Result<()> { - self.skip_whitespace(); - if self.pos < self.input.len() { - bail!("unexpected characters at end of arithmetic expression: {:?}", - &self.input[self.pos..]); + fn peek(&self) -> Option { + self.chars.get(self.pos).map(|(_, c)| *c) + } + + fn peek_at(&self, n: usize) -> Option { + self.chars.get(self.pos + n).map(|(_, c)| *c) + } + + fn advance(&mut self) -> Option { + let c = self.peek(); + if c.is_some() { + self.pos += 1; } - Ok(()) + c } - /// Parse comparison operators (lowest precedence): >, <, >=, <=, ==, != - /// Returns 1 for true, 0 for false. - fn parse_comparison(&mut self) -> Result { - let mut left = self.parse_expr()?; + fn slice(&self, start: usize, end: usize) -> &'a str { + let end_byte = self.chars.get(end).map(|(b, _)| *b).unwrap_or(self.text.len()); + let start_byte = self.chars.get(start).map(|(b, _)| *b).unwrap_or(self.text.len()); + &self.text[start_byte..end_byte] + } + fn skip_ws(&mut self) { + while matches!(self.peek(), Some(c) if c.is_whitespace()) { + self.pos += 1; + } + } + + fn tokenize(mut self) -> Result, ArithError> { + let mut out = Vec::new(); loop { - self.skip_whitespace(); - match (self.peek_ahead(0), self.peek_ahead(1)) { - // Two-character operators must be checked first - (Some('>'), Some('=')) => { - self.advance(); // consume '>' - self.advance(); // consume '=' - let right = self.parse_expr()?; - left = if left >= right { 1 } else { 0 }; + self.skip_ws(); + let Some(c) = self.peek() else { break }; + let start = self.pos; + let start_byte = self.byte_pos(); + let kind = match c { + '0'..='9' => self.lex_number()?, + '$' => self.lex_dollar()?, + c if c.is_ascii_alphabetic() || c == '_' => self.lex_ident(), + '(' => { self.advance(); TokKind::LParen } + ')' => { self.advance(); TokKind::RParen } + '[' => { self.advance(); TokKind::LBracket } + ']' => { self.advance(); TokKind::RBracket } + '?' => { self.advance(); TokKind::Question } + ':' => { self.advance(); TokKind::Colon } + '~' => { self.advance(); TokKind::Tilde } + '!' => { + self.advance(); + if self.peek() == Some('=') { + self.advance(); + TokKind::Op(BinOp::Ne) + } else { + TokKind::Bang + } } - (Some('<'), Some('=')) => { - self.advance(); // consume '<' - self.advance(); // consume '=' - let right = self.parse_expr()?; - left = if left <= right { 1 } else { 0 }; + '+' => { self.advance(); self.reject_compound_or("+")?; TokKind::Op(BinOp::Add) } + '-' => { self.advance(); self.reject_compound_or("-")?; TokKind::Op(BinOp::Sub) } + '*' => { + self.advance(); + if self.peek() == Some('*') { + self.advance(); + TokKind::Op(BinOp::Pow) + } else { + TokKind::Op(BinOp::Mul) + } } - (Some('='), Some('=')) => { - self.advance(); // consume '=' - self.advance(); // consume '=' - let right = self.parse_expr()?; - left = if left == right { 1 } else { 0 }; + '/' => { self.advance(); TokKind::Op(BinOp::Div) } + '%' => { self.advance(); TokKind::Op(BinOp::Rem) } + '<' => { + self.advance(); + if self.peek() == Some('<') { + self.advance(); + if self.peek() == Some('<') { + return Err(ArithError::new( + "`<<<` is a here-string, not an operator; write `<<` to shift", + start..self.pos + 1, + )); + } + TokKind::Op(BinOp::Shl) + } else if self.peek() == Some('=') { + self.advance(); + TokKind::Op(BinOp::Le) + } else { + TokKind::Op(BinOp::Lt) + } } - (Some('!'), Some('=')) => { - self.advance(); // consume '!' - self.advance(); // consume '=' - let right = self.parse_expr()?; - left = if left != right { 1 } else { 0 }; + '>' => { + self.advance(); + if self.peek() == Some('>') { + self.advance(); + if self.peek() == Some('>') { + return Err(ArithError::new( + "`>>>` is not an operator; write `>>`", + start..self.pos + 1, + )); + } + TokKind::Op(BinOp::Shr) + } else if self.peek() == Some('=') { + self.advance(); + TokKind::Op(BinOp::Ge) + } else { + TokKind::Op(BinOp::Gt) + } } - // Single-character operators - (Some('>'), _) => { - self.advance(); // consume '>' - let right = self.parse_expr()?; - left = if left > right { 1 } else { 0 }; + '=' => { + self.advance(); + if self.peek() == Some('=') { + self.advance(); + TokKind::Op(BinOp::Eq) + } else { + return Err(ArithError::new( + format!( + "`{}` assigns inside `$(( ))`; write `name=$((rhs))`, or `==` to compare", + self.slice(start, self.pos) + ), + start..self.pos, + )); + } } - (Some('<'), _) => { - self.advance(); // consume '<' - let right = self.parse_expr()?; - left = if left < right { 1 } else { 0 }; + '&' => { + self.advance(); + if self.peek() == Some('&') { + self.advance(); + TokKind::Op(BinOp::And) + } else { + TokKind::Op(BinOp::BitAnd) + } } - _ => break, - } + '|' => { + self.advance(); + if self.peek() == Some('|') { + self.advance(); + TokKind::Op(BinOp::Or) + } else { + TokKind::Op(BinOp::BitOr) + } + } + '^' => { self.advance(); TokKind::Op(BinOp::BitXor) } + ',' => { + return Err(ArithError::new( + "`,` is not an operator; one expression per `$(( ))`", + start..self.pos + 1, + )); + } + other => { + return Err(ArithError::new( + format!("`{other}` cannot start a value"), + start..self.pos + 1, + )); + } + }; + out.push(Tok { kind, span: start_byte..self.byte_pos() }); } + Ok(out) + } - Ok(left) + /// After consuming `+`/`-`, refuse `++`/`--`/`+=`/`-=` outright — kaish + /// has no assignment or increment inside `$(( ))`. + fn reject_compound_or(&mut self, sym: &str) -> Result<(), ArithError> { + let start = self.pos - 1; + if self.peek() == Some(sym.chars().next().unwrap_or(' ')) { + self.advance(); + let name_hint = "name"; + return Err(ArithError::new( + format!( + "`{}{sym}` assigns inside `$(( ))`; write `{name_hint} = {name_hint} {sym1} 1`", + sym, + sym1 = sym, + ), + start..self.pos, + )); + } + if self.peek() == Some('=') { + self.advance(); + return Err(ArithError::new( + format!( + "`{sym}=` assigns inside `$(( ))`; write `name=$((name {sym} rhs))`", + ), + start..self.pos, + )); + } + Ok(()) } - /// Parse an expression: handles + and - (lowest precedence) - fn parse_expr(&mut self) -> Result { - let mut left = self.parse_term()?; + fn lex_ident(&mut self) -> TokKind { + let start = self.pos; + while matches!(self.peek(), Some(c) if c.is_ascii_alphanumeric() || c == '_') { + self.pos += 1; + } + TokKind::Ident(self.slice(start, self.pos).to_string()) + } + /// Consume a run of base-`base` digits (case-insensitive letters past + /// `9`), erroring loud on `_` or a digit too large for `base`. Returns + /// the checked magnitude and the run's end index (== start if empty). + fn consume_digits(&mut self, base: u32, lit_start: usize) -> Result<(u64, usize), ArithError> { + let digits_start = self.pos; + let mut mag: u64 = 0; loop { - match self.peek() { - Some('+') => { - self.advance(); - let right = self.parse_term()?; - left = left.checked_add(right) - .context("arithmetic overflow in addition")?; - } - Some('-') => { - self.advance(); - let right = self.parse_term()?; - left = left.checked_sub(right) - .context("arithmetic overflow in subtraction")?; + let Some(c) = self.peek() else { break }; + if c == '_' { + return Err(ArithError::new( + format!("`{}` contains `_`; remove it", self.slice(lit_start, self.pos + 1)), + lit_start..self.byte_pos() + c.len_utf8(), + )); + } + if !c.is_ascii_alphanumeric() { + break; + } + let digit_val = match c { + '0'..='9' => c as u32 - '0' as u32, + 'a'..='z' => c as u32 - 'a' as u32 + 10, + 'A'..='Z' => c as u32 - 'A' as u32 + 10, + _ => unreachable!("ascii_alphanumeric"), + }; + if digit_val >= base { + self.pos += 1; + return Err(ArithError::new( + format!( + "`{c}` is not a digit in `{}`; use digits valid for base {base}", + self.slice(lit_start, self.pos) + ), + lit_start..self.byte_pos(), + )); + } + mag = mag + .checked_mul(base as u64) + .and_then(|m| m.checked_add(digit_val as u64)) + .ok_or_else(|| { + ArithError::new( + format!("`{}` {INTEGER_OUT_OF_RANGE}", self.slice(lit_start, self.pos + 1)), + lit_start..self.byte_pos(), + ) + })?; + self.pos += 1; + } + Ok((mag, digits_start)) + } + + fn lex_number(&mut self) -> Result { + let start = self.pos; + + // `0x` / `0X` hex. + if self.peek() == Some('0') && matches!(self.peek_at(1), Some('x' | 'X')) { + self.pos += 2; + let prefix = self.slice(start, self.pos).to_string(); + let (mag, digits_start) = self.consume_digits(16, start)?; + if digits_start == self.pos { + return Err(ArithError::new( + format!("`{prefix}` has no digits; add digits after `{prefix}`"), + start..self.pos, + )); + } + return Ok(TokKind::Number(mag)); + } + + // `0b` / `0o` — not a kaish base spelling. + if self.peek() == Some('0') && matches!(self.peek_at(1), Some('b' | 'B' | 'o' | 'O')) { + let kind_char = self.peek_at(1).unwrap_or('b'); + self.pos += 2; + let digits_start = self.pos; + while matches!(self.peek(), Some(c) if c.is_ascii_alphanumeric()) { + self.pos += 1; + } + let digits = self.slice(digits_start, self.pos); + let full = self.slice(start, self.pos); + let (base, word) = if matches!(kind_char, 'b' | 'B') { (2, "binary") } else { (8, "octal") }; + return Err(ArithError::new( + format!("`{full}` is not a kaish base spelling; write `{base}#{digits}` for {word}"), + start..self.pos, + )); + } + + // Plain decimal run — either a bare decimal literal, or the base + // number before `#`. + let (base_mag, digits_start) = self.consume_digits(10, start)?; + debug_assert!(digits_start == start); + + if self.peek() == Some('#') { + self.advance(); // consume '#' + let base = base_mag as u32; + if !(2..=36).contains(&base) { + return Err(ArithError::new( + format!("base `{base_mag}` is outside 2..=36"), + start..self.pos, + )); + } + if matches!(self.peek(), Some('+') | Some('-')) { + let sign = self.peek().unwrap(); + let sign_start = self.pos; + self.pos += 1; + while matches!(self.peek(), Some(c) if c.is_ascii_alphanumeric()) { + self.pos += 1; } - _ => break, + let lit = self.slice(start, self.pos); + return Err(ArithError::new( + format!("`{lit}` puts `{sign}` after `#`; write `{sign}{base}#{}`", self.slice(sign_start + 1, self.pos)), + start..self.pos, + )); + } + if self.peek() == Some('$') { + let expansion = self.lex_expansion_body()?; + return Ok(TokKind::BasedExpansion { base, expansion: Box::new(expansion) }); } + let prefix = self.slice(start, self.pos).to_string(); + let (mag, bdigits_start) = self.consume_digits(base, start)?; + if bdigits_start == self.pos { + return Err(ArithError::new( + format!("`{prefix}` has no digits; add digits after `{prefix}`"), + start..self.pos, + )); + } + return Ok(TokKind::Number(mag)); } - Ok(left) + let text = self.slice(start, self.pos); + if crate::lexer::is_leading_zero_numeral(text) { + let decimal = leading_zero_decimal(text).unwrap_or_else(|| "0".to_string()); + return Err(ArithError::new( + format!( + "`{text}` has a leading zero — kaish reads no octal; write `8#{}` for octal or `{decimal}` for decimal", + text.trim_start_matches('0') + ), + start..self.pos, + )); + } + Ok(TokKind::Number(base_mag)) } - /// Parse a term: handles * / % (higher precedence) - fn parse_term(&mut self) -> Result { - let mut left = self.parse_unary()?; + /// `$name`, `$?`, `$$`, `${...}`, `$(...)`, `$((...))` starting at the + /// current `$`. + fn lex_dollar(&mut self) -> Result { + Ok(TokKind::Expansion(self.lex_expansion_body()?)) + } - loop { - match self.peek() { - Some('*') => { - self.advance(); - let right = self.parse_unary()?; - left = left.checked_mul(right) - .context("arithmetic overflow in multiplication")?; - } - Some('/') => { - self.advance(); - let right = self.parse_unary()?; - if right == 0 { - bail!("division by zero"); + /// Same as [`Self::lex_dollar`] but returning the bare [`Expansion`], + /// for `base#$name` and `base#$(...)`. + fn lex_expansion_body(&mut self) -> Result { + let dollar_start = self.pos; + self.advance(); // consume '$' + match self.peek() { + Some('?') => { self.advance(); Ok(Expansion::LastExitCode) } + Some('$') => { self.advance(); Ok(Expansion::CurrentPid) } + Some('(') if self.peek_at(1) == Some('(') => { + self.pos += 2; + let inner_start = self.pos; + let mut depth = 0i32; + let mut closed = false; + while let Some(c) = self.peek() { + match c { + '(' => { depth += 1; self.pos += 1; } + ')' => { + if depth > 0 { + depth -= 1; + self.pos += 1; + } else if self.peek_at(1) == Some(')') { + self.pos += 2; + closed = true; + break; + } else { + self.pos += 1; + } + } + _ => { self.pos += 1; } } - left = left.checked_div(right) - .context("arithmetic overflow in division")?; } - Some('%') => { - self.advance(); - let right = self.parse_unary()?; - if right == 0 { - bail!("modulo by zero"); + if !closed { + return Err(ArithError::new( + format!("`{}` has no closing `)`", self.slice(dollar_start, self.pos)), + dollar_start..self.byte_pos(), + )); + } + let inner_end = self.pos - 2; + let inner_text = self.slice(inner_start, inner_end).to_string(); + let inner = parse(&inner_text)?; + Ok(Expansion::Nested(Box::new(inner))) + } + Some('(') => { + self.advance(); // consume '(' + let remainder_start_byte = self.byte_pos(); + let remainder = &self.text[remainder_start_byte..]; + let toks = crate::lexer::tokenize(remainder).map_err(|_| { + ArithError::new( + format!("`{}` has no closing `)`", self.slice(dollar_start, self.pos)), + dollar_start..self.text.len(), + ) + })?; + let toks: Vec<(crate::lexer::Token, crate::parser::Span)> = toks + .into_iter() + .map(|sp| (sp.token, (sp.span.start..sp.span.end).into())) + .collect(); + let close = crate::parser::find_cmd_subst_close(&toks).ok_or_else(|| { + ArithError::new( + format!("`{}` has no closing `)`", self.slice(dollar_start, self.pos)), + dollar_start..self.text.len(), + ) + })?; + let close_span = toks[close].1; + let close_start: usize = close_span.start; + let close_end: usize = close_span.end; + let cmd_text = &remainder[..close_start]; + // Advance past the command text plus its closing `)`. + let consumed_bytes = close_end; + let mut consumed_chars = 0usize; + let mut byte_count = 0usize; + while byte_count < consumed_bytes && self.pos + consumed_chars < self.chars.len() { + byte_count += self.chars[self.pos + consumed_chars].1.len_utf8(); + consumed_chars += 1; + } + self.pos += consumed_chars; + match crate::parser::parse(cmd_text) { + Ok(program) => Ok(Expansion::CommandSubst(program.statements)), + Err(_) => Err(ArithError::new( + format!("syntax error in command substitution: $({cmd_text})"), + dollar_start..self.byte_pos(), + )), + } + } + Some('{') => { + self.advance(); // consume '{' + let body_start = self.pos; + let mut depth = 1i32; + while depth > 0 { + match self.peek() { + Some('{') => { depth += 1; self.pos += 1; } + Some('}') => { depth -= 1; self.pos += 1; } + Some(_) => { self.pos += 1; } + None => { + return Err(ArithError::new( + format!("`{}` has no closing `}}`", self.slice(dollar_start, self.pos)), + dollar_start..self.byte_pos(), + )); + } } - left = left.checked_rem(right) - .context("arithmetic overflow in modulo")?; } - _ => break, + let body = self.slice(body_start, self.pos - 1).to_string(); + parse_braced_body(&body, dollar_start..self.byte_pos()) + } + // `$1`, `$2`, … — positional parameters. A leading digit is + // otherwise not a valid identifier start, so it is unambiguous + // here: bash allows only a single digit unbraced, but kaish + // reads the whole run (`${10}` still works too). + Some(c) if c.is_ascii_alphabetic() || c == '_' || c.is_ascii_digit() => { + let start = self.pos; + while matches!(self.peek(), Some(c) if c.is_ascii_alphanumeric() || c == '_') { + self.pos += 1; + } + Ok(Expansion::Var(self.slice(start, self.pos).to_string())) + } + _ => Err(ArithError::new( + format!("`{}` cannot start a value", self.slice(dollar_start, self.pos + 1)), + dollar_start..self.byte_pos() + 1, + )), + } + } +} + +/// Message for a numeral outside i64 range — shared text with the lexer. +use crate::lexer::INTEGER_OUT_OF_RANGE; + +fn split_name_and_brackets(text: &str) -> Option<(String, String)> { + let bracket_start = text.find('[').unwrap_or(text.len()); + let name = &text[..bracket_start]; + if name.is_empty() || !name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') + || name.chars().next().is_some_and(|c| c.is_ascii_digit()) + { + return None; + } + Some((name.to_string(), text[bracket_start..].to_string())) +} + +fn parse_braced_body(body: &str, span: Range) -> Result { + if body == "?" { + return Ok(Expansion::LastExitCode); + } + if body == "$" { + return Ok(Expansion::CurrentPid); + } + let bytes = body.as_bytes(); + let mut depth = 0i32; + let mut default_at = None; + let mut i = 0; + while i < bytes.len() { + match bytes[i] { + b'[' => depth += 1, + b']' => depth -= 1, + b':' if depth == 0 && bytes.get(i + 1) == Some(&b'-') => { + default_at = Some(i); + break; } + _ => {} } + i += 1; + } + if let Some(idx) = default_at { + let root_and_sub = &body[..idx]; + let default = body[idx + 2..].to_string(); + let Some((root, brackets)) = split_name_and_brackets(root_and_sub) else { + return Err(ArithError::new(format!("`{{{body}}}` is not valid inside `$(( ))`"), span)); + }; + return Ok(Expansion::BracedDefault { root, brackets, default }); + } + let Some((root, brackets)) = split_name_and_brackets(body) else { + return Err(ArithError::new(format!("`{{{body}}}` is not valid inside `$(( ))`"), span)); + }; + if brackets.is_empty() { + Ok(Expansion::Var(root)) + } else { + Ok(Expansion::BracedPath { root, brackets }) + } +} + +fn tokenize(text: &str) -> Result, ArithError> { + Tokenizer::new(text).tokenize() +} +// ═══════════════════════════════════════════════════════════════════ +// Parser (precedence climbing over the EBNF, high to low: unary, `**`, +// `* / %`, `+ -`, `<< >>`, `< <= > >=`, `== !=`, `&`, `^`, `|`, `&&`, +// `||`, `?:`) +// ═══════════════════════════════════════════════════════════════════ + +struct Parser { + toks: Vec, + pos: usize, + depth: usize, + end: usize, +} + +impl Parser { + fn new(toks: Vec, end: usize) -> Self { + Self { toks, pos: 0, depth: 0, end } + } + + fn peek(&self) -> Option<&TokKind> { + self.toks.get(self.pos).map(|t| &t.kind) + } + + fn peek_span(&self) -> Range { + self.toks.get(self.pos).map(|t| t.span.clone()).unwrap_or(self.end..self.end) + } + + fn advance(&mut self) -> Option { + let t = self.toks.get(self.pos).cloned(); + if t.is_some() { + self.pos += 1; + } + t + } + + fn enter(&mut self) -> Result<(), ArithError> { + self.depth += 1; + if self.depth > MAX_DEPTH { + return Err(ArithError::new("more than 256 nested arithmetic forms", self.peek_span())); + } + Ok(()) + } + + fn leave(&mut self) { + self.depth -= 1; + } + + fn eat_op(&mut self, want: BinOp) -> bool { + if self.peek() == Some(&TokKind::Op(want)) { + self.pos += 1; + true + } else { + false + } + } + + fn left_assoc( + &mut self, + ops: &[BinOp], + mut next: impl FnMut(&mut Self) -> Result, + ) -> Result { + let mut left = next(self)?; + loop { + let matched = ops.iter().copied().find(|op| self.peek() == Some(&TokKind::Op(*op))); + let Some(op) = matched else { break }; + self.pos += 1; + let right = next(self)?; + left = ArithExpr::Binary { op, left: Box::new(left), right: Box::new(right) }; + } Ok(left) } - /// Parse unary operators: + and - prefix - fn parse_unary(&mut self) -> Result { - match self.peek() { - Some('+') => { - self.advance(); - self.parse_unary() + fn parse_conditional(&mut self) -> Result { + self.enter()?; + let cond = self.parse_logical_or()?; + let result = if self.peek() == Some(&TokKind::Question) { + self.pos += 1; + let then_branch = self.parse_conditional()?; + match self.peek() { + Some(&TokKind::Colon) => self.pos += 1, + _ => { + return Err(ArithError::new("`?` has no matching `:`", self.peek_span())); + } } - Some('-') => { - self.advance(); - let val = self.parse_unary()?; - val.checked_neg().context("arithmetic overflow in negation") + let else_branch = self.parse_conditional()?; + ArithExpr::Ternary { + cond: Box::new(cond), + then_branch: Box::new(then_branch), + else_branch: Box::new(else_branch), } - _ => self.parse_primary(), + } else { + cond + }; + self.leave(); + Ok(result) + } + + fn parse_logical_or(&mut self) -> Result { + self.left_assoc(&[BinOp::Or], Self::parse_logical_and) + } + + fn parse_logical_and(&mut self) -> Result { + self.left_assoc(&[BinOp::And], Self::parse_bitor) + } + + fn parse_bitor(&mut self) -> Result { + self.left_assoc(&[BinOp::BitOr], Self::parse_bitxor) + } + + fn parse_bitxor(&mut self) -> Result { + self.left_assoc(&[BinOp::BitXor], Self::parse_bitand) + } + + fn parse_bitand(&mut self) -> Result { + self.left_assoc(&[BinOp::BitAnd], Self::parse_equality) + } + + fn parse_equality(&mut self) -> Result { + self.left_assoc(&[BinOp::Eq, BinOp::Ne], Self::parse_relational) + } + + fn parse_relational(&mut self) -> Result { + self.left_assoc(&[BinOp::Le, BinOp::Ge, BinOp::Lt, BinOp::Gt], Self::parse_shift) + } + + fn parse_shift(&mut self) -> Result { + self.left_assoc(&[BinOp::Shl, BinOp::Shr], Self::parse_additive) + } + + fn parse_additive(&mut self) -> Result { + self.left_assoc(&[BinOp::Add, BinOp::Sub], Self::parse_multiplicative) + } + + fn parse_multiplicative(&mut self) -> Result { + self.left_assoc(&[BinOp::Mul, BinOp::Div, BinOp::Rem], Self::parse_power) + } + + fn parse_power(&mut self) -> Result { + let base = self.parse_unary()?; + if self.eat_op(BinOp::Pow) { + self.enter()?; + let exp = self.parse_power()?; + self.leave(); + Ok(ArithExpr::Binary { op: BinOp::Pow, left: Box::new(base), right: Box::new(exp) }) + } else { + Ok(base) } } - /// Parse primary: numbers, variables, parenthesized expressions - fn parse_primary(&mut self) -> Result { - self.skip_whitespace(); + /// `i64::MIN`'s magnitude, `9223372036854775808`, has no representation + /// as a positive `i64` — it is legal only as the direct operand of a + /// single unary minus. + const MIN_MAGNITUDE: u64 = 9_223_372_036_854_775_808; - match self.peek() { - Some('(') => { - self.advance(); // consume '(' - let val = self.parse_expr()?; - match self.peek() { - Some(')') => { - self.advance(); - Ok(val) + fn parse_unary(&mut self) -> Result { + self.enter()?; + let result = match self.peek() { + Some(&TokKind::Op(BinOp::Sub)) => { + self.pos += 1; + if let Some(&TokKind::Number(mag)) = self.peek() { + if mag == Self::MIN_MAGNITUDE { + self.pos += 1; + self.leave(); + return Ok(ArithExpr::Int(i64::MIN)); } - _ => bail!("expected ')' in arithmetic expression"), } + let operand = self.parse_unary()?; + ArithExpr::Unary { op: UnOp::Neg, operand: Box::new(operand) } } - Some('$') => { - // $VAR, ${VAR}, $?, $$, ${?}, ${$} syntax - self.advance(); // consume '$' + Some(&TokKind::Op(BinOp::Add)) => { + self.pos += 1; + self.parse_unary()? + } + Some(&TokKind::Bang) => { + self.pos += 1; + let operand = self.parse_unary()?; + ArithExpr::Unary { op: UnOp::Not, operand: Box::new(operand) } + } + Some(&TokKind::Tilde) => { + self.pos += 1; + let operand = self.parse_unary()?; + ArithExpr::Unary { op: UnOp::BitNot, operand: Box::new(operand) } + } + _ => self.parse_primary()?, + }; + self.leave(); + Ok(result) + } - // Special case: $? (last exit code) - if self.peek() == Some('?') { - self.advance(); // consume '?' - return Ok(self.scope.last_result().code); + fn parse_primary(&mut self) -> Result { + let span = self.peek_span(); + match self.advance().map(|t| t.kind) { + Some(TokKind::Number(mag)) => int_from_magnitude(mag, false, span), + Some(TokKind::BasedExpansion { base, expansion }) => { + Ok(ArithExpr::BasedExpansion { base, expansion }) + } + Some(TokKind::Expansion(e)) => Ok(ArithExpr::Expansion(e)), + Some(TokKind::Ident(name)) => { + if self.peek() == Some(&TokKind::LBracket) { + let mut indices = Vec::new(); + while self.peek() == Some(&TokKind::LBracket) { + self.pos += 1; + self.enter()?; + let index = self.parse_conditional()?; + self.leave(); + match self.peek() { + Some(&TokKind::RBracket) => self.pos += 1, + _ => { + return Err(ArithError::new( + "`[` has no matching `]`", + self.peek_span(), + )); + } + } + indices.push(index); + } + Ok(ArithExpr::Subscript { root: name, indices }) + } else { + Ok(ArithExpr::Expansion(Expansion::Var(name))) } - - // Special case: $$ (current PID) - if self.peek() == Some('$') { - self.advance(); // consume second '$' - return Ok(self.scope.pid() as i64); + } + Some(TokKind::LParen) => { + self.enter()?; + if self.peek() == Some(&TokKind::RParen) { + self.leave(); + self.pos += 1; + return Err(ArithError::new("`()` has no expression", span)); + } + let inner = self.parse_conditional()?; + self.leave(); + match self.peek() { + Some(&TokKind::RParen) => { + self.pos += 1; + Ok(inner) + } + _ => Err(ArithError::new("`(` has no closing `)`", span)), } + } + Some(TokKind::RParen) => Err(ArithError::new("`)` has no matching `(`", span)), + Some(other) => Err(ArithError::new(format!("`{other}` cannot start a value"), span)), + None => Err(ArithError::new("`$(( ))` has no expression; write a number or an expression", span)), + } + } +} - let var_name = if self.peek() == Some('{') { - self.advance(); // consume '{' +fn int_from_magnitude(mag: u64, negative: bool, span: Range) -> Result { + let max = if negative { Parser::MIN_MAGNITUDE } else { i64::MAX as u64 }; + if mag > max { + return Err(ArithError::new(format!("`{mag}` {INTEGER_OUT_OF_RANGE}"), span)); + } + if negative && mag == Parser::MIN_MAGNITUDE { + return Ok(ArithExpr::Int(i64::MIN)); + } + Ok(ArithExpr::Int(if negative { -(mag as i64) } else { mag as i64 })) +} - // Special case: ${?} (last exit code, braced form) - if self.peek() == Some('?') { - self.advance(); // consume '?' - if self.peek() != Some('}') { - bail!("expected '}}' after ${{?}} in arithmetic"); - } - self.advance(); // consume '}' - return Ok(self.scope.last_result().code); - } +pub(crate) fn parse(text: &str) -> Result { + let toks = tokenize(text)?; + if toks.is_empty() { + return Err(ArithError::new( + "`$(( ))` has no expression; write a number or an expression", + 0..text.len(), + )); + } + let end = text.len(); + let mut parser = Parser::new(toks, end); + let expr = parser.parse_conditional()?; + if let Some(extra) = parser.peek() { + if extra == &TokKind::RParen { + return Err(ArithError::new( + format!("`)` has no matching `(` in `{text}`"), + parser.peek_span(), + )); + } + return Err(ArithError::new(format!("`{extra}` is not valid inside `$(( ))`"), parser.peek_span())); + } + Ok(expr) +} - // Special case: ${$} (current PID, braced form) - if self.peek() == Some('$') { - self.advance(); // consume '$' - if self.peek() != Some('}') { - bail!("expected '}}' after ${{$}} in arithmetic"); - } - self.advance(); // consume '}' - return Ok(self.scope.pid() as i64); - } +// ═══════════════════════════════════════════════════════════════════ +// Pure operator evaluation — shared by the sync and async walkers +// ═══════════════════════════════════════════════════════════════════ - let name = self.parse_identifier()?; - // Collection subscript path: `${p[port]}`, `${a[b][0]}`. - if self.peek() == Some('[') { - return self.eval_braced_path(&name); - } - if self.peek() != Some('}') { - bail!("expected '}}' after variable name in arithmetic"); +fn shift_count_error(count: i64) -> ArithError { + ArithError::new(format!("shift count `{count}` is outside 0..=63"), 0..0) +} + +fn overflow(l: i64, op: BinOp, r: i64) -> ArithError { + ArithError::new(format!("`{l} {} {r}` does not fit in a 64-bit integer", op.symbol()), 0..0) +} + +pub(crate) fn apply_binary(op: BinOp, l: i64, r: i64) -> Result { + match op { + BinOp::Add => l.checked_add(r).ok_or_else(|| overflow(l, op, r)), + BinOp::Sub => l.checked_sub(r).ok_or_else(|| overflow(l, op, r)), + BinOp::Mul => l.checked_mul(r).ok_or_else(|| overflow(l, op, r)), + BinOp::Div => { + if r == 0 { + return Err(ArithError::new(format!("`{l} / 0` divides by zero"), 0..0)); + } + l.checked_div(r).ok_or_else(|| overflow(l, op, r)) + } + BinOp::Rem => { + if r == 0 { + return Err(ArithError::new(format!("`{l} % 0` divides by zero"), 0..0)); + } + l.checked_rem(r).ok_or_else(|| overflow(l, op, r)) + } + BinOp::Pow => { + if r < 0 { + return Err(ArithError::new(format!("exponent `{r}` is negative; use 0 or greater"), 0..0)); + } + match l { + 0 => Ok(if r == 0 { 1 } else { 0 }), + 1 => Ok(1), + -1 => Ok(if r % 2 == 0 { 1 } else { -1 }), + _ => { + if r > u32::MAX as i64 { + return Err(overflow(l, op, r)); } - self.advance(); // consume '}' - name - } else { - self.parse_identifier()? - }; - self.get_var_value(&var_name) - } - Some(c) if c.is_ascii_digit() => { - self.parse_number() - } - Some(c) if c.is_ascii_alphabetic() || c == '_' => { - // Bare variable name (bash allows this in $(( ))) - let var_name = self.parse_identifier()?; - if self.peek() == Some('[') { - // Bare subscript path `xs[i]` — decision B. - return self.eval_bare_subscript_path(&var_name); + l.checked_pow(r as u32).ok_or_else(|| overflow(l, op, r)) } - self.get_var_value(&var_name) } - Some(c) => bail!("unexpected character in arithmetic expression: {:?}", c), - None => bail!("unexpected end of arithmetic expression"), } + BinOp::Shl => { + if !(0..=63).contains(&r) { + return Err(shift_count_error(r)); + } + let factor: i128 = 1i128 << r; + let result = (l as i128) * factor; + i64::try_from(result).map_err(|_| overflow(l, op, r)) + } + BinOp::Shr => { + if !(0..=63).contains(&r) { + return Err(shift_count_error(r)); + } + Ok(l >> r) + } + BinOp::Lt => Ok((l < r) as i64), + BinOp::Le => Ok((l <= r) as i64), + BinOp::Gt => Ok((l > r) as i64), + BinOp::Ge => Ok((l >= r) as i64), + BinOp::Eq => Ok((l == r) as i64), + BinOp::Ne => Ok((l != r) as i64), + BinOp::BitAnd => Ok(l & r), + BinOp::BitXor => Ok(l ^ r), + BinOp::BitOr => Ok(l | r), + BinOp::And | BinOp::Or => unreachable!("short-circuit ops are handled by the tree walk"), } +} - fn parse_number(&mut self) -> Result { - let start = self.pos; - while self.pos < self.input.len() { - let ch = self.input.as_bytes()[self.pos]; - if ch.is_ascii_digit() { - self.pos += 1; - } else { - break; - } +pub(crate) fn apply_unary(op: UnOp, v: i64) -> Result { + match op { + UnOp::Neg => v + .checked_neg() + .ok_or_else(|| ArithError::new(format!("`-{v}` does not fit in a 64-bit integer"), 0..0)), + UnOp::Not => Ok(if v == 0 { 1 } else { 0 }), + UnOp::BitNot => Ok(!v), + } +} + +fn truthy(v: i64) -> bool { + v != 0 +} + +// ═══════════════════════════════════════════════════════════════════ +// Coercion (Value → i64) +// ═══════════════════════════════════════════════════════════════════ + +fn expression_like(s: &str) -> bool { + let bytes = s.as_bytes(); + bytes.iter().enumerate().any(|(i, &b)| match b { + b'+' | b'-' => i > 0, + b'*' | b'/' | b'%' | b'<' | b'>' | b'=' | b'&' | b'|' | b'^' | b'!' | b'~' | b'?' | b':' | b'(' | b')' => true, + _ => false, + }) +} + +/// What a piece of text is, as a signed numeral in decimal/hex/`base#` +/// spelling — the core `parse_numeric_string` and the command-output +/// coercion below share, so the sign/leading-zero/tokenize logic exists +/// once. +enum Numeral { + Ok(i64), + Empty, + ExpressionLike, + LeadingZero, + NotANumber, + OutOfRange, +} + +fn read_numeral(text: &str) -> Numeral { + let trimmed = text.trim(); + if trimmed.is_empty() { + return Numeral::Empty; + } + if expression_like(trimmed) { + return Numeral::ExpressionLike; + } + let (neg, digits) = match trimmed.strip_prefix('-') { + Some(rest) => (true, rest), + None => (false, trimmed.strip_prefix('+').unwrap_or(trimmed)), + }; + if digits.is_empty() { + return Numeral::NotANumber; + } + if crate::lexer::is_leading_zero_numeral(digits) { + return Numeral::LeadingZero; + } + match tokenize(digits) { + Ok(toks) if toks.len() == 1 => match &toks[0].kind { + TokKind::Number(mag) => match int_from_magnitude(*mag, neg, 0..0) { + Ok(ArithExpr::Int(n)) => Numeral::Ok(n), + Ok(_) => unreachable!("int_from_magnitude only returns Int"), + Err(_) => Numeral::OutOfRange, + }, + _ => Numeral::NotANumber, + }, + _ => Numeral::NotANumber, + } +} + +fn parse_numeric_string(s: &str, name: &str) -> Result { + match read_numeral(s) { + Numeral::Ok(n) => Ok(n), + Numeral::Empty | Numeral::ExpressionLike => Err(ArithError::new( + format!( + "`{name}` holds `{s}`; a variable is a value, not an expression — write it inside `$(( ))`" + ), + 0..0, + )), + Numeral::LeadingZero => Err(ArithError::new( + format!( + "`{name}` holds `{s}` (leading zero) — kaish reads no octal; write `10#${name}` for decimal or `8#${name}` for octal" + ), + 0..0, + )), + Numeral::NotANumber => { + Err(ArithError::new(format!("`{name}` holds `{s}`, which is not a number"), 0..0)) } - let num_str = &self.input[start..self.pos]; - // bash reads `010` as octal and answers 9; decimal would answer 11. - // Refuse rather than answer a third number. - if crate::lexer::is_leading_zero_numeral(num_str) { - let trimmed = num_str.trim_start_matches('0'); - let trimmed = if trimmed.is_empty() { "0" } else { trimmed }; - bail!( - "`{num_str}` is text (leading zero) and kaish reads no octal — write \ - `{trimmed}` for the decimal value" - ); + Numeral::OutOfRange => { + Err(ArithError::new(format!("`{name}` holds `{s}`, outside the 64-bit range"), 0..0)) } - // The loop above admits only `[0-9]+`, so overflow is the only way - // this parse fails — same limit the lexer names, same words. - num_str.parse().context(crate::lexer::INTEGER_OUT_OF_RANGE) } +} - fn parse_identifier(&mut self) -> Result { - let start = self.pos; - while self.pos < self.input.len() { - let ch = self.input.as_bytes()[self.pos]; - if ch.is_ascii_alphanumeric() || ch == b'_' { - self.pos += 1; +/// Coerce a `$(...)` operand's printed text — the command must print exactly +/// one integer. +fn parse_command_output(text: &str, cmd: &str) -> Result { + match read_numeral(text) { + Numeral::Ok(n) => Ok(n), + Numeral::Empty => Err(ArithError::new( + format!("`{cmd}` printed nothing; the command must print one integer"), + 0..0, + )), + Numeral::ExpressionLike | Numeral::NotANumber | Numeral::LeadingZero | Numeral::OutOfRange => { + Err(ArithError::new( + format!("`{cmd}` printed `{text}`; the command must print one integer"), + 0..0, + )) + } + } +} + +pub(crate) fn value_to_arith(value: &Value, name: &str) -> Result { + match value { + Value::Int(n) => Ok(*n), + Value::Bool(b) => Ok(if *b { 1 } else { 0 }), + Value::Float(f) => { + if !f.is_finite() || f.fract() != 0.0 { + Err(ArithError::new(format!("`{name}` holds `{f}`; arithmetic is integer-only"), 0..0)) + } else if *f < i64::MIN as f64 || *f > i64::MAX as f64 { + Err(ArithError::new(format!("`{name}` holds `{f}`, outside the 64-bit range"), 0..0)) } else { - break; + Ok(*f as i64) } } - if start == self.pos { - bail!("expected identifier in arithmetic expression"); + Value::String(s) => parse_numeric_string(s, name), + Value::Null => Err(ArithError::new(format!("`{name}` is null; set it to an integer"), 0..0)), + Value::Json(serde_json::Value::Array(_)) => { + Err(ArithError::new(format!("`{name}` is a list; index a number field"), 0..0)) + } + Value::Json(serde_json::Value::Object(_)) => { + Err(ArithError::new(format!("`{name}` is a record; index a number field"), 0..0)) + } + Value::Json(_) => Err(ArithError::new( + format!("`{name}` holds `{}`, which is not a number", value_to_string(value)), + 0..0, + )), + Value::Bytes(b) => { + Err(ArithError::new(format!("`{name}` holds {} bytes; decode them first", b.len()), 0..0)) } - Ok(self.input[start..self.pos].to_string()) } +} - fn get_var_value(&self, name: &str) -> Result { - // Check for positional parameters ($0, $1, $2, ... $9, etc.) - // Name is just the digits when called from `$1` or `${1}` parsing - if let Ok(index) = name.parse::() { - if let Some(pos_val) = self.scope.get_positional(index) { - if let Some(decimal) = leading_zero_decimal(pos_val) { - anyhow::bail!( - "${index} holds `{pos_val}`, which is text (leading zero) — kaish reads \ - no octal; write `{decimal}` for the decimal value" - ); - } - return pos_val.parse().with_context(|| { - format!("${} has non-numeric value: {:?}", index, pos_val) - }); - } - return Ok(0); // Unset positional defaults to 0 - } - - // Regular variable lookup - match self.scope.get(name).cloned() { - Some(value) => self.value_to_arith(&value, name), - None => Ok(0), // Unset variables default to 0 in arithmetic - } - } - - /// Resolve a subscripted variable path (`${p[port]}`) and coerce to an - /// integer. Reuses the real path resolver, so scalar unwrap and the loud - /// path errors are identical to `${p[port]}` outside arithmetic. - fn eval_braced_path(&mut self, root: &str) -> Result { - let mut brackets = String::new(); - while self.peek() == Some('[') { - brackets.push('['); - self.advance(); // consume '[' - let mut depth = 1; - while depth > 0 { - match self.advance() { - Some('[') => { - depth += 1; - brackets.push('['); - } - Some(']') => { - depth -= 1; - brackets.push(']'); - } - Some(c) => brackets.push(c), - None => bail!("unterminated subscript in arithmetic"), +fn unset_error(name: &str) -> ArithError { + let message = match name { + "RANDOM" => "`$RANDOM` has no value in kaish; write `$(random --max 100)`".to_string(), + "SECONDS" => { + "`$SECONDS` has no value in kaish; write `start=$(date +%s)` and `$(( $(date +%s) - start ))`" + .to_string() + } + _ => format!("`{name}` is unset; set it before `$(( ))` or write `${{{name}:-0}}`"), + }; + ArithError::new(message, 0..0) +} + +fn resolve_var_sync(scope: &Scope, name: &str) -> Result { + // `$1`, `$2`, … reach the same variable slot bash gives them: text, + // coerced by the same rules as any other string operand. + if let Ok(index) = name.parse::() { + return match scope.get_positional(index) { + Some(s) => parse_numeric_string(s, name), + None => Err(unset_error(name)), + }; + } + match scope.get(name) { + Some(value) => value_to_arith(value, name), + None => Err(unset_error(name)), + } +} + +fn braced_path_value(scope: &Scope, root: &str, brackets: &str) -> Result { + let raw = format!("${{{root}{brackets}}}"); + let path: VarPath = crate::parser::parse_varpath(&raw); + scope.resolve_path(&path).map_err(|e| match e { + crate::interpreter::PathError::UndefinedRoot(_) => unset_error(root), + crate::interpreter::PathError::Absence(msg) | crate::interpreter::PathError::Shape(msg) => { + ArithError::new(msg, 0..0) + } + }) +} + +fn subscript_path(root: &str, indices: &[i64]) -> VarPath { + let mut raw = format!("${{{root}"); + for idx in indices { + raw.push('['); + raw.push_str(&idx.to_string()); + raw.push(']'); + } + raw.push('}'); + crate::parser::parse_varpath(&raw) +} + +fn resolve_subscript_sync(scope: &Scope, root: &str, indices: &[i64]) -> Result { + let path = subscript_path(root, indices); + let value = scope.resolve_path(&path).map_err(|e| match e { + crate::interpreter::PathError::UndefinedRoot(_) => unset_error(root), + crate::interpreter::PathError::Absence(msg) | crate::interpreter::PathError::Shape(msg) => { + ArithError::new(msg, 0..0) + } + })?; + value_to_arith(&value, root) +} + +fn based_value(base: u32, text: &str, name: &str) -> Result { + let trimmed = text.trim(); + let (neg, digits) = match trimmed.strip_prefix('-') { + Some(rest) => (true, rest), + None => (false, trimmed.strip_prefix('+').unwrap_or(trimmed)), + }; + if digits.is_empty() { + return Err(ArithError::new(format!("`{name}` printed `{text}`; the command must print one integer"), 0..0)); + } + let mut mag: u64 = 0; + for c in digits.chars() { + if !c.is_ascii_alphanumeric() { + return Err(ArithError::new(format!("`{name}` printed `{text}`, which is not a number"), 0..0)); + } + let digit_val = match c { + '0'..='9' => c as u32 - '0' as u32, + 'a'..='z' => c as u32 - 'a' as u32 + 10, + 'A'..='Z' => c as u32 - 'A' as u32 + 10, + _ => unreachable!(), + }; + if digit_val >= base { + return Err(ArithError::new(format!("`{name}` printed `{text}`, which is not a number"), 0..0)); + } + mag = mag + .checked_mul(base as u64) + .and_then(|m| m.checked_add(digit_val as u64)) + .ok_or_else(|| ArithError::new(format!("`{text}` {INTEGER_OUT_OF_RANGE}"), 0..0))?; + } + match int_from_magnitude(mag, neg, 0..0)? { + ArithExpr::Int(n) => Ok(n), + _ => unreachable!(), + } +} + +// ═══════════════════════════════════════════════════════════════════ +// Sync evaluator — used where no `$(...)` is reachable. Hits a +// `CommandSubst` leaf only if the walk actually reaches one; the caller +// is expected not to call this when `contains_command_subst()` is true. +// ═══════════════════════════════════════════════════════════════════ + +fn needs_async(what: &str) -> ArithError { + ArithError::new(format!("`{what}` needs the async evaluator"), 0..0) +} + +fn resolve_expansion_sync(e: &Expansion, scope: &Scope) -> Result { + match e { + Expansion::Var(name) => resolve_var_sync(scope, name), + Expansion::BracedPath { root, brackets } => { + let v = braced_path_value(scope, root, brackets)?; + value_to_arith(&v, root) + } + Expansion::BracedDefault { root, brackets, default } => { + let resolved = if brackets.is_empty() { scope.get(root).cloned() } else { + braced_path_value(scope, root, brackets).ok() + }; + match resolved { + Some(Value::Null) | None => { + let default_expr = parse(default)?; + eval_sync(&default_expr, scope) } + Some(v) => value_to_arith(&v, root), } } - if self.peek() != Some('}') { - bail!("expected '}}' after subscripted variable in arithmetic"); - } - self.advance(); // consume '}' - - let raw = format!("${{{root}{brackets}}}"); - let path = crate::parser::parse_varpath(&raw); - let value = self.scope.resolve_path(&path).map_err(|e| match e { - crate::interpreter::PathError::UndefinedRoot(_) => { - anyhow::anyhow!("undefined variable in arithmetic: {root}") - } - crate::interpreter::PathError::Absence(msg) - | crate::interpreter::PathError::Shape(msg) => anyhow::anyhow!(msg), - })?; - self.value_to_arith(&value, root) - } - - /// Resolve a BARE subscripted path in arithmetic (`xs[i]`, `xs[0]`, - /// `xs[i+1]`, `xs[-1]`) and coerce to an integer. - /// - /// Decision B: inside `$(( … ))` a bracket's contents are a numeric - /// expression, so a bareword subscript is the VARIABLE `i` (evaluated here), - /// NOT the literal key — the exact opposite of the interpolation form - /// `${xs[i]}`, which stays a literal key via `eval_braced_path`. Each - /// subscript is evaluated as a nested arithmetic expression to an integer - /// index; chained subscripts (`grid[i][j]`) walk left to right. - fn eval_bare_subscript_path(&mut self, root: &str) -> Result { - let mut segments = vec![VarSegment::Field(root.to_string())]; - while self.peek() == Some('[') { - self.advance(); // consume '[' - let index = self.parse_comparison()?; // the inner is a numeric expr - self.skip_whitespace(); - if self.peek() != Some(']') { - bail!("expected ']' to close subscript in arithmetic"); - } - self.advance(); // consume ']' - segments.push(VarSegment::Index(index)); - } - let path = VarPath { segments }; - let value = self.scope.resolve_path(&path).map_err(|e| match e { - crate::interpreter::PathError::UndefinedRoot(_) => { - anyhow::anyhow!("undefined variable in arithmetic: {root}") - } - crate::interpreter::PathError::Absence(msg) - | crate::interpreter::PathError::Shape(msg) => anyhow::anyhow!(msg), - })?; - self.value_to_arith(&value, root) - } - - /// Coerce a resolved value to an integer for arithmetic. - fn value_to_arith(&self, value: &Value, name: &str) -> Result { - match value { - Value::Int(n) => Ok(*n), - Value::String(s) => { - // `x=007` stores the string, and reading it back is a number - // position like any other. - if let Some(decimal) = leading_zero_decimal(s) { - anyhow::bail!( - "variable '{name}' holds `{s}`, which is text (leading zero) — kaish \ - reads no octal; write `{decimal}` for the decimal value" - ); - } - s.parse().with_context(|| format!( - "variable '{}' has non-numeric value: {:?}", name, s - )) + Expansion::LastExitCode => Ok(scope.last_result().code), + Expansion::CurrentPid => Ok(scope.pid() as i64), + Expansion::CommandSubst(_) => Err(needs_async("$(...)")), + Expansion::Nested(inner) => eval_sync(inner, scope), + } +} + +fn resolve_based_sync(base: u32, e: &Expansion, scope: &Scope) -> Result { + match e { + Expansion::CommandSubst(_) => Err(needs_async("$(...)")), + _ => { + let n = resolve_expansion_sync(e, scope)?; + based_value(base, &n.to_string(), "expansion") + } + } +} + +pub(crate) fn eval_sync(expr: &ArithExpr, scope: &Scope) -> Result { + match expr { + ArithExpr::Int(n) => Ok(*n), + ArithExpr::Expansion(e) => resolve_expansion_sync(e, scope), + ArithExpr::Subscript { root, indices } => { + let mut idx_vals = Vec::with_capacity(indices.len()); + for idx in indices { + idx_vals.push(eval_sync(idx, scope)?); + } + resolve_subscript_sync(scope, root, &idx_vals) + } + ArithExpr::BasedExpansion { base, expansion } => resolve_based_sync(*base, expansion, scope), + ArithExpr::Unary { op, operand } => apply_unary(*op, eval_sync(operand, scope)?), + ArithExpr::Binary { op: BinOp::And, left, right } => { + let l = eval_sync(left, scope)?; + if !truthy(l) { Ok(0) } else { Ok(if truthy(eval_sync(right, scope)?) { 1 } else { 0 }) } + } + ArithExpr::Binary { op: BinOp::Or, left, right } => { + let l = eval_sync(left, scope)?; + if truthy(l) { Ok(1) } else { Ok(if truthy(eval_sync(right, scope)?) { 1 } else { 0 }) } + } + ArithExpr::Binary { op, left, right } => { + apply_binary(*op, eval_sync(left, scope)?, eval_sync(right, scope)?) + } + ArithExpr::Ternary { cond, then_branch, else_branch } => { + if truthy(eval_sync(cond, scope)?) { + eval_sync(then_branch, scope) + } else { + eval_sync(else_branch, scope) } - Value::Float(f) => Ok(*f as i64), - Value::Bool(b) => Ok(if *b { 1 } else { 0 }), - Value::Null => Ok(0), // null coerces to 0 in arithmetic - Value::Json(_) => anyhow::bail!("variable '{}' is JSON, not a number", name), - Value::Bytes(_) => anyhow::bail!("variable '{}' is binary data, not a number", name), } } } +/// Tokenize, parse, and evaluate `text` (the content of `$(( ))`) with no +/// `$(...)` support — the fast path used where an async evaluator isn't +/// available. A reachable `$(...)` errors loudly rather than silently +/// resolving to nothing. +pub fn eval_arithmetic(text: &str, scope: &Scope) -> Result { + let expr = parse(text)?; + eval_sync(&expr, scope) +} + #[cfg(test)] mod tests { use super::*; fn eval(expr: &str) -> i64 { let scope = Scope::new(); - eval_arithmetic(expr, &scope).expect("eval should succeed") + eval_arithmetic(expr, &scope).unwrap_or_else(|e| panic!("eval {expr:?} failed: {e}")) + } + + fn err(expr: &str) -> String { + let scope = Scope::new(); + eval_arithmetic(expr, &scope).expect_err("expected an error").message } - fn eval_with_var(expr: &str, name: &str, value: i64) -> i64 { + fn eval_with(expr: &str, setup: impl FnOnce(&mut Scope)) -> i64 { let mut scope = Scope::new(); - scope.set(name, Value::Int(value)); - eval_arithmetic(expr, &scope).expect("eval should succeed") + setup(&mut scope); + eval_arithmetic(expr, &scope).unwrap_or_else(|e| panic!("eval {expr:?} failed: {e}")) } + fn err_with(expr: &str, setup: impl FnOnce(&mut Scope)) -> String { + let mut scope = Scope::new(); + setup(&mut scope); + eval_arithmetic(expr, &scope).expect_err("expected an error").message + } + + // ── literals & bases ── #[test] - fn test_simple_integers() { + fn decimal() { assert_eq!(eval("42"), 42); assert_eq!(eval("0"), 0); - assert_eq!(eval("12345"), 12345); } - // ── Decision B: a bare subscript in arithmetic is a numeric expression ── - // `$(( xs[i] ))` reads variable `i` (the opposite of `${xs[i]}`, a literal - // key). Each bracket's inner is evaluated arithmetically to an index. + #[test] + fn hex() { + assert_eq!(eval("0xff"), 255); + assert_eq!(eval("0XFF"), 255); + } - fn eval_with_scope(expr: &str, setup: impl FnOnce(&mut Scope)) -> Result { - let mut scope = Scope::new(); - setup(&mut scope); - eval_arithmetic(expr, &scope) + #[test] + fn based() { + assert_eq!(eval("16#ff"), 255); + assert_eq!(eval("8#17"), 15); + assert_eq!(eval("2#1011"), 11); + assert_eq!(eval("36#z"), 35); } #[test] - fn bare_subscript_index_is_a_variable() { - // xs = [10, 20, 30]; i = 1 → xs[i] == 20 - let r = eval_with_scope("xs[i]", |s| { - s.set("xs", Value::Json(serde_json::json!([10, 20, 30]))); - s.set("i", Value::Int(1)); - }) - .expect("bare xs[i] should resolve via variable i"); - assert_eq!(r, 20); + fn negative_hex_and_based() { + assert_eq!(eval("-0xff"), -255); + assert_eq!(eval("- 16#ff"), -255); } #[test] - fn bare_subscript_literal_index() { - let r = eval_with_scope("xs[0] + 1", |s| { - s.set("xs", Value::Json(serde_json::json!([10, 20, 30]))); - }) - .expect("xs[0] + 1"); - assert_eq!(r, 11); + fn sign_after_hash_is_an_error() { + let msg = err("16#-ff"); + assert!(msg.contains("puts") && msg.contains('#'), "{msg}"); } #[test] - fn bare_subscript_inner_is_an_expression() { - // xs[i + 1] with i = 0 → xs[1] == 20 - let r = eval_with_scope("xs[i + 1]", |s| { - s.set("xs", Value::Json(serde_json::json!([10, 20, 30]))); - s.set("i", Value::Int(0)); - }) - .expect("xs[i + 1]"); - assert_eq!(r, 20); + fn leading_zero_is_an_error() { + let msg = err("010"); + assert!(msg.contains("leading zero"), "{msg}"); + assert!(msg.contains("8#10"), "{msg}"); + assert!(msg.contains('9') || msg.contains("10"), "{msg}"); } #[test] - fn bare_subscript_negative_index() { - let r = eval_with_scope("xs[-1]", |s| { - s.set("xs", Value::Json(serde_json::json!([10, 20, 30]))); - }) - .expect("xs[-1]"); - assert_eq!(r, 30); + fn zero_alone_is_fine() { + assert_eq!(eval("0 + 1"), 1); } #[test] - fn bare_subscript_out_of_bounds_is_loud() { - let r = eval_with_scope("xs[9]", |s| { - s.set("xs", Value::Json(serde_json::json!([10, 20]))); - }); - assert!(r.is_err(), "out-of-bounds index must be a loud error"); + fn zero_b_and_zero_o_are_not_kaish_spellings() { + let msg = err("0b101"); + assert!(msg.contains("2#101"), "{msg}"); + let msg = err("0o17"); + assert!(msg.contains("8#17"), "{msg}"); } #[test] - fn test_addition() { - assert_eq!(eval("1 + 2"), 3); - assert_eq!(eval("10 + 20 + 30"), 60); + fn base_out_of_range() { + let msg = err("1#5"); + assert!(msg.contains("outside 2..=36"), "{msg}"); + let msg = err("37#5"); + assert!(msg.contains("outside 2..=36"), "{msg}"); } #[test] - fn test_subtraction() { - assert_eq!(eval("10 - 3"), 7); - assert_eq!(eval("100 - 50 - 25"), 25); + fn bad_digit_for_base() { + let msg = err("2#5"); + assert!(msg.contains("not a digit"), "{msg}"); } #[test] - fn test_multiplication() { - assert_eq!(eval("3 * 4"), 12); - assert_eq!(eval("2 * 3 * 4"), 24); + fn no_digits_after_prefix() { + let msg = err("0x"); + assert!(msg.contains("no digits"), "{msg}"); + let msg = err("16#"); + assert!(msg.contains("no digits"), "{msg}"); } #[test] - fn test_division() { - assert_eq!(eval("10 / 2"), 5); - assert_eq!(eval("100 / 10 / 2"), 5); + fn underscore_in_literal() { + let msg = err("1_000"); + assert!(msg.contains('_'), "{msg}"); } #[test] - fn test_modulo() { - assert_eq!(eval("10 % 3"), 1); - assert_eq!(eval("17 % 5"), 2); + fn out_of_range_literal() { + let msg = err("9223372036854775808 + 1"); + assert!(msg.contains("does not fit"), "{msg}"); } #[test] - fn test_precedence() { - assert_eq!(eval("2 + 3 * 4"), 14); // Not 20 - assert_eq!(eval("10 - 6 / 2"), 7); // Not 2 + fn min_literal_only_as_direct_unary_operand() { + assert_eq!(eval("-9223372036854775808"), i64::MIN); + let msg = err("9223372036854775808"); + assert!(msg.contains("does not fit"), "{msg}"); + let msg = err("- -9223372036854775808"); + assert!(msg.contains("does not fit"), "{msg}"); } + // ── operators & precedence ── #[test] - fn test_parentheses() { - assert_eq!(eval("(2 + 3) * 4"), 20); - assert_eq!(eval("((1 + 2) * (3 + 4))"), 21); + fn basic_ops() { + assert_eq!(eval("5 + 3 * 2"), 11); + assert_eq!(eval("10 / 3"), 3); + assert_eq!(eval("-7 % 3"), -1); + assert_eq!(eval("2 ** 10"), 1024); } #[test] - fn test_unary_minus() { - assert_eq!(eval("-5"), -5); - assert_eq!(eval("10 + -3"), 7); - assert_eq!(eval("--5"), 5); + fn precedence_examples() { + assert_eq!(eval("1 << 2 + 1"), 8); + assert_eq!(eval("5 & 3 == 3"), 1); + assert_eq!(eval("2 ** 3 ** 2"), 512); + assert_eq!(eval("-2 ** 2"), 4); + assert_eq!(eval("1 ? 2 : 3 ? 4 : 5"), 2); } #[test] - fn test_unary_plus() { - assert_eq!(eval("+5"), 5); - assert_eq!(eval("++5"), 5); + fn comparisons_return_one_or_zero() { + assert_eq!(eval("5 > 3"), 1); + assert_eq!(eval("3 > 5"), 0); } #[test] - fn test_whitespace() { - assert_eq!(eval(" 1 + 2 "), 3); - assert_eq!(eval("1+2"), 3); + fn bitwise() { + assert_eq!(eval("6 & 3"), 2); + assert_eq!(eval("6 | 1"), 7); + assert_eq!(eval("6 ^ 3"), 5); + assert_eq!(eval("~0"), -1); } #[test] - fn test_variable_dollar() { - assert_eq!(eval_with_var("$X", "X", 10), 10); - assert_eq!(eval_with_var("$X + 5", "X", 10), 15); + fn shifts() { + assert_eq!(eval("1 << 4"), 16); + assert_eq!(eval("-8 >> 1"), -4); } #[test] - fn test_variable_dollar_braces() { - assert_eq!(eval_with_var("${X}", "X", 10), 10); - assert_eq!(eval_with_var("${X} * 2", "X", 10), 20); + fn shift_count_out_of_range() { + let msg = err("1 << 64"); + assert!(msg.contains("outside 0..=63"), "{msg}"); + let msg = err("1 << -1"); + assert!(msg.contains("outside 0..=63"), "{msg}"); } #[test] - fn test_variable_bare() { - assert_eq!(eval_with_var("X", "X", 10), 10); - assert_eq!(eval_with_var("X + Y", "X", 10), 10); // Y is unset = 0 + fn short_circuit_and_or() { + assert_eq!(eval("0 && 1"), 0); + assert_eq!(eval("1 && 1"), 1); + assert_eq!(eval("1 || 0"), 1); + assert_eq!(eval("0 || 0"), 0); } #[test] - fn test_unset_variable() { - let scope = Scope::new(); - let result = eval_arithmetic("UNDEFINED", &scope).expect("should succeed"); - assert_eq!(result, 0); // Unset variables default to 0 + fn ternary_selects_unnormalized_value() { + assert_eq!(eval("1 ? 42 : 7"), 42); + assert_eq!(eval("0 ? 42 : 7"), 7); } + // ── overflow ── #[test] - fn test_division_by_zero() { - let scope = Scope::new(); - let result = eval_arithmetic("10 / 0", &scope); - assert!(result.is_err()); + fn overflow_each_op() { + assert!(err("9223372036854775807 + 1").contains("does not fit")); + assert!(err("-9223372036854775808 - 1").contains("does not fit")); + assert!(err("9223372036854775807 * 2").contains("does not fit")); + assert!(err("-9223372036854775808 / -1").contains("does not fit")); + assert!(err("2 ** 63").contains("does not fit")); + assert!(err("1 << 63").contains("does not fit")); } #[test] - fn test_modulo_by_zero() { - let scope = Scope::new(); - let result = eval_arithmetic("10 % 0", &scope); - assert!(result.is_err()); + fn division_and_modulo_by_zero() { + assert!(err("10 / 0").contains("divides by zero")); + assert!(err("10 % 0").contains("divides by zero")); } #[test] - fn test_complex_expression() { - assert_eq!(eval("(1 + 2) * (3 + 4) - 5"), 16); + fn division_truncates_toward_zero() { + assert_eq!(eval("7 / 2"), 3); + assert_eq!(eval("-7 / 2"), -3); } - // Comparison operator tests #[test] - fn test_greater_than() { - assert_eq!(eval("5 > 3"), 1); - assert_eq!(eval("3 > 5"), 0); - assert_eq!(eval("5 > 5"), 0); + fn negative_exponent() { + assert!(err("2 ** -1").contains("negative")); } + // ── variables ── #[test] - fn test_less_than() { - assert_eq!(eval("3 < 5"), 1); - assert_eq!(eval("5 < 3"), 0); - assert_eq!(eval("5 < 5"), 0); + fn bare_and_dollar_variable() { + assert_eq!(eval_with("count + 1", |s| s.set("count", Value::Int(4))), 5); + assert_eq!(eval_with("$count + 1", |s| s.set("count", Value::Int(4))), 5); } #[test] - fn test_greater_or_equal() { - assert_eq!(eval("5 >= 3"), 1); - assert_eq!(eval("5 >= 5"), 1); - assert_eq!(eval("3 >= 5"), 0); + fn unset_variable_is_an_error() { + let msg = err("missing + 1"); + assert!(msg.contains("unset"), "{msg}"); + assert!(msg.contains(":-0"), "{msg}"); } #[test] - fn test_less_or_equal() { - assert_eq!(eval("3 <= 5"), 1); - assert_eq!(eval("5 <= 5"), 1); - assert_eq!(eval("5 <= 3"), 0); + fn random_and_seconds_name_their_fix() { + let msg = err("RANDOM % 10"); + assert!(msg.contains("random --max"), "{msg}"); + let msg = err("SECONDS"); + assert!(msg.contains("date +%s"), "{msg}"); } #[test] - fn test_equal() { - assert_eq!(eval("5 == 5"), 1); - assert_eq!(eval("5 == 3"), 0); + fn null_variable_is_an_error() { + let msg = err_with("x", |s| s.set("x", Value::Null)); + assert!(msg.contains("null"), "{msg}"); } #[test] - fn test_not_equal() { - assert_eq!(eval("5 != 3"), 1); - assert_eq!(eval("5 != 5"), 0); + fn float_variable_errors() { + let msg = err_with("x", |s| s.set("x", Value::Float(2.7))); + assert!(msg.contains("integer-only"), "{msg}"); } #[test] - fn test_comparison_with_arithmetic() { - assert_eq!(eval("(2 + 3) > 4"), 1); - assert_eq!(eval("10 / 2 == 5"), 1); - assert_eq!(eval("3 * 4 >= 12"), 1); - assert_eq!(eval("10 - 5 < 6"), 1); + fn integral_float_coerces() { + assert_eq!(eval_with("x + 1", |s| s.set("x", Value::Float(100.0))), 101); } #[test] - fn test_comparison_with_variables() { - assert_eq!(eval_with_var("X > 5", "X", 10), 1); - assert_eq!(eval_with_var("X == 10", "X", 10), 1); - assert_eq!(eval_with_var("X <= 10", "X", 10), 1); + fn string_value_is_parsed() { + assert_eq!(eval_with("x", |s| s.set("x", Value::String("0xff".to_string()))), 255); + assert_eq!(eval_with("mask & 16#0f", |s| s.set("mask", Value::String("0xff".to_string()))), 15); } #[test] - fn test_chained_comparison() { - // Note: chained comparisons work left-to-right, not mathematically - // (5 > 3) > 2 = 1 > 2 = 0 - assert_eq!(eval("5 > 3 > 2"), 0); - // (5 > 3) == 1 = 1 == 1 = 1 - assert_eq!(eval("5 > 3 == 1"), 1); + fn string_with_leading_zero_names_the_fix() { + let msg = err_with("x", |s| s.set("x", Value::String("08".to_string()))); + assert!(msg.contains("10#$x") || msg.contains("leading zero"), "{msg}"); + } + + #[test] + fn string_expression_names_the_fix() { + let msg = err_with("x", |s| s.set("x", Value::String("1 + 2".to_string()))); + assert!(msg.contains("not an expression"), "{msg}"); + } + + #[test] + fn string_non_numeric_is_an_error() { + let msg = err_with("x", |s| s.set("x", Value::String("abc".to_string()))); + assert!(msg.contains("not a number"), "{msg}"); + } + + #[test] + fn list_and_record_error() { + let msg = err_with("x", |s| s.set("x", Value::Json(serde_json::json!([1, 2])))); + assert!(msg.contains("list"), "{msg}"); + let msg = err_with("x", |s| s.set("x", Value::Json(serde_json::json!({"a": 1})))); + assert!(msg.contains("record"), "{msg}"); + } + + #[test] + fn last_exit_code_and_pid() { + let mut scope = Scope::new(); + scope.set_last_result(crate::interpreter::ExecResult::success("x").with_code(3)); + assert_eq!(eval_arithmetic("$?", &scope).unwrap(), 3); + assert_eq!(eval_arithmetic("$$", &scope).unwrap(), scope.pid() as i64); + } + + // ── subscripts (Decision B: bare `[...]` is an expression) ── + #[test] + fn bare_subscript_is_a_variable_expression() { + let r = eval_with("xs[i]", |s| { + s.set("xs", Value::Json(serde_json::json!([10, 20, 30]))); + s.set("i", Value::Int(1)); + }); + assert_eq!(r, 20); + } + + #[test] + fn bare_subscript_literal_and_expression_index() { + let r = eval_with("xs[0] + 1", |s| s.set("xs", Value::Json(serde_json::json!([10, 20, 30])))); + assert_eq!(r, 11); + let r = eval_with("xs[i + 1]", |s| { + s.set("xs", Value::Json(serde_json::json!([10, 20, 30]))); + s.set("i", Value::Int(0)); + }); + assert_eq!(r, 20); + } + + #[test] + fn braced_path_reads_a_literal_key() { + let r = eval_with("${c[port]}", |s| s.set("c", Value::Json(serde_json::json!({"port": 8080})))); + assert_eq!(r, 8080); + } + + // ── default expansion ── + #[test] + fn default_used_when_unset() { + assert_eq!(eval("${limit:-0} + 1"), 1); + } + + #[test] + fn default_not_used_when_set() { + assert_eq!(eval_with("${limit:-0} + 1", |s| s.set("limit", Value::Int(9))), 10); + } + + // ── nested arithmetic ── + #[test] + fn nested_arithmetic() { + assert_eq!(eval("$(( 1 + 2 )) * 4"), 12); + } + + #[test] + fn newline_inside_is_whitespace() { + assert_eq!(eval("1 +\n2"), 3); + } + + // ── structural errors ── + #[test] + fn empty_is_an_error() { + let msg = err(""); + assert!(msg.contains("no expression"), "{msg}"); + } + + #[test] + fn empty_group_is_an_error() { + let msg = err("()"); + assert!(msg.contains("no expression"), "{msg}"); + } + + #[test] + fn missing_close_paren() { + let msg = err("(1 + 2"); + assert!(msg.contains("closing"), "{msg}"); + } + + #[test] + fn extra_close_paren() { + let msg = err("1 + 2)"); + assert!(msg.contains("matching"), "{msg}"); + } + + #[test] + fn ternary_without_colon() { + let msg = err("1 ? 2"); + assert!(msg.contains(':'), "{msg}"); + } + + #[test] + fn not_operators_are_diagnosed() { + assert!(err("1 <<< 2").contains("here-string")); + assert!(err("1 >>> 2").contains(">>")); + assert!(err("x = 5").contains("assigns")); + assert!(err("x += 1").contains("assigns")); + assert!(err("x++").contains("assigns")); + assert!(err("1, 2").contains("one expression")); + } + + #[test] + fn depth_cap() { + let mut src = String::new(); + for _ in 0..300 { + src.push('('); + } + src.push('1'); + for _ in 0..300 { + src.push(')'); + } + let msg = err(&src); + assert!(msg.contains("256"), "{msg}"); } } diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index 82ea1a75..9d94a2d4 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -3445,7 +3445,7 @@ impl CmdSubstFrames { /// Plain-slice twin of [`cmd_subst_body_tokens`]'s live chumsky capture, used /// only by `validate_cmd_subst_bodies`'s error-path fallback — see that /// function's doc comment for why a second, non-chumsky scan exists at all. -fn find_cmd_subst_close(tokens: &[(Token, Span)]) -> Option { +pub(crate) fn find_cmd_subst_close(tokens: &[(Token, Span)]) -> Option { let mut tracker = CmdSubstFrames::default(); (0..tokens.len()).find(|&i| { let next = tokens.get(i + 1).map(|(t, _)| t); diff --git a/crates/kaish-kernel/tests/correctness_oneoffs_tests.rs b/crates/kaish-kernel/tests/correctness_oneoffs_tests.rs index f304e53d..c6142f9b 100644 --- a/crates/kaish-kernel/tests/correctness_oneoffs_tests.rs +++ b/crates/kaish-kernel/tests/correctness_oneoffs_tests.rs @@ -200,7 +200,7 @@ async fn interpolated_arithmetic_division_by_zero_is_loud() { "division by zero inside a string must fail loudly, not silently splice in \"\"" ); assert!( - result.err.contains("division by zero"), + result.err.contains("divides by zero"), "got: {}", result.err ); diff --git a/crates/kaish-kernel/tests/heredoc_fragment_tests.rs b/crates/kaish-kernel/tests/heredoc_fragment_tests.rs index 17dae1aa..ea27a351 100644 --- a/crates/kaish-kernel/tests/heredoc_fragment_tests.rs +++ b/crates/kaish-kernel/tests/heredoc_fragment_tests.rs @@ -542,20 +542,23 @@ fn spaced_session_state_inside_arithmetic_still_refuses() { } /// The control for the test above, and the one that makes it mean something: -/// a *spaced* ordinary variable must still expand. `$(( $ COUNT + 1 ))` reads -/// COUNT and prints 42 when executed (verified against the binary), so a -/// scanner that treated the space itself as unnameable would refuse a body -/// that expands correctly — and the refusal test alone cannot tell the two -/// apart, because refusing everything passes it. -#[test] -fn a_spaced_ordinary_variable_inside_arithmetic_still_expands() { - assert_eq!( - expand( - "python3 < Date: Thu, 27 Aug 2026 09:51:40 -0400 Subject: [PATCH 03/15] arithmetic: run $(...) through the async evaluator, and (( )) as a command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit $(( )) has always been sync-only: eval_arithmetic() takes a &Scope and returns i64 straight through, so a $(...) operand had nowhere to run a command from. The kernel's async evaluator (kernel.rs) already owns command execution, so this adds a boxed recursive async walk over the parsed ArithExpr tree — eval_arith_expr_async and eval_arith_expansion_async — that mirrors eval_expr_async's own Pin> recursion pattern for the same reason: the recursion is unbounded by the type system, so it can't live in a fixed-depth stack frame. Laziness (a $(...) on the unselected side of &&/||/?: must not run) falls out of the walk's own control flow rather than needing separate machinery: the short-circuit arms simply never call eval_arith_expr_async on the branch they don't take, so its $(...) never executes. Each leaf takes its own short scope read lock instead of one held across the whole walk, because Expansion::CommandSubst runs through execute_block_capturing, which takes its own scope lock internally — a lock held here across that await would deadlock. A tree with no $(...) anywhere (ArithExpr::contains_command_subst()) still takes the sync fast path under one lock. Building this surfaced a real bug in the based-expansion path (base#, e.g. 10#$m): resolve_based_sync converted the expansion through the FULL arithmetic coercion first and then stringified the result, so a leading-zero variable (m="08") hit the leading-zero refusal before 10#$m ever got a chance to read it as decimal — defeating the one thing that form exists to fix. Rewritten as expansion_text_sync/eval_arith_expansion_text_async: the expansion's rendered VALUE, never re-coerced through the numeral rules. Also found while writing the integration tests: the tokenizer had no float/exponent detection at all, so $((1.5)) and $((1e3)) failed on the '.' or 'e' as "cannot start a value" instead of the documented "arithmetic is integer-only" — consume_digits (used for hex/based digit bodies, where an invalid letter IS an error) was also being used for the plain decimal run, where a trailing e/x/b/o needs to be a clean stop, not an error. Split into consume_decimal_digits for that case and added the float-shape check ahead of it. Bare (( expr )) as a command/condition is the sibling of [[ ]]: a new Stmt::Arith/Expr::Arith AST pair, parsed from a new ArithCond token the lexer's existing scanner produces the same way it already produces Arithmetic for $((( — extract_arithmetic now takes a marker_len (2 for bare (( vs 3 for $(() and an is_condition flag that ScanOutput.arithmetics carries through to marker resolution. Exit 0 when nonzero, 1 when zero; an evaluation fault (division by zero, an unset variable) is exit 2 with the arithmetic error as the message rather than aborting the statement list — a deliberate divergence from [[ ]]'s condition errors, which do abort, because a bare (( )) command has an exit-code channel a condition doesn't: in if/while position an evaluation fault still propagates hard, same as [[ ]] already does there. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 135 +++++- crates/kaish-kernel/src/ast/plan.rs | 7 + crates/kaish-kernel/src/ast/sexpr.rs | 2 + crates/kaish-kernel/src/ast/types.rs | 7 + crates/kaish-kernel/src/interpreter/eval.rs | 11 + crates/kaish-kernel/src/kernel.rs | 309 ++++++++++++- crates/kaish-kernel/src/lexer.rs | 77 +++- crates/kaish-kernel/src/parser.rs | 25 +- crates/kaish-kernel/src/validator/walker.rs | 5 +- crates/kaish-kernel/tests/arithmetic_tests.rs | 431 ++++++++++++++++++ 10 files changed, 966 insertions(+), 43 deletions(-) create mode 100644 crates/kaish-kernel/tests/arithmetic_tests.rs diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index e535b0c5..1dba411f 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -466,6 +466,37 @@ impl<'a> Tokenizer<'a> { Ok((mag, digits_start)) } + /// Consume a run of plain `0`-`9` digits, erroring loud on `_`. Unlike + /// [`Self::consume_digits`], a non-digit letter (`e`, `x`, …) is a clean + /// stop, not an error — the base-10 run is used both as a full decimal + /// literal and as the base number before `#`, and the caller decides + /// what a trailing `e3`/`.5`/`#` means. + fn consume_decimal_digits(&mut self, lit_start: usize) -> Result<(u64, usize), ArithError> { + let digits_start = self.pos; + let mut mag: u64 = 0; + loop { + let Some(c) = self.peek() else { break }; + if c == '_' { + return Err(ArithError::new( + format!("`{}` contains `_`; remove it", self.slice(lit_start, self.pos + 1)), + lit_start..self.byte_pos() + c.len_utf8(), + )); + } + if !c.is_ascii_digit() { + break; + } + let digit_val = c as u64 - '0' as u64; + mag = mag.checked_mul(10).and_then(|m| m.checked_add(digit_val)).ok_or_else(|| { + ArithError::new( + format!("`{}` {INTEGER_OUT_OF_RANGE}", self.slice(lit_start, self.pos + 1)), + lit_start..self.byte_pos(), + ) + })?; + self.pos += 1; + } + Ok((mag, digits_start)) + } + fn lex_number(&mut self) -> Result { let start = self.pos; @@ -502,9 +533,41 @@ impl<'a> Tokenizer<'a> { // Plain decimal run — either a bare decimal literal, or the base // number before `#`. - let (base_mag, digits_start) = self.consume_digits(10, start)?; + let (base_mag, digits_start) = self.consume_decimal_digits(start)?; debug_assert!(digits_start == start); + // Float/exponent shape (`1.5`, `1e3`, `1E-3`): not a kaish spelling + // — checked before `#` and leading-zero, since a numeral can't be + // both a based prefix and a float. + let looks_like_float = (self.peek() == Some('.') + && matches!(self.peek_at(1), Some(c) if c.is_ascii_digit())) + || (matches!(self.peek(), Some('e' | 'E')) + && (matches!(self.peek_at(1), Some(c) if c.is_ascii_digit()) + || (matches!(self.peek_at(1), Some('+' | '-')) + && matches!(self.peek_at(2), Some(c) if c.is_ascii_digit())))); + if looks_like_float { + if self.peek() == Some('.') { + self.pos += 1; + while matches!(self.peek(), Some(c) if c.is_ascii_digit()) { + self.pos += 1; + } + } + if matches!(self.peek(), Some('e' | 'E')) { + self.pos += 1; + if matches!(self.peek(), Some('+' | '-')) { + self.pos += 1; + } + while matches!(self.peek(), Some(c) if c.is_ascii_digit()) { + self.pos += 1; + } + } + let text = self.slice(start, self.pos); + return Err(ArithError::new( + format!("`{text}` is not an integer; arithmetic is integer-only"), + start..self.pos, + )); + } + if self.peek() == Some('#') { self.advance(); // consume '#' let base = base_mag as u32; @@ -1190,7 +1253,7 @@ fn parse_numeric_string(s: &str, name: &str) -> Result { /// Coerce a `$(...)` operand's printed text — the command must print exactly /// one integer. -fn parse_command_output(text: &str, cmd: &str) -> Result { +pub(crate) fn parse_command_output(text: &str, cmd: &str) -> Result { match read_numeral(text) { Numeral::Ok(n) => Ok(n), Numeral::Empty => Err(ArithError::new( @@ -1237,7 +1300,7 @@ pub(crate) fn value_to_arith(value: &Value, name: &str) -> Result ArithError { +pub(crate) fn unset_error(name: &str) -> ArithError { let message = match name { "RANDOM" => "`$RANDOM` has no value in kaish; write `$(random --max 100)`".to_string(), "SECONDS" => { @@ -1249,7 +1312,7 @@ fn unset_error(name: &str) -> ArithError { ArithError::new(message, 0..0) } -fn resolve_var_sync(scope: &Scope, name: &str) -> Result { +pub(crate) fn resolve_var_sync(scope: &Scope, name: &str) -> Result { // `$1`, `$2`, … reach the same variable slot bash gives them: text, // coerced by the same rules as any other string operand. if let Ok(index) = name.parse::() { @@ -1264,7 +1327,7 @@ fn resolve_var_sync(scope: &Scope, name: &str) -> Result { } } -fn braced_path_value(scope: &Scope, root: &str, brackets: &str) -> Result { +pub(crate) fn braced_path_value(scope: &Scope, root: &str, brackets: &str) -> Result { let raw = format!("${{{root}{brackets}}}"); let path: VarPath = crate::parser::parse_varpath(&raw); scope.resolve_path(&path).map_err(|e| match e { @@ -1286,7 +1349,7 @@ fn subscript_path(root: &str, indices: &[i64]) -> VarPath { crate::parser::parse_varpath(&raw) } -fn resolve_subscript_sync(scope: &Scope, root: &str, indices: &[i64]) -> Result { +pub(crate) fn resolve_subscript_sync(scope: &Scope, root: &str, indices: &[i64]) -> Result { let path = subscript_path(root, indices); let value = scope.resolve_path(&path).map_err(|e| match e { crate::interpreter::PathError::UndefinedRoot(_) => unset_error(root), @@ -1297,19 +1360,24 @@ fn resolve_subscript_sync(scope: &Scope, root: &str, indices: &[i64]) -> Result< value_to_arith(&value, root) } -fn based_value(base: u32, text: &str, name: &str) -> Result { +/// Read `text` as digits in `base` — the evaluation half of `base#` +/// (`2#$BITS`, `10#$(date +%m)`). `text` is the expansion's rendered VALUE, +/// never re-coerced through the normal numeral rules first: that coercion is +/// exactly what a leading-zero string (`m="08"`) needs `10#$m` to escape, so +/// routing through it here would defeat the form's only purpose. +pub(crate) fn based_value(base: u32, text: &str) -> Result { let trimmed = text.trim(); let (neg, digits) = match trimmed.strip_prefix('-') { Some(rest) => (true, rest), None => (false, trimmed.strip_prefix('+').unwrap_or(trimmed)), }; if digits.is_empty() { - return Err(ArithError::new(format!("`{name}` printed `{text}`; the command must print one integer"), 0..0)); + return Err(ArithError::new(format!("`{text}` has no digits"), 0..0)); } let mut mag: u64 = 0; for c in digits.chars() { if !c.is_ascii_alphanumeric() { - return Err(ArithError::new(format!("`{name}` printed `{text}`, which is not a number"), 0..0)); + return Err(ArithError::new(format!("`{c}` is not a digit in `{text}`; use digits valid for base {base}"), 0..0)); } let digit_val = match c { '0'..='9' => c as u32 - '0' as u32, @@ -1318,7 +1386,7 @@ fn based_value(base: u32, text: &str, name: &str) -> Result { _ => unreachable!(), }; if digit_val >= base { - return Err(ArithError::new(format!("`{name}` printed `{text}`, which is not a number"), 0..0)); + return Err(ArithError::new(format!("`{c}` is not a digit in `{text}`; use digits valid for base {base}"), 0..0)); } mag = mag .checked_mul(base as u64) @@ -1367,16 +1435,53 @@ fn resolve_expansion_sync(e: &Expansion, scope: &Scope) -> Result Result { +/// The expansion's rendered VALUE, for `base#` — not its +/// arithmetically-coerced number. A `String` value's text passes through +/// untouched (leading zero included); other values render through the same +/// `value_to_string` interpolation uses. +pub(crate) fn expansion_text_sync(e: &Expansion, scope: &Scope) -> Result { match e { - Expansion::CommandSubst(_) => Err(needs_async("$(...)")), - _ => { - let n = resolve_expansion_sync(e, scope)?; - based_value(base, &n.to_string(), "expansion") + Expansion::Var(name) => { + if let Ok(index) = name.parse::() { + return match scope.get_positional(index) { + Some(s) => Ok(s.to_string()), + None => Err(unset_error(name)), + }; + } + match scope.get(name) { + Some(v) => Ok(value_to_string(v)), + None => Err(unset_error(name)), + } + } + Expansion::BracedPath { root, brackets } => { + braced_path_value(scope, root, brackets).map(|v| value_to_string(&v)) + } + Expansion::BracedDefault { root, brackets, default } => { + let resolved = if brackets.is_empty() { + scope.get(root).cloned() + } else { + braced_path_value(scope, root, brackets).ok() + }; + match resolved { + Some(Value::Null) | None => { + let default_expr = parse(default)?; + Ok(eval_sync(&default_expr, scope)?.to_string()) + } + Some(v) => Ok(value_to_string(&v)), + } } + Expansion::LastExitCode => Ok(scope.last_result().code.to_string()), + Expansion::CurrentPid => Ok(scope.pid().to_string()), + Expansion::CommandSubst(_) => Err(needs_async("$(...)")), + Expansion::Nested(inner) => Ok(eval_sync(inner, scope)?.to_string()), } } +fn resolve_based_sync(base: u32, e: &Expansion, scope: &Scope) -> Result { + let text = expansion_text_sync(e, scope)?; + based_value(base, &text) +} + pub(crate) fn eval_sync(expr: &ArithExpr, scope: &Scope) -> Result { match expr { ArithExpr::Int(n) => Ok(*n), diff --git a/crates/kaish-kernel/src/ast/plan.rs b/crates/kaish-kernel/src/ast/plan.rs index 30c17bc2..ca621073 100644 --- a/crates/kaish-kernel/src/ast/plan.rs +++ b/crates/kaish-kernel/src/ast/plan.rs @@ -439,6 +439,10 @@ fn collect_stmt<'a>(stmt: &'a Stmt, background: bool, out: &mut Collected<'a>) { collect_block(&def.body, background, out) } Stmt::Test(t) => collect_test(t, background, out), + // Mirrors `Expr::Arithmetic`: free-variable reads only, same as a + // bare `$(( ))` — a `$(...)` operand inside is not walked into + // `PlannedCommand`s (a narrower surface than `Test`'s). + Stmt::Arith(expr) => out.read_arithmetic(expr), Stmt::AndChain { left, right } | Stmt::OrChain { left, right } => { collect_stmt(left, background, out); collect_stmt(right, background, out); @@ -534,6 +538,7 @@ fn collect_expr<'a>(expr: &'a Expr, background: bool, out: &mut Collected<'a>) { } Expr::VarRef(path) | Expr::VarLength(path) => out.read_path(path), Expr::Arithmetic(e) => out.read_arithmetic(e), + Expr::Arith(e) => out.read_arithmetic(e), // Special forms ($1, $@, $#, $?, $$) are not session variables; an // embedder cannot peek them with `get_var`, so they are not listed. Expr::Literal(_) @@ -608,6 +613,7 @@ pub(crate) fn render_stmt(stmt: &Stmt) -> String { Stmt::Exit(e) => render_keyword("exit", e.as_ref().map(|e| render_expr(e))), Stmt::ToolDef(def) => render_tooldef(def), Stmt::Test(t) => format!("[[ {} ]]", render_test(t)), + Stmt::Arith(e) => format!("(({e}))"), Stmt::AndChain { left, right } => { format!("{} && {}", render_stmt(left), render_stmt(right)) } @@ -897,6 +903,7 @@ pub(crate) fn render_expr(expr: &Expr) -> String { format!("${{{}:-{}}}", render_varpath(path), render_parts(default)) } Expr::Arithmetic(e) => format!("$(({e}))"), + Expr::Arith(e) => format!("(({e}))"), // Render the source text, not `value`'s canonical form — that is what // this variant is for. Expr::NumericLiteral { raw, .. } => raw.clone(), diff --git a/crates/kaish-kernel/src/ast/sexpr.rs b/crates/kaish-kernel/src/ast/sexpr.rs index 735f9641..2620b0b8 100644 --- a/crates/kaish-kernel/src/ast/sexpr.rs +++ b/crates/kaish-kernel/src/ast/sexpr.rs @@ -54,6 +54,7 @@ pub fn format_stmt(stmt: &Stmt) -> String { }, Stmt::ToolDef(tool) => format_tooldef(tool), Stmt::Test(test_expr) => format!("(test {})", format_test_expr(test_expr)), + Stmt::Arith(expr_str) => format!("(arith \"{}\")", expr_str), Stmt::AndChain { left, right } => { format!("(and-chain {} {})", format_stmt(left), format_stmt(right)) } @@ -314,6 +315,7 @@ pub fn format_expr(expr: &Expr) -> String { format!("(var-default {} ({}))", format_varpath(path), default_parts.join(" ")) } Expr::Arithmetic(expr_str) => format!("(arithmetic \"{}\")", expr_str), + Expr::Arith(expr_str) => format!("(arith \"{}\")", expr_str), Expr::Command(cmd) => format_command(cmd), Expr::LastExitCode => "(last-exit-code)".to_string(), Expr::CurrentPid => "(current-pid)".to_string(), diff --git a/crates/kaish-kernel/src/ast/types.rs b/crates/kaish-kernel/src/ast/types.rs index 87f5de9e..3e9a21aa 100644 --- a/crates/kaish-kernel/src/ast/types.rs +++ b/crates/kaish-kernel/src/ast/types.rs @@ -38,6 +38,9 @@ pub enum Stmt { ToolDef(ToolDef), /// Test expression: `[[ -f path ]]` or `[[ $X == "value" ]]` Test(TestExpr), + /// Bare arithmetic condition as a command: `(( expr ))`. Exit 0 when the + /// value is nonzero, 1 when zero — the sibling of `Test`. + Arith(String), /// Statement chain with `&&`: run right only if left succeeds AndChain { left: Box, right: Box }, /// Statement chain with `||`: run right only if left fails @@ -68,6 +71,7 @@ impl Stmt { Stmt::Exit(_) => "exit", Stmt::ToolDef(_) => "tooldef", Stmt::Test(_) => "test", + Stmt::Arith(_) => "arith", Stmt::AndChain { .. } => "and_chain", Stmt::OrChain { .. } => "or_chain", Stmt::EnvScoped { .. } => "env_scoped", @@ -377,6 +381,9 @@ pub enum Expr { VarWithDefault { path: VarPath, default: Vec }, /// Arithmetic expansion: `$((expr))` - evaluates to integer Arithmetic(String), + /// Bare arithmetic condition: `(( expr ))` in `if`/`while` condition + /// position — truthy when the value is nonzero, the sibling of `Test`. + Arith(String), /// Command as condition: `if grep -q pattern file; then` - exit code determines truthiness Command(Command), /// Last exit code: `$?` diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index adb5e425..88ade16f 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -219,6 +219,7 @@ impl<'a> Evaluator<'a> { Expr::VarLength(path) => self.eval_var_length(path), Expr::VarWithDefault { path, default } => self.eval_var_with_default(path, default), Expr::Arithmetic(expr_str) => self.eval_arithmetic(expr_str), + Expr::Arith(expr_str) => self.eval_arith_cond(expr_str), Expr::Command(cmd) => self.eval_command(cmd), Expr::LastExitCode => self.eval_last_exit_code(), Expr::CurrentPid => self.eval_current_pid(), @@ -307,6 +308,16 @@ impl<'a> Evaluator<'a> { .map_err(|e| EvalError::ArithmeticError(e.to_string())) } + /// Evaluate a bare `(( expr ))` condition: true when the value is + /// nonzero. The sibling of [`Self::eval_test`], same coercion as + /// `$(( ))` (`eval_arithmetic` above) — only the truthiness wrapper + /// differs. + fn eval_arith_cond(&mut self, expr_str: &str) -> EvalResult { + arithmetic::eval_arithmetic(expr_str, self.scope) + .map(|n| Value::Bool(n != 0)) + .map_err(|e| EvalError::ArithmeticError(e.to_string())) + } + /// Evaluate a test expression `[[ ... ]]` to a boolean value. fn eval_test(&mut self, test_expr: &TestExpr) -> EvalResult { let result = match test_expr { diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index 9d0466ee..6045f1e4 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -3073,6 +3073,27 @@ impl Kernel { } Ok(ControlFlow::ok(result)) } + // `(( expr ))` — the sibling of `Test` above, but an evaluation + // fault (division by zero, an unset variable) does not abort + // the statement list the way a bad `[[ ]]` comparison does: it + // is exit 2 with the arithmetic error as the message, same as + // any other command that ran and failed. + Stmt::Arith(expr_str) => { + let result = match self.eval_arithmetic_async(expr_str).await { + Ok(n) if n != 0 => ExecResult::success(""), + Ok(_) => ExecResult::failure(1, ""), + Err(e) => ExecResult::failure(2, e.to_string()), + }; + self.update_last_result(&result).await; + if !result.ok() { + let scope = self.scope.read().await; + if scope.error_exit_enabled() { + let code = result.code; + return Ok(ControlFlow::Exit { code, result }); + } + } + Ok(ControlFlow::ok(result)) + } Stmt::EnvScoped { assignments, body } => { // Inline env prefix (`NAME=value ... command`): apply the // assignments as EXPORTED vars in a fresh frame so the command @@ -4209,6 +4230,18 @@ impl Kernel { Expr::Test(test_expr) => { Ok(Value::Bool(self.eval_test_async(test_expr).await?)) } + // `(( expr ))` in condition position (`if`/`while`). Unlike the + // standalone `Stmt::Arith` form, a condition has no exit-code + // channel separate from true/false, so an evaluation fault + // propagates like `Expr::Test`'s does — it aborts the + // enclosing statement rather than silently reading false. + Expr::Arith(expr_str) => { + let n = self + .eval_arithmetic_async(expr_str) + .await + .context("arithmetic condition")?; + Ok(Value::Bool(n != 0)) + } Expr::Positional(n) => { let scope = self.scope.read().await; match scope.get_positional(*n) { @@ -4244,10 +4277,7 @@ impl Kernel { } } Expr::Arithmetic(expr_str) => { - let scope = self.scope.read().await; - crate::arithmetic::eval_arithmetic(expr_str, &scope) - .map(Value::Int) - .map_err(|e| anyhow::anyhow!("arithmetic error: {}", e)) + self.eval_arithmetic_async(expr_str).await.map(Value::Int) } Expr::Command(cmd) => { // A command in expression position — an `if`/`while` @@ -4541,10 +4571,7 @@ impl Kernel { // `"$((1/0))"` — `echo "value: $((1/0))"` printed "value: " // at exit 0 instead of failing. Matches the bare (non-string) // `Expr::Arithmetic` arm above, which already propagates. - let scope = self.scope.read().await; - crate::arithmetic::eval_arithmetic(expr, &scope) - .map(|value| value.to_string()) - .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + self.eval_arithmetic_async(expr).await.map(|value| value.to_string()) } StringPart::CommandSubst(stmts) => { // Snapshot scope, cwd, and session config — command @@ -4926,6 +4953,272 @@ impl Kernel { Ok(result) } + /// Evaluate `$(( text ))`'s content. Takes the sync fast path + /// (`arithmetic::eval_sync` under one scope read lock) when no `$(...)` + /// is reachable in the parsed tree; otherwise walks it with + /// [`Self::eval_arith_expr_async`], which can run a `$(...)` operand and + /// never runs one on the unselected side of `&&`/`||`/`?:`. + async fn eval_arithmetic_async(&self, text: &str) -> Result { + let ast = crate::arithmetic::parse(text).map_err(|e| anyhow::anyhow!("arithmetic error: {e}"))?; + if ast.contains_command_subst() { + self.eval_arith_expr_async(&ast).await + } else { + let scope = self.scope.read().await; + crate::arithmetic::eval_sync(&ast, &scope).map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + } + + /// Async recursive walk over a parsed `$(( ))` tree. Boxed for the same + /// reason as [`Self::eval_expr_async`]: the recursion is unbounded by + /// the type system, so a fixed-depth stack frame can't hold it. Each + /// leaf takes its own short `self.scope` read lock rather than one held + /// across the whole walk — `Expansion::CommandSubst` runs through + /// [`Self::execute_block_capturing`], which takes its own scope lock + /// internally, so a lock held here across that await would deadlock. + fn eval_arith_expr_async<'a>( + &'a self, + expr: &'a crate::arithmetic::ArithExpr, + ) -> std::pin::Pin> + Send + 'a>> { + use crate::arithmetic::{ArithExpr, BinOp}; + Box::pin(async move { + match expr { + ArithExpr::Int(n) => Ok(*n), + ArithExpr::Expansion(e) => self.eval_arith_expansion_async(e).await, + ArithExpr::Subscript { root, indices } => { + let mut values = Vec::with_capacity(indices.len()); + for index in indices { + values.push(self.eval_arith_expr_async(index).await?); + } + let scope = self.scope.read().await; + crate::arithmetic::resolve_subscript_sync(&scope, root, &values) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + ArithExpr::BasedExpansion { base, expansion } => { + let text = self.eval_arith_expansion_text_async(expansion).await?; + crate::arithmetic::based_value(*base, &text) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + ArithExpr::Unary { op, operand } => { + let v = self.eval_arith_expr_async(operand).await?; + crate::arithmetic::apply_unary(*op, v).map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + // `&&`/`||` short-circuit: the unselected side's `$(...)` + // must not run (docs/LANGUAGE.md, "Operators"). + ArithExpr::Binary { op: BinOp::And, left, right } => { + if self.eval_arith_expr_async(left).await? == 0 { + Ok(0) + } else { + Ok(if self.eval_arith_expr_async(right).await? != 0 { 1 } else { 0 }) + } + } + ArithExpr::Binary { op: BinOp::Or, left, right } => { + if self.eval_arith_expr_async(left).await? != 0 { + Ok(1) + } else { + Ok(if self.eval_arith_expr_async(right).await? != 0 { 1 } else { 0 }) + } + } + ArithExpr::Binary { op, left, right } => { + let l = self.eval_arith_expr_async(left).await?; + let r = self.eval_arith_expr_async(right).await?; + crate::arithmetic::apply_binary(*op, l, r).map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + ArithExpr::Ternary { cond, then_branch, else_branch } => { + if self.eval_arith_expr_async(cond).await? != 0 { + self.eval_arith_expr_async(then_branch).await + } else { + self.eval_arith_expr_async(else_branch).await + } + } + } + }) + } + + fn eval_arith_expansion_async<'a>( + &'a self, + expansion: &'a crate::arithmetic::Expansion, + ) -> std::pin::Pin> + Send + 'a>> { + use crate::arithmetic::Expansion; + Box::pin(async move { + match expansion { + Expansion::Var(name) => { + let scope = self.scope.read().await; + crate::arithmetic::resolve_var_sync(&scope, name) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + Expansion::BracedPath { root, brackets } => { + let scope = self.scope.read().await; + let value = crate::arithmetic::braced_path_value(&scope, root, brackets) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}"))?; + crate::arithmetic::value_to_arith(&value, root) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + Expansion::BracedDefault { root, brackets, default } => { + let resolved = { + let scope = self.scope.read().await; + if brackets.is_empty() { + scope.get(root).cloned() + } else { + crate::arithmetic::braced_path_value(&scope, root, brackets).ok() + } + }; + match resolved { + Some(Value::Null) | None => { + let default_expr = crate::arithmetic::parse(default) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}"))?; + self.eval_arith_expr_async(&default_expr).await + } + Some(value) => crate::arithmetic::value_to_arith(&value, root) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")), + } + } + Expansion::LastExitCode => { + let scope = self.scope.read().await; + Ok(scope.last_result().code) + } + Expansion::CurrentPid => { + let scope = self.scope.read().await; + Ok(scope.pid() as i64) + } + Expansion::CommandSubst(stmts) => self.run_arith_command_subst(stmts).await, + Expansion::Nested(inner) => self.eval_arith_expr_async(inner).await, + } + }) + } + + /// The expansion's rendered VALUE, for `base#` — mirrors + /// [`crate::arithmetic::expansion_text_sync`], async so `$(...)` can + /// run for real. Never routes through [`Self::eval_arith_expansion_async`] + /// (the arithmetically-coerced form): that coercion refuses a leading + /// zero, which is exactly what `10#$m`/`10#$(date +%m)` exist to escape. + fn eval_arith_expansion_text_async<'a>( + &'a self, + expansion: &'a crate::arithmetic::Expansion, + ) -> std::pin::Pin> + Send + 'a>> { + use crate::arithmetic::Expansion; + Box::pin(async move { + match expansion { + Expansion::Var(name) => { + let scope = self.scope.read().await; + if let Ok(index) = name.parse::() { + return match scope.get_positional(index) { + Some(s) => Ok(s.to_string()), + None => Err(anyhow::anyhow!( + "arithmetic error: {}", + crate::arithmetic::unset_error(name) + )), + }; + } + match scope.get(name) { + Some(v) => Ok(value_to_string(v)), + None => Err(anyhow::anyhow!( + "arithmetic error: {}", + crate::arithmetic::unset_error(name) + )), + } + } + Expansion::BracedPath { root, brackets } => { + let scope = self.scope.read().await; + crate::arithmetic::braced_path_value(&scope, root, brackets) + .map(|v| value_to_string(&v)) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + Expansion::BracedDefault { root, brackets, default } => { + let resolved = { + let scope = self.scope.read().await; + if brackets.is_empty() { + scope.get(root).cloned() + } else { + crate::arithmetic::braced_path_value(&scope, root, brackets).ok() + } + }; + match resolved { + Some(Value::Null) | None => { + let default_expr = crate::arithmetic::parse(default) + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}"))?; + let n = self.eval_arith_expr_async(&default_expr).await?; + Ok(n.to_string()) + } + Some(value) => Ok(value_to_string(&value)), + } + } + Expansion::LastExitCode => { + let scope = self.scope.read().await; + Ok(scope.last_result().code.to_string()) + } + Expansion::CurrentPid => { + let scope = self.scope.read().await; + Ok(scope.pid().to_string()) + } + Expansion::CommandSubst(stmts) => self.run_arith_command_subst_text(stmts).await, + Expansion::Nested(inner) => { + let n = self.eval_arith_expr_async(inner).await?; + Ok(n.to_string()) + } + } + }) + } + + /// Run a `$(...)` operand inside `$(( ))`. Mirrors `Expr::CommandSubst`'s + /// isolation (scope/cwd/config snapshot-and-restore, stderr forwarded to + /// the enclosing statement) — the same substitution mechanism, just + /// coerced to an integer instead of spliced in as text. + async fn run_arith_command_subst(&self, stmts: &[Stmt]) -> Result { + let text = self.run_arith_command_subst_text(stmts).await?; + crate::arithmetic::parse_command_output(&text, "$(...)") + .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) + } + + /// Run a `$(...)` operand and return its printed text — the shared half + /// of [`Self::run_arith_command_subst`] (a bare operand, coerced to an + /// integer) and the `base#$(...)` case (the text is read as digits in a + /// base, never coerced first — see [`crate::arithmetic::based_value`]). + async fn run_arith_command_subst_text(&self, stmts: &[Stmt]) -> Result { + let saved_scope = Box::new(self.scope.read().await.clone()); + let saved_ec = { + let ec = self.exec_ctx.read().await; + ( + ec.cwd.clone(), + ec.prev_cwd.clone(), + ec.aliases.clone(), + ec.ignore_config.clone(), + ec.output_limit.clone(), + ) + }; + + let run_result = self.execute_block_capturing(stmts).await; + + { + let mut scope = self.scope.write().await; + *scope = *saved_scope; + if let Ok(ref r) = run_result { + scope.set_last_result(r.clone()); + scope.note_cmdsubst_code(r.code); + } + } + { + let mut ec = self.exec_ctx.write().await; + let (cwd, prev_cwd, aliases, ignore_config, output_limit) = saved_ec; + ec.cwd = cwd; + ec.prev_cwd = prev_cwd; + ec.aliases = aliases; + ec.ignore_config = ignore_config; + ec.output_limit = output_limit; + } + + if let Ok(ref r) = run_result { + self.emit_cmdsubst_stderr(&r.err).await; + } + + let result = run_result?; + if result.out_bytes().is_some() { + return Err(anyhow::anyhow!( + "arithmetic error: `$(...)` printed binary data; the command must print one integer" + )); + } + Ok(result.text_out().trim_end_matches('\n').to_string()) + } + /// Execute the `source` / `.` command to include and run a script. /// /// Unlike regular tool execution, `source` executes in the CURRENT scope, diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index d178ca5c..725e8d30 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -503,6 +503,11 @@ pub enum Token { /// Contains the expression string between `$((` and `))`. Arithmetic(String), + /// Bare `(( expr ))` arithmetic condition: synthesized by preprocessing, + /// the same way as `Arithmetic` but for the unquoted, `$`-less form. + /// Contains the expression string between the two `(` and the two `)`. + ArithCond(String), + /// Command substitution start: `$(` - begins a command substitution #[token("$(")] CmdSubstStart, @@ -905,7 +910,8 @@ impl Token { | Token::Newline | Token::LineContinuation | Token::CmdSubstStart - | Token::DotDotDot => TokenCategory::Punctuation, + | Token::DotDotDot + | Token::ArithCond(_) => TokenCategory::Punctuation, // Glob words (merged tokens containing wildcards) Token::GlobWord(_) => TokenCategory::Path, @@ -1330,6 +1336,7 @@ impl fmt::Display for Token { Token::Question => write!(f, "?"), Token::GlobWord(s) => write!(f, "GLOB({})", s), Token::Arithmetic(s) => write!(f, "ARITHMETIC({})", s), + Token::ArithCond(s) => write!(f, "((ARITHCOND({})))", s), Token::CmdSubstStart => write!(f, "$("), Token::LongFlag(s) => write!(f, "--{}", s), Token::ShortFlag(s) => write!(f, "-{}", s), @@ -1558,8 +1565,12 @@ struct PendingHeredoc { /// markers and correct spans. struct ScanOutput { text: String, - /// (marker, expression) pairs, indexed by `ReplacementKind::Arith`. - arithmetics: Vec<(String, String)>, + /// (marker, expression, is_condition) triples, indexed by + /// `ReplacementKind::Arith`. `is_condition` is set only for a bare + /// `(( expr ))` at the top level — `$((expr))` is always `false`, and a + /// quoted string's own `$((` never sets it (the condition form has no + /// meaning inside a string). + arithmetics: Vec<(String, String, bool)>, /// Heredoc extracts, indexed by `ReplacementKind::HeredocIntro`. heredocs: Vec, replacements: Vec, @@ -1581,7 +1592,7 @@ fn scan(source: &str) -> Result> { }; let mut out = String::with_capacity(source.len()); - let mut arithmetics: Vec<(String, String)> = Vec::new(); + let mut arithmetics: Vec<(String, String, bool)> = Vec::new(); let mut heredocs: Vec = Vec::new(); let mut replacements: Vec = Vec::new(); let mut pending: Vec = Vec::new(); @@ -1654,6 +1665,8 @@ fn scan(source: &str) -> Result> { &mut out, &mut arithmetics, &mut replacements, + 3, + false, )?; continue; } @@ -1732,6 +1745,26 @@ fn scan(source: &str) -> Result> { &mut out, &mut arithmetics, &mut replacements, + 3, + false, + )?; + } + // Bare `((expr))` — an arithmetic condition, the sibling of + // `[[ ]]`. Unlike `$((`, there is no sigil to make this + // unambiguous; it is safe because kaish gives a lone `(` no + // other meaning at this (unquoted, top-level) position — see + // `Stmt::Arith`/`Expr::Arith` in `ast/types.rs`. + '(' if i + 1 < n && chars[i + 1].1 == '(' => { + extract_arithmetic( + &chars, + &mut i, + pos, + total_len, + &mut out, + &mut arithmetics, + &mut replacements, + 2, + true, )?; } '$' if i + 1 < n && chars[i + 1].1 == '{' => { @@ -1944,11 +1977,13 @@ fn extract_arithmetic( start_pos: usize, total_len: usize, out: &mut String, - arithmetics: &mut Vec<(String, String)>, + arithmetics: &mut Vec<(String, String, bool)>, replacements: &mut Vec, + marker_len: usize, + is_condition: bool, ) -> Result<(), Spanned> { let n = chars.len(); - *i += 3; // consume `$((` + *i += marker_len; // consume `$((` (3) or bare `((` (2) let mut expr = String::new(); let mut depth = 0usize; @@ -2010,7 +2045,7 @@ fn extract_arithmetic( new_len: marker.len(), kind: ReplacementKind::Arith(arithmetics.len()), }); - arithmetics.push((marker.clone(), expr)); + arithmetics.push((marker.clone(), expr, is_condition)); out.push_str(&marker); Ok(()) } @@ -2368,7 +2403,8 @@ fn resolve_markers( } match (&spanned.token, contained.as_slice()) { - // Exact cover by a single arithmetic marker → Arithmetic token. + // Exact cover by a single arithmetic marker → Arithmetic token, + // or ArithCond for a bare `((expr))` condition. (Token::Ident(_), [m]) if matches!(m.kind, ReplacementKind::Arith(_)) && m.new_start == span.start @@ -2377,10 +2413,13 @@ fn resolve_markers( let ReplacementKind::Arith(idx) = m.kind else { unreachable!("guarded by matches! above") }; - result.push(Spanned::new( - Token::Arithmetic(scan.arithmetics[idx].1.clone()), - span, - )); + let (_, expr, is_condition) = &scan.arithmetics[idx]; + let token = if *is_condition { + Token::ArithCond(expr.clone()) + } else { + Token::Arithmetic(expr.clone()) + }; + result.push(Spanned::new(token, span)); } // Exact cover by a heredoc marker → HereDoc token (the parser @@ -2420,7 +2459,8 @@ fn resolve_markers( // never does. unreachable!("heredoc marker inside string content") }; - let (marker, expr) = &scan.arithmetics[idx]; + let (marker, expr, is_condition) = &scan.arithmetics[idx]; + debug_assert!(!is_condition, "a bare `((` condition never scans inside a string"); content = content.replacen(marker, &format!("${{__ARITH:{}__}}", expr), 1); } @@ -2444,10 +2484,13 @@ fn resolve_markers( } match m.kind { ReplacementKind::Arith(idx) => { - result.push(Spanned::new( - Token::Arithmetic(scan.arithmetics[idx].1.clone()), - m.new_start..m.new_start + m.new_len, - )); + let (_, expr, is_condition) = &scan.arithmetics[idx]; + let token = if *is_condition { + Token::ArithCond(expr.clone()) + } else { + Token::Arithmetic(expr.clone()) + }; + result.push(Spanned::new(token, m.new_start..m.new_start + m.new_len)); } ReplacementKind::HeredocIntro(_) | ReplacementKind::Elision => { // Heredoc markers are always delimited by the diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index 9d94a2d4..114a57c5 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -1571,6 +1571,7 @@ where return_stmt, exit_stmt, test_expr_stmt_parser().map(Stmt::Test), + arith_cond_parser().map(Stmt::Arith), // Note: 'true' and 'false' are handled by command_parser/pipeline_parser pipeline_parser(choice(( compound.map(|s| PipelineStage::Compound(Box::new(s))), @@ -2204,6 +2205,7 @@ fn stmt_has_ambiguous_stdin(stmt: &Stmt) -> bool { | Stmt::Return(_) | Stmt::Exit(_) | Stmt::Test(_) + | Stmt::Arith(_) | Stmt::Empty => false, } } @@ -2699,6 +2701,18 @@ where /// - Comparisons: `[[ $X == "value" ]]`, `[[ $NUM -gt 5 ]]` /// - Compound: `[[ -f a && -d b ]]`, `[[ -z x || -n y ]]`, `[[ ! -f file ]]` /// +/// `(( expr ))` — the sibling of `test_expr_stmt_parser` for a bare +/// arithmetic condition. The lexer already extracted the text between the +/// two `(` and the two `)` into one `ArithCond` token (mirroring `$((`'s +/// `Arithmetic` token); this parser only unwraps it. +fn arith_cond_parser<'tokens, I>( +) -> impl Parser<'tokens, I, String, extra::Err>> + Clone +where + I: ValueInput<'tokens, Token = Token, Span = Span>, +{ + select! { Token::ArithCond(expr) => expr }.labelled("arithmetic condition") +} + /// Precedence (highest to lowest): `!` > `&&` > `||` fn test_expr_stmt_parser<'tokens, I>( ) -> impl Parser<'tokens, I, TestExpr, extra::Err>> + Clone @@ -2866,12 +2880,15 @@ where // [[ ]] test expression - wrap as Expr::Test let test_expr_condition = test_expr_stmt_parser().map(|test| Expr::Test(Box::new(test))); + // (( expr )) arithmetic condition - the sibling of [[ ]] above. + let arith_condition = arith_cond_parser().map(Expr::Arith); + // Command as condition (includes true/false/: as command names) // The command's exit code determines truthiness (0 = true, non-zero = false) let command_condition = command_parser().map(Expr::Command); - // Base: test expr OR command - let base = choice((test_expr_condition, command_condition)); + // Base: test expr OR arithmetic condition OR command + let base = choice((test_expr_condition, arith_condition, command_condition)); // `!` negates the command that follows it, BEFORE `&&`/`||` fold below — // bash reads `! true && true` as `(! true) && true`. Repeated so `! ! x` @@ -3613,6 +3630,10 @@ fn is_word_token(tok: &Token) -> bool { | Token::LBrace | Token::RBrace | Token::LBracket | Token::RBracket | Token::LParen | Token::RParen => false, + // `(( ))` is a whole command/condition, like `[[ ]]` — never a word + // an argument would glue onto. + Token::ArithCond(_) => false, + // `$(` opens a balanced group, not a single-token word — `word_unit` // handles it by scanning to the matching `)`. Token::CmdSubstStart => false, diff --git a/crates/kaish-kernel/src/validator/walker.rs b/crates/kaish-kernel/src/validator/walker.rs index 7b3eff59..c3af23a1 100644 --- a/crates/kaish-kernel/src/validator/walker.rs +++ b/crates/kaish-kernel/src/validator/walker.rs @@ -105,6 +105,9 @@ impl<'a> Validator<'a> { } Stmt::ToolDef(tool_def) => self.validate_tool_def(tool_def), Stmt::Test(test_expr) => self.validate_test(test_expr), + // Arithmetic parsing (and the base-cap/depth-cap checks it + // carries) happens at runtime, same as `Expr::Arithmetic` below. + Stmt::Arith(_) => {} Stmt::AndChain { left, right } | Stmt::OrChain { left, right } => { self.validate_stmt(left); self.validate_stmt(right); @@ -603,7 +606,7 @@ impl<'a> Validator<'a> { Expr::VarWithDefault { .. } => { // Don't warn — the default handles the undefined/absent case. } - Expr::Arithmetic(_) => { + Expr::Arithmetic(_) | Expr::Arith(_) => { // Arithmetic parsing is done at runtime } Expr::Command(cmd) => self.validate_command(cmd), diff --git a/crates/kaish-kernel/tests/arithmetic_tests.rs b/crates/kaish-kernel/tests/arithmetic_tests.rs new file mode 100644 index 00000000..17602e59 --- /dev/null +++ b/crates/kaish-kernel/tests/arithmetic_tests.rs @@ -0,0 +1,431 @@ +//! `$(( ))` and bare `(( ))`: every documented example, every named error +//! fix, and the precedence/coercion tables from `docs/LANGUAGE.md`. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use kaish_kernel::{Kernel, KernelConfig}; + +mod common; + +async fn run(source: &str) -> (i64, String, String) { + let k = Kernel::new(KernelConfig::isolated()).expect("kernel"); + let r = k.execute(source).await.expect("kernel execute"); + (r.code, r.text_out().trim().to_string(), r.err.clone()) +} + +/// Every diagnostic a failing statement produces, however it refused. +async fn err_of(source: &str) -> String { + let k = Kernel::new(KernelConfig::isolated()).expect("kernel").into_arc(); + match k.execute(source).await { + Ok(r) => { + assert!(!r.ok(), "{source:?} should fail"); + format!("{}{}", r.text_out(), r.err) + } + Err(e) => format!("{e:?}"), + } +} + +async fn ok(source: &str, expected: &str) { + let (code, out, err) = run(source).await; + assert_eq!(code, 0, "{source:?} must run: {err:?}"); + assert_eq!(out, expected, "{source:?}"); +} + +// ── docs/LANGUAGE.md "Arithmetic" — every example with its # result ──────── + +#[tokio::test] +async fn language_md_examples() { + for (source, expected) in [ + ("echo $((5 + 3 * 2))", "11"), + ("echo $((10 / 3))", "3"), + ("echo $((-7 % 3))", "-1"), + ("echo $((2 ** 10))", "1024"), + ("echo $((0xff))", "255"), + ("echo $((16#ff))", "255"), + ("echo $((8#17))", "15"), + ("echo $((2#1011))", "11"), + ("echo $((36#z))", "35"), + ("echo $((-0xff))", "-255"), + ("count=4; echo $((count + 1))", "5"), + ("count=4; echo $(($count + 1))", "5"), + (r#"mask="0xff"; echo $((mask & 16#0f))"#, "15"), + ("echo $(( ${limit:-0} + 1 ))", "1"), + ("echo $((2 ** 3 ** 2))", "512"), + ("echo $((-2 ** 2))", "4"), + ("echo $((1 << 2 + 1))", "8"), + ("echo $((5 & 3 == 3))", "1"), + ("echo $((5 > 3))", "1"), + ("echo $(( (8 & 8) != 0 ))", "1"), + ] { + ok(source, expected).await; + } +} + +#[tokio::test] +async fn assignment_counter_idiom() { + ok("x=1; x=$((x + 1)); echo $x", "2").await; +} + +#[tokio::test] +async fn a_command_prints_one_integer_operand() { + ok("echo $(( $(printf 6) * 2 ))", "12").await; +} + +#[tokio::test] +async fn ternary_picks_the_larger_value() { + ok("a=3; b=9; echo $((a > b ? a : b))", "9").await; +} + +#[tokio::test] +async fn arithmetic_as_a_condition() { + ok( + "i=1; while (( i <= 5 )); do echo $i; i=$((i + 1)); done", + "1\n2\n3\n4\n5", + ) + .await; + ok("n=4; if (( n % 2 == 0 )); then echo even; else echo odd; fi", "even").await; + ok("n=5; if (( n % 2 == 0 )); then echo even; else echo odd; fi", "odd").await; +} + +// ── "A bare number follows JSON rules" — the octal paragraph ─────────────── + +#[tokio::test] +async fn bare_number_section_examples() { + let text = err_of("echo $((010 + 1))").await; + assert!(text.contains("leading zero")); + let text = err_of("[[ 010 -eq 10 ]]").await; + assert!(!text.is_empty(), "010 must still be refused as a numeral"); + let text = err_of("x=010; echo $((x))").await; + assert!(text.contains("10#$x") || text.contains("8#$x"), "{text:?}"); +} + +// ── Not supported, and what to write ──────────────────────────────────────── + +#[tokio::test] +async fn not_supported_table() { + for source in ["echo $((x++))", "echo $((x += 1))", "echo $((x = 5))"] { + let text = err_of(source).await; + assert!(text.contains("assigns"), "{source:?}: {text:?}"); + } + let text = err_of("echo $((a, b))").await; + assert!(text.contains("one expression"), "{text:?}"); + for source in ["echo $((1.5))", "echo $((1e3))"] { + let text = err_of(source).await; + assert!(text.contains("integer-only"), "{source:?}: {text:?}"); + } + let text = err_of("echo $((1 <<< 2))").await; + assert!(text.contains("here-string"), "{text:?}"); + let text = err_of("echo $(( ))").await; + assert!(text.contains("no expression"), "{text:?}"); + for source in ["echo $((0x))", "echo $((16#))"] { + let text = err_of(source).await; + assert!(text.contains("no digits"), "{source:?}: {text:?}"); + } +} + +// ── Coercion table ────────────────────────────────────────────────────────── + +#[tokio::test] +async fn coercion_int_bool_float() { + ok("x=5; echo $((x))", "5").await; + ok("x=true; echo $((x))", "1").await; + ok("x=false; echo $((x))", "0").await; + ok("x=$(fromjson 100.0); echo $((x + 1))", "101").await; + ok("x=$(fromjson 1e10); echo $((x))", "10000000000").await; +} + +#[tokio::test] +async fn coercion_float_errors() { + for source in ["x=2.7; echo $((x))", "x=$(fromjson 1e20); echo $((x))"] { + let text = err_of(source).await; + assert!(!text.is_empty(), "{source:?} must refuse"); + } +} + +#[tokio::test] +async fn coercion_string_forms() { + ok(r#"x="0xff"; echo $((x))"#, "255").await; + ok(r#"x="16#ff"; echo $((x))"#, "255").await; + ok(r#"x=" 5 "; echo $((x))"#, "5").await; + ok(r#"x="-5"; echo $((x))"#, "-5").await; +} + +#[tokio::test] +async fn coercion_leading_zero_string() { + let text = err_of(r#"x="08"; echo $((x))"#).await; + assert!(text.contains("leading zero"), "{text:?}"); +} + +#[tokio::test] +async fn coercion_empty_and_non_numeric_and_expression_strings() { + let text = err_of(r#"x=""; echo $((x))"#).await; + assert!(!text.is_empty()); + let text = err_of(r#"x="abc"; echo $((x))"#).await; + assert!(text.contains("not a number"), "{text:?}"); + let text = err_of(r#"x="1 + 2"; echo $((x))"#).await; + assert!(text.contains("not an expression"), "{text:?}"); +} + +#[tokio::test] +async fn coercion_null_and_unset() { + let text = err_of("x=null; x=$(fromjson null); echo $((x))").await; + assert!(text.contains("null"), "{text:?}"); + let text = err_of("echo $((missing))").await; + assert!(text.contains("unset"), "{text:?}"); +} + +#[tokio::test] +async fn coercion_list_record_bytes() { + let text = err_of("x=[1 2]; echo $((x))").await; + assert!(text.contains("list"), "{text:?}"); + let text = err_of("x={a: 1}; echo $((x))").await; + assert!(text.contains("record"), "{text:?}"); +} + +// ── Precedence ─────────────────────────────────────────────────────────── + +#[tokio::test] +async fn precedence_table() { + ok("echo $((1 << 2 + 1))", "8").await; + ok("echo $((5 & 3 == 3))", "1").await; + ok("echo $((2 ** 3 ** 2))", "512").await; + ok("echo $((-2 ** 2))", "4").await; + ok("echo $((1 ? 2 : 3 ? 4 : 5))", "2").await; +} + +// ── Overflow at each operator ──────────────────────────────────────────── + +#[tokio::test] +async fn overflow_at_each_operator() { + for source in [ + "echo $((9223372036854775807 + 1))", + "echo $((-9223372036854775808 - 1))", + "echo $((9223372036854775807 * 2))", + "echo $((-9223372036854775808 / -1))", + "echo $((2 ** 63))", + "echo $((1 << 63))", + ] { + let text = err_of(source).await; + assert!(text.contains("does not fit"), "{source:?}: {text:?}"); + } +} + +#[tokio::test] +async fn min_literal_only_as_direct_unary_operand() { + ok("echo $((-9223372036854775808))", "-9223372036854775808").await; + let text = err_of("echo $((9223372036854775808))").await; + assert!(text.contains("does not fit"), "{text:?}"); +} + +// ── Lazy $(...) — the unselected side must not run ────────────────────── + +#[tokio::test] +async fn lazy_command_substitution_does_not_run_on_the_skipped_side() { + let (code, out, err) = run("echo $((1 || $(echo side >&2; echo 1)))").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "1"); + assert!(err.is_empty(), "the skipped $() must not run: {err:?}"); + + let (code, out, err) = run("echo $((0 && $(echo side >&2; echo 1)))").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "0"); + assert!(err.is_empty(), "the skipped $() must not run: {err:?}"); + + let (code, out, err) = run("echo $((1 ? 2 : $(echo side >&2; echo 3)))").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "2"); + assert!(err.is_empty(), "the skipped ternary branch must not run: {err:?}"); +} + +#[tokio::test] +async fn selected_side_command_substitution_still_runs() { + // `||` normalizes to 1/0 like any other comparison-shaped operator — the + // selected side's $(echo 5) still RUNS (unlike the lazy test above), it + // just doesn't splice its value in unnormalized the way `?:` does. + let (code, out, err) = run("echo $((0 || $(echo 5)))").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "1"); +} + +// ── Based expansions ───────────────────────────────────────────────────── + +#[tokio::test] +async fn based_expansion_from_a_variable() { + ok(r#"BITS="1011"; echo $((2#$BITS))"#, "11").await; + ok(r#"MODE="755"; echo $((8#$MODE))"#, "493").await; +} + +#[tokio::test] +async fn based_expansion_from_a_command() { + ok(r#"echo $((10#$(printf 08)))"#, "8").await; +} + +// ── Nesting ─────────────────────────────────────────────────────────────── + +#[tokio::test] +async fn nested_arithmetic() { + ok("echo $(( $((1+2)) * 4 ))", "12").await; +} + +#[tokio::test] +async fn newline_inside_arithmetic() { + ok("echo $((1 +\n2))", "3").await; +} + +#[tokio::test] +async fn comment_after_arithmetic_is_a_shell_comment() { + ok("echo $((16#ff)) # comment", "255").await; +} + +#[tokio::test] +async fn quoted_arithmetic_interpolates() { + ok(r#"echo "$((0xff))""#, "255").await; +} + +// ── `[[ ]]` still refuses a leading zero ──────────────────────────────── + +#[tokio::test] +async fn test_expr_leading_zero_still_refused() { + let text = err_of("[[ 010 -eq 8 ]]").await; + assert!(!text.is_empty()); +} + +// ── Bare `(( ))` as a command ──────────────────────────────────────────── + +#[tokio::test] +async fn bare_arith_command_exit_codes() { + let (code, _, _) = run("(( 3 > 2 ))").await; + assert_eq!(code, 0); + let (code, _, _) = run("(( 1 > 2 ))").await; + assert_eq!(code, 1); + let (code, out, err) = run("(( 1/0 ))").await; + assert_eq!(code, 2, "out={out:?} err={err:?}"); + assert!(err.contains("divides by zero"), "{err:?}"); +} + +#[tokio::test] +async fn bare_arith_chains_with_and_or() { + ok("(( 1 > 0 )) && echo yes", "yes").await; + ok("(( 0 > 1 )) || echo fallback", "fallback").await; +} + +// ── Depth cap ──────────────────────────────────────────────────────────── + +#[tokio::test] +async fn depth_cap_is_enforced() { + let mut expr = String::new(); + for _ in 0..300 { + expr.push('('); + } + expr.push('1'); + for _ in 0..300 { + expr.push(')'); + } + let text = err_of(&format!("echo $(({expr}))")).await; + assert!(text.contains("256"), "{text:?}"); +} + +// ── Plan rendering: $(( )) text is verbatim ────────────────────────────── + +#[tokio::test] +async fn plan_renders_arithmetic_text_verbatim() { + use kaish_kernel::plan_program; + let plans = plan_program("echo $(( 1 + 2 ))").expect("parses"); + let rendered = &plans[0].plan.rendered; + assert!(rendered.contains("$(( 1 + 2 ))"), "{rendered:?}"); +} + +#[tokio::test] +async fn plan_renders_bare_arith_condition_verbatim() { + use kaish_kernel::plan_program; + let plans = plan_program("(( i <= 5 ))").expect("parses"); + let rendered = &plans[0].plan.rendered; + assert!(rendered.contains("(( i <= 5 ))"), "{rendered:?}"); +} + +// ── The 5-model panel's 14 tasks (fixed inputs replace date/random) ────── + +#[tokio::test] +async fn panel_task_1_hex_string_to_decimal() { + ok(r#"HEX="0xff"; echo $((HEX))"#, "255").await; +} + +#[tokio::test] +async fn panel_task_2_binary_string_to_decimal() { + ok(r#"BITS="1011"; echo $((2#$BITS))"#, "11").await; +} + +#[tokio::test] +async fn panel_task_3_octal_mode_string_to_decimal() { + ok(r#"MODE="755"; echo $((8#$MODE))"#, "493").await; +} + +/// Task 4 (`$RANDOM`) needs the `random` builtin, which ships in a separate +/// PR — this asserts the refusal names that exact fix instead of a value. +#[tokio::test] +async fn panel_task_4_random_names_its_fix() { + let text = err_of("echo $((RANDOM % 100))").await; + assert!(text.contains("random --max"), "{text:?}"); +} + +/// Task 5 (`sleep 1` timing via `$SECONDS`) — asserts the refusal names the +/// `date +%s` idiom instead of running a real timed sleep. +#[tokio::test] +async fn panel_task_5_seconds_names_its_fix() { + let text = err_of("echo $((SECONDS))").await; + assert!(text.contains("date +%s"), "{text:?}"); +} + +#[tokio::test] +async fn panel_task_6_count_with_while() { + ok( + "i=1; while (( i <= 5 )); do echo $i; i=$((i + 1)); done", + "1\n2\n3\n4\n5", + ) + .await; +} + +#[tokio::test] +async fn panel_task_7_integer_percentage() { + ok("echo $((7 * 100 / 9))", "77").await; +} + +#[tokio::test] +async fn panel_task_8_even_or_odd() { + ok("N=4; if (( N % 2 == 0 )); then echo even; else echo odd; fi", "even").await; + ok("N=7; if (( N % 2 == 0 )); then echo even; else echo odd; fi", "odd").await; +} + +#[tokio::test] +async fn panel_task_9_bit_test() { + ok("FLAGS=0x0c; echo $(( (FLAGS & 8) != 0 ))", "1").await; + ok("FLAGS=0x04; echo $(( (FLAGS & 8) != 0 ))", "0").await; +} + +/// Task 10 (`date +%m`, next month) — a fixed month string stands in for +/// the live date so the answer is deterministic. +#[tokio::test] +async fn panel_task_10_next_month_from_a_fixed_month() { + ok(r#"m="08"; echo $((10#$m % 12 + 1))"#, "9").await; + ok(r#"m="12"; echo $((10#$m % 12 + 1))"#, "1").await; +} + +#[tokio::test] +async fn panel_task_11_power_of_two() { + ok("echo $((2 ** 40))", "1099511627776").await; +} + +#[tokio::test] +async fn panel_task_12_larger_of_two() { + ok("A=3; B=7; echo $((A > B ? A : B))", "7").await; + ok("A=9; B=2; echo $((A > B ? A : B))", "9").await; +} + +#[tokio::test] +async fn panel_task_13_bytes_to_kilobytes() { + ok("TOTAL_BYTES=1536000; echo $((TOTAL_BYTES / 1024))", "1500").await; +} + +#[tokio::test] +async fn panel_task_14_decimal_to_hex() { + ok("printf '%x' 255", "ff").await; +} From 73468107dd3fb3c6a858685479906998a034b008 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 10:03:35 -0400 Subject: [PATCH 04/15] Adversarial cases for $(( )) from the spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviewers derived edge cases directly from the spec text — base boundaries, the i64::MIN/MAX seam, precedence combinations, and laziness beyond the $(...) case already covered — meant to catch exactly the blind spots a fresh implementation has. Checking each row against the spec (rather than trusting the row) before pinning it found two real bugs and two rows that disagreed with the spec: - i64::checked_rem returns None for MIN % -1, not Some(0) — it is defined in terms of the division, which overflows, even though division by a divisor of magnitude 1 never has a remainder for any dividend. apply_binary's Rem arm now special-cases r == -1 before calling checked_rem, since checked_rem is simply wrong on this input. - 8#$(echo 17) as a based-expansion's $(...) operand failed to close ("$( has no closing )") whenever the substitution was followed by more arithmetic text, e.g. $(( $(echo 17) + 1 )). The scanner tokenized the raw remainder with the general kaish lexer to find the matching ')' (the pattern parser.rs's own quoted-string interpolation uses for the same job) — but text after the substitution inside $(( )) is ARITHMETIC syntax ('+', '**', …), which the general lexer doesn't tokenize outside $(( )) at all, so it errored on the operators past the close before ever finding it. Replaced with a character-level scan (paren depth plus single/ double-quote awareness, matching the risk profile $((' own extract_arithmetic already accepts) that never asks the general lexer to make sense of arithmetic text. - digits=-ff; echo $((16#$digits)) — a reviewer expected the sign to be refused the way 16#-ff (a literal sign after #) is. The spec's coercion table says a based-expansion's text is "digits only (optional sign)": the sign is part of the accepted VALUE text, a different rule from a sign appearing in SOURCE after #. Pinned the spec's answer (-255), noted in the test. - echo $(( true + false )) — a reviewer expected 1 (true/false as literals). The grammar has no keyword form inside $(( )): a bare word is always `reference = identifier`, a variable name. true and false are builtin commands in kaish, not auto-bound session variables, so bare true is simply unset — pinned the spec's answer (an error naming true as unset) over the reviewer's expectation. Every other row (i64::MIN edges across every base, precedence towers, sign+base variable coercion, the parens-break-the-unary-exception case) already matched on the first run and needed no code change, which is itself the useful signal: they're now pinned so a future change that breaks one says so immediately instead of surfacing as a one-off bug report. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 112 ++++-- crates/kaish-kernel/src/parser.rs | 2 +- .../tests/arithmetic_adversarial_tests.rs | 323 ++++++++++++++++++ 3 files changed, 406 insertions(+), 31 deletions(-) create mode 100644 crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 1dba411f..e0bfdcf4 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -669,38 +669,80 @@ impl<'a> Tokenizer<'a> { } Some('(') => { self.advance(); // consume '(' - let remainder_start_byte = self.byte_pos(); - let remainder = &self.text[remainder_start_byte..]; - let toks = crate::lexer::tokenize(remainder).map_err(|_| { - ArithError::new( - format!("`{}` has no closing `)`", self.slice(dollar_start, self.pos)), - dollar_start..self.text.len(), - ) - })?; - let toks: Vec<(crate::lexer::Token, crate::parser::Span)> = toks - .into_iter() - .map(|sp| (sp.token, (sp.span.start..sp.span.end).into())) - .collect(); - let close = crate::parser::find_cmd_subst_close(&toks).ok_or_else(|| { - ArithError::new( + // A character-level scan, not a re-tokenize of the + // remainder: text after this substitution's own `)` is + // ARITHMETIC syntax (`+`, `**`, …), which kaish's general + // lexer does not tokenize at all outside `$(( ))` — handing + // it a remainder like "echo 1) + 2" made `lexer::tokenize` + // fail on the `+` before the close was ever found. Quotes + // are tracked (a literal `(`/`)` inside one does not count), + // matching the risk `extract_arithmetic` already accepts + // for `$((`'s own scan; a `\`-escaped quote is honored. + let cmd_start = self.pos; + let mut depth = 0i32; + let closed = loop { + match self.peek() { + None => break false, + Some('\\') => { + self.pos += 1; + if self.peek().is_some() { + self.pos += 1; + } + } + Some('\'') => { + self.pos += 1; + while matches!(self.peek(), Some(c) if c != '\'') { + self.pos += 1; + } + if self.peek() == Some('\'') { + self.pos += 1; + } else { + break false; + } + } + Some('"') => { + self.pos += 1; + loop { + match self.peek() { + None => break, + Some('\\') => { + self.pos += 1; + if self.peek().is_some() { + self.pos += 1; + } + } + Some('"') => { + self.pos += 1; + break; + } + Some(_) => self.pos += 1, + } + } + } + Some('(') => { + depth += 1; + self.pos += 1; + } + Some(')') => { + if depth > 0 { + depth -= 1; + self.pos += 1; + } else { + break true; + } + } + Some(_) => self.pos += 1, + } + }; + if !closed { + return Err(ArithError::new( format!("`{}` has no closing `)`", self.slice(dollar_start, self.pos)), - dollar_start..self.text.len(), - ) - })?; - let close_span = toks[close].1; - let close_start: usize = close_span.start; - let close_end: usize = close_span.end; - let cmd_text = &remainder[..close_start]; - // Advance past the command text plus its closing `)`. - let consumed_bytes = close_end; - let mut consumed_chars = 0usize; - let mut byte_count = 0usize; - while byte_count < consumed_bytes && self.pos + consumed_chars < self.chars.len() { - byte_count += self.chars[self.pos + consumed_chars].1.len_utf8(); - consumed_chars += 1; + dollar_start..self.byte_pos(), + )); } - self.pos += consumed_chars; - match crate::parser::parse(cmd_text) { + let cmd_text = self.slice(cmd_start, self.pos).to_string(); + self.pos += 1; // consume the ')' + match crate::parser::parse(&cmd_text) { Ok(program) => Ok(Expansion::CommandSubst(program.statements)), Err(_) => Err(ArithError::new( format!("syntax error in command substitution: $({cmd_text})"), @@ -1111,6 +1153,15 @@ pub(crate) fn apply_binary(op: BinOp, l: i64, r: i64) -> Result if r == 0 { return Err(ArithError::new(format!("`{l} % 0` divides by zero"), 0..0)); } + // `i64::checked_rem` returns `None` for `MIN % -1` too — it is + // defined in terms of the division, which overflows, even + // though the remainder itself (0, any divisor of ±1 divides + // evenly) always fits. Division by ±1 never has a remainder, + // for any `l`, so this is a real answer, not a special case + // bolted onto an edge — checked_rem is just wrong here. + if r == -1 { + return Ok(0); + } l.checked_rem(r).ok_or_else(|| overflow(l, op, r)) } BinOp::Pow => { @@ -1919,3 +1970,4 @@ mod tests { assert!(msg.contains("256"), "{msg}"); } } + diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index 114a57c5..7f8968c1 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -3462,7 +3462,7 @@ impl CmdSubstFrames { /// Plain-slice twin of [`cmd_subst_body_tokens`]'s live chumsky capture, used /// only by `validate_cmd_subst_bodies`'s error-path fallback — see that /// function's doc comment for why a second, non-chumsky scan exists at all. -pub(crate) fn find_cmd_subst_close(tokens: &[(Token, Span)]) -> Option { +fn find_cmd_subst_close(tokens: &[(Token, Span)]) -> Option { let mut tracker = CmdSubstFrames::default(); (0..tokens.len()).find(|&i| { let next = tokens.get(i + 1).map(|(t, _)| t); diff --git a/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs b/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs new file mode 100644 index 00000000..bae184e9 --- /dev/null +++ b/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs @@ -0,0 +1,323 @@ +//! Adversarial `$(( ))` cases two reviewers derived from the spec, meant to +//! catch a fresh implementation's blind spots — overflow at the exact +//! boundary, precedence combinations, and the coercion table's corners. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use kaish_kernel::{Kernel, KernelConfig}; + +mod common; + +async fn run(source: &str) -> (i64, String, String) { + let k = Kernel::new(KernelConfig::isolated()).expect("kernel"); + let r = k.execute(source).await.expect("kernel execute"); + (r.code, r.text_out().trim().to_string(), r.err.clone()) +} + +/// Every diagnostic a failing statement produces, however it refused. +async fn err_of(source: &str) -> String { + let k = Kernel::new(KernelConfig::isolated()).expect("kernel").into_arc(); + match k.execute(source).await { + Ok(r) => { + assert!(!r.ok(), "{source:?} should fail"); + format!("{}{}", r.text_out(), r.err) + } + Err(e) => format!("{e:?}"), + } +} + +async fn ok(source: &str, expected: &str) { + let (code, out, err) = run(source).await; + assert_eq!(code, 0, "{source:?} must run: {err:?}"); + assert_eq!(out, expected, "{source:?}"); +} + +async fn errs(source: &str, needle: &str) { + let text = err_of(source).await; + assert!(text.contains(needle), "{source:?}: expected {needle:?} in {text:?}"); +} + +// ── Bases, signs, and based-expansions ────────────────────────────────── + +#[tokio::test] +async fn signed_hex_and_based_literals_combine() { + ok("echo $(( - 16#Ff + + 0X10 ))", "-239").await; +} + +#[tokio::test] +async fn based_expansion_from_a_variable_and_a_command() { + ok("digits=7f; echo $((16#$digits + 1))", "128").await; + ok("echo $((8#$(echo 17) + 1))", "16").await; +} + +/// The spec's coercion table says a based-expansion's text is "digits only +/// (optional sign)" — a sign IS part of the accepted text, so +/// `16#$digits` with `digits` holding `-ff` is -255, not a refusal. A +/// reviewer expected an error naming `-16#ff` (the LITERAL sign-after-`#` +/// refusal, `16#-ff`), but that rule is about the sign appearing in SOURCE +/// TEXT after `#`; here the sign arrives inside the expansion's VALUE, +/// which the spec explicitly allows. Pinning the spec's answer. +/// +/// (Written with a quoted assignment — `digits=-ff` unquoted hits an +/// unrelated, pre-existing parser gap: a bareword assignment value that +/// starts with `-` is misparsed as a command, not this rewrite's doing.) +#[tokio::test] +async fn based_expansion_text_may_carry_a_sign() { + ok(r#"digits="-ff"; echo $((16#$digits))"#, "-255").await; +} + +#[tokio::test] +async fn based_expansion_needs_digits_after_hash() { + errs("echo $((2# 101))", "digits after").await; +} + +#[tokio::test] +async fn leading_zero_names_the_octal_fix() { + errs("echo $((077))", "8#77").await; +} + +#[tokio::test] +async fn whitespace_padded_signed_based_string_variable() { + ok(r#"x=' -16#F '; echo "$((x + 16#1))""#, "-14").await; +} + +#[tokio::test] +async fn default_expression_can_itself_be_a_base_literal() { + ok("echo $((${missing:-0Xf} + 1))", "16").await; +} + +#[tokio::test] +async fn plain_string_base_spelling_and_signed_whitespace_padded() { + ok(r#"var="16#10"; echo $(( var + 1 ))"#, "17").await; + ok(r#"v=" -16#Ff "; echo $(( v ))"#, "-255").await; +} + +#[tokio::test] +async fn command_output_hex_with_sign() { + ok(r#"echo $(( $(echo "-0x10") * 2 ))"#, "-32").await; +} + +// ── i64::MIN / i64::MAX boundary ──────────────────────────────────────── + +#[tokio::test] +async fn integral_float_at_min_converts() { + ok("x=$(fromjson -9223372036854775808.0); echo $((x))", "-9223372036854775808").await; +} + +#[tokio::test] +async fn min_literal_direct_unary_operand_every_base() { + ok("echo $(( - 9223372036854775808 ))", "-9223372036854775808").await; + ok("echo $((-0x8000000000000000))", "-9223372036854775808").await; +} + +#[tokio::test] +async fn min_magnitude_positive_is_out_of_range() { + errs("echo $((0X8000000000000000))", "64-bit").await; +} + +#[tokio::test] +async fn parens_break_the_direct_unary_minus_exception() { + errs("echo $((-(9223372036854775808)))", "64-bit").await; + errs("echo $((-(-9223372036854775808)))", "64-bit").await; +} + +#[tokio::test] +async fn max_literal_via_hex() { + ok("echo $((16#7fffffffffffffff))", "9223372036854775807").await; +} + +#[tokio::test] +async fn min_div_neg_one_overflows() { + errs("echo $(((-9223372036854775808) / (-1)))", "64-bit").await; +} + +/// `i64::checked_rem` returns `None` for `MIN % -1` too — it is defined via +/// the division, which overflows, even though the remainder (0; any +/// divisor of magnitude 1 divides evenly) always fits. This is a real bug +/// this test file caught: `apply_binary`'s `Rem` arm special-cases `r == +/// -1` now instead of trusting `checked_rem`. +#[tokio::test] +async fn min_rem_neg_one_is_zero() { + ok("echo $(((-9223372036854775808) % (-1)))", "0").await; +} + +#[tokio::test] +async fn power_just_past_the_boundary_overflows() { + errs("echo $((3037000500 ** 2))", "64-bit").await; +} + +#[tokio::test] +async fn negative_two_to_the_63_is_exactly_min() { + // (-2) ** 63 == i64::MIN exactly; checked_pow must reach it without a + // spurious intermediate overflow (verified directly against + // i64::checked_pow before trusting the arithmetic evaluator here). + ok("echo $((-2 ** 63))", "-9223372036854775808").await; +} + +#[tokio::test] +async fn one_past_min_magnitude_is_out_of_range() { + errs("echo $((-9223372036854775809))", "64-bit").await; +} + +#[tokio::test] +async fn min_times_neg_one_overflows() { + errs("echo $((-9223372036854775808 * -1))", "64-bit").await; +} + +#[tokio::test] +async fn min_magnitude_plus_one_literal_is_out_of_range() { + errs("echo $(( 9223372036854775808 - 1 ))", "64-bit").await; +} + +// ── Shifts ─────────────────────────────────────────────────────────────── + +#[tokio::test] +async fn arithmetic_right_shift_sign_extends() { + ok("echo $((-1 >> 63))", "-1").await; +} + +#[tokio::test] +async fn nested_arithmetic_as_a_shift_count() { + errs("echo $((1 << $((32 + 32))))", "0..=63").await; +} + +// ── Laziness beyond $(...) — the skipped branch must not even error ──── + +#[tokio::test] +async fn ternary_skipped_branch_does_not_divide_by_zero() { + ok("echo $(( 0 ? 1/0 : 42 ))", "42").await; +} + +#[tokio::test] +async fn and_or_skipped_side_command_substitution_does_not_run() { + let (code, out, err) = run("echo $((0 && $(echo nope >&2; echo 1)))").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "0"); + assert!(!err.contains("nope"), "the skipped $() must not run: {err:?}"); + + let (code, out, err) = run("echo $((1 || $(echo nope >&2; echo 1)))").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "1"); + assert!(!err.contains("nope"), "the skipped $() must not run: {err:?}"); +} + +#[tokio::test] +async fn nested_ternary_skips_both_unselected_command_substitutions() { + let (code, out, err) = + run("echo $((1 ? 0 : 1 ? $(echo nope >&2; echo 1) : $(echo bad >&2; echo 1)))").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "0"); + assert!(!err.contains("nope") && !err.contains("bad"), "{err:?}"); +} + +// ── Precedence combinations ────────────────────────────────────────────── + +#[tokio::test] +async fn power_binds_tighter_than_multiplication_with_nested_arithmetic() { + ok("echo $((3 * $((1 + 2)) ** 2))", "27").await; +} + +#[tokio::test] +async fn bitnot_binds_tighter_than_power() { + ok("echo $((~1 ** 3))", "-8").await; +} + +#[tokio::test] +async fn bitand_tighter_than_xor_tighter_than_or() { + ok("echo $(( 3 | 5 ^ 6 & 10 ))", "7").await; +} + +#[tokio::test] +async fn double_negative_power_towers() { + ok("echo $((-2 ** 3 ** 2))", "-512").await; +} + +#[tokio::test] +async fn division_and_modulo_toward_zero_and_dividend_sign() { + ok("echo $(( -7 / 3 + -7 % 3 ))", "-3").await; +} + +#[tokio::test] +async fn based_literal_in_a_sum() { + ok("echo $(( 2 + 3#10 ))", "5").await; +} + +// ── Not-a-token / structural garbage ───────────────────────────────────── + +#[tokio::test] +async fn zero_b_and_zero_o_name_the_kaish_spelling() { + errs("echo $((0b101))", "2#101").await; + errs("echo $((0o77))", "8#77").await; +} + +#[tokio::test] +async fn based_prefix_alone_has_no_digits() { + errs("echo $((16#))", "digits after").await; +} + +#[tokio::test] +async fn whitespace_splitting_the_hash_operator_is_an_error() { + assert!(!err_of("echo $((16 # ff))").await.is_empty()); +} + +#[tokio::test] +async fn a_bare_hash_cannot_start_a_value() { + assert!(!err_of("echo $((#12))").await.is_empty()); +} + +#[tokio::test] +async fn hash_is_never_a_comment_inside_arithmetic() { + assert!(!err_of("echo $((2#10 # comment))").await.is_empty()); +} + +#[tokio::test] +async fn missing_operands_are_errors() { + assert!(!err_of("echo $((1 + ))").await.is_empty()); + assert!(!err_of("echo $(( + ))").await.is_empty()); +} + +#[tokio::test] +async fn trailing_garbage_after_a_numeral_is_an_error() { + assert!(!err_of("echo $((1 + 2a))").await.is_empty()); + assert!(!err_of("echo $((12abc))").await.is_empty()); +} + +#[tokio::test] +async fn float_literal_names_integer_only() { + errs("echo $((12.0))", "integer").await; +} + +#[tokio::test] +async fn negative_exponent_names_the_fix() { + errs("echo $((2 ** -1))", "negative").await; +} + +#[tokio::test] +async fn power_overflow_names_the_limit() { + errs("echo $((2 ** 100))", "64-bit").await; +} + +#[tokio::test] +async fn shift_count_out_of_range_both_directions() { + assert!(!err_of("echo $((1 << -1))").await.is_empty()); + assert!(!err_of("echo $((1 << 64))").await.is_empty()); +} + +#[tokio::test] +async fn command_output_that_is_an_expression_is_refused() { + assert!(!err_of(r#"echo $(( $(echo "1 + 2") ))"#).await.is_empty()); +} + +// ── Bare identifiers are variables, not keywords ───────────────────────── + +/// A reviewer expected `true + false` to read as the boolean literals (1 + +/// 0 = 1). Under the spec's grammar, though, a bare word inside `$(( ))` +/// is always `reference = identifier` — a variable name — never a keyword; +/// only a variable that HOLDS a `Bool` value coerces through the +/// Int/Bool/Float/String/Null table. `true` and `false` are builtin +/// COMMANDS in kaish, not auto-bound session variables, so `true` here is +/// simply unset. Pinning the spec's answer (an error naming `true` as +/// unset) over the reviewer's expectation. +#[tokio::test] +async fn bare_true_false_are_variable_names_not_literals() { + errs("echo $(( true + false ))", "unset").await; +} From d848608b5e41d6436763752ac210e3ee5d60fc3a Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 10:04:46 -0400 Subject: [PATCH 05/15] changelog: $(( )) rewrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added and Changed bullets for the arithmetic rewrite — bases, the full operator set, (( )) as a command, and the deliberate bash divergences (overflow/unset/empty as errors, strings as values, lazy $(...) in a skipped branch). Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c58bd76c..ff577879 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,9 +15,17 @@ breaking entries are marked **BREAKING**. an `ExecContext` can spawn a child with the kernel's external-command discipline. `tools::DEFAULT_KILL_GRACE` is 2s. Only a struct literal changes. +- **Arithmetic diverges from bash on purpose** — overflow, an unset/empty + operand, and a leading zero are errors, never a wrap or 0; strings are + values, not expressions; a skipped `&&`/`||`/`?:` branch's `$(...)` never + runs. + ### Added - **`random` builtin** — `random [--min N] [--max N]` prints one uniformly chosen integer, typed; the default range is bash's `$RANDOM` (0 to 32767). +- **`$(( ))` reads another base** (`0x`, `base#digits`, `base#$var`) and + does checked 64-bit arithmetic with the full C operator set through `?:`; + `$(...)` is an operand; bare `(( expr ))` is a condition, like `[[ ]]`. - **Wrapped commands** (`kaish_kernel::tools::wrapped`, `subprocess` feature): register an external program as a tool with a declared grammar. Verbs and flags From d4a658944bc80328441da0bf1c899ec32072f65c Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 10:27:31 -0400 Subject: [PATCH 06/15] Arithmetic errors name the source they refuse x=5; echo $((x++)) named a hardcoded "name" placeholder instead of x, and the wrong fix shape for the compound and plain-assignment forms: reject_compound_or built its message from the operator symbol alone, never looking at the identifier the operator was actually attached to. The tokenizer already emits x as its own Ident token before seeing ++/+=/=, so the name is sitting right there in the token stream (out, the vec of tokens already produced) by the time the error fires - it was just never read. preceding_name reads it for the postfix/compound/plain forms (x++, x += 2, x = 2); consume_following_name scans forward for the prefix form (++x, where the name has not been tokenized yet). Each error now quotes the real matched source and substitutes the real name into the fix: x=$((x + 1)) for x++/++x, x=$((x - 1)) for x--/--x, x=$((x + rhs-source-text)) for x += rhs, and x=rhs, or `==` to compare for x = rhs. $((1 + )) and $(( + )) both fell back to the same "$(( )) has no expression" text the fully-empty $(( )) uses, which is misleading - the expression is not empty, an operand is missing after a real operator. left_assoc, parse_power's `**` handling, and parse_unary's four prefix operators now check whether the operator they just consumed was the last token, and if so raise a specific error naming the operator and (for a binary operator, which has a left side to show) the source text consumed so far: `+` has no right operand in `1 + `; add an integer expression after `+`. A bare unary operator with nothing after it (parsed with no left-hand context at all) gets the sibling wording without the "right"/"in {expr}" clause: `+` has no operand; add an integer expression after `+`. Parser now carries the arithmetic source text alongside its token stream so these messages can quote it. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 198 +++++++++++++++--- .../tests/arithmetic_adversarial_tests.rs | 4 +- crates/kaish-kernel/tests/arithmetic_tests.rs | 25 +++ 3 files changed, 193 insertions(+), 34 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index e0bfdcf4..5a181437 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -282,8 +282,16 @@ impl<'a> Tokenizer<'a> { TokKind::Bang } } - '+' => { self.advance(); self.reject_compound_or("+")?; TokKind::Op(BinOp::Add) } - '-' => { self.advance(); self.reject_compound_or("-")?; TokKind::Op(BinOp::Sub) } + '+' => { + self.advance(); + self.reject_compound_or('+', start_byte, &out)?; + TokKind::Op(BinOp::Add) + } + '-' => { + self.advance(); + self.reject_compound_or('-', start_byte, &out)?; + TokKind::Op(BinOp::Sub) + } '*' => { self.advance(); if self.peek() == Some('*') { @@ -337,12 +345,23 @@ impl<'a> Tokenizer<'a> { self.advance(); TokKind::Op(BinOp::Eq) } else { + let end = self.text.len(); + let rhs = self.text[self.byte_pos()..].trim(); + if let Some((name, name_start)) = Self::preceding_name(&out) { + let source = &self.text[name_start..end]; + return Err(ArithError::new( + format!( + "`{source}` assigns inside `$(( ))`; write `{name}={rhs}`, or `==` to compare" + ), + name_start..end, + )); + } + let source = &self.text[start_byte..end]; return Err(ArithError::new( format!( - "`{}` assigns inside `$(( ))`; write `name=$((rhs))`, or `==` to compare", - self.slice(start, self.pos) + "`{source}` assigns inside `$(( ))`; write `name=rhs`, or `==` to compare" ), - start..self.pos, + start_byte..end, )); } } @@ -385,27 +404,75 @@ impl<'a> Tokenizer<'a> { /// After consuming `+`/`-`, refuse `++`/`--`/`+=`/`-=` outright — kaish /// has no assignment or increment inside `$(( ))`. - fn reject_compound_or(&mut self, sym: &str) -> Result<(), ArithError> { - let start = self.pos - 1; - if self.peek() == Some(sym.chars().next().unwrap_or(' ')) { + /// The identifier that ends immediately before `op_start_byte` — the + /// `x` in `x++`/`x += 2`/`x = 2`, read from the token already emitted + /// (the operator has not been pushed onto `out` yet). + fn preceding_name(out: &[Tok]) -> Option<(String, usize)> { + match out.last() { + Some(Tok { kind: TokKind::Ident(name), span }) => Some((name.clone(), span.start)), + _ => None, + } + } + + /// The identifier starting at the current position — the `x` in + /// `++x`/`--x`, where the operator precedes the name. Consumes it: + /// this is only called on a path that is about to return `Err`, so + /// leaving `self.pos` past it does not affect anything further. + fn consume_following_name(&mut self) -> Option<(String, usize)> { + let start = self.pos; + if !matches!(self.peek(), Some(c) if c.is_ascii_alphabetic() || c == '_') { + return None; + } + while matches!(self.peek(), Some(c) if c.is_ascii_alphanumeric() || c == '_') { + self.pos += 1; + } + Some((self.slice(start, self.pos).to_string(), self.byte_pos())) + } + + /// After consuming `+`/`-`, refuse `++`/`--`/`+=`/`-=` outright — kaish + /// has no assignment or increment inside `$(( ))`. Names the real + /// identifier — from the token just emitted for `x++`/`x+=` (postfix), + /// or scanned forward for `++x` (prefix) — rather than a placeholder. + fn reject_compound_or(&mut self, sym: char, op_start_byte: usize, out: &[Tok]) -> Result<(), ArithError> { + let step = if sym == '+' { "+ 1" } else { "- 1" }; + if self.peek() == Some(sym) { self.advance(); - let name_hint = "name"; + let end = self.byte_pos(); + if let Some((name, name_start)) = Self::preceding_name(out) { + let source = &self.text[name_start..end]; + return Err(ArithError::new( + format!("`{source}` assigns inside `$(( ))`; write `{name}=$(({name} {step}))`"), + name_start..end, + )); + } + if let Some((name, name_end)) = self.consume_following_name() { + let source = &self.text[op_start_byte..name_end]; + return Err(ArithError::new( + format!("`{source}` assigns inside `$(( ))`; write `{name}=$(({name} {step}))`"), + op_start_byte..name_end, + )); + } + let source = &self.text[op_start_byte..end]; return Err(ArithError::new( - format!( - "`{}{sym}` assigns inside `$(( ))`; write `{name_hint} = {name_hint} {sym1} 1`", - sym, - sym1 = sym, - ), - start..self.pos, + format!("`{source}` assigns inside `$(( ))`; write `name=$((name {step}))`"), + op_start_byte..end, )); } if self.peek() == Some('=') { self.advance(); + let end = self.text.len(); + let rhs = self.text[self.byte_pos()..].trim(); + if let Some((name, name_start)) = Self::preceding_name(out) { + let source = &self.text[name_start..end]; + return Err(ArithError::new( + format!("`{source}` assigns inside `$(( ))`; write `{name}=$(({name} {sym} {rhs}))`"), + name_start..end, + )); + } + let source = &self.text[op_start_byte..end]; return Err(ArithError::new( - format!( - "`{sym}=` assigns inside `$(( ))`; write `name=$((name {sym} rhs))`", - ), - start..self.pos, + format!("`{source}` assigns inside `$(( ))`; write `name=$((name {sym} rhs))`"), + op_start_byte..end, )); } Ok(()) @@ -859,11 +926,38 @@ struct Parser { pos: usize, depth: usize, end: usize, + /// The arithmetic source, kept so a "no operand" error can quote the + /// text consumed so far (`{expr}` in the spec's error table). + text: String, } impl Parser { - fn new(toks: Vec, end: usize) -> Self { - Self { toks, pos: 0, depth: 0, end } + fn new(toks: Vec, end: usize, text: &str) -> Self { + Self { toks, pos: 0, depth: 0, end, text: text.to_string() } + } + + /// `` `{op}` has no right operand in `{expr}` `` — the operator was the + /// last token; `expr` is the whole source (trailing whitespace + /// included, as the spec's own example shows: "1 + " — this only fires + /// when the operator was the LAST token, so the whole text amounts to + /// "everything through end of input"). + fn missing_right_operand(&self, op: &str, op_span: &Range) -> ArithError { + ArithError::new( + format!( + "`{op}` has no right operand in `{}`; add an integer expression after `{op}`", + self.text + ), + op_span.clone(), + ) + } + + /// `` `{op}` has no operand `` — a unary/power operator with nothing at + /// all after it (no left operand to show, unlike the binary case). + fn missing_operand(&self, op: &str, op_span: &Range) -> ArithError { + ArithError::new( + format!("`{op}` has no operand; add an integer expression after `{op}`"), + op_span.clone(), + ) } fn peek(&self) -> Option<&TokKind> { @@ -894,15 +988,6 @@ impl Parser { self.depth -= 1; } - fn eat_op(&mut self, want: BinOp) -> bool { - if self.peek() == Some(&TokKind::Op(want)) { - self.pos += 1; - true - } else { - false - } - } - fn left_assoc( &mut self, ops: &[BinOp], @@ -912,7 +997,11 @@ impl Parser { loop { let matched = ops.iter().copied().find(|op| self.peek() == Some(&TokKind::Op(*op))); let Some(op) = matched else { break }; + let op_span = self.peek_span(); self.pos += 1; + if self.pos >= self.toks.len() { + return Err(self.missing_right_operand(op.symbol(), &op_span)); + } let right = next(self)?; left = ArithExpr::Binary { op, left: Box::new(left), right: Box::new(right) }; } @@ -986,7 +1075,12 @@ impl Parser { fn parse_power(&mut self) -> Result { let base = self.parse_unary()?; - if self.eat_op(BinOp::Pow) { + if self.peek() == Some(&TokKind::Op(BinOp::Pow)) { + let op_span = self.peek_span(); + self.pos += 1; + if self.pos >= self.toks.len() { + return Err(self.missing_right_operand("**", &op_span)); + } self.enter()?; let exp = self.parse_power()?; self.leave(); @@ -1005,7 +1099,12 @@ impl Parser { self.enter()?; let result = match self.peek() { Some(&TokKind::Op(BinOp::Sub)) => { + let op_span = self.peek_span(); self.pos += 1; + if self.pos >= self.toks.len() { + self.leave(); + return Err(self.missing_operand("-", &op_span)); + } if let Some(&TokKind::Number(mag)) = self.peek() { if mag == Self::MIN_MAGNITUDE { self.pos += 1; @@ -1017,16 +1116,31 @@ impl Parser { ArithExpr::Unary { op: UnOp::Neg, operand: Box::new(operand) } } Some(&TokKind::Op(BinOp::Add)) => { + let op_span = self.peek_span(); self.pos += 1; + if self.pos >= self.toks.len() { + self.leave(); + return Err(self.missing_operand("+", &op_span)); + } self.parse_unary()? } Some(&TokKind::Bang) => { + let op_span = self.peek_span(); self.pos += 1; + if self.pos >= self.toks.len() { + self.leave(); + return Err(self.missing_operand("!", &op_span)); + } let operand = self.parse_unary()?; ArithExpr::Unary { op: UnOp::Not, operand: Box::new(operand) } } Some(&TokKind::Tilde) => { + let op_span = self.peek_span(); self.pos += 1; + if self.pos >= self.toks.len() { + self.leave(); + return Err(self.missing_operand("~", &op_span)); + } let operand = self.parse_unary()?; ArithExpr::Unary { op: UnOp::BitNot, operand: Box::new(operand) } } @@ -1112,7 +1226,7 @@ pub(crate) fn parse(text: &str) -> Result { )); } let end = text.len(); - let mut parser = Parser::new(toks, end); + let mut parser = Parser::new(toks, end, text); let expr = parser.parse_conditional()?; if let Some(extra) = parser.peek() { if extra == &TokKind::RParen { @@ -1956,6 +2070,26 @@ mod tests { assert!(err("1, 2").contains("one expression")); } + #[test] + fn assignment_errors_name_the_real_tokens_not_a_placeholder() { + assert_eq!(err("x++"), "`x++` assigns inside `$(( ))`; write `x=$((x + 1))`"); + assert_eq!(err("++x"), "`++x` assigns inside `$(( ))`; write `x=$((x + 1))`"); + assert_eq!(err("x--"), "`x--` assigns inside `$(( ))`; write `x=$((x - 1))`"); + assert_eq!(err("--x"), "`--x` assigns inside `$(( ))`; write `x=$((x - 1))`"); + assert_eq!(err("x += 2"), "`x += 2` assigns inside `$(( ))`; write `x=$((x + 2))`"); + assert_eq!(err("x -= 3"), "`x -= 3` assigns inside `$(( ))`; write `x=$((x - 3))`"); + assert_eq!(err("x = 2"), "`x = 2` assigns inside `$(( ))`; write `x=2`, or `==` to compare"); + } + + #[test] + fn missing_operand_names_the_source_consumed_so_far() { + assert_eq!( + err("1 + "), + "`+` has no right operand in `1 + `; add an integer expression after `+`" + ); + assert_eq!(err(" + "), "`+` has no operand; add an integer expression after `+`"); + } + #[test] fn depth_cap() { let mut src = String::new(); diff --git a/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs b/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs index bae184e9..a0005baf 100644 --- a/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs +++ b/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs @@ -271,8 +271,8 @@ async fn hash_is_never_a_comment_inside_arithmetic() { #[tokio::test] async fn missing_operands_are_errors() { - assert!(!err_of("echo $((1 + ))").await.is_empty()); - assert!(!err_of("echo $(( + ))").await.is_empty()); + errs("echo $((1 + ))", "has no right operand").await; + errs("echo $(( + ))", "has no operand").await; } #[tokio::test] diff --git a/crates/kaish-kernel/tests/arithmetic_tests.rs b/crates/kaish-kernel/tests/arithmetic_tests.rs index 17602e59..87875fc6 100644 --- a/crates/kaish-kernel/tests/arithmetic_tests.rs +++ b/crates/kaish-kernel/tests/arithmetic_tests.rs @@ -122,6 +122,31 @@ async fn not_supported_table() { } } +/// Round-2 review: these error texts used to print a hardcoded `name`/`rhs` +/// placeholder (and the wrong shape for `+=`/`=`) instead of the tokens the +/// author actually typed. +#[tokio::test] +async fn assignment_errors_name_the_real_source() { + let text = err_of("echo $((x++))").await; + assert!(text.contains("`x++`") && text.contains("x=$((x + 1))"), "{text:?}"); + let text = err_of("echo $((++x))").await; + assert!(text.contains("`++x`") && text.contains("x=$((x + 1))"), "{text:?}"); + let text = err_of("echo $((x--))").await; + assert!(text.contains("`x--`") && text.contains("x=$((x - 1))"), "{text:?}"); + let text = err_of("echo $((x += 2))").await; + assert!(text.contains("`x += 2`") && text.contains("x=$((x + 2))"), "{text:?}"); + let text = err_of("echo $((x = 2))").await; + assert!(text.contains("`x = 2`") && text.contains("write `x=2`, or `==` to compare"), "{text:?}"); +} + +#[tokio::test] +async fn missing_operand_errors_are_specific() { + let text = err_of("echo $((1 + ))").await; + assert!(text.contains("has no right operand"), "{text:?}"); + let text = err_of("echo $(( + ))").await; + assert!(text.contains("has no operand"), "{text:?}"); +} + // ── Coercion table ────────────────────────────────────────────────────────── #[tokio::test] From 4797a8c189b526154af7c16f3c6660c7d1098956 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 10:30:43 -0400 Subject: [PATCH 07/15] The text after # is digits, whether typed or expanded $((08#17)) and $((010#5)) both read the base as decimal (8, 10) and ran, silently accepting a leading zero on the base itself - the 2..=36 range check never looked at how the base number was spelled, only its value. lex_number now checks the base's own source text for a leading zero before consuming '#', naming it: `08` is not a base spelling; write the base without a leading zero. d="-ff"; echo $((16#$d)) evaluated to -255. The literal form, 16#-ff, has always been refused (naming the fix -16#ff) because a sign after # puts the sign in the wrong place - it belongs outside the base prefix, negating the whole based number, not inside the digit run. based_value's coercion of an EXPANDED value never applied that same rule: it stripped a leading sign from the expansion's text and applied it, so the same "sign inside #" mistake succeeded whenever it arrived through a variable or $(...) instead of source text. There is one rule for the text after #, not two: digits only, whichever way they arrive. based_value now refuses a leading sign there too, and expansion_label names where the value came from for the message - `d` holds `-ff`; the digits after `#` take no sign; write `-16#ff` for a variable, `$(...)` printed `-ff`; ... for a command's output. (The coercion table's "digits only, optional sign" line describes a plain STRING's own coercion for a bare $((x)) - x="-ff" is still -255 - not a base# operand; the two share `base#digits` syntax but are different rules, and conflating them was the earlier mistake.) The adversarial test this surfaced in now asserts the refusal instead of -255. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 63 ++++++++++++++++--- crates/kaish-kernel/src/kernel.rs | 3 +- .../tests/arithmetic_adversarial_tests.rs | 20 +++--- crates/kaish-kernel/tests/arithmetic_tests.rs | 14 +++++ 4 files changed, 83 insertions(+), 17 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 5a181437..370cca41 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -636,6 +636,13 @@ impl<'a> Tokenizer<'a> { } if self.peek() == Some('#') { + let base_text = self.slice(start, self.pos); + if base_text.len() > 1 && base_text.starts_with('0') { + return Err(ArithError::new( + format!("`{base_text}` is not a base spelling; write the base without a leading zero"), + start..self.pos, + )); + } self.advance(); // consume '#' let base = base_mag as u32; if !(2..=36).contains(&base) { @@ -1525,17 +1532,46 @@ pub(crate) fn resolve_subscript_sync(scope: &Scope, root: &str, indices: &[i64]) value_to_arith(&value, root) } +/// The name and verb an error about `base#`'s VALUE uses to +/// describe where the value came from — `` `m` holds `08` `` versus +/// `` `$(...)` printed `08` ``, matching the phrasing the rest of the +/// coercion errors already use for a variable vs. a command's output. +pub(crate) fn expansion_label(e: &Expansion) -> (String, &'static str) { + match e { + Expansion::Var(name) => (name.clone(), "holds"), + Expansion::BracedPath { root, .. } | Expansion::BracedDefault { root, .. } => { + (root.clone(), "holds") + } + Expansion::LastExitCode => ("$?".to_string(), "holds"), + Expansion::CurrentPid => ("$$".to_string(), "holds"), + Expansion::CommandSubst(_) => ("$(...)".to_string(), "printed"), + Expansion::Nested(_) => ("$((...))".to_string(), "holds"), + } +} + /// Read `text` as digits in `base` — the evaluation half of `base#` /// (`2#$BITS`, `10#$(date +%m)`). `text` is the expansion's rendered VALUE, /// never re-coerced through the normal numeral rules first: that coercion is /// exactly what a leading-zero string (`m="08"`) needs `10#$m` to escape, so /// routing through it here would defeat the form's only purpose. -pub(crate) fn based_value(base: u32, text: &str) -> Result { +/// +/// A sign in `text` is refused, not applied — the same rule as the literal +/// form (`16#-ff` is refused, naming `-16#ff`): the digits after `#` take +/// no sign, whether the `#` came with the sign in source text or the sign +/// arrived inside an expansion's value. `label`/`verb` name where the value +/// came from (see [`expansion_label`]) for that refusal's message. +pub(crate) fn based_value(base: u32, text: &str, label: &str, verb: &str) -> Result { let trimmed = text.trim(); - let (neg, digits) = match trimmed.strip_prefix('-') { - Some(rest) => (true, rest), - None => (false, trimmed.strip_prefix('+').unwrap_or(trimmed)), - }; + if let Some(stripped) = trimmed.strip_prefix('-').or_else(|| trimmed.strip_prefix('+')) { + let sign = &trimmed[..1]; + return Err(ArithError::new( + format!( + "`{label}` {verb} `{text}`; the digits after `#` take no sign — write `{sign}{base}#{stripped}`" + ), + 0..0, + )); + } + let digits = trimmed; if digits.is_empty() { return Err(ArithError::new(format!("`{text}` has no digits"), 0..0)); } @@ -1558,7 +1594,7 @@ pub(crate) fn based_value(base: u32, text: &str) -> Result { .and_then(|m| m.checked_add(digit_val as u64)) .ok_or_else(|| ArithError::new(format!("`{text}` {INTEGER_OUT_OF_RANGE}"), 0..0))?; } - match int_from_magnitude(mag, neg, 0..0)? { + match int_from_magnitude(mag, false, 0..0)? { ArithExpr::Int(n) => Ok(n), _ => unreachable!(), } @@ -1644,7 +1680,8 @@ pub(crate) fn expansion_text_sync(e: &Expansion, scope: &Scope) -> Result Result { let text = expansion_text_sync(e, scope)?; - based_value(base, &text) + let (label, verb) = expansion_label(e); + based_value(base, &text, &label, verb) } pub(crate) fn eval_sync(expr: &ArithExpr, scope: &Scope) -> Result { @@ -2090,6 +2127,18 @@ mod tests { assert_eq!(err(" + "), "`+` has no operand; add an integer expression after `+`"); } + #[test] + fn a_leading_zero_base_is_refused() { + assert_eq!(err("08#17"), "`08` is not a base spelling; write the base without a leading zero"); + assert_eq!(err("010#5"), "`010` is not a base spelling; write the base without a leading zero"); + } + + #[test] + fn based_expansion_digits_take_no_sign() { + let msg = err_with("16#$d", |s| s.set("d", Value::String("-ff".to_string()))); + assert_eq!(msg, "`d` holds `-ff`; the digits after `#` take no sign — write `-16#ff`"); + } + #[test] fn depth_cap() { let mut src = String::new(); diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index 6045f1e4..632435b7 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -4995,7 +4995,8 @@ impl Kernel { } ArithExpr::BasedExpansion { base, expansion } => { let text = self.eval_arith_expansion_text_async(expansion).await?; - crate::arithmetic::based_value(*base, &text) + let (label, verb) = crate::arithmetic::expansion_label(expansion); + crate::arithmetic::based_value(*base, &text, &label, verb) .map_err(|e| anyhow::anyhow!("arithmetic error: {e}")) } ArithExpr::Unary { op, operand } => { diff --git a/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs b/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs index a0005baf..4939b9ea 100644 --- a/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs +++ b/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs @@ -49,20 +49,22 @@ async fn based_expansion_from_a_variable_and_a_command() { ok("echo $((8#$(echo 17) + 1))", "16").await; } -/// The spec's coercion table says a based-expansion's text is "digits only -/// (optional sign)" — a sign IS part of the accepted text, so -/// `16#$digits` with `digits` holding `-ff` is -255, not a refusal. A -/// reviewer expected an error naming `-16#ff` (the LITERAL sign-after-`#` -/// refusal, `16#-ff`), but that rule is about the sign appearing in SOURCE -/// TEXT after `#`; here the sign arrives inside the expansion's VALUE, -/// which the spec explicitly allows. Pinning the spec's answer. +/// One rule for the text after `#`, whether it is typed or expanded: the +/// digits take no sign. `16#-ff` (a sign in SOURCE text after `#`) was +/// always refused, naming `-16#ff`; `16#$digits` with `digits` holding +/// `-ff` used to accept the sign arriving through the expansion's VALUE +/// (round-2 review corrected an earlier reading of the coercion table's +/// "optional sign" — that clause describes a plain STRING's own coercion, +/// `x="-ff"` for a bare `$((x))`, not a based-expansion operand). Both +/// forms now refuse alike, naming the same fix. /// /// (Written with a quoted assignment — `digits=-ff` unquoted hits an /// unrelated, pre-existing parser gap: a bareword assignment value that /// starts with `-` is misparsed as a command, not this rewrite's doing.) #[tokio::test] -async fn based_expansion_text_may_carry_a_sign() { - ok(r#"digits="-ff"; echo $((16#$digits))"#, "-255").await; +async fn based_expansion_text_takes_no_sign() { + errs(r#"digits="-ff"; echo $((16#$digits))"#, "-16#ff").await; + errs(r#"echo $((16#$(printf -- "-ff")))"#, "-16#ff").await; } #[tokio::test] diff --git a/crates/kaish-kernel/tests/arithmetic_tests.rs b/crates/kaish-kernel/tests/arithmetic_tests.rs index 87875fc6..59c37817 100644 --- a/crates/kaish-kernel/tests/arithmetic_tests.rs +++ b/crates/kaish-kernel/tests/arithmetic_tests.rs @@ -147,6 +147,20 @@ async fn missing_operand_errors_are_specific() { assert!(text.contains("has no operand"), "{text:?}"); } +#[tokio::test] +async fn a_leading_zero_base_is_refused() { + for source in ["echo $((08#17))", "echo $((010#5))"] { + let text = err_of(source).await; + assert!(text.contains("without a leading zero"), "{source:?}: {text:?}"); + } +} + +#[tokio::test] +async fn based_expansion_digits_take_no_sign() { + let text = err_of(r#"d="-ff"; echo $((16#$d))"#).await; + assert!(text.contains("take no sign") && text.contains("-16#ff"), "{text:?}"); +} + // ── Coercion table ────────────────────────────────────────────────────────── #[tokio::test] From 5fd0c8ba86a5301809a6a7017aef5e130668f823 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 10:39:19 -0400 Subject: [PATCH 08/15] A plan's free variables come from the arithmetic parser, not a text scan read_arithmetic scanned $(( ))'s raw text for anything identifier- shaped and called each one a variable read. That is wrong wherever an identifier-looking substring isn't one: echo $((16#ff + 1)) reported free_variables: ["ff"], and $((0xff)) would report ["xff"] the same way - a base literal's own digits, not a variable. free_variables is an embedder contract (kaijutsu reads it to decide what a script needs before running it), so this was a wrong answer a consumer could act on, not a cosmetic one. Parses the text with the real arithmetic parser and walks the tree instead: every Ref name (bare or $-prefixed) is a read, including a bare subscript's root AND its index expression (xs[i] reads both xs and i - Decision B, the index is itself arithmetic, unlike ${xs[i]}, where i is a literal key and only xs is read). A based literal's own digits (16#ff, 0xff) contribute nothing - they were never a Ref to begin with. base#$var still reads var; $?/$$ are excluded the same way Expr::LastExitCode/CurrentPid already are elsewhere in this file, and so is a bare digit name ($1) - a positional parameter, not a session variable, matching Expr::Positional's exclusion. RANDOM and SECONDS are ordinary bare names here: planning is static and cannot know they will be unset at eval time, and a plan consumer may set them, so they are reported like any other read, not special-cased out. A $(...) operand's own reads ride along through collect_block, the same walker a bare $(...) already goes through outside arithmetic, so the two never drift apart by hand. A ${...}/${...:-...} operand reuses parse_varpath + the existing read_path, the same path ${x} takes everywhere else in this file. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/ast/plan.rs | 144 +++++++++++++++++++++++++--- 1 file changed, 133 insertions(+), 11 deletions(-) diff --git a/crates/kaish-kernel/src/ast/plan.rs b/crates/kaish-kernel/src/ast/plan.rs index ca621073..46661bc2 100644 --- a/crates/kaish-kernel/src/ast/plan.rs +++ b/crates/kaish-kernel/src/ast/plan.rs @@ -334,20 +334,81 @@ impl<'a> Collected<'a> { } } - /// Record every identifier in an arithmetic expression as a read. - /// kaish arithmetic is numbers, variables (bare or `${name}`), and - /// operators — an identifier token is always a variable. + /// Record every variable a `$(( ))` (or bare `(( ))`) reads: every + /// `Ref` name — bare or `$`-prefixed, a bare subscript's root AND its + /// index expression (`xs[i]` reads both `xs` and `i` — Decision B, the + /// index is itself arithmetic) — plus a `${...}`/`base#$var`/nested + /// `$((...))` operand's own reads, and a `$(...)` operand's reads (via + /// [`collect_block`], the same walker a bare `$(...)` already goes + /// through, so the two agree by construction rather than by two + /// implementations staying in sync by hand). `$?`/`$$`/a positional + /// parameter are not session variables, matching every other reader of + /// them in this file. Parses the text with the real arithmetic parser + /// rather than scanning for identifier-shaped substrings — the old + /// scan read `ff` out of `16#ff` and `xff` out of `0xff` as if they + /// were variables. A syntax error inside the text reads nothing: the + /// statement's own parse already failed loudly if this text was + /// invalid, so a plan never reaches an unparseable arithmetic body. fn read_arithmetic(&mut self, expr: &str) { - let mut name = String::new(); - for c in expr.chars() { - if c == '_' || c.is_ascii_alphabetic() || (!name.is_empty() && c.is_ascii_digit()) { - name.push(c); - } else if !name.is_empty() { - self.reads.insert(std::mem::take(&mut name)); + if let Ok(parsed) = crate::arithmetic::parse(expr) { + self.read_arith_expr(&parsed); + } + } + + fn read_arith_expr(&mut self, expr: &crate::arithmetic::ArithExpr) { + use crate::arithmetic::ArithExpr; + match expr { + ArithExpr::Int(_) => {} + ArithExpr::Expansion(e) => self.read_arith_expansion(e), + ArithExpr::Subscript { root, indices } => { + self.reads.insert(root.clone()); + for index in indices { + self.read_arith_expr(index); + } + } + ArithExpr::BasedExpansion { expansion, .. } => self.read_arith_expansion(expansion), + ArithExpr::Unary { operand, .. } => self.read_arith_expr(operand), + ArithExpr::Binary { left, right, .. } => { + self.read_arith_expr(left); + self.read_arith_expr(right); + } + ArithExpr::Ternary { cond, then_branch, else_branch } => { + self.read_arith_expr(cond); + self.read_arith_expr(then_branch); + self.read_arith_expr(else_branch); } } - if !name.is_empty() { - self.reads.insert(name); + } + + fn read_arith_expansion(&mut self, e: &crate::arithmetic::Expansion) { + use crate::arithmetic::Expansion; + match e { + // A bare `$1` is a positional parameter, not a session + // variable — same exclusion `collect_expr` applies to + // `Expr::Positional` below. + Expansion::Var(name) => { + if name.parse::().is_err() { + self.reads.insert(name.clone()); + } + } + Expansion::BracedPath { root, brackets } => { + let raw = format!("${{{root}{brackets}}}"); + self.read_path(&crate::parser::parse_varpath(&raw)); + } + Expansion::BracedDefault { root, brackets, default } => { + let raw = format!("${{{root}{brackets}}}"); + self.read_path(&crate::parser::parse_varpath(&raw)); + if let Ok(parsed) = crate::arithmetic::parse(default) { + self.read_arith_expr(&parsed); + } + } + Expansion::LastExitCode | Expansion::CurrentPid => {} + Expansion::CommandSubst(stmts) => { + let mut inner = Collected::default(); + collect_block(stmts, false, &mut inner); + self.reads.extend(inner.reads); + } + Expansion::Nested(inner) => self.read_arith_expr(inner), } } } @@ -1284,6 +1345,67 @@ mod tests { assert!(plan.bound_variables.is_empty()); } + // ── Arithmetic reads via the real parser, not a text scan ── + // + // The scan used to treat any identifier-shaped substring as a + // variable, so a base literal's own digits (`ff` in `16#ff`, `xff` in + // `0xff`) were reported as free variables `get_var` can never resolve + // — a plan consumer (kaijutsu) reading this to decide what a script + // needs before running it got a wrong answer, not a cosmetic one. + + #[test] + fn a_based_literal_reads_nothing() { + assert!(plan_of("echo $((16#ff + 1))").free_variables.is_empty()); + } + + #[test] + fn a_hex_literal_reads_nothing_but_a_real_operand_still_does() { + assert_eq!(plan_of("echo $((0xff + x))").free_variables, vec!["x"]); + } + + #[test] + fn based_expansion_reads_the_variable_not_the_base() { + assert_eq!(plan_of("echo $((10#$m % 12))").free_variables, vec!["m"]); + } + + #[test] + fn ternary_reads_both_branches_deduped_and_sorted() { + assert_eq!(plan_of("echo $((a > b ? a : b))").free_variables, vec!["a", "b"]); + } + + #[test] + fn a_bare_subscript_reads_the_root_and_the_index_variable() { + // Decision B: `xs[i]` reads `i` as a variable (the index is itself + // arithmetic) as well as `xs` — unlike `${xs[i]}`, where `i` is a + // literal key and only `xs` is read. + assert_eq!(plan_of("echo $((xs[i] + 1))").free_variables, vec!["i", "xs"]); + assert_eq!(plan_of("echo ${xs[i]}").free_variables, vec!["xs"]); + } + + #[test] + fn last_exit_code_and_pid_are_not_session_variables() { + assert!(plan_of("echo $(($? + $$))").free_variables.is_empty()); + } + + #[test] + fn random_and_seconds_are_free_variables_like_any_other_name() { + // Planning is static — it cannot know RANDOM/SECONDS will be + // unset at eval time, and a plan consumer may set them, so they + // are reported exactly like any other bare name. + assert_eq!(plan_of("echo $((RANDOM % 10))").free_variables, vec!["RANDOM"]); + } + + #[test] + fn command_substitution_inside_arithmetic_contributes_its_own_reads() { + assert_eq!(plan_of("echo $((1 + $(echo $y)))").free_variables, vec!["y"]); + } + + #[test] + fn a_bare_arith_condition_reads_like_any_other_arithmetic() { + let plan = plan_of("while (( i <= n )); do :; done"); + assert_eq!(plan.free_variables, vec!["i", "n"]); + } + #[test] fn a_subscripted_assignment_binds_the_root_and_reads_the_subscript() { let plan = plan_of("counts[$key]=1"); From 250ec008f27f83ff4b9065c576f15c808cd0892d Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 10:54:36 -0400 Subject: [PATCH 09/15] Arithmetic passes clippy and rustdoc under -D warnings consume_digits and consume_decimal_digits (arithmetic.rs:495,544) both wrote loop { let Some(c) = self.peek() else { break }; ... }, which clippy::while_let_loop flags as a while-let loop wearing a loop's clothes; rewritten as while let Some(c) = self.peek() { ... } with no behavior change (the body is identical, just no longer re-testing a break condition the while-let already expresses). The sign-after-# diagnostic (arithmetic.rs:655) matched matches!(self.peek(), Some('+') | Some('-')) and then called self.peek().unwrap() to get the char back, which clippy::unwrap_used denies even though the unwrap can't panic here - the guard makes it provably safe, which is exactly the case the lint can't see and the project's house rule doesn't special-case. Restructured so the value is in hand from the match itself: if let Some(sign @ ('+' | '-')) = self.peek() binds sign directly, no unwrap needed. extract_arithmetic (lexer.rs:1974) had grown to 9 parameters as this branch added marker_len and is_condition to support bare (( )) - one over clippy::too_many_arguments' limit of 7. The three scanner output buffers (out, arithmetics, replacements) it appends to are always passed together at all three call sites, so they group naturally: a new ScanBuffers<'a> struct holds all three behind one &mut, dropping the signature to 7. Rustdoc (-D warnings) failed separately: crates/kaish-kernel/src/ arithmetic.rs's module doc linked [`tokenize`], [`Tok`], [`parse`], [`ArithExpr`], [`eval_sync`] - all private items, so `cargo doc` (public docs only) can't resolve them. Swept every `///`/`//!` this branch added (grep for `[\`` and `](` across every touched file) for the same shape: a handful more in arithmetic.rs, kernel.rs, interpreter/eval.rs, and ast/plan.rs linked private methods the same way. None of those broke the gate (the containing items are private too, so rustdoc never rendered them), but they were the same mistake waiting to surface if visibility ever changed, so rewritten as plain code spans throughout rather than left half-fixed. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 21 ++++----- crates/kaish-kernel/src/ast/plan.rs | 2 +- crates/kaish-kernel/src/interpreter/eval.rs | 2 +- crates/kaish-kernel/src/kernel.rs | 14 +++--- crates/kaish-kernel/src/lexer.rs | 48 +++++++++++++-------- 5 files changed, 49 insertions(+), 38 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 370cca41..8890a212 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -2,8 +2,8 @@ //! reading. //! //! Three stages, kept separate so each can be tested on its own: -//! [`tokenize`] (text → [`Tok`]), [`parse`] ([`Tok`] → [`ArithExpr`]), and -//! evaluation ([`eval_sync`] for a scope with no `$(...)` reachable, and +//! `tokenize` (text → `Tok`), `parse` (`Tok` → `ArithExpr`), and +//! evaluation (`eval_sync` for a scope with no `$(...)` reachable, and //! `Kernel::eval_arith_async` in `kernel.rs` for the general case). //! //! Supports: decimal/hex/`base#digits` literals (base 2..=36), the full C @@ -94,7 +94,7 @@ pub(crate) enum UnOp { /// A `$(...)`/`${...}`/`$name`/`$?`/`$$`/`$((...))` operand, still /// unresolved. Evaluating one is the only place `$(( ))` needs the async -/// evaluator — everything else in [`ArithExpr`] is pure. +/// evaluator — everything else in `ArithExpr` is pure. #[derive(Debug, Clone, PartialEq)] pub(crate) enum Expansion { /// Bare `x` or `$x` — the whole value. @@ -492,8 +492,7 @@ impl<'a> Tokenizer<'a> { fn consume_digits(&mut self, base: u32, lit_start: usize) -> Result<(u64, usize), ArithError> { let digits_start = self.pos; let mut mag: u64 = 0; - loop { - let Some(c) = self.peek() else { break }; + while let Some(c) = self.peek() { if c == '_' { return Err(ArithError::new( format!("`{}` contains `_`; remove it", self.slice(lit_start, self.pos + 1)), @@ -534,15 +533,14 @@ impl<'a> Tokenizer<'a> { } /// Consume a run of plain `0`-`9` digits, erroring loud on `_`. Unlike - /// [`Self::consume_digits`], a non-digit letter (`e`, `x`, …) is a clean + /// `Self::consume_digits`, a non-digit letter (`e`, `x`, …) is a clean /// stop, not an error — the base-10 run is used both as a full decimal /// literal and as the base number before `#`, and the caller decides /// what a trailing `e3`/`.5`/`#` means. fn consume_decimal_digits(&mut self, lit_start: usize) -> Result<(u64, usize), ArithError> { let digits_start = self.pos; let mut mag: u64 = 0; - loop { - let Some(c) = self.peek() else { break }; + while let Some(c) = self.peek() { if c == '_' { return Err(ArithError::new( format!("`{}` contains `_`; remove it", self.slice(lit_start, self.pos + 1)), @@ -651,8 +649,7 @@ impl<'a> Tokenizer<'a> { start..self.pos, )); } - if matches!(self.peek(), Some('+') | Some('-')) { - let sign = self.peek().unwrap(); + if let Some(sign @ ('+' | '-')) = self.peek() { let sign_start = self.pos; self.pos += 1; while matches!(self.peek(), Some(c) if c.is_ascii_alphanumeric()) { @@ -699,7 +696,7 @@ impl<'a> Tokenizer<'a> { Ok(TokKind::Expansion(self.lex_expansion_body()?)) } - /// Same as [`Self::lex_dollar`] but returning the bare [`Expansion`], + /// Same as `Self::lex_dollar` but returning the bare `Expansion`, /// for `base#$name` and `base#$(...)`. fn lex_expansion_body(&mut self) -> Result { let dollar_start = self.pos; @@ -1559,7 +1556,7 @@ pub(crate) fn expansion_label(e: &Expansion) -> (String, &'static str) { /// form (`16#-ff` is refused, naming `-16#ff`): the digits after `#` take /// no sign, whether the `#` came with the sign in source text or the sign /// arrived inside an expansion's value. `label`/`verb` name where the value -/// came from (see [`expansion_label`]) for that refusal's message. +/// came from (see `expansion_label`) for that refusal's message. pub(crate) fn based_value(base: u32, text: &str, label: &str, verb: &str) -> Result { let trimmed = text.trim(); if let Some(stripped) = trimmed.strip_prefix('-').or_else(|| trimmed.strip_prefix('+')) { diff --git a/crates/kaish-kernel/src/ast/plan.rs b/crates/kaish-kernel/src/ast/plan.rs index 46661bc2..2aa8d876 100644 --- a/crates/kaish-kernel/src/ast/plan.rs +++ b/crates/kaish-kernel/src/ast/plan.rs @@ -339,7 +339,7 @@ impl<'a> Collected<'a> { /// index expression (`xs[i]` reads both `xs` and `i` — Decision B, the /// index is itself arithmetic) — plus a `${...}`/`base#$var`/nested /// `$((...))` operand's own reads, and a `$(...)` operand's reads (via - /// [`collect_block`], the same walker a bare `$(...)` already goes + /// `collect_block`, the same walker a bare `$(...)` already goes /// through, so the two agree by construction rather than by two /// implementations staying in sync by hand). `$?`/`$$`/a positional /// parameter are not session variables, matching every other reader of diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index 88ade16f..9f7d4324 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -309,7 +309,7 @@ impl<'a> Evaluator<'a> { } /// Evaluate a bare `(( expr ))` condition: true when the value is - /// nonzero. The sibling of [`Self::eval_test`], same coercion as + /// nonzero. The sibling of `Self::eval_test`, same coercion as /// `$(( ))` (`eval_arithmetic` above) — only the truthiness wrapper /// differs. fn eval_arith_cond(&mut self, expr_str: &str) -> EvalResult { diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index 632435b7..ff776afe 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -4956,7 +4956,7 @@ impl Kernel { /// Evaluate `$(( text ))`'s content. Takes the sync fast path /// (`arithmetic::eval_sync` under one scope read lock) when no `$(...)` /// is reachable in the parsed tree; otherwise walks it with - /// [`Self::eval_arith_expr_async`], which can run a `$(...)` operand and + /// `Self::eval_arith_expr_async`, which can run a `$(...)` operand and /// never runs one on the unselected side of `&&`/`||`/`?:`. async fn eval_arithmetic_async(&self, text: &str) -> Result { let ast = crate::arithmetic::parse(text).map_err(|e| anyhow::anyhow!("arithmetic error: {e}"))?; @@ -4969,11 +4969,11 @@ impl Kernel { } /// Async recursive walk over a parsed `$(( ))` tree. Boxed for the same - /// reason as [`Self::eval_expr_async`]: the recursion is unbounded by + /// reason as `Self::eval_expr_async`: the recursion is unbounded by /// the type system, so a fixed-depth stack frame can't hold it. Each /// leaf takes its own short `self.scope` read lock rather than one held /// across the whole walk — `Expansion::CommandSubst` runs through - /// [`Self::execute_block_capturing`], which takes its own scope lock + /// `Self::execute_block_capturing`, which takes its own scope lock /// internally, so a lock held here across that await would deadlock. fn eval_arith_expr_async<'a>( &'a self, @@ -5088,8 +5088,8 @@ impl Kernel { } /// The expansion's rendered VALUE, for `base#` — mirrors - /// [`crate::arithmetic::expansion_text_sync`], async so `$(...)` can - /// run for real. Never routes through [`Self::eval_arith_expansion_async`] + /// `crate::arithmetic::expansion_text_sync`, async so `$(...)` can + /// run for real. Never routes through `Self::eval_arith_expansion_async` /// (the arithmetically-coerced form): that coercion refuses a leading /// zero, which is exactly what `10#$m`/`10#$(date +%m)` exist to escape. fn eval_arith_expansion_text_async<'a>( @@ -5171,9 +5171,9 @@ impl Kernel { } /// Run a `$(...)` operand and return its printed text — the shared half - /// of [`Self::run_arith_command_subst`] (a bare operand, coerced to an + /// of `Self::run_arith_command_subst` (a bare operand, coerced to an /// integer) and the `base#$(...)` case (the text is read as digits in a - /// base, never coerced first — see [`crate::arithmetic::based_value`]). + /// base, never coerced first — see `crate::arithmetic::based_value`). async fn run_arith_command_subst_text(&self, stmts: &[Stmt]) -> Result { let saved_scope = Box::new(self.scope.read().await.clone()); let saved_ec = { diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 725e8d30..63dc8e4b 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -1662,9 +1662,11 @@ fn scan(source: &str) -> Result> { &mut i, dpos, total_len, - &mut out, - &mut arithmetics, - &mut replacements, + &mut ScanBuffers { + out: &mut out, + arithmetics: &mut arithmetics, + replacements: &mut replacements, + }, 3, false, )?; @@ -1742,9 +1744,11 @@ fn scan(source: &str) -> Result> { &mut i, pos, total_len, - &mut out, - &mut arithmetics, - &mut replacements, + &mut ScanBuffers { + out: &mut out, + arithmetics: &mut arithmetics, + replacements: &mut replacements, + }, 3, false, )?; @@ -1760,9 +1764,11 @@ fn scan(source: &str) -> Result> { &mut i, pos, total_len, - &mut out, - &mut arithmetics, - &mut replacements, + &mut ScanBuffers { + out: &mut out, + arithmetics: &mut arithmetics, + replacements: &mut replacements, + }, 2, true, )?; @@ -1971,14 +1977,22 @@ fn copy_substitution_verbatim(chars: &[(usize, char)], i: &mut usize, out: &mut } } +/// The scanner buffers `extract_arithmetic` appends to, grouped so the +/// call carries one thing instead of three — `scan`'s own locals (`out`, +/// `arithmetics`, `replacements`) still own the data; this just borrows +/// all three for the one call. +struct ScanBuffers<'a> { + out: &'a mut String, + arithmetics: &'a mut Vec<(String, String, bool)>, + replacements: &'a mut Vec, +} + fn extract_arithmetic( chars: &[(usize, char)], i: &mut usize, start_pos: usize, total_len: usize, - out: &mut String, - arithmetics: &mut Vec<(String, String, bool)>, - replacements: &mut Vec, + buffers: &mut ScanBuffers<'_>, marker_len: usize, is_condition: bool, ) -> Result<(), Spanned> { @@ -2038,15 +2052,15 @@ fn extract_arithmetic( let end_pos = if *i < n { chars[*i].0 } else { total_len }; let marker = format!("__KAISH_ARITH_{}__", unique_marker_id()); - replacements.push(Replacement { + buffers.replacements.push(Replacement { orig_start: start_pos, orig_len: end_pos - start_pos, - new_start: out.len(), + new_start: buffers.out.len(), new_len: marker.len(), - kind: ReplacementKind::Arith(arithmetics.len()), + kind: ReplacementKind::Arith(buffers.arithmetics.len()), }); - arithmetics.push((marker.clone(), expr, is_condition)); - out.push_str(&marker); + buffers.arithmetics.push((marker.clone(), expr, is_condition)); + buffers.out.push_str(&marker); Ok(()) } From 4e12017435e02ded9fb697cbad8c954073667d61 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 11:08:14 -0400 Subject: [PATCH 10/15] A float at 2^63 is out of range, and a default may hold a command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit value_to_arith's Float branch checked `*f > i64::MAX as f64`, but i64::MAX (2^63 - 1) has no exact f64 representation this close to the limit and rounds UP to 2^63 when cast — the same value 2^63 itself is. A strict `>` against that rounded bound let a Float holding exactly 2^63 through as in-range, and the saturating `as i64` cast then silently answered i64::MAX instead of refusing. The suggested round-trip fix (cast to i64, cast back, compare) turns out not to catch this specific input either: i64::MAX and 2^63 collide to the same f64 bit pattern near this magnitude (verified directly against rustc), so the saturated i64::MAX round-trips right back to 2^63 and looks equal. The actual fix is the boundary itself: compare against the literal 9_223_372_036_854_775_808.0 (2^63, exactly representable) with `>=` instead of `>` against the rounded i64::MAX. Any float this large cannot distinguish i64::MAX from one past it, so refusing the whole ambiguous boundary is correct, not just convenient — there is no way to recover which one the caller meant. contains_command_subst treated Expansion::BracedDefault as never containing a `$(...)`, so `${x:-$(cmd)}` inside `$(( ))` picked the sync fast path and failed with "needs the async evaluator" - the default's text is unparsed at that point, and the check never looked inside it. expansion_has now parses the default and asks the same question of the result. That surfaced a second bug once the async path was actually reached: BracedDefault's fallback evaluated the default text as a full arithmetic expression unconditionally, so `${m:-$(echo 08)}` used as a base# operand ran the command through the same leading-zero refusal a bare arithmetic operand gets - defeating the whole point of base# reading raw digit text. A default that is itself a single expansion now stays in TEXT mode (matching `expansion_text_sync`'s existing `$var`/`$(...)` handling) and only gets evaluated as a real expression when it actually contains operators (`${x:-1 + 2}`). needs_async's message named itself ("`$(...)` needs the async evaluator") - an internal detail, not something a user should read. The fix above makes it unreachable from every path this crate wires up (contains_command_subst now routes anything holding a `$(...)`, including inside a default, to the async walker before eval_sync ever runs); it stays reachable only if some future caller invokes eval_sync directly on such a tree without that check. Reworded to match EvalError::NoExecutor's existing wording for the identical situation elsewhere in the interpreter, with a comment explaining why it should no longer fire. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 64 ++++++++++++++++--- crates/kaish-kernel/src/kernel.rs | 14 +++- crates/kaish-kernel/tests/arithmetic_tests.rs | 30 +++++++++ 3 files changed, 98 insertions(+), 10 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 8890a212..7bc9b1cd 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -138,8 +138,15 @@ impl ArithExpr { match e { Expansion::CommandSubst(_) => true, Expansion::Nested(inner) => inner.contains_command_subst(), - Expansion::BracedDefault { .. } - | Expansion::Var(_) + // The default is unparsed text at this point (parsing + // happens only if it is actually reached, at eval time) — + // parse it here just to answer the question. A parse + // failure changes nothing: eval will hit the identical + // parse error on either path. + Expansion::BracedDefault { default, .. } => parse(default) + .map(|parsed| parsed.contains_command_subst()) + .unwrap_or(false), + Expansion::Var(_) | Expansion::BracedPath { .. } | Expansion::LastExitCode | Expansion::CurrentPid => false, @@ -1445,7 +1452,15 @@ pub(crate) fn value_to_arith(value: &Value, name: &str) -> Result { if !f.is_finite() || f.fract() != 0.0 { Err(ArithError::new(format!("`{name}` holds `{f}`; arithmetic is integer-only"), 0..0)) - } else if *f < i64::MIN as f64 || *f > i64::MAX as f64 { + // The upper bound is the literal 2^63, not `i64::MAX as f64`: + // i64::MAX (2^63 - 1) has no exact f64 representation at this + // magnitude, so casting it to f64 ALSO rounds up to 2^63 — a + // strict `>` against that rounded value let `f == 2^63` + // through, and the saturating `as i64` below silently + // returned i64::MAX. A float this large cannot distinguish + // i64::MAX from one past it, so `>=` refuses the whole + // ambiguous boundary instead of guessing. + } else if *f < i64::MIN as f64 || *f >= 9_223_372_036_854_775_808.0 { Err(ArithError::new(format!("`{name}` holds `{f}`, outside the 64-bit range"), 0..0)) } else { Ok(*f as i64) @@ -1603,8 +1618,16 @@ pub(crate) fn based_value(base: u32, text: &str, label: &str, verb: &str) -> Res // is expected not to call this when `contains_command_subst()` is true. // ═══════════════════════════════════════════════════════════════════ +/// `contains_command_subst()` routes a tree holding this to the async +/// walker before eval_sync ever runs, so this is reachable only when a +/// caller invokes the sync evaluator directly without that check — the +/// message matches `EvalError::NoExecutor`'s wording for the same +/// situation elsewhere in the interpreter, not an internal name. fn needs_async(what: &str) -> ArithError { - ArithError::new(format!("`{what}` needs the async evaluator"), 0..0) + ArithError::new( + format!("`{what}` must be resolved by the async evaluator before sync evaluation"), + 0..0, + ) } fn resolve_expansion_sync(e: &Expansion, scope: &Scope) -> Result { @@ -1661,10 +1684,16 @@ pub(crate) fn expansion_text_sync(e: &Expansion, scope: &Scope) -> Result { - let default_expr = parse(default)?; - Ok(eval_sync(&default_expr, scope)?.to_string()) - } + // A default that is itself a single expansion (`$(cmd)`, + // `$var`, …) stays in TEXT mode — `10#${m:-$(date +%m)}` + // needs the same "read raw digits" treatment `10#$m` gets, + // not the leading-zero refusal a full arithmetic operand + // would apply. A default with real operators (`1 + 2`) is + // genuinely an expression and is evaluated as one. + Some(Value::Null) | None => match parse(default)? { + ArithExpr::Expansion(e) => expansion_text_sync(&e, scope), + default_expr => Ok(eval_sync(&default_expr, scope)?.to_string()), + }, Some(v) => Ok(value_to_string(&v)), } } @@ -1974,6 +2003,25 @@ mod tests { assert_eq!(eval_with("x + 1", |s| s.set("x", Value::Float(100.0))), 101); } + #[test] + fn float_at_2_63_is_out_of_range() { + // i64::MAX has no exact f64 representation and rounds UP to 2^63 + // when cast — the same rounding that makes 2^63 itself look like + // it fits if the bound is compared as `i64::MAX as f64`. + let msg = err_with("x", |s| s.set("x", Value::Float(9223372036854775808.0))); + assert!(msg.contains("64-bit"), "{msg}"); + } + + #[test] + fn float_at_min_still_converts() { + assert_eq!(eval_with("x", |s| s.set("x", Value::Float(-9223372036854775808.0))), i64::MIN); + } + + #[test] + fn negative_zero_float_converts_to_zero() { + assert_eq!(eval_with("x", |s| s.set("x", Value::Float(-0.0))), 0); + } + #[test] fn string_value_is_parsed() { assert_eq!(eval_with("x", |s| s.set("x", Value::String("0xff".to_string()))), 255); diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index ff776afe..ad7d739e 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -5134,11 +5134,21 @@ impl Kernel { } }; match resolved { + // Stays in TEXT mode when the default is itself a + // single expansion — see the sync twin, + // `expansion_text_sync`, for why. Some(Value::Null) | None => { let default_expr = crate::arithmetic::parse(default) .map_err(|e| anyhow::anyhow!("arithmetic error: {e}"))?; - let n = self.eval_arith_expr_async(&default_expr).await?; - Ok(n.to_string()) + match default_expr { + crate::arithmetic::ArithExpr::Expansion(e) => { + self.eval_arith_expansion_text_async(&e).await + } + default_expr => { + let n = self.eval_arith_expr_async(&default_expr).await?; + Ok(n.to_string()) + } + } } Some(value) => Ok(value_to_string(&value)), } diff --git a/crates/kaish-kernel/tests/arithmetic_tests.rs b/crates/kaish-kernel/tests/arithmetic_tests.rs index 59c37817..a6c4b914 100644 --- a/crates/kaish-kernel/tests/arithmetic_tests.rs +++ b/crates/kaish-kernel/tests/arithmetic_tests.rs @@ -172,6 +172,22 @@ async fn coercion_int_bool_float() { ok("x=$(fromjson 1e10); echo $((x))", "10000000000").await; } +/// Round-5 review: `i64::MAX as f64` rounds up to 2^63 (i64::MAX has no +/// exact f64 representation this close to the limit), so a strict `>` +/// against that rounded bound let a Float holding exactly 2^63 through, +/// and the saturating cast then silently answered i64::MAX. +#[tokio::test] +async fn float_at_the_64_bit_boundary() { + let text = err_of("x=$(fromjson 9223372036854775808.0); echo $((x))").await; + assert!(text.contains("64-bit"), "{text:?}"); + ok( + "x=$(fromjson -9223372036854775808.0); echo $((x))", + "-9223372036854775808", + ) + .await; + ok("x=$(fromjson -0.0); echo $((x))", "0").await; +} + #[tokio::test] async fn coercion_float_errors() { for source in ["x=2.7; echo $((x))", "x=$(fromjson 1e20); echo $((x))"] { @@ -298,6 +314,20 @@ async fn based_expansion_from_a_command() { ok(r#"echo $((10#$(printf 08)))"#, "8").await; } +/// Round-5 review: `${x:-$(cmd)}` took the sync fast path and failed with +/// an internal "needs the async evaluator" message — `contains_command_subst` +/// never looked inside a default's text for a `$(...)`. +#[tokio::test] +async fn a_default_may_hold_a_command_substitution() { + ok("echo $(( ${x:-$(echo 5)} + 1 ))", "6").await; + ok("x=3; echo $(( ${x:-$(echo 5)} + 1 ))", "4").await; + // A based-expansion's default stays in TEXT mode (like `10#$var` + // already does), not the arithmetic operand's leading-zero refusal — + // `08` from the fallback command reads as decimal, same as if `m` + // held it directly. + ok(r#"echo $((10#${m:-$(echo 08)}))"#, "8").await; +} + // ── Nesting ─────────────────────────────────────────────────────────────── #[tokio::test] From 848fb5fb64288b04515cd663d8ab94e1abfe0b75 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 11:12:51 -0400 Subject: [PATCH 11/15] Tests for the power special cases, bytes, and (( $(...) )) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coverage gaps review found: 0 ** 0 = 1, 0 ** 5 = 0, 1 ** 999 = 1, and (-1) ** 3 = -1 with and without parens (unary binds tighter than **, same rule -2 ** 2 = 4 already pins) — all already correct, now pinned. 2 ** 4294967296 (one past the u32 exponent cap) already named the 64-bit limit. A Value::Bytes variable (head -c off the synthetic /dev/urandom, no localfs/subprocess feature needed) already refused — coercion_list_record_bytes claimed to cover this in its own name and didn't. Bare (( $(echo 3) > 2 )) already exits 0 - the condition form needed the same async-path fix $(( )) as a value already got. Corrected a false claim in read_arithmetic's doc comment: it said an unparsable arithmetic body could not reach a plan because the statement's own parse would have already failed. It doesn't - the shell parser and validator both defer arithmetic to runtime, so a syntactically-valid statement with a broken $(( )) body reaches the plan walk fine. Corrected to say what actually happens: no free variables from that expression, and the statement itself fails loudly when it runs. Trimmed three comments per CLAUDE.md's no-narrative rule: the $(...) scan's history (why it isn't a re-tokenize) down to the rule itself; the Rem/checked_rem rationale down to one line; and preceding_name's doc comment, which had picked up two sentences that belong to reject_compound_or (already stated correctly on reject_compound_or itself) from an earlier round's edit. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 21 ++++--------------- crates/kaish-kernel/src/ast/plan.rs | 8 ++++--- .../tests/arithmetic_adversarial_tests.rs | 16 ++++++++++++++ crates/kaish-kernel/tests/arithmetic_tests.rs | 12 +++++++++++ 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 7bc9b1cd..416b7f0e 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -409,8 +409,6 @@ impl<'a> Tokenizer<'a> { Ok(out) } - /// After consuming `+`/`-`, refuse `++`/`--`/`+=`/`-=` outright — kaish - /// has no assignment or increment inside `$(( ))`. /// The identifier that ends immediately before `op_start_byte` — the /// `x` in `x++`/`x += 2`/`x = 2`, read from the token already emitted /// (the operator has not been pushed onto `out` yet). @@ -747,15 +745,9 @@ impl<'a> Tokenizer<'a> { } Some('(') => { self.advance(); // consume '(' - // A character-level scan, not a re-tokenize of the - // remainder: text after this substitution's own `)` is - // ARITHMETIC syntax (`+`, `**`, …), which kaish's general - // lexer does not tokenize at all outside `$(( ))` — handing - // it a remainder like "echo 1) + 2" made `lexer::tokenize` - // fail on the `+` before the close was ever found. Quotes - // are tracked (a literal `(`/`)` inside one does not count), - // matching the risk `extract_arithmetic` already accepts - // for `$((`'s own scan; a `\`-escaped quote is honored. + // Character-level, quote-aware scan — re-tokenizing the + // remainder chokes on arithmetic syntax the general lexer + // doesn't know outside `$(( ))`. let cmd_start = self.pos; let mut depth = 0i32; let closed = loop { @@ -1278,12 +1270,7 @@ pub(crate) fn apply_binary(op: BinOp, l: i64, r: i64) -> Result if r == 0 { return Err(ArithError::new(format!("`{l} % 0` divides by zero"), 0..0)); } - // `i64::checked_rem` returns `None` for `MIN % -1` too — it is - // defined in terms of the division, which overflows, even - // though the remainder itself (0, any divisor of ±1 divides - // evenly) always fits. Division by ±1 never has a remainder, - // for any `l`, so this is a real answer, not a special case - // bolted onto an edge — checked_rem is just wrong here. + // checked_rem returns None for MIN % -1; the answer is 0. if r == -1 { return Ok(0); } diff --git a/crates/kaish-kernel/src/ast/plan.rs b/crates/kaish-kernel/src/ast/plan.rs index 2aa8d876..daa702c2 100644 --- a/crates/kaish-kernel/src/ast/plan.rs +++ b/crates/kaish-kernel/src/ast/plan.rs @@ -346,9 +346,11 @@ impl<'a> Collected<'a> { /// them in this file. Parses the text with the real arithmetic parser /// rather than scanning for identifier-shaped substrings — the old /// scan read `ff` out of `16#ff` and `xff` out of `0xff` as if they - /// were variables. A syntax error inside the text reads nothing: the - /// statement's own parse already failed loudly if this text was - /// invalid, so a plan never reaches an unparseable arithmetic body. + /// were variables. The shell parser and validator both defer + /// arithmetic to runtime — an unparsable body is syntactically valid + /// shell — so a syntax error here reads no variables rather than + /// failing the plan; the statement itself still fails loudly when it + /// runs. fn read_arithmetic(&mut self, expr: &str) { if let Ok(parsed) = crate::arithmetic::parse(expr) { self.read_arith_expr(&parsed); diff --git a/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs b/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs index 4939b9ea..6afa66ab 100644 --- a/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs +++ b/crates/kaish-kernel/tests/arithmetic_adversarial_tests.rs @@ -298,6 +298,22 @@ async fn power_overflow_names_the_limit() { errs("echo $((2 ** 100))", "64-bit").await; } +#[tokio::test] +async fn power_special_cases() { + ok("echo $((0 ** 0))", "1").await; + ok("echo $((0 ** 5))", "0").await; + ok("echo $((1 ** 999))", "1").await; + ok("echo $(((-1) ** 3))", "-1").await; + // Unary binds tighter than `**` (same rule as `-2 ** 2` = 4), so this + // parses identically to the parenthesized form above. + ok("echo $((-1 ** 3))", "-1").await; +} + +#[tokio::test] +async fn a_huge_exponent_overflows_before_computing_anything() { + errs("echo $((2 ** 4294967296))", "64-bit").await; +} + #[tokio::test] async fn shift_count_out_of_range_both_directions() { assert!(!err_of("echo $((1 << -1))").await.is_empty()); diff --git a/crates/kaish-kernel/tests/arithmetic_tests.rs b/crates/kaish-kernel/tests/arithmetic_tests.rs index a6c4b914..ca79823d 100644 --- a/crates/kaish-kernel/tests/arithmetic_tests.rs +++ b/crates/kaish-kernel/tests/arithmetic_tests.rs @@ -234,6 +234,10 @@ async fn coercion_list_record_bytes() { assert!(text.contains("list"), "{text:?}"); let text = err_of("x={a: 1}; echo $((x))").await; assert!(text.contains("record"), "{text:?}"); + // `head -c` off the synthetic /dev/urandom produces a real Value::Bytes + // with no localfs/subprocess feature needed. + let text = err_of("x=$(head -c 4 /dev/urandom); echo $((x))").await; + assert!(text.contains("bytes"), "{text:?}"); } // ── Precedence ─────────────────────────────────────────────────────────── @@ -371,6 +375,14 @@ async fn bare_arith_command_exit_codes() { assert!(err.contains("divides by zero"), "{err:?}"); } +/// The bare `(( ))` command form also needs the async evaluator for a +/// `$(...)` operand, not just `$(( ))` used as a value. +#[tokio::test] +async fn bare_arith_command_runs_a_command_substitution() { + let (code, _, _) = run("(( $(echo 3) > 2 ))").await; + assert_eq!(code, 0); +} + #[tokio::test] async fn bare_arith_chains_with_and_or() { ok("(( 1 > 0 )) && echo yes", "yes").await; From 18b5f548efcc5f1009e649a7ae2f73941f3786eb Mon Sep 17 00:00:00 2001 From: A Tobey Date: Fri, 28 Aug 2026 09:08:32 -0400 Subject: [PATCH 12/15] The bytes coercion test uses bytes, not chance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `coercion_list_record_bytes` reached the `Value::Bytes` arm by capturing `$(head -c 4 /dev/urandom)`, on the theory that four random bytes are not valid UTF-8. They are, often enough: the capture then binds a String and the error reads "which is not a number" instead of naming bytes. Twelve runs of the command produced one such miss, so the test failed roughly once in twelve — it went red once during this branch's own verification while nothing in the tree had changed. No shell command in kaish produces Bytes deterministically: `printf` does not interpret `\xff` as a raw byte, and `/dev/zero` decodes to a String of NULs. The arm is reachable without a shell at all, so the assertion moves to the unit tests beside the list and record cases, where `err_with` sets `x` to `Value::Bytes` directly. The integration test keeps list and record and says in a comment why bytes is not tested there. Co-Authored-By: Claude Opus 5 --- crates/kaish-kernel/src/arithmetic.rs | 4 +++- crates/kaish-kernel/tests/arithmetic_tests.rs | 9 ++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 416b7f0e..439b3895 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -2034,11 +2034,13 @@ mod tests { } #[test] - fn list_and_record_error() { + fn list_record_and_bytes_error() { let msg = err_with("x", |s| s.set("x", Value::Json(serde_json::json!([1, 2])))); assert!(msg.contains("list"), "{msg}"); let msg = err_with("x", |s| s.set("x", Value::Json(serde_json::json!({"a": 1})))); assert!(msg.contains("record"), "{msg}"); + let msg = err_with("x", |s| s.set("x", Value::Bytes(vec![0xff, 0xfe, 0x00, 0x01]))); + assert!(msg.contains("bytes"), "{msg}"); } #[test] diff --git a/crates/kaish-kernel/tests/arithmetic_tests.rs b/crates/kaish-kernel/tests/arithmetic_tests.rs index ca79823d..003f1fff 100644 --- a/crates/kaish-kernel/tests/arithmetic_tests.rs +++ b/crates/kaish-kernel/tests/arithmetic_tests.rs @@ -229,15 +229,14 @@ async fn coercion_null_and_unset() { } #[tokio::test] -async fn coercion_list_record_bytes() { +async fn coercion_list_and_record() { let text = err_of("x=[1 2]; echo $((x))").await; assert!(text.contains("list"), "{text:?}"); let text = err_of("x={a: 1}; echo $((x))").await; assert!(text.contains("record"), "{text:?}"); - // `head -c` off the synthetic /dev/urandom produces a real Value::Bytes - // with no localfs/subprocess feature needed. - let text = err_of("x=$(head -c 4 /dev/urandom); echo $((x))").await; - assert!(text.contains("bytes"), "{text:?}"); + // The Bytes arm is unit-tested in arithmetic.rs: no shell command produces + // Bytes deterministically, and /dev/urandom decodes as UTF-8 often enough + // to fail this assertion roughly once in twelve runs. } // ── Precedence ─────────────────────────────────────────────────────────── From ac091081ad93370c9364e0bebbd8e1dc7e00c1a9 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Fri, 28 Aug 2026 09:26:48 -0400 Subject: [PATCH 13/15] A base past 2^32 truncated to a base that fit, and computed wrong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `base#digits` range-checked the base by narrowing `base_mag: u64` to `u32` with `as u32`, then testing the narrowed value against 2..=36. For any base of the form k*2^32 + b with b in 2..=36, the truncation drops the high bits and the narrowed value passes the check — kaish then evaluated the literal in base b instead of refusing it. Verified on the built binary before the fix: `$((4294967298#10))` printed `2`, `$((4294967330#10))` printed `34`, `$((8589934594#10))` printed `2` again — silent wrong answers, the exact class kaish refuses. The error text on the next line already named the true `base_mag`, so the full-value check was clearly intended and just ran after the damage. Fix: range-check `base_mag` (the u64) against 2..=36 before narrowing. The same tokenizer path backs the typed literal, `base#$VAR` expansion, and string-variable coercion, so one fix covers all three; tests added for each. While in the area: `read_numeral` (the string-to-number coercion behind a variable held as arithmetic, e.g. `x=0b101; echo $((x))`) caught every tokenizer `Err` and flattened it to a generic "is not a number", discarding a fix the tokenizer had already worked out — `0b101` names `2#101`, `1_000` names the `_` to remove, `1e3` names kaish's integer-only rule, and none of that reached the user. Genuine non-numerals like `abc` have no tokenizer fix to lose and keep the generic message unchanged. `Numeral` gained a `NotANumberWithFix` variant carrying the tokenizer's message, and `parse_numeric_string` now composes it with the variable's name and value instead of discarding it. Co-Authored-By: Claude Opus 5 --- crates/kaish-kernel/src/arithmetic.rs | 93 ++++++++++++++++++++++++--- 1 file changed, 84 insertions(+), 9 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 439b3895..df67d129 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -647,13 +647,16 @@ impl<'a> Tokenizer<'a> { )); } self.advance(); // consume '#' - let base = base_mag as u32; - if !(2..=36).contains(&base) { + // Range-check the full u64 before narrowing: `as u32` on + // k*2^32 + b (b in 2..=36) truncates to b and passes the + // check, silently evaluating in base b instead of refusing. + if !(2..=36).contains(&base_mag) { return Err(ArithError::new( format!("base `{base_mag}` is outside 2..=36"), start..self.pos, )); } + let base = base_mag as u32; if let Some(sign @ ('+' | '-')) = self.peek() { let sign_start = self.pos; self.pos += 1; @@ -1356,6 +1359,11 @@ enum Numeral { ExpressionLike, LeadingZero, NotANumber, + /// The tokenizer refused with a message that already names a fix + /// (`0b101` → `2#101`, `1_000` → remove the `_`, `1e3` → integer-only). + /// Carries that message so the caller can keep it instead of a generic + /// "is not a number". + NotANumberWithFix(String), OutOfRange, } @@ -1386,7 +1394,8 @@ fn read_numeral(text: &str) -> Numeral { }, _ => Numeral::NotANumber, }, - _ => Numeral::NotANumber, + Ok(_) => Numeral::NotANumber, + Err(e) => Numeral::NotANumberWithFix(e.message), } } @@ -1408,6 +1417,9 @@ fn parse_numeric_string(s: &str, name: &str) -> Result { Numeral::NotANumber => { Err(ArithError::new(format!("`{name}` holds `{s}`, which is not a number"), 0..0)) } + Numeral::NotANumberWithFix(fix) => { + Err(ArithError::new(format!("`{name}` holds `{s}`; {fix}"), 0..0)) + } Numeral::OutOfRange => { Err(ArithError::new(format!("`{name}` holds `{s}`, outside the 64-bit range"), 0..0)) } @@ -1423,12 +1435,14 @@ pub(crate) fn parse_command_output(text: &str, cmd: &str) -> Result { - Err(ArithError::new( - format!("`{cmd}` printed `{text}`; the command must print one integer"), - 0..0, - )) - } + Numeral::ExpressionLike + | Numeral::NotANumber + | Numeral::NotANumberWithFix(_) + | Numeral::LeadingZero + | Numeral::OutOfRange => Err(ArithError::new( + format!("`{cmd}` printed `{text}`; the command must print one integer"), + 0..0, + )), } } @@ -1834,6 +1848,42 @@ mod tests { assert!(msg.contains("not a digit"), "{msg}"); } + // A base of the form k*2^32 + b (b in 2..=36) used to truncate through + // `as u32` BEFORE the range check, landing in range and silently + // computing as base b instead of refusing. Covers the typed literal, + // the `base#$VAR` expansion form, and a string variable holding the + // same spelling. + #[test] + fn base_out_of_range_survives_u32_truncation() { + for (expr, true_base) in [ + ("4294967298#10", "4294967298"), + ("4294967299#10", "4294967299"), + ("4294967330#10", "4294967330"), + ("8589934594#10", "8589934594"), + ] { + let msg = err(expr); + assert!(msg.contains("outside 2..=36"), "{expr}: {msg}"); + assert!(msg.contains(true_base), "{expr}: {msg}"); + } + } + + #[test] + fn based_expansion_out_of_range_survives_u32_truncation() { + let msg = err_with("4294967298#$d", |s| s.set("d", Value::String("10".to_string()))); + assert!(msg.contains("outside 2..=36"), "{msg}"); + assert!(msg.contains("4294967298"), "{msg}"); + } + + #[test] + fn string_variable_based_literal_base_overflow_does_not_compute() { + let mut scope = Scope::new(); + scope.set("x", Value::String("4294967298#10".to_string())); + assert!( + eval_arithmetic("x", &scope).is_err(), + "a u32-truncated out-of-range base must refuse, not silently compute a value" + ); + } + #[test] fn no_digits_after_prefix() { let msg = err("0x"); @@ -2033,6 +2083,31 @@ mod tests { assert!(msg.contains("not a number"), "{msg}"); } + // `read_numeral` used to flatten every tokenizer `Err` to a generic + // "is not a number", discarding a fix the tokenizer already named. + // These three spellings each carry a real fix; `abc` above has none + // and must keep the generic message. + #[test] + fn string_binary_spelling_names_the_fix() { + let msg = err_with("x", |s| s.set("x", Value::String("0b101".to_string()))); + assert!(msg.contains("`x`") && msg.contains("0b101"), "{msg}"); + assert!(msg.contains("2#101"), "{msg}"); + } + + #[test] + fn string_underscore_digit_group_names_the_fix() { + let msg = err_with("x", |s| s.set("x", Value::String("1_000".to_string()))); + assert!(msg.contains("`x`") && msg.contains("1_000"), "{msg}"); + assert!(msg.contains("remove it"), "{msg}"); + } + + #[test] + fn string_float_spelling_names_the_fix() { + let msg = err_with("x", |s| s.set("x", Value::String("1e3".to_string()))); + assert!(msg.contains("`x`") && msg.contains("1e3"), "{msg}"); + assert!(msg.contains("integer-only"), "{msg}"); + } + #[test] fn list_record_and_bytes_error() { let msg = err_with("x", |s| s.set("x", Value::Json(serde_json::json!([1, 2])))); From f4de5c5a4c6832b2ba777a2e7abde0da704f280e Mon Sep 17 00:00:00 2001 From: A Tobey Date: Fri, 28 Aug 2026 09:26:59 -0400 Subject: [PATCH 14/15] A $() inside $(( )) leaked the sync evaluator's internal refusal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scatter/gather's reduced sync arg binder handles a bare `$(...)` flag value correctly: `scatter --limit $(echo 2)` fails with a message naming the fix ("assign it to a variable first"). But when a `$(...)` sits inside a `$((...))` instead (`--limit $(( $(echo 2) ))`), the same binder called `arithmetic::eval_sync` directly, without first checking whether the parsed expression could even reach a command substitution. `eval_sync` refuses that shape with its own internal message ("`$(...)` must be resolved by the async evaluator before sync evaluation"), written for a caller that is expected to have already routed around it — never meant to reach a user, and it did: `printf "a\nb\n" | scatter --as H --limit $(( $(echo 2) )) | echo "$H" | gather` printed that sentence verbatim as `scatter: arithmetic error: ...`. Fix: both arithmetic arms (the bare `Expr::Arithmetic` flag value and the quoted `StringPart::Arithmetic` interpolated value) now parse the expression, check `contains_command_subst`, and report the same message their sibling bare-`$(...)` arm already gives, so the two spellings behave alike. The messages were lifted into two shared helpers (`command_subst_flag_value_message`, `command_subst_interpolated_value_message`) reused by both the arithmetic and the plain command-substitution arms, rather than writing the text a second time. Co-Authored-By: Claude Opus 5 --- crates/kaish-kernel/src/scheduler/pipeline.rs | 59 ++++++++++++++----- .../tests/scatter_gather_jsonl_tests.rs | 44 ++++++++++++++ 2 files changed, 89 insertions(+), 14 deletions(-) diff --git a/crates/kaish-kernel/src/scheduler/pipeline.rs b/crates/kaish-kernel/src/scheduler/pipeline.rs index f7302e5a..03a182d9 100644 --- a/crates/kaish-kernel/src/scheduler/pipeline.rs +++ b/crates/kaish-kernel/src/scheduler/pipeline.rs @@ -1073,6 +1073,23 @@ pub async fn build_tool_args( .map_err(|e| e.to_string()) } +/// Message for a `$(...)` used directly as a scatter/gather flag value +/// (`--limit $(...)`) — also reused when a `$((...))` flag value reaches +/// one (`--limit $(( $(...) ))`), so the two spellings behave alike. +fn command_subst_flag_value_message() -> String { + "command substitution `$(...)` is not supported in a scatter/gather flag value here; \ + assign it to a variable first (e.g. `n=$(...); scatter --limit $n`)" + .to_string() +} + +/// Message for a `$(...)` inside an interpolated flag value +/// (`--as "W$(...)"`) — also reused when a quoted `$((...))` reaches one. +fn command_subst_interpolated_value_message() -> String { + "command substitution `$(...)` is not supported inside a scatter/gather \ + flag's interpolated value here; assign it to a variable first" + .to_string() +} + /// Simple expression evaluation for args (without full scope access). /// /// `Ok(None)` means "not representable in this reduced sync context" (only @@ -1130,9 +1147,21 @@ pub(crate) fn eval_simple_expr(expr: &Expr, ctx: &ExecContext) -> Result arithmetic::eval_arithmetic(expr_str, &ctx.scope) - .map(|n| Some(Value::Int(n))) - .map_err(|e| format!("arithmetic error: {e}")), + // + // A `$(...)` reachable inside the expression (`$(( $(echo 2) ))`) + // can't run here either — the sync evaluator's own internal + // refusal for that case is not user-facing wording, so it's + // checked for and reported the same as the bare `$(...)` arm below. + Expr::Arithmetic(expr_str) => { + let parsed = arithmetic::parse(expr_str).map_err(|e| format!("arithmetic error: {e}"))?; + if parsed.contains_command_subst() { + Err(command_subst_flag_value_message()) + } else { + arithmetic::eval_sync(&parsed, &ctx.scope) + .map(|n| Some(Value::Int(n))) + .map_err(|e| format!("arithmetic error: {e}")) + } + } Expr::HereDocBody { parts, strip_tabs } => { // Heredoc body materialization for redirect targets. `<<-` tab // stripping applies to the literal source, not to tabs from a @@ -1155,11 +1184,7 @@ pub(crate) fn eval_simple_expr(expr: &Expr, ctx: &ExecContext) -> Result Err( - "command substitution `$(...)` is not supported in a scatter/gather flag value here; \ - assign it to a variable first (e.g. `n=$(...); scatter --limit $n`)" - .to_string(), - ), + Expr::CommandSubst(_) | Expr::Command(_) => Err(command_subst_flag_value_message()), _ => Ok(None), // Binary ops need more context } } @@ -1233,7 +1258,17 @@ fn eval_string_parts_sync(parts: &[crate::ast::StringPart], ctx: &ExecContext) - // the real arithmetic error. Matches the bare // `Expr::Arithmetic` arm above and the async // `eval_string_part_async` (kernel.rs). - let value = arithmetic::eval_arithmetic(expr, &ctx.scope) + // + // A `$(...)` reachable inside the expression can't run here + // either (see the bare `Expr::Arithmetic` arm above) — check + // for it and report the same message as the `CommandSubst` + // arm below, rather than the sync evaluator's internal + // refusal. + let parsed = arithmetic::parse(expr).map_err(|e| format!("arithmetic error: {e}"))?; + if parsed.contains_command_subst() { + return Err(command_subst_interpolated_value_message()); + } + let value = arithmetic::eval_sync(&parsed, &ctx.scope) .map_err(|e| format!("arithmetic error: {e}"))?; result.push_str(&value.to_string()); } @@ -1243,11 +1278,7 @@ fn eval_string_parts_sync(parts: &[crate::ast::StringPart], ctx: &ExecContext) - // loud instead of silently splicing in nothing. // `scatter --as "W$(suffix)"` used to bind the plain "W" // with the substitution silently dropped. - return Err( - "command substitution `$(...)` is not supported inside a scatter/gather \ - flag's interpolated value here; assign it to a variable first" - .to_string(), - ); + return Err(command_subst_interpolated_value_message()); } crate::ast::StringPart::LastExitCode => { result.push_str(&ctx.scope.last_result().code.to_string()); diff --git a/crates/kaish-kernel/tests/scatter_gather_jsonl_tests.rs b/crates/kaish-kernel/tests/scatter_gather_jsonl_tests.rs index 29cb9afc..528957ab 100644 --- a/crates/kaish-kernel/tests/scatter_gather_jsonl_tests.rs +++ b/crates/kaish-kernel/tests/scatter_gather_jsonl_tests.rs @@ -564,6 +564,50 @@ async fn scatter_quoted_arithmetic_flag_value_division_by_zero_is_loud() { ); } +// ═══════════════════════════════════════════════════════════════════════ +// A `$(...)` reachable INSIDE `$((...))` (`--limit $(( $(echo 2) ))`) can't +// be evaluated by the reduced sync arg binder either — the sync arithmetic +// evaluator's own internal refusal ("`$(...)` must be resolved by the async +// evaluator before sync evaluation") used to leak straight to the user +// instead of the same "assign it to a variable first" message the bare +// `$(...)` arm already gives for `--limit $(echo 2)`. +// ═══════════════════════════════════════════════════════════════════════ + +#[tokio::test] +async fn scatter_bare_arithmetic_flag_value_command_subst_is_not_internal_wording() { + let k = kernel_at(tempdir().unwrap().path()); + let r = run_full(&k, "seq 1 3 | scatter --limit $(( $(echo 2) )) | echo $ITEM | gather").await; + assert_ne!(r.code, 0, "a $(...) inside $((...)) must fail loud: {r:?}"); + assert!( + !r.err.contains("async evaluator") && !r.err.contains("sync evaluation"), + "internal evaluator wording leaked to the user: {}", + r.err + ); + assert!( + r.err.contains("scatter/gather flag value") + && r.err.contains("assign it to a variable first"), + "expected the same message the bare $(...) arm gives, got: {}", + r.err + ); +} + +#[tokio::test] +async fn scatter_quoted_arithmetic_flag_value_command_subst_is_not_internal_wording() { + let k = kernel_at(tempdir().unwrap().path()); + let r = run_full(&k, r#"seq 1 3 | scatter --limit "$(( $(echo 2) ))" | echo $ITEM | gather"#).await; + assert_ne!(r.code, 0, "a $(...) inside a quoted $((...)) must fail loud: {r:?}"); + assert!( + !r.err.contains("async evaluator") && !r.err.contains("sync evaluation"), + "internal evaluator wording leaked to the user: {}", + r.err + ); + assert!( + r.err.contains("flag's interpolated value") && r.err.contains("assign it to a variable first"), + "expected the same message the quoted $(...) arm gives, got: {}", + r.err + ); +} + #[tokio::test] async fn scatter_bare_arithmetic_flag_value_binds_successfully() { // Regression guard, pairing with the loud-error test above: a VALID bare From 4e7b85a912621fdc3a7651ef69c4ba6ea33e3826 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Fri, 28 Aug 2026 09:31:02 -0400 Subject: [PATCH 15/15] A refused numeral is quoted as the user wrote it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Propagating the tokenizer's error through a variable (the previous commit) made a pre-existing truncation visible: `x="1_000"` reported "`1_` contains `_`", quoting only as far as the offending character. The typed form had always done the same — `$((1_000))` said `1_`, and `$((12_345_6))` said `12_`. Both underscore sites sliced the literal at `self.pos + 1`, the scan position, rather than at the end of the numeral. `numeral_run_end` walks the rest of the run so the message names what was written. The same slice appeared in the two INTEGER_OUT_OF_RANGE messages in the same functions, and is corrected with it: `16#ffffffffffffffffff` was reported one digit short, because the scan stops on the digit that overflows. A refused value is quoted whole, whatever the reason for the refusal. The old test asserted only that the message contained an underscore, so it passed against `1_`. It now pins the whole literal for the decimal, multi-group, and based spellings. Co-Authored-By: Claude Opus 5 --- crates/kaish-kernel/src/arithmetic.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index df67d129..8ed48a48 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -256,6 +256,17 @@ impl<'a> Tokenizer<'a> { &self.text[start_byte..end_byte] } + /// End of the numeral run starting at `from` — digits, letters, and `_`. + /// An error quotes the literal the user wrote, not the prefix scanned so + /// far, so `1_000` is not reported as `1_`. + fn numeral_run_end(&self, from: usize) -> usize { + let mut end = from; + while self.chars.get(end).is_some_and(|(_, c)| c.is_ascii_alphanumeric() || *c == '_') { + end += 1; + } + end + } + fn skip_ws(&mut self) { while matches!(self.peek(), Some(c) if c.is_whitespace()) { self.pos += 1; @@ -500,7 +511,7 @@ impl<'a> Tokenizer<'a> { while let Some(c) = self.peek() { if c == '_' { return Err(ArithError::new( - format!("`{}` contains `_`; remove it", self.slice(lit_start, self.pos + 1)), + format!("`{}` contains `_`; remove it", self.slice(lit_start, self.numeral_run_end(self.pos))), lit_start..self.byte_pos() + c.len_utf8(), )); } @@ -528,7 +539,7 @@ impl<'a> Tokenizer<'a> { .and_then(|m| m.checked_add(digit_val as u64)) .ok_or_else(|| { ArithError::new( - format!("`{}` {INTEGER_OUT_OF_RANGE}", self.slice(lit_start, self.pos + 1)), + format!("`{}` {INTEGER_OUT_OF_RANGE}", self.slice(lit_start, self.numeral_run_end(self.pos))), lit_start..self.byte_pos(), ) })?; @@ -548,7 +559,7 @@ impl<'a> Tokenizer<'a> { while let Some(c) = self.peek() { if c == '_' { return Err(ArithError::new( - format!("`{}` contains `_`; remove it", self.slice(lit_start, self.pos + 1)), + format!("`{}` contains `_`; remove it", self.slice(lit_start, self.numeral_run_end(self.pos))), lit_start..self.byte_pos() + c.len_utf8(), )); } @@ -558,7 +569,7 @@ impl<'a> Tokenizer<'a> { let digit_val = c as u64 - '0' as u64; mag = mag.checked_mul(10).and_then(|m| m.checked_add(digit_val)).ok_or_else(|| { ArithError::new( - format!("`{}` {INTEGER_OUT_OF_RANGE}", self.slice(lit_start, self.pos + 1)), + format!("`{}` {INTEGER_OUT_OF_RANGE}", self.slice(lit_start, self.numeral_run_end(self.pos))), lit_start..self.byte_pos(), ) })?; @@ -1893,9 +1904,11 @@ mod tests { } #[test] - fn underscore_in_literal() { - let msg = err("1_000"); - assert!(msg.contains('_'), "{msg}"); + fn underscore_in_literal_quotes_the_whole_literal() { + // Not `1_`: the message names the literal the user wrote. + assert!(err("1_000").contains("`1_000`"), "{}", err("1_000")); + assert!(err("12_345_6").contains("`12_345_6`"), "{}", err("12_345_6")); + assert!(err("16#f_f").contains("`16#f_f`"), "{}", err("16#f_f")); } #[test]