diff --git a/CHANGELOG.md b/CHANGELOG.md index d46dd675..3c16ecea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,41 @@ breaking entries are marked **BREAKING**. parameter's aliases. `help kj` and every wrapped command showed "No parameters." before. +- **A numeral argv word like `-0` or `007` lost its sign or leading zeros** + when re-serialized from its typed value; `xargs -0 rm -f` ran as + `xargs 0 rm -f`. External-command argv and `plan`/`--plan` now keep the + source text exactly. + +- **`007` is now the string `007`, not the number 7** — a leading zero is + not a JSON number, and `fromjson` already refused it. `-0`, `0.10`, and + `1.0` are unaffected; they stay numbers. + +- **Where kaish needs a number, a leading zero is an error** — `break 007`, + `$((010 + 1))`, `[[ 010 -eq 10 ]]`, and a list index name the number to + write; kaish reads no octal. + +- **An integer past 64 bits names the limit and the fix** instead of + "invalid number", in both a bare numeral and `$(( ))`. + +- **Arithmetic refuses a leading zero however it arrives** — `$((010 + 1))` + and `x=010; $((x))` both name the decimal to write. Reading the text as + decimal answered 10 where bash answers 8. + +- **A record key like `"007"` could not be read back** — `${r[007]}` resolved + to index 7 and `r[007]=v` was a parse error. Read and write now classify + every numeral subscript alike, including `[-0]` and `[1.0]`. + +- **A slice bound with a leading zero was silent** — `${xs[007:2]}` sliced + from 7, inverted the range, and returned an empty list with exit 0. + +- **`echo hi > -0` created a file named `0`** while `--plan` reported the + target as `-0`. Redirect targets keep the source text, so a plan document + and the run it describes agree. + +- **A builtin's argv lost a numeral's exact text too** — `echo -0` printed + `0`, `echo 0.10` printed `0.1`. `echo`, function calls, and script + `$1`/`$2` now keep the source word, matching the external-command fix + above. - **File tests on virtual and real paths** — `-w` claimed read-only mounts and root-owned files were writable, and `-x` denied that a memory-backed directory is searchable. `-w`/`-r`/`-x` now answer from the owning mount plus diff --git a/crates/kaish-help/content/en/syntax.md b/crates/kaish-help/content/en/syntax.md index 06fbbb65..24484401 100644 --- a/crates/kaish-help/content/en/syntax.md +++ b/crates/kaish-help/content/en/syntax.md @@ -10,6 +10,33 @@ PI=3.14159 # float ENABLED=true # boolean (only true/false) ``` +## Numbers + +```sh +echo 007 # the string 007 — a leading zero is not a JSON number +chmod 0644 f # the string 0644 — a mode keeps every digit you typed +echo -0 # the number 0 — valid JSON; -0 and 0 are the same number +echo "-0" # the string -0 — quote it to keep those two characters +break 7 # a loop count is a number — `break 007` is an error +``` + +`007`, `010`, and `00` are strings — `fromjson '007'` already refuses them +as invalid JSON, and a bare number now agrees. `-0`, `0.10`, and `1.0` are +numbers; kaish prints back the word you typed wherever it crosses into argv +or a plan. Once the number moves through a variable, arithmetic, or +`--json`, it is a plain typed number again: `x=-0; echo $x` prints `0`. +Quote a number to keep it a string on purpose. + +Where kaish needs a number, a leading zero is an error that names the +number to write: a `break`/`continue` count, arithmetic, a numeric +comparison, and a list index. kaish reads no octal, so `$((010 + 1))` is +an error rather than 9 (bash's answer) or 11 (the decimal one). A record +key is text, so `${r[007]}` reads the `"007"` key. + +A bare integer must fit in 64 bits (`-9223372036854775808` to +`9223372036854775807`); a longer numeral is an error naming the limit — +quote it to keep the text. + ## Expansion ```sh diff --git a/crates/kaish-help/src/fragments.rs b/crates/kaish-help/src/fragments.rs index bbadeeb3..293d4203 100644 --- a/crates/kaish-help/src/fragments.rs +++ b/crates/kaish-help/src/fragments.rs @@ -303,6 +303,34 @@ COUNT=42 # integer PI=3.14159 # float ENABLED=true # boolean (only true/false) ```"#, + ), + syntax_section( + "numbers", + "Numbers", + r#"```sh +echo 007 # the string 007 — a leading zero is not a JSON number +chmod 0644 f # the string 0644 — a mode keeps every digit you typed +echo -0 # the number 0 — valid JSON; -0 and 0 are the same number +echo "-0" # the string -0 — quote it to keep those two characters +break 7 # a loop count is a number — `break 007` is an error +``` + +`007`, `010`, and `00` are strings — `fromjson '007'` already refuses them +as invalid JSON, and a bare number now agrees. `-0`, `0.10`, and `1.0` are +numbers; kaish prints back the word you typed wherever it crosses into argv +or a plan. Once the number moves through a variable, arithmetic, or +`--json`, it is a plain typed number again: `x=-0; echo $x` prints `0`. +Quote a number to keep it a string on purpose. + +Where kaish needs a number, a leading zero is an error that names the +number to write: a `break`/`continue` count, arithmetic, a numeric +comparison, and a list index. kaish reads no octal, so `$((010 + 1))` is +an error rather than 9 (bash's answer) or 11 (the decimal one). A record +key is text, so `${r[007]}` reads the `"007"` key. + +A bare integer must fit in 64 bits (`-9223372036854775808` to +`9223372036854775807`); a longer numeral is an error naming the limit — +quote it to keep the text."#, ), syntax_section( "expansion", diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 0bb84425..7f8b1b3d 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -34,6 +34,19 @@ pub fn eval_arithmetic(expr: &str, scope: &Scope) -> Result { 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. +/// +/// The suggestion keeps the sign: `-007` is not fixed by writing `7`. +pub(crate) fn leading_zero_decimal(text: &str) -> Option { + if !crate::lexer::is_leading_zero_numeral(text) { + return None; + } + let sign = if text.starts_with('-') { "-" } else { "" }; + let digits = text.trim_start_matches('-').trim_start_matches('0'); + Some(format!("{sign}{}", if digits.is_empty() { "0" } else { digits })) +} + /// Simple recursive descent parser for arithmetic expressions. struct ArithParser<'a> { input: &'a str, @@ -311,7 +324,19 @@ impl<'a> ArithParser<'a> { } } let num_str = &self.input[start..self.pos]; - num_str.parse().context("invalid number in arithmetic expression") + // 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" + ); + } + // 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 { @@ -335,6 +360,12 @@ impl<'a> ArithParser<'a> { // 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) }); @@ -427,7 +458,14 @@ impl<'a> ArithParser<'a> { match value { Value::Int(n) => Ok(*n), Value::String(s) => { - // Try to parse string as integer + // `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 )) diff --git a/crates/kaish-kernel/src/ast/plan.rs b/crates/kaish-kernel/src/ast/plan.rs index 73bd0e7d..30c17bc2 100644 --- a/crates/kaish-kernel/src/ast/plan.rs +++ b/crates/kaish-kernel/src/ast/plan.rs @@ -537,6 +537,7 @@ fn collect_expr<'a>(expr: &'a Expr, background: bool, out: &mut Collected<'a>) { // Special forms ($1, $@, $#, $?, $$) are not session variables; an // embedder cannot peek them with `get_var`, so they are not listed. Expr::Literal(_) + | Expr::NumericLiteral { .. } | Expr::Positional(_) | Expr::AllArgs | Expr::ArgCount @@ -896,6 +897,9 @@ pub(crate) fn render_expr(expr: &Expr) -> String { format!("${{{}:-{}}}", render_varpath(path), render_parts(default)) } Expr::Arithmetic(e) => format!("$(({e}))"), + // Render the source text, not `value`'s canonical form — that is what + // this variant is for. + Expr::NumericLiteral { raw, .. } => raw.clone(), Expr::Command(cmd) => render_command(cmd), Expr::LastExitCode => "$?".to_string(), Expr::CurrentPid => "$$".to_string(), diff --git a/crates/kaish-kernel/src/ast/sexpr.rs b/crates/kaish-kernel/src/ast/sexpr.rs index ba29a227..735f9641 100644 --- a/crates/kaish-kernel/src/ast/sexpr.rs +++ b/crates/kaish-kernel/src/ast/sexpr.rs @@ -272,6 +272,9 @@ pub fn format_expr(expr: &Expr) -> String { match expr { Expr::Not(inner) => format!("(not {})", format_expr(inner)), Expr::Literal(value) => format_value(value), + Expr::NumericLiteral { value, raw } => { + format!("(numeric-literal {} raw={:?})", format_value(value), raw) + } Expr::VarRef(path) => format!("(varref {})", format_varpath(path)), Expr::Interpolated(parts) => { let parts_str: Vec = parts diff --git a/crates/kaish-kernel/src/ast/types.rs b/crates/kaish-kernel/src/ast/types.rs index 085d57c0..87f5de9e 100644 --- a/crates/kaish-kernel/src/ast/types.rs +++ b/crates/kaish-kernel/src/ast/types.rs @@ -393,6 +393,17 @@ pub enum Expr { /// Record literal: `{name: amy, role: maintainer}`, `{port:8080}` (colon /// may be spaced or unspaced). Value-position only, same as `ListLiteral`. RecordLiteral(Vec), + /// A numeral whose own `Display` does not reproduce the source text it + /// was written as — `-0`, `0.10`, `1.0`. `value` is the typed value: + /// arithmetic, comparisons, `set x = -0`, and `--json` all see a real + /// `Int`/`Float`. `raw` is the exact source text, used by argv and plan + /// rendering, so `xargs -0 rm -f` keeps its `-0`. + /// + /// A canonical numeral (`-1`, `42`) stays a plain `Literal`, and a leading + /// zero (`007`) is a `Literal(String)` — see `lexer::Token::NumericLiteral`, + /// which this mirrors, and `docs/LANGUAGE.md`, "A bare number follows JSON + /// rules". + NumericLiteral { value: Value, raw: String }, } /// One element of a list literal. diff --git a/crates/kaish-kernel/src/dispatch.rs b/crates/kaish-kernel/src/dispatch.rs index 21b5cc5f..e1cf3a0a 100644 --- a/crates/kaish-kernel/src/dispatch.rs +++ b/crates/kaish-kernel/src/dispatch.rs @@ -316,6 +316,9 @@ impl BackendDispatcher { Expr::Literal(Value::String(s)) => argv.push(s.clone()), Expr::Literal(Value::Int(i)) => argv.push(i.to_string()), Expr::Literal(Value::Float(f)) => argv.push(f.to_string()), + // Kept in sync with kernel.rs::build_args_flat, which + // pushes `raw` for the same reason. + Expr::NumericLiteral { raw, .. } => argv.push(raw.clone()), Expr::VarRef(path) => { if let Ok(v) = ctx.scope.resolve_path(path) { if let Some(msg) = crate::interpreter::structured_boundary_error("a command argument", &v) { diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index 526990f4..adb5e425 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -181,6 +181,9 @@ impl<'a> Evaluator<'a> { // condition holds no command substitution. Expr::Not(inner) => Ok(Value::Bool(!is_truthy(&self.eval(inner)?))), Expr::Literal(value) => self.eval_literal(value), + // Typed evaluation only needs `value`. `raw` is for argv and plan + // text sinks, which read the `Expr` directly. + Expr::NumericLiteral { value, .. } => self.eval_literal(value), Expr::VarRef(path) => self.eval_var_ref(path), Expr::Interpolated(parts) => self.eval_interpolated(parts), Expr::HereDocBody { parts, strip_tabs } => { @@ -600,7 +603,6 @@ impl<'a> Evaluator<'a> { } -/// Convert a Value to its string representation for interpolation. /// Coerce a Value into an exit code (i64) for `return`/`exit`. /// /// Bash semantics: `return $(echo 42)` works because the captured text "42" @@ -614,7 +616,11 @@ pub fn value_to_exit_code(value: &Value) -> anyhow::Result { Value::String(s) => { let trimmed = s.trim(); trimmed.parse::().map_err(|_| { - anyhow::anyhow!("numeric argument required: {:?}", s) + if is_i64_overflow_shape(trimmed) { + anyhow::anyhow!("`{trimmed}`, which {}", crate::lexer::INTEGER_OUT_OF_RANGE) + } else { + anyhow::anyhow!("numeric argument required: {:?}", s) + } }) } Value::Null | Value::Json(_) | Value::Bytes(_) => { @@ -623,6 +629,14 @@ pub fn value_to_exit_code(value: &Value) -> anyhow::Result { } } +/// True for a string shaped like `-?[0-9]+` — the only shape whose `i64` +/// parse can fail exclusively by overflow. Shared by `value_to_exit_code` +/// and `value_to_num` so both name the same 64-bit limit the same way. +fn is_i64_overflow_shape(t: &str) -> bool { + let digits = t.strip_prefix('-').unwrap_or(t); + !digits.is_empty() && digits.bytes().all(|b| b.is_ascii_digit()) +} + /// Length of a value for `${#…}`: element count for a list, key count for a /// record, and the CHARACTER count (Unicode scalar values) of the string form /// for any scalar (unchanged for non-collections). The single source of truth @@ -778,6 +792,7 @@ pub fn scalar_test_operand_error(op_symbol: &str, value: &Value) -> Option String { match value { Value::Null => "null".to_string(), @@ -1176,25 +1191,63 @@ fn compare_values(left: &Value, right: &Value) -> EvalResult } } -/// Coerce a value to a number for arithmetic test ops (`-eq`/`-gt`/…). -/// -/// `String` operands are parsed as `i64` then `f64` (matching POSIX `[[ ]]` -/// arithmetic context). Non-numeric strings and non-numeric types error. +/// An integer or float result from `value_to_num`. enum Num { Int(i64), Float(f64), } +/// Coerce a value to a number for arithmetic test ops (`-eq`/`-gt`/…). +/// +/// A `String` operand: a leading zero refuses; then `i64`; then `f64`, but +/// only for a float spelling (`.`/`e`/`E`) and only when the result is +/// finite. An all-digit string that overflows `i64` names the 64-bit +/// limit rather than falling through to `f64`. Other strings and +/// non-numeric types error. fn value_to_num(value: &Value) -> EvalResult { match value { Value::Int(n) => Ok(Num::Int(*n)), Value::Float(f) => Ok(Num::Float(*f)), Value::String(s) => { let t = s.trim(); + // Same refusal as `$((010))`: a leading zero is text, not decimal 10 or octal 8. + if let Some(decimal) = arithmetic::leading_zero_decimal(t) { + return Err(EvalError::TypeError { + expected: "a number", + got: format!( + "`{t}`, which is text (leading zero) — kaish reads no octal; write \ + `{decimal}` for the decimal value" + ), + }); + } if let Ok(n) = t.parse::() { - Ok(Num::Int(n)) - } else if let Ok(f) = t.parse::() { - Ok(Num::Float(f)) + return Ok(Num::Int(n)); + } + // A float spelling (`1.5`, `1e3`) falls to f64 as before. An + // integer-shaped string that only failed above by overflow must + // not silently round through f64 — both sides of a comparison + // past i64::MAX round to the same f64, so `9223372036854775808 + // -eq 9223372036854775807` would answer true. + let looks_like_float = t.contains(['.', 'e', 'E']); + if looks_like_float + && let Ok(f) = t.parse::() + { + // kaish numbers are JSON numbers, and JSON has no infinity — + // `1e309` parses to f64::INFINITY without an Err, so a + // magnitude past the f64 range needs its own check here. + if !f.is_finite() { + return Err(EvalError::TypeError { + expected: "a number", + got: format!("`{t}`, which is outside the 64-bit float range"), + }); + } + return Ok(Num::Float(f)); + } + if is_i64_overflow_shape(t) { + Err(EvalError::TypeError { + expected: "a number", + got: format!("`{t}`, which {}", crate::lexer::INTEGER_OUT_OF_RANGE), + }) } else { Err(EvalError::TypeError { expected: "numeric operand", diff --git a/crates/kaish-kernel/src/interpreter/scope.rs b/crates/kaish-kernel/src/interpreter/scope.rs index 224c251f..a1abe73f 100644 --- a/crates/kaish-kernel/src/interpreter/scope.rs +++ b/crates/kaish-kernel/src/interpreter/scope.rs @@ -112,12 +112,43 @@ fn classify_index(json: &serde_json::Value, i: i64, path: &str) -> Result Option { + let mut changed = false; + let fixed = key + .split(':') + .map(|part| { + if !crate::lexer::is_leading_zero_numeral(part) { + return part.to_string(); + } + changed = true; + let sign = if part.starts_with('-') { "-" } else { "" }; + let digits = part.trim_start_matches('-').trim_start_matches('0'); + format!("{sign}{}", if digits.is_empty() { "0" } else { digits }) + }) + .collect::>() + .join(":"); + changed.then_some(fixed) +} + /// Classify a record key against an object. A bareword/string key on a list is /// an error; key *presence* is checked when the step is applied ([`descend`]), /// not here — the read/write split lives in that leaf policy. fn classify_key(json: &serde_json::Value, key: &str, path: &str) -> Result { match json { serde_json::Value::Object(_) => Ok(Step::Key(key.to_string())), + serde_json::Value::Array(_) if let Some(fix) = without_leading_zeros(key) => { + // Name the leading zero, or the message reads as a type confusion + // the author never made. + Err(PathError::Shape(format!( + "${{{path}[{key}]}}: `{key}` is text (leading zero) and a list is indexed by \ + number — write ${{{path}[{fix}]}}" + ))) + } serde_json::Value::Array(_) => Err(PathError::Shape(format!( "${{{path}[{key}]}}: string key on a list — use an integer index" ))), diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index 3eef9141..9d0466ee 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -3479,6 +3479,8 @@ impl Kernel { Expr::Literal(Value::Float(f)) => f.to_string(), Expr::Literal(Value::Bool(b)) => b.to_string(), Expr::Literal(Value::Null) => "null".to_string(), + // Show the source text, not the typed value. + Expr::NumericLiteral { raw, .. } => raw.clone(), Expr::VarRef(path) => { let mut name = String::new(); for (i, seg) in path.segments.iter().enumerate() { @@ -3914,6 +3916,13 @@ impl Kernel { continue; } } + // The exact word the author typed reaches the external + // process, matching what `ast::plan::render_expr` showed. + // Skips `eval_expr_async`: no `Value` reproduces `raw`. + if let Expr::NumericLiteral { raw, .. } = expr { + argv.push(raw.clone()); + continue; + } let value = self.eval_expr_async(expr).await?; // Decision D: a bare collection can't cross the external // process boundary as an argv element — refuse rather than @@ -3929,6 +3938,10 @@ impl Kernel { argv.push(value_to_text_sink(&value).map_err(|e| anyhow::anyhow!("{e}"))?); } Arg::Named { key, value } => { + if let Expr::NumericLiteral { raw, .. } = value { + argv.push(format!("--{key}={raw}")); + continue; + } let val = self.eval_expr_async(value).await?; if let Some(msg) = crate::interpreter::structured_boundary_error("a command argument", &val) { return Err(anyhow::anyhow!(msg)); @@ -3938,6 +3951,10 @@ impl Kernel { argv.push(format!("--{key}={val_str}")); } Arg::WordAssign { key, value } => { + if let Expr::NumericLiteral { raw, .. } = value { + argv.push(format!("{key}={raw}")); + continue; + } let val = self.eval_expr_async(value).await?; if let Some(msg) = crate::interpreter::structured_boundary_error("a command argument", &val) { return Err(anyhow::anyhow!(msg)); @@ -4045,6 +4062,9 @@ impl Kernel { Ok(Value::Bool(!is_truthy(&value))) } Expr::Literal(value) => Ok(value.clone()), + // Typed evaluation only needs `value`; `raw` is for argv and + // plan text sinks that read the `Expr` directly. + Expr::NumericLiteral { value, .. } => Ok(value.clone()), Expr::VarRef(path) => { let scope = self.scope.read().await; match scope.resolve_path(path) { @@ -4668,9 +4688,18 @@ impl Kernel { let saved = scope.save_positional(); // Set up new positional parameters ($0 = function name, $1, $2, ... = args) + // `$1` is a text sink like any argv word, and `value_to_string` + // cannot reproduce the source text — see `ToolArgs::positional_raw`. let positional_args: Vec = tool_args.positional .iter() - .map(value_to_string) + .enumerate() + .map(|(i, v)| { + tool_args + .positional_raw + .get(&i) + .cloned() + .unwrap_or_else(|| value_to_string(v)) + }) .collect(); scope.set_positional(&def.name, positional_args); @@ -5135,9 +5164,17 @@ impl Kernel { } // Set up positional parameters ($0 = script name, $1, $2, ... = args) + // Same source-text fidelity as the function-call site above. let positional_args: Vec = tool_args.positional .iter() - .map(value_to_string) + .enumerate() + .map(|(i, v)| { + tool_args + .positional_raw + .get(&i) + .cloned() + .unwrap_or_else(|| value_to_string(v)) + }) .collect(); isolated_scope.set_positional(name, positional_args); @@ -6213,6 +6250,11 @@ pub(crate) async fn bind_tool_args( // Nothing evaluated means no word, matching the // typed path's `if let Some(value)`. if let Some(value) = source.eval(expr).await? { + // Recorded at the index the push below lands + // at — see `ToolArgs::words_raw`. + if let Expr::NumericLiteral { raw, .. } = expr { + tool_args.words_raw.insert(words.len(), raw.clone()); + } words.push(apply_tilde_expansion(value, home.as_deref())); } } @@ -6247,12 +6289,18 @@ pub(crate) async fn bind_tool_args( } // Loud on binary (GH #116): reassembling `--k=$BIN` as text // hands the tool a placeholder that looks like data. A bare - // binary word is fine — it stays typed. - let val_str = crate::interpreter::value_to_text_sink_named( - &val, - "a --key=value argument", - ) - .map_err(|e| anyhow::anyhow!("{e}"))?; + // binary word is fine — it stays typed. The whole word is + // composed here, so no later render step could reach a + // `words_raw` entry. + let val_str = if let Expr::NumericLiteral { raw, .. } = value { + raw.clone() + } else { + crate::interpreter::value_to_text_sink_named( + &val, + "a --key=value argument", + ) + .map_err(|e| anyhow::anyhow!("{e}"))? + }; words.push(Value::String(format!("--{key}={val_str}"))); } Arg::WordAssign { key, value } => { @@ -6260,11 +6308,15 @@ pub(crate) async fn bind_tool_args( anyhow::anyhow!("verbatim key=value could not be evaluated in this context") })?; let val = apply_tilde_expansion(val, home.as_deref()); - let val_str = crate::interpreter::value_to_text_sink_named( - &val, - "a key=value argument", - ) - .map_err(|e| anyhow::anyhow!("{e}"))?; + let val_str = if let Expr::NumericLiteral { raw, .. } = value { + raw.clone() + } else { + crate::interpreter::value_to_text_sink_named( + &val, + "a key=value argument", + ) + .map_err(|e| anyhow::anyhow!("{e}"))? + }; words.push(Value::String(format!("{key}={val_str}"))); } Arg::DoubleDash => { @@ -6309,6 +6361,11 @@ pub(crate) async fn bind_tool_args( ) })?; let value = apply_tilde_expansion(value, home.as_deref()); + if let Expr::NumericLiteral { raw, .. } = expr { + tool_args + .positional_raw + .insert(tool_args.positional.len(), raw.clone()); + } tool_args.positional.push(value); } } @@ -6319,6 +6376,13 @@ pub(crate) async fn bind_tool_args( ) })?; let value = apply_tilde_expansion(value, home.as_deref()); + // `test`'s numeric operators still get the real + // `value`; a text consumer gets `raw`. + if let Expr::NumericLiteral { raw, .. } = expr { + tool_args + .positional_raw + .insert(tool_args.positional.len(), raw.clone()); + } tool_args.positional.push(value); } } @@ -6335,12 +6399,17 @@ pub(crate) async fn bind_tool_args( let val = apply_tilde_expansion(val, home.as_deref()); // Loud on binary (GH #116): `test --k=$BIN` must not // silently reassemble the placeholder into the raw-argv - // positional stream `test` binds against. - let val_str = crate::interpreter::value_to_text_sink_named( - &val, - "a --key=value argument", - ) - .map_err(|e| anyhow::anyhow!("{e}"))?; + // positional stream `test` binds against. Source text + // wins, as in the Verbatim binder's `Arg::Named` arm. + let val_str = if let Expr::NumericLiteral { raw, .. } = value { + raw.clone() + } else { + crate::interpreter::value_to_text_sink_named( + &val, + "a --key=value argument", + ) + .map_err(|e| anyhow::anyhow!("{e}"))? + }; tool_args .positional .push(Value::String(format!("--{key}={val_str}"))); @@ -6352,11 +6421,15 @@ pub(crate) async fn bind_tool_args( let val = apply_tilde_expansion(val, home.as_deref()); // Loud on binary (GH #116): same reasoning as the Named // arm above, for the bare `key=value` raw-argv form. - let val_str = crate::interpreter::value_to_text_sink_named( - &val, - "a key=value argument", - ) - .map_err(|e| anyhow::anyhow!("{e}"))?; + let val_str = if let Expr::NumericLiteral { raw, .. } = value { + raw.clone() + } else { + crate::interpreter::value_to_text_sink_named( + &val, + "a key=value argument", + ) + .map_err(|e| anyhow::anyhow!("{e}"))? + }; tool_args .positional .push(Value::String(format!("{key}={val_str}"))); @@ -6437,6 +6510,14 @@ pub(crate) async fn bind_tool_args( } if let Some(value) = source.eval(expr).await? { let value = apply_tilde_expansion(value, home.as_deref()); + // The path `echo` reads: it takes `args.positional` + // directly, never the clap-parsed field, so a plain + // `Value::Int` here could not reproduce `-0`. + if let Expr::NumericLiteral { raw, .. } = expr { + tool_args + .positional_raw + .insert(tool_args.positional.len(), raw.clone()); + } tool_args.positional.push(value); } } @@ -6448,11 +6529,15 @@ pub(crate) async fn bind_tool_args( // `--key=value`, the same collapse the `WordAssign` arm // below does for `A=1` (GH #189). The value still expands. if past_double_dash { - let val_str = crate::interpreter::value_to_text_sink_named( - &val, - "a --key=value operand after `--`", - ) - .map_err(|e| anyhow::anyhow!("{e}"))?; + let val_str = if let Expr::NumericLiteral { raw, .. } = value { + raw.clone() + } else { + crate::interpreter::value_to_text_sink_named( + &val, + "a --key=value operand after `--`", + ) + .map_err(|e| anyhow::anyhow!("{e}"))? + }; tool_args .positional .push(Value::String(format!("--{key}={val_str}"))); @@ -6507,6 +6592,12 @@ pub(crate) async fn bind_tool_args( } // Value::Bool(false): absent == false, nothing to insert. } else { + // A named value is normally read off the clap-parsed + // field, built from `to_argv()` — see + // `ToolArgs::named_raw`. + if let Expr::NumericLiteral { raw, .. } = value { + tool_args.named_raw.insert(key.clone(), raw.clone()); + } tool_args.named.insert(key.clone(), val); } } @@ -6527,18 +6618,26 @@ pub(crate) async fn bind_tool_args( // named-assignment path `past_double_dash` exists to // suppress for flags right above this arm. if accepts_word_assign && !past_double_dash { + if let Expr::NumericLiteral { raw, .. } = value { + tool_args.named_raw.insert(key.clone(), raw.clone()); + } tool_args.named.insert(key.clone(), val); } else { // Stringify "key=value" and pass as a positional. // Matches bash: `cat foo=bar` opens a file named `foo=bar`. // Loud on binary (GH #116): `cat foo=$BIN`/`dd if=$BIN` // must not silently become a path/operand literally named - // `foo=[binary: N bytes]`. - let val_str = crate::interpreter::value_to_text_sink_named( - &val, - "a key=value argument", - ) - .map_err(|e| anyhow::anyhow!("{e}"))?; + // `foo=[binary: N bytes]`. Source text wins, as in + // every other composed-string arm. + let val_str = if let Expr::NumericLiteral { raw, .. } = value { + raw.clone() + } else { + crate::interpreter::value_to_text_sink_named( + &val, + "a key=value argument", + ) + .map_err(|e| anyhow::anyhow!("{e}"))? + }; tool_args.positional.push(Value::String(format!("{key}={val_str}"))); } } @@ -6786,7 +6885,15 @@ pub(crate) async fn bind_tool_args( tool_args.positional.len() }; + // This block reindexes `positional`, and `positional_raw` is keyed + // by index — a value that moves must carry its raw text along or the + // map mislabels some other positional's text as this one's. + // `old_raw` is keyed by the pre-drain index; both destinations insert + // under the index the value actually lands at. + let old_raw = std::mem::take(&mut tool_args.positional_raw); let mut remaining = Vec::new(); + let mut remaining_raw: std::collections::BTreeMap = + std::collections::BTreeMap::new(); let mut positional_iter = tool_args.positional.drain(..).enumerate(); for param in &schema.params { @@ -6799,10 +6906,16 @@ pub(crate) async fn bind_tool_args( loop { match positional_iter.next() { Some((idx, val)) if idx < pre_dash_count => { + if let Some(raw) = old_raw.get(&idx) { + tool_args.named_raw.insert(param.name.clone(), raw.clone()); + } tool_args.named.insert(param.name.clone(), val); break; } - Some((_, val)) => { + Some((idx, val)) => { + if let Some(raw) = old_raw.get(&idx) { + remaining_raw.insert(remaining.len(), raw.clone()); + } remaining.push(val); } None => break, @@ -6810,8 +6923,14 @@ pub(crate) async fn bind_tool_args( } } - remaining.extend(positional_iter.map(|(_, v)| v)); + for (idx, val) in positional_iter { + if let Some(raw) = old_raw.get(&idx) { + remaining_raw.insert(remaining.len(), raw.clone()); + } + remaining.push(val); + } tool_args.positional = remaining; + tool_args.positional_raw = remaining_raw; } Ok(tool_args) diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 115ff047..d178ca5c 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -38,6 +38,7 @@ use std::fmt; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::UNIX_EPOCH; use kaish_types::clock::system_now; +use kaish_types::Value; /// Global counter for generating unique markers across all tokenize calls. static MARKER_COUNTER: AtomicU64 = AtomicU64::new(0); @@ -87,6 +88,11 @@ pub enum LexerError { UnterminatedVarRef, InvalidEscape, InvalidNumber, + /// An integer numeral parsed but did not fit in `i64`. The regex behind + /// `lex_int`/`parse_int` admits only `-?[0-9]+`, so overflow is the only + /// way that parse fails — this is a distinct variant so the message can + /// name the limit instead of just saying "invalid". + IntegerOutOfRange, InvalidFloatNoLeading, InvalidFloatNoTrailing, /// Nesting depth exceeded (too many nested parentheses in arithmetic). @@ -129,6 +135,12 @@ pub enum LexerError { HashInsideWord, } +/// Message for a numeral outside i64 range. Shared with `arithmetic::parse_number` +/// so the lexer and `$(( ))` name the same limit in the same words. +pub(crate) const INTEGER_OUT_OF_RANGE: &str = + "does not fit in a 64-bit integer (-9223372036854775808..9223372036854775807); \ + quote it to keep the text"; + impl fmt::Display for LexerError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -143,6 +155,7 @@ impl fmt::Display for LexerError { } LexerError::InvalidEscape => write!(f, "invalid escape sequence"), LexerError::InvalidNumber => write!(f, "invalid number"), + LexerError::IntegerOutOfRange => write!(f, "{INTEGER_OUT_OF_RANGE}"), LexerError::InvalidFloatNoLeading => write!(f, "float must have leading digit"), LexerError::InvalidFloatNoTrailing => write!(f, "float must have trailing digit"), LexerError::NestingTooDeep => write!(f, "nesting depth exceeded (max {})", MAX_PAREN_DEPTH), @@ -217,6 +230,15 @@ pub struct HereDocData { pub body_start_offset: usize, } +/// A numeral's typed value and its verbatim source text — see +/// `Token::NumericLiteral`. Logos requires a single-field variant payload, +/// hence the wrapper struct (the same shape as `HereDocData`, above). +#[derive(Debug, Clone, PartialEq)] +pub struct NumericLiteralData { + pub value: Value, + pub raw: String, +} + /// A word is anything that is not whitespace and not an operator, so the /// bareword and path rules below admit `\u{80}-\u{10FFFF}` — this file's /// spelling of "any non-ASCII scalar value" — alongside their ASCII classes. @@ -640,6 +662,21 @@ pub enum Token { #[regex(r"-?[0-9]+\.[0-9]+", lex_float)] Float(f64), + /// A plain `Int`/`Float` whose own `Display` does not reproduce the source + /// text it was lexed from — a negative zero (`-0`, `-0.0`) or a + /// non-canonical trailing fraction digit (`0.10`, `1.0`). Carries the + /// typed value, so arithmetic and `--json` still see a real number, and + /// the verbatim text, so argv and plan rendering reproduce what was typed. + /// See `docs/LANGUAGE.md`, "A bare number follows JSON rules". + /// + /// A leading zero (`007`) is a different case, reclassified to + /// `NumberIdent` — see `has_invalid_leading_zero`. + /// + /// Never produced by logos. `preserve_numeric_source_text` synthesizes it + /// as the LAST step of `tokenize_impl`, after the fusion passes, which + /// match `Int`/`Float` directly. The common case pays nothing. + NumericLiteral(NumericLiteralData), + // ═══════════════════════════════════════════════════════════════════ // Invalid patterns (caught before valid tokens for better errors) // ═══════════════════════════════════════════════════════════════════ @@ -830,7 +867,9 @@ impl Token { Token::String(_) | Token::SingleString(_) | Token::HereDoc(_) => TokenCategory::String, // Numbers - Token::Int(_) | Token::Float(_) | Token::Arithmetic(_) => TokenCategory::Number, + Token::Int(_) | Token::Float(_) | Token::Arithmetic(_) | Token::NumericLiteral(_) => { + TokenCategory::Number + } // Variables Token::VarRef(_) @@ -1055,7 +1094,16 @@ fn lex_var_length(lex: &mut logos::Lexer) -> String { /// Lex an integer literal. fn lex_int(lex: &mut logos::Lexer) -> Result { - lex.slice().parse().map_err(|_| LexerError::InvalidNumber) + let slice = lex.slice(); + // A leading-zero numeral is text, not a number, whether or not its + // digits fit in i64 — `09223372036854775808` is text (leading zero) + // before it is ever an overflow. `preserve_numeric_source_text` reads + // the source span, not this value, to reclassify the token into + // `NumberIdent`, so any placeholder here is discarded. + if has_invalid_leading_zero(slice) { + return Ok(0); + } + slice.parse().map_err(|_| LexerError::IntegerOutOfRange) } /// Lex a float literal. @@ -1305,6 +1353,7 @@ impl fmt::Display for Token { Token::VarLength(v) => write!(f, "${{#{}}}", v), Token::Int(n) => write!(f, "INT({})", n), Token::Float(n) => write!(f, "FLOAT({})", n), + Token::NumericLiteral(d) => write!(f, "NUMERICLITERAL({:?}, raw={:?})", d.value, d.raw), Token::Path(s) => write!(f, "PATH({})", s), Token::Ident(s) => write!(f, "IDENT({})", s), Token::NumberIdent(s) => write!(f, "NUMIDENT({})", s), @@ -1392,6 +1441,7 @@ impl Token { | Token::Arithmetic(_) | Token::Int(_) | Token::Float(_) + | Token::NumericLiteral(_) | Token::True | Token::False | Token::VarRef(_) @@ -3501,12 +3551,90 @@ fn tokenize_impl( }) .collect(); - Ok(merge_glob_adjacent( - merge_colon_adjacent(merge_flag_metachar_adjacent(mapped), source), + Ok(preserve_numeric_source_text( + merge_glob_adjacent( + merge_colon_adjacent(merge_flag_metachar_adjacent(mapped), source), + source, + ), source, )) } +/// True when a numeral's integer part is not a valid JSON number (RFC 8259: +/// `int = zero / (digit1-9 *DIGIT)`) — more than one digit and a leading `0`: +/// `007`, `010`, `-022`, and the integer part of `007.5`. A lone `0` (`0`, +/// `-0`, `0.5`) is the `zero` alternative and is fine. +/// +/// `fromjson '007'` is already a parse error; the lexer now agrees. +fn has_invalid_leading_zero(raw: &str) -> bool { + let unsigned = raw.strip_prefix('-').unwrap_or(raw); + let int_part = unsigned.split('.').next().unwrap_or(unsigned); + int_part.len() > 1 && int_part.starts_with('0') +} + +/// True when a word is a numeral in every respect except its leading zero — +/// `007`, `010`, `-022`, `007.5`. These lex as [`Token::NumberIdent`]. +/// +/// Callers use this where kaish needs a number and got text, so the error can +/// name the leading zero rather than report a shape mismatch. A word that is +/// not otherwise a numeral (`007abc`) answers false: there is no number the +/// author might have meant. +pub(crate) fn is_leading_zero_numeral(word: &str) -> bool { + let unsigned = word.strip_prefix('-').unwrap_or(word); + let mut parts = unsigned.split('.'); + let int_part = parts.next().unwrap_or(""); + let frac_part = parts.next(); + if parts.next().is_some() { + return false; + } + let digits_only = |s: &str| !s.is_empty() && s.bytes().all(|b| b.is_ascii_digit()); + if !digits_only(int_part) { + return false; + } + if let Some(frac) = frac_part + && !digits_only(frac) + { + return false; + } + has_invalid_leading_zero(word) +} + +/// Reclassify a plain `Int`/`Float` token from its own source text, into +/// [`Token::NumberIdent`] or [`Token::NumericLiteral`]. The common case +/// (`-1`, `42`, `3.14`) pays one string comparison and stays unchanged. +/// +/// Runs as the LAST step of `tokenize_impl`, after every fusion pass: +/// `is_colon_mergeable` matches `Int` and `Float` directly and +/// `is_glob_mergeable` matches `Int`, so a numeral must still present its +/// ordinary shape while fusion decides. +/// Spans are original-source coordinates by now, so `source[span]` is the +/// exact word the author typed. +fn preserve_numeric_source_text(tokens: Vec>, source: &str) -> Vec> { + tokens + .into_iter() + .map(|t| { + let (value, canonical): (Value, String) = match &t.token { + Token::Int(n) => (Value::Int(*n), n.to_string()), + Token::Float(n) => (Value::Float(*n), n.to_string()), + _ => return t, + }; + let Some(raw) = source.get(t.span.start..t.span.end) else { + return t; + }; + if has_invalid_leading_zero(raw) { + return Spanned::new(Token::NumberIdent(raw.to_string()), t.span); + } + if raw == canonical { + return t; + } + Spanned::new( + Token::NumericLiteral(NumericLiteralData { value, raw: raw.to_string() }), + t.span, + ) + }) + .collect() +} + /// Extract the string content from a string token (removes quotes, processes escapes). pub fn parse_string_literal(source: &str) -> Result { // Remove surrounding quotes @@ -3642,7 +3770,7 @@ pub fn parse_var_ref(source: &str) -> Result, LexerError> { /// Parse an integer literal. pub fn parse_int(source: &str) -> Result { - source.parse().map_err(|_| LexerError::InvalidNumber) + source.parse().map_err(|_| LexerError::IntegerOutOfRange) } /// Parse a float literal. diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index e8b61216..82ea1a75 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -461,9 +461,14 @@ fn parse_subscript(inner: &str) -> VarSegment { // isn't a numeric slice falls through to a bareword key (`["a:b"]` covers // colon-bearing keys explicitly). if let Some((lhs, rhs)) = inner.split_once(':') { + // A slice bound is a number position: `[007:2]` sliced from 7 and + // silently returned an empty list. Refusing it falls through to a + // bareword key, which `scope.rs` reports against the container. let bound = |s: &str| -> Option> { if s.is_empty() { Some(None) + } else if lexer::is_leading_zero_numeral(s) { + None } else { s.parse::().ok().map(Some) } @@ -472,8 +477,11 @@ fn parse_subscript(inner: &str) -> VarSegment { return VarSegment::Slice(start, end); } } - // Integer index: `[0]`, `[-1]`. - if let Ok(i) = inner.parse::() { + // Integer index: `[0]`, `[-1]`. `[007]` is text and falls through to the + // bareword key below. + if !lexer::is_leading_zero_numeral(inner) + && let Ok(i) = inner.parse::() + { return VarSegment::Index(i); } // Bareword literal key: `[name]`, `[content-type]`. @@ -1289,6 +1297,12 @@ fn parse_tokens( if let Err(specific) = validate_heredoc_bodies(&tokens) { return specific; } + // The count grammar matches `Token::Int`, which `break 007` no longer + // produces, so chumsky blames the whole statement alternative set. + let error_starts: Vec = errs.iter().map(|e| e.span().start).collect(); + if let Err(specific) = validate_leading_zero_counts(&tokens, &error_starts) { + return specific; + } // `reject_glued_args` raises this from inside a `try_map` wrapping the // whole argv, where chumsky's alt bookkeeping swaps in a shallower // sibling's span — `git show HEAD:x.py` reported at `show`. Re-derive @@ -1373,18 +1387,28 @@ where recursive(|stmt| { let terminator = choice((just(Token::Newline), just(Token::Semi))).repeated(); + // A loop count is an integer. `NumericLiteral` is here because `-0` is + // one — a valid count whose source text does not round-trip, so it + // lexes as that variant rather than `Int` and would otherwise stop + // parsing. + let loop_count = select! { + Token::Int(n) => n as usize, + Token::NumericLiteral(data) if matches!(data.value, Value::Int(_)) => { + match data.value { + Value::Int(n) => n as usize, + _ => unreachable!("guarded by the select! pattern above"), + } + }, + }; + // break [N] - break out of N levels of loops (default 1) let break_stmt = just(Token::Break) - .ignore_then( - select! { Token::Int(n) => n as usize }.or_not() - ) + .ignore_then(loop_count.or_not()) .map(Stmt::Break); // continue [N] - continue to next iteration, skipping N levels (default 1) let continue_stmt = just(Token::Continue) - .ignore_then( - select! { Token::Int(n) => n as usize }.or_not() - ) + .ignore_then(loop_count.or_not()) .map(Stmt::Continue); // return [expr] - return from a tool @@ -1608,6 +1632,13 @@ where select! { Token::SingleString(s) => VarSegment::Key(s) }, select! { Token::Int(n) => VarSegment::Index(n) }, select! { Token::Ident(s) => parse_subscript(&s) }, + // Without this arm `r[007]=v` was a parse error while `${r[007]}` + // read fine. Both are the same text key now. + select! { Token::NumberIdent(s) => parse_subscript(&s) }, + // Same split for `r[-0]=v` and `r[1.0]=v`. Classifying from the raw + // text is what makes read and write agree: it is the same string the + // read path hands `parse_subscript`. + select! { Token::NumericLiteral(d) => parse_subscript(&d.raw) }, )); just(Token::LBracket) @@ -3117,6 +3148,7 @@ where var_expr_parser(), interpolated_string_parser(), literal_parser().map(Expr::Literal), + numeric_literal_parser(), // Glob patterns before ident (GlobWord is more specific) glob_pattern, // Bare identifiers become string literals (shell barewords) @@ -3545,6 +3577,7 @@ fn is_word_token(tok: &Token) -> bool { | Token::SingleString(_) | Token::VarRef(_) | Token::SimpleVarRef(_) | Token::Positional(_) | Token::AllArgs | Token::ArgCount | Token::LastExitCode | Token::CurrentPid | Token::VarLength(_) | Token::Int(_) | Token::Float(_) + | Token::NumericLiteral(_) | Token::NumberIdent(_) | Token::DashNumWord(_) | Token::AtWord(_) | Token::Path(_) | Token::Ident(_) => true, @@ -3673,6 +3706,82 @@ fn glue_candidate_units(tokens: &[(Token, Span)]) -> Vec { /// a shape just leaves the grammar's span in place. The scope is "report the /// right span for a rejection that already happened", never "change what is /// rejected". +/// `break 007` no longer matches the count grammar, so chumsky reports the +/// miss against every statement alternative — a long message that never +/// mentions the zero. +/// +/// Runs only after the grammar has already failed, and only when the grammar's +/// own error sits exactly on that numeral. Like [`validate_glued_args`], this +/// may reword a diagnosis and must never author one: without the position gate +/// it scanned the whole stream and would blame `echo break 007` (where `break` +/// is an argv word the grammar rejected for its own reasons) or answer a real +/// error elsewhere on the line with this message instead. +fn validate_leading_zero_counts( + tokens: &[(Token, Span)], + error_starts: &[usize], +) -> Result<(), Vec> { + for (i, pair) in tokens.windows(2).enumerate() { + let keyword = match &pair[0].0 { + Token::Break => "break", + Token::Continue => "continue", + _ => continue, + }; + // `break` is also a bareword an argument list accepts, and `echo break + // 007` fails ON the numeral just like the statement does. Only a + // `break` in statement position is the one this message describes. + let at_statement_start = match i.checked_sub(1).map(|prev| &tokens[prev].0) { + None => true, + Some( + Token::Newline + | Token::Semi + | Token::DoubleSemi + | Token::Do + | Token::Then + | Token::Else + | Token::LBrace + | Token::And + | Token::Or, + ) => true, + Some(_) => false, + }; + if !at_statement_start { + continue; + } + let Token::NumberIdent(word) = &pair[1].0 else { + continue; + }; + if !lexer::is_leading_zero_numeral(word) { + continue; + } + // The grammar must have failed ON this numeral. Anywhere else and the + // verdict belongs to whatever the grammar was actually judging. + if !error_starts.contains(&pair[1].1.start) { + continue; + } + // Keep the sign: `break -022` is not fixed by writing `break 22`. + let sign = if word.starts_with('-') { "-" } else { "" }; + let digits = word.trim_start_matches('-').trim_start_matches('0'); + let count = format!("{sign}{}", if digits.is_empty() { "0" } else { digits }); + // The count grammar takes only a whole number — `007.5` trims to + // `7.5`, which is itself a parse error, so the fix cannot be the + // trimmed value. Name the integer part instead. + let message = if let Some((int_part, _)) = count.split_once('.') { + let int_part = if int_part.is_empty() || int_part == "-" { "0" } else { int_part }; + format!( + "`{keyword}` takes a whole-number loop count and `{word}` is text (leading \ + zero) — write a whole number such as `{keyword} {int_part}`" + ) + } else { + format!( + "`{keyword}` takes a loop count and `{word}` is text (leading zero) — write \ + `{keyword} {count}`" + ) + }; + return Err(vec![ParseError { span: pair[1].1, message }]); + } + Ok(()) +} + fn validate_glued_args( tokens: &[(Token, Span)], from_offset: usize, @@ -3808,6 +3917,23 @@ where .boxed() } +/// A numeral whose source text does not round-trip through its own typed +/// `Display` — a negative zero, a leading zero, or a non-canonical trailing +/// fraction digit. See `lexer::Token::NumericLiteral`. Kept separate from +/// `literal_parser` because it produces `Expr::NumericLiteral` directly +/// (carrying `raw` alongside `value`), not a bare `Value` for `Expr::Literal` +/// to wrap. +fn numeric_literal_parser<'tokens, I>( +) -> impl Parser<'tokens, I, Expr, extra::Err>> + Clone +where + I: ValueInput<'tokens, Token = Token, Span = Span>, +{ + select! { + Token::NumericLiteral(d) => Expr::NumericLiteral { value: d.value, raw: d.raw }, + } + .labelled("literal") +} + /// Identifier parser. fn ident_parser<'tokens, I>( ) -> impl Parser<'tokens, I, String, extra::Err>> + Clone diff --git a/crates/kaish-kernel/src/scheduler/pipeline.rs b/crates/kaish-kernel/src/scheduler/pipeline.rs index acb45fd4..f7302e5a 100644 --- a/crates/kaish-kernel/src/scheduler/pipeline.rs +++ b/crates/kaish-kernel/src/scheduler/pipeline.rs @@ -245,6 +245,14 @@ async fn eval_redirect_target( ctx: &ExecContext, dispatcher: &dyn CommandDispatcher, ) -> Result { + // A numeral whose source text does not round-trip through its own typed + // `Display` — `> -0` wrote a file named `0`, while `--plan` reported the + // target as `-0`. `plan_redirect_target` reads `raw` for exactly this, so + // read it here too or the plan document describes a write that never + // happens. A literal needs no evaluation, so there is nothing to skip. + if let Expr::NumericLiteral { raw, .. } = expr { + return Ok(raw.clone()); + } let value = dispatcher .eval_expr(expr, ctx) .await diff --git a/crates/kaish-kernel/src/tools/builtin/echo.rs b/crates/kaish-kernel/src/tools/builtin/echo.rs index 08d5c41b..aa575925 100644 --- a/crates/kaish-kernel/src/tools/builtin/echo.rs +++ b/crates/kaish-kernel/src/tools/builtin/echo.rs @@ -55,7 +55,15 @@ impl Tool for Echo { // should be is silent corruption. (Path-coercing builtins like `mkdir` // and env export remain deferred — the binary-at-text-sinks cluster.) let mut words = Vec::with_capacity(args.positional.len()); - for value in &args.positional { + for (i, value) in args.positional.iter().enumerate() { + // A non-canonical numeral (`-0`, `0.10`, `1.0`) keeps its own + // source text: `Value::Int`/`Float` has no way to reproduce it + // once typed (no negative zero, no trailing `.0`) — see + // `ToolArgs::positional_raw`. + if let Some(raw) = args.positional_raw.get(&i) { + words.push(raw.clone()); + continue; + } match crate::interpreter::value_to_text_sink(value) { Ok(s) => words.push(s), Err(e) => return ExecResult::failure(1, format!("echo: {e}")), diff --git a/crates/kaish-kernel/src/tools/builtin/exec.rs b/crates/kaish-kernel/src/tools/builtin/exec.rs index 506e72f8..872debf1 100644 --- a/crates/kaish-kernel/src/tools/builtin/exec.rs +++ b/crates/kaish-kernel/src/tools/builtin/exec.rs @@ -131,10 +131,16 @@ impl Tool for Exec { // `[binary: N bytes]` placeholder) — exec's argv crosses the same // process boundary as any other external command's. let mut argv: Vec = Vec::with_capacity(args.positional.len().saturating_sub(1)); - for v in args.positional.iter().skip(1) { + for (i, v) in args.positional.iter().enumerate().skip(1) { if let Some(msg) = crate::interpreter::structured_boundary_error("a command argument", v) { return ExecResult::failure(1, format!("exec: {msg}")); } + // `exec /bin/echo -0` must reach the process as the same argv + // `/bin/echo -0` does, so the source text wins here too. + if let Some(raw) = args.positional_raw.get(&i) { + argv.push(raw.clone()); + continue; + } match crate::interpreter::value_to_text_sink(v) { Ok(s) => argv.push(s), Err(e) => return ExecResult::failure(1, format!("exec: {e}")), diff --git a/crates/kaish-kernel/src/validator/walker.rs b/crates/kaish-kernel/src/validator/walker.rs index 54e5d1dd..7b3eff59 100644 --- a/crates/kaish-kernel/src/validator/walker.rs +++ b/crates/kaish-kernel/src/validator/walker.rs @@ -572,6 +572,7 @@ impl<'a> Validator<'a> { match expr { Expr::Not(inner) => self.validate_expr(inner), Expr::Literal(_) => {} + Expr::NumericLiteral { .. } => {} Expr::VarRef(path) => self.validate_var_ref(path), Expr::Interpolated(parts) => { for part in parts { @@ -1177,6 +1178,13 @@ fn bind_value_or_flag( fn expr_to_placeholder(expr: &Expr) -> Value { match expr { Expr::Literal(val) => val.clone(), + // A numeral whose source text doesn't round-trip through `Display` + // (`0.0`, `-0`, `007`) lexes as `NumericLiteral`, not `Literal` — the + // same split `kernel.rs`'s runtime binder reads. Without this arm a + // `Tool::validate` that matches `Value::Int`/`Value::Float` never + // sees the value at all; it gets the `` placeholder below + // and the check silently never fires (missed `seq 1 0.0 10`). + Expr::NumericLiteral { value, .. } => value.clone(), Expr::Interpolated(parts) if parts.len() == 1 => { if let StringPart::Literal(s) = &parts[0] { Value::String(s.clone()) diff --git a/crates/kaish-kernel/tests/builtin_fidelity_tests.rs b/crates/kaish-kernel/tests/builtin_fidelity_tests.rs index e7a512e5..afe32c55 100644 --- a/crates/kaish-kernel/tests/builtin_fidelity_tests.rs +++ b/crates/kaish-kernel/tests/builtin_fidelity_tests.rs @@ -636,3 +636,30 @@ async fn jq_arg_two_value_form_still_binds_name_and_value() { assert_eq!(code, 0, "out={out:?}"); assert_eq!(out.trim(), "\"kaish\"", "--arg NAME VAL binds both slots: {out:?}"); } + +// ───────── builtin echo keeps a non-canonical numeral's source text ───────── +// `echo -0` used to print `0`: the builtin reads `args.positional` directly +// as a typed `Value` (never the clap-parsed field, which is a +// validation-only sink), and `Value::Int` has no way to reproduce `-0` — +// the external-command fix (`kernel.rs::build_args_flat`) never touches +// this path at all, since `echo` is a builtin, not a spawned process. +// `ToolArgs::positional_raw` closes the same class of bug for builtin argv +// that build_args_flat closed for external-command argv. + +#[tokio::test] +async fn echo_builtin_preserves_noncanonical_numeral_source_text() { + let (out, code) = run("echo -0 0.10 1.0 -0.0", "").await; + assert_eq!(code, 0, "out={out:?}"); + assert_eq!( + out.trim(), + "-0 0.10 1.0 -0.0", + "the builtin must print the exact source words, not their typed re-serialization" + ); +} + +#[tokio::test] +async fn echo_builtin_still_prints_canonical_numerals_plainly() { + let (out, code) = run("echo -1 -5 -0.5 42", "").await; + assert_eq!(code, 0, "out={out:?}"); + assert_eq!(out.trim(), "-1 -5 -0.5 42"); +} diff --git a/crates/kaish-kernel/tests/external_command_tests.rs b/crates/kaish-kernel/tests/external_command_tests.rs index 7493427e..e81800db 100644 --- a/crates/kaish-kernel/tests/external_command_tests.rs +++ b/crates/kaish-kernel/tests/external_command_tests.rs @@ -51,6 +51,27 @@ async fn external_command_with_args() { assert!(result.ok()); } +/// The real argv an external process receives must be the exact source +/// text of a numeral argv word, not a re-serialized typed value. +/// +/// `/bin/echo` (an absolute path, so it always resolves externally, past the +/// builtin `echo`) prints back whatever `execve` actually received. Before +/// the fix, `-0` planned AND executed as a bare `0` — `Value::Int` has no +/// negative zero, so the sign was gone before `build_args_flat` ever ran — +/// which is the same silent corruption `xargs -0 rm -f` suffers on its +/// null-delimiter flag. +#[tokio::test] +async fn external_command_argv_preserves_noncanonical_numeral_source_text() { + let kernel = repl_kernel(); + let result = kernel.execute("/bin/echo -0 007 -0.0").await.unwrap(); + assert!(result.ok(), "echo should succeed: {:?}", result); + assert_eq!( + result.text_out().trim(), + "-0 007 -0.0", + "the external argv must be the exact source words, not their typed re-serialization" + ); +} + #[tokio::test] async fn large_buffered_stdin_does_not_deadlock() { // A buffered String stdin used to be write_all'd INLINE, before the diff --git a/crates/kaish-kernel/tests/kernel_error_tests.rs b/crates/kaish-kernel/tests/kernel_error_tests.rs index aecd0e6e..a99c757d 100644 --- a/crates/kaish-kernel/tests/kernel_error_tests.rs +++ b/crates/kaish-kernel/tests/kernel_error_tests.rs @@ -224,6 +224,31 @@ async fn validation_issue_about_a_command_carries_its_name() { ); } +/// `0.0`, `-0`, and `0.00` all spell zero without matching a canonical +/// `Value::Int`/`Value::Float` `Display` round-trip — the lexer keeps the +/// source text, so validation sees `Expr::NumericLiteral`, not the plain +/// `Expr::Literal` the canonical `seq 1 0 10` above produces. Every +/// spelling of zero must be caught before execution, the same as the +/// canonical one. +#[rstest] +#[case("seq 1 0.0 10")] +#[case("seq 1 -0 10")] +#[case("seq 1 0.00 10")] +#[tokio::test] +async fn seq_zero_increment_is_caught_at_every_spelling(#[case] script: &str) { + let kernel = make_kernel(); + let err = kernel.execute(script).await.expect_err(&format!("`{script}` must be rejected")); + + let KernelError::Validation { issues, .. } = err else { + panic!("`{script}` must be KernelError::Validation, not {err:?}"); + }; + + assert!( + issues.iter().any(|i| i.code == kaish_kernel::validator::IssueCode::SeqZeroIncrement), + "`{script}` must raise SeqZeroIncrement: {issues:?}" + ); +} + /// A validation issue that is not about any command — `break` outside a /// loop is a language-level statement, not a command invocation — reports /// the command as genuinely absent, never an empty string or a guess. diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs new file mode 100644 index 00000000..d0ecf587 --- /dev/null +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -0,0 +1,446 @@ +//! A leading zero makes a word text; where kaish needs a number, it is an error. +//! +//! `007` is not a JSON number (RFC 8259 admits `0` or a nonzero leading digit), +//! and `fromjson '007'` has always refused it. The lexer now agrees, so a +//! leading-zero numeral is text everywhere a word is text — `chmod 0644`, +//! `echo 007` — and the positions that need a real number say so instead of +//! reinterpreting the digits. +//! +//! The failure this replaces was silent in both directions: `echo 007` printed +//! `7`, and `$((010 + 1))` answered 11 where bash answers 9. +#![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:?}"), + } +} + +// ── A word is text, and keeps the text that was typed ────────────────────── + +#[tokio::test] +async fn a_leading_zero_word_keeps_its_digits() { + for (source, expected) in + [("echo 007", "007"), ("echo 0644", "0644"), ("echo 00", "00"), ("echo 007.5", "007.5")] + { + let (code, out, err) = run(source).await; + assert_eq!(code, 0, "{source:?} must run: {err:?}"); + assert_eq!(out, expected, "{source:?} lost the text that was typed"); + } +} + +/// The mode operand is the case that decides this: it is by far the most +/// common leading-zero word in real shell, and it must cost nothing. +#[tokio::test] +async fn a_mode_operand_survives_as_typed() { + let (code, out, _) = run("echo 0755 0644 0000").await; + assert_eq!(code, 0); + assert_eq!(out, "0755 0644 0000"); +} + +#[tokio::test] +async fn a_leading_zero_word_is_a_string_not_a_number() { + let (_, out, _) = run("echo $(typeof 007)").await; + assert_eq!(out, "string", "007 must not type as a number"); + let (_, out, _) = run("echo $(typeof 7)").await; + assert_eq!(out, "number", "an ordinary numeral is untouched"); +} + +// ── Where kaish needs a number, the leading zero is named ────────────────── + +#[tokio::test] +async fn break_and_continue_name_the_leading_zero() { + for (source, fix) in [ + ("for i in 1 2; do break 007; done", "write `break 7`"), + ("for i in 1 2; do continue 010; done", "write `continue 10`"), + ] { + let text = err_of(source).await; + assert!(text.contains("(leading zero)"), "must name the cause: {text:?}"); + assert!(text.contains(fix), "must name the fix: {text:?}"); + } +} + +/// The validator that produces the message above runs only after the grammar +/// has already failed. A loop count that parses must never reach it. +#[tokio::test] +async fn an_ordinary_loop_count_still_parses() { + let (code, out, err) = run("for i in 1 2 3; do break 2; done; echo done").await; + assert_eq!(code, 0, "break 2 must still parse: {err:?}"); + assert_eq!(out, "done"); +} + +/// `-0` is a valid count and a valid JSON number, but its source text does not +/// round-trip, so it lexes as `NumericLiteral` rather than `Int`. The count +/// grammar matched only `Int`, which turned `break -0` into a parse error -- +/// a regression this rule introduced and nothing else caught. +#[tokio::test] +async fn a_negative_zero_count_still_parses() { + for source in [ + "for i in 1 2 3; do break -0; done; echo done", + "for i in 1 2 3; do continue -0; done; echo done", + ] { + let (code, out, err) = run(source).await; + assert_eq!(code, 0, "-0 is a number, not a leading zero: {source} {err:?}"); + assert_eq!(out, "done", "{source}"); + } +} + +/// The message may reword a diagnosis and must never author one. `break` is +/// also a bareword an argument list accepts, and `echo break 007` fails ON the +/// numeral exactly like the statement does, so position in the token stream is +/// not enough to tell them apart. +#[tokio::test] +async fn the_count_message_never_speaks_for_an_argument() { + for source in ["echo break 007", "echo continue 007"] { + let text = err_of(source).await; + assert!( + !text.contains("takes a loop count"), + "{source:?} is not a loop statement: {text:?}" + ); + } +} + +/// And it must not answer a real error elsewhere on the line with this one. +#[tokio::test] +async fn a_real_error_elsewhere_still_wins() { + let text = err_of("if true; then echo hi; done +break 007").await; + assert!(text.contains("found 'done'"), "the grammar's own error must stand: {text:?}"); + assert!(!text.contains("takes a loop count"), "must not mask it: {text:?}"); +} + +/// Every statement-start context the gate admits, so narrowing it cannot +/// quietly stop diagnosing the case it exists for. +#[tokio::test] +async fn the_count_message_reaches_every_statement_position() { + for source in [ + "for i in 1 2; do break 007; done", + "for i in 1 2; do echo a; break 007; done", + "while true; do +break 007 +done", + "for i in 1 2; do if true; then break 007; fi; done", + "for i in 1 2; do true && break 007; done", + "for i in 1 2; do continue 007; done", + ] { + let text = err_of(source).await; + assert!(text.contains("takes a loop count"), "{source:?} must be diagnosed: {text:?}"); + } +} + +/// `break -022` is not fixed by writing `break 22`. +#[tokio::test] +async fn the_suggested_count_keeps_its_sign() { + let text = err_of("for i in 1 2; do break -022; done").await; + assert!(text.contains("write `break -22`"), "the sign must survive: {text:?}"); +} + +/// `break 007.5` used to suggest `break 7.5` — the fraction survives the +/// zero-trim, but the count grammar takes only a whole number, so the +/// suggestion was itself a parse error. The message must not repeat that +/// mistake, and `break 007` (a genuine integer) must be unaffected. +#[tokio::test] +async fn the_suggested_count_is_never_a_fraction() { + let text = err_of("for i in 1 2; do break 007.5; done").await; + assert!(text.contains("whole-number"), "must say a whole number is needed: {text:?}"); + assert!(!text.contains("write `break 7.5`"), "must not suggest a fraction: {text:?}"); + + let text = err_of("for i in 1 2; do break 007; done").await; + assert!(text.contains("write `break 7`"), "an integer count is unaffected: {text:?}"); +} + +/// bash reads `010` as octal and answers 9; kaish reads no octal and would +/// answer 11. Answering a different number than the shell the author learned +/// is the outcome worth refusing. +#[tokio::test] +async fn arithmetic_refuses_a_leading_zero_rather_than_reading_decimal() { + let text = err_of("echo $((010 + 1))").await; + assert!(text.contains("(leading zero)"), "must name the cause: {text:?}"); + assert!(text.contains("no octal"), "must say kaish reads no octal: {text:?}"); + assert!(text.contains("write `10`"), "must name the fix: {text:?}"); + assert!(!text.contains("11"), "must not answer 11: {text:?}"); +} + +#[tokio::test] +async fn ordinary_arithmetic_is_untouched() { + let (code, out, err) = run("echo $((10 + 1))").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "11"); +} + +#[tokio::test] +async fn a_list_index_names_the_leading_zero() { + let text = err_of("xs=[10 20 30]; echo ${xs[007]}").await; + assert!(text.contains("(leading zero)"), "must name the cause: {text:?}"); + assert!(text.contains("${xs[7]}"), "must name the fix: {text:?}"); +} + +// ── Read and write agree on the same subscript ───────────────────────────── + +/// `${r[007]}` read fine while `r[007]=v` was a parse error, and the read +/// resolved to index 7 — so a record whose key really is "007" could not be +/// reached by the name it was stored under. +#[tokio::test] +async fn a_record_key_that_is_a_leading_zero_numeral_round_trips() { + let (code, out, err) = run(r#"r={"007":9}; echo ${r[007]}"#).await; + assert_eq!(code, 0, "reading a 007 key must work: {err:?}"); + assert_eq!(out, "9"); + + let (code, out, err) = run("r={}; r[007]=nine; echo ${r[007]}").await; + assert_eq!(code, 0, "writing a 007 key must work: {err:?}"); + assert_eq!(out, "nine", "the write and the read must name the same key"); +} + +/// A slice carries two number positions, and the leading zero was silent in +/// both: `${xs[007:2]}` sliced from 7, inverted the range, and returned an +/// empty list. An empty result is the one wrong answer a caller cannot tell +/// from a correct one. +#[tokio::test] +async fn a_slice_bound_refuses_a_leading_zero() { + for (source, fix) in [ + ("xs=[1 2 3]; echo ${xs[007:2]}", "${xs[7:2]}"), + ("xs=[1 2 3]; echo ${xs[0:007]}", "${xs[0:7]}"), + ] { + let text = err_of(source).await; + assert!(text.contains("(leading zero)"), "must name the cause: {text:?}"); + assert!(text.contains(fix), "must name the fix: {text:?}"); + } +} + +/// The slice grammar is easy to break while refusing one spelling of it. +#[tokio::test] +async fn every_ordinary_slice_spelling_still_works() { + for (source, expected) in [ + ("xs=[1 2 3]; echo ${xs[0:2]}", "[1,2]"), + ("xs=[1 2 3]; echo ${xs[:2]}", "[1,2]"), + ("xs=[1 2 3]; echo ${xs[1:]}", "[2,3]"), + ("xs=[1 2 3]; echo ${xs[-2:]}", "[2,3]"), + ] { + let (code, out, err) = run(source).await; + assert_eq!(code, 0, "{source:?} must run: {err:?}"); + assert_eq!(out, expected, "{source:?}"); + } +} + +/// A variable holding `010` is a number position when it reaches arithmetic, +/// and parsing it decimal answers 10 where bash answers 8. The literal case +/// was already refused; this is the same numeral arriving by another road. +#[tokio::test] +async fn arithmetic_refuses_a_leading_zero_that_arrives_in_a_variable() { + for (source, decimal) in + [("x=010; echo $((x))", "write `10`"), ("x=007; echo $((x + 1))", "write `7`")] + { + let text = err_of(source).await; + assert!(text.contains("(leading zero)"), "must name the cause: {text:?}"); + assert!(text.contains(decimal), "must name the fix: {text:?}"); + } + let (code, out, err) = run("x=10; echo $((x + 1))").await; + assert_eq!(code, 0, "an ordinary variable must still work: {err:?}"); + assert_eq!(out, "11"); +} + +/// `${r[-0]}` read as index 0 while `r[-0]=v` was a parse error, and `${r[1.0]}` +/// read as a key while the write refused. Neither is a leading-zero numeral — +/// they are numerals whose source text does not round-trip — but they reach +/// the subscript through the same token, so the two sides must classify alike. +#[tokio::test] +async fn read_and_write_classify_every_numeral_subscript_alike() { + let read = err_of("r={}; echo ${r[-0]}").await; + let write = err_of("r={}; r[-0]=v").await; + assert!(read.contains("integer index on a record"), "read: {read:?}"); + assert!(write.contains("integer index on a record"), "write must agree: {write:?}"); + + let (code, out, err) = run("r={}; r[1.0]=v; echo ${r[1.0]}").await; + assert_eq!(code, 0, "a 1.0 key must round-trip: {err:?}"); + assert_eq!(out, "v"); +} + +#[tokio::test] +async fn an_ordinary_index_is_untouched() { + let (code, out, err) = run("xs=[10 20 30]; echo ${xs[1]}").await; + assert_eq!(code, 0, "{err:?}"); + assert_eq!(out, "20"); +} + +// ── The plan document and the run agree ──────────────────────────────────── + +/// `--plan` reported the redirect target as `-0` while execution created a +/// file named `0`. A plan that describes a write that never happens is the +/// one failure a plan consumer cannot detect for itself. +#[cfg(feature = "localfs")] +#[tokio::test] +async fn a_redirect_target_writes_the_file_the_plan_names() { + for target in ["-0", "007", "0.10", "1.0"] { + let dir = tempfile::tempdir().expect("tempdir"); + let kernel = common::kernel_at(dir.path()); + + let planned = kernel + .plan_program(&format!("echo hi > {target}")) + .expect("plan") + .first() + .map(|s| s.plan.rendered.clone()) + .expect("one statement"); + assert!( + planned.ends_with(target), + "the plan must name {target:?} as typed, got {planned:?}" + ); + + let (_, code) = common::run(&kernel, &format!("echo hi > {target}")).await; + assert!( + dir.path().join(target).exists(), + "the plan promised {target:?} (exit {code}); the run created {:?}", + std::fs::read_dir(dir.path()) + .expect("read_dir") + .filter_map(|e| e.ok().map(|e| e.file_name())) + .collect::>() + ); + } +} + +// ── A comparison operand is a number position too ────────────────────────── + +/// `[[ 010 -eq 10 ]]` was true: the string parsed as decimal 10, the number +/// arithmetic refuses to answer. bash answers 8 here (octal), so this is the +/// same three-answers case as `$((010))`, and it takes the same refusal. +#[tokio::test] +async fn numeric_comparison_refuses_a_leading_zero_rather_than_reading_decimal() { + for source in [ + "[[ 010 -eq 10 ]]", + "test 010 -eq 10", + "[[ 10 -lt 0100 ]]", + "x=010; [[ $x -eq 10 ]]", + "x=-007; test $x -eq -7", + r#"[[ "01" -eq "1" ]]"#, + r#"X="01"; [[ "$X" -eq 1 ]]"#, + ] { + let text = err_of(source).await; + assert!(text.contains("(leading zero)"), "{source:?} must name the cause: {text:?}"); + assert!(text.contains("no octal"), "{source:?} must say kaish reads no octal: {text:?}"); + assert!( + text.contains("write `10`") + || text.contains("write `100`") + || text.contains("write `-7`") + || text.contains("write `1`"), + "{source:?} must name the fix: {text:?}" + ); + } +} + +#[tokio::test] +async fn ordinary_numeric_comparison_is_untouched() { + for source in [ + "[[ 10 -eq 10 ]]", + "[[ 0 -eq 0 ]]", + "[[ -0 -eq 0 ]]", + "[[ 0.5 -gt 0 ]]", + "[[ 0.10 -lt 1 ]]", + "test 100 -gt 10", + "x=$(fromjson 10); [[ $x -eq 10 ]]", + ] { + let (code, _, err) = run(source).await; + assert_eq!(code, 0, "{source:?} must be true: {err:?}"); + } +} + +// ── A numeral kaish cannot hold names the limit and the fix ──────────────── + +/// One past `i64::MAX` was "invalid number" with nothing to do about it. The +/// numeral is a valid JSON number, so the error names the limit kaish adds and +/// the quoting that keeps the text. +#[tokio::test] +async fn an_integer_past_64_bits_names_the_limit_and_the_fix() { + for source in ["echo 9223372036854775808", "echo -9223372036854775809", "x=18446744073709551616"] { + let text = err_of(source).await; + assert!(text.contains("64-bit"), "{source:?} must name the limit: {text:?}"); + assert!(text.contains("quote"), "{source:?} must name the fix: {text:?}"); + assert!(!text.contains("invalid number"), "{source:?} must not say only 'invalid': {text:?}"); + } + let (code, out, _) = run("echo \"18446744073709551616\"").await; + assert_eq!((code, out.as_str()), (0, "18446744073709551616"), "the quoted form is the fix"); + let (code, out, _) = run("echo 9223372036854775807 -9223372036854775808").await; + assert_eq!((code, out.as_str()), (0, "9223372036854775807 -9223372036854775808")); +} + +/// A leading zero makes a word text before overflow ever gets to matter — +/// `09223372036854775808` is one past `i64::MAX`, but it is text (leading +/// zero) first, same as `007`, not a 64-bit refusal. +#[tokio::test] +async fn a_leading_zero_numeral_past_64_bits_is_still_text() { + let (code, out, err) = run("echo 09223372036854775808").await; + assert_eq!(code, 0, "must run: {err:?}"); + assert_eq!(out, "09223372036854775808", "leading zero wins over overflow"); + let (_, out, _) = run("echo $(typeof 09223372036854775808)").await; + assert_eq!(out, "string", "09223372036854775808 must not type as a number"); + + // The un-zeroed overflow still refuses. + let text = err_of("echo 9223372036854775808").await; + assert!(text.contains("64-bit"), "must still name the 64-bit limit: {text:?}"); +} + +/// `$(( … ))` parses its own numerals separately from the lexer, and used to +/// say only "invalid number" for an overflowing literal. It now names the +/// same 64-bit limit the lexer does. +#[tokio::test] +async fn arithmetic_overflow_literal_names_the_64_bit_limit() { + let text = err_of("echo $((9223372036854775808))").await; + assert!(text.contains("64-bit"), "must name the limit: {text:?}"); + assert!(!text.contains("invalid number"), "must not say only 'invalid': {text:?}"); + + // Overflow from addition is a different failure and keeps its own wording. + let text = err_of("echo $((9223372036854775807 + 1))").await; + assert!(text.contains("overflow"), "addition overflow must still say overflow: {text:?}"); +} + +// ── `value_to_num` does not round an out-of-range string through f64 ─────── + +/// `value_to_num`'s `String` arm fell to an `f64` parse whenever the `i64` +/// parse failed — including on an integer-shaped string that only failed +/// because it overflowed. Both sides of the comparison below round to 2^63 +/// in f64, so the comparison answered true for two numbers that are not +/// equal. An all-digit string that overflows i64 must refuse instead. +#[tokio::test] +async fn an_overflowing_integer_string_is_refused_not_rounded_through_float() { + let text = + err_of(r#"x="9223372036854775808"; [[ $x -eq 9223372036854775807 ]]"#).await; + assert!(text.contains("64-bit"), "must name the 64-bit limit: {text:?}"); + + // A genuine float spelling still falls to f64 as before. + let (code, _, err) = run(r#"[[ "1.5" -gt 1 ]]"#).await; + assert_eq!(code, 0, "a float string must still compare: {err:?}"); + let (code, _, err) = run(r#"[[ "1e3" -eq 1000 ]]"#).await; + assert_eq!(code, 0, "an exponent string must still compare: {err:?}"); +} + +// ── `-0` keeps source text at argv, canonicalizes once it moves ──────────── + +/// `-0` is a valid JSON number, not a leading-zero refusal (`has_invalid_ +/// leading_zero` only fires past one digit). `docs/LANGUAGE.md` documents +/// this split — argv keeps the typed word, a variable canonicalizes — but +/// it had no test pinning either half. +#[tokio::test] +async fn negative_zero_keeps_source_text_at_argv_and_canonicalizes_through_a_variable() { + let (code, out, err) = run("echo -0").await; + assert_eq!((code, out.as_str()), (0, "-0"), "argv keeps the typed word: {err:?}"); + + let (code, out, err) = run("x=-0; echo $x").await; + assert_eq!((code, out.as_str()), (0, "0"), "a variable prints the canonical form: {err:?}"); + + let (code, out, err) = run("x=-0; typeof $x").await; + assert_eq!((code, out.as_str()), (0, "number"), "-0 is a number, not text: {err:?}"); +} diff --git a/crates/kaish-kernel/tests/lexer_tests.rs b/crates/kaish-kernel/tests/lexer_tests.rs index 9f386514..b588386d 100644 --- a/crates/kaish-kernel/tests/lexer_tests.rs +++ b/crates/kaish-kernel/tests/lexer_tests.rs @@ -131,6 +131,14 @@ fn format_token(token: &Token) -> String { format!("FLOAT({}.0)", s) } } + // A numeral whose source text does not round-trip through its typed + // `Display` (`-0`, `007`, `0.0`, `1.0`, …) — `raw` is the exact + // source, so it needs none of `Float`'s reconstruction above. + Token::NumericLiteral(d) => match &d.value { + kaish_kernel::ast::Value::Int(_) => format!("INT({})", d.raw), + kaish_kernel::ast::Value::Float(_) => format!("FLOAT({})", d.raw), + other => panic!("format_token: NumericLiteral wrapping unexpected {other:?}"), + }, // Identifiers and paths Token::Ident(s) => format!("IDENT({})", s), @@ -236,8 +244,12 @@ fn lexer_identifiers(#[case] input: &str, #[case] expected: &[&str]) { // Digit-leading bare words are valid argv tokens: SHA prefixes (019dda1c), // UUIDs, version-ish identifiers. Lex them as NumberIdent so the parser -// can treat them as bareword strings. (Pure digit sequences still lex as -// Int — at least one alpha character is required to land here.) +// can treat them as bareword strings via THIS regex — it requires at +// least one alpha character to match, so a pure digit run (`123`) still +// lexes as Int here. A pure digit run CAN still end up NumberIdent, just +// not through this regex: a leading zero with more digits (`007`) is not +// a JSON number and is reclassified to NumberIdent by a later pass — see +// `lexer_leading_zero_is_not_a_number`, below. #[rstest] #[case::numident_hex("019dda1c", &["NUMIDENT(019dda1c)"])] #[case::numident_alpha_after_digit("123abc", &["NUMIDENT(123abc)"])] @@ -280,6 +292,35 @@ fn lexer_integers(#[case] input: &str, #[case] expected: &[&str]) { run_lexer_test(input, expected); } +/// One past `i64::MAX` fits the `-?[0-9]+` regex but not the type it lexes +/// into — `IntegerOutOfRange` names the limit instead of a generic failure. +#[rstest] +#[case::int_overflow_positive("9223372036854775808")] +#[case::int_overflow_negative("-9223372036854775809")] +fn lexer_integer_overflow_is_out_of_range(#[case] input: &str) { + run_lexer_error_variant(input, LexerError::IntegerOutOfRange); +} + +// A leading zero followed by another digit is not a JSON number (RFC 8259: +// `int = zero / (digit1-9 *DIGIT)`) — kaish already agrees for `fromjson` +// (`fromjson '007'` is a loud parse error), and there is no reason for a +// reader to expect `007` to mean the number 7. It lexes as a bareword +// string instead, the same shape `NumberIdent` already gives a digit run +// with a trailing letter (`019dda1c`) — see `preserve_numeric_source_text`. +// A LONE `0` is the JSON `zero` production and stays `Int`; so does a +// leading zero that got fused into a larger word (`a:007`, `007*` — +// Bug 4 below), since by then it is not a standalone numeral at all. +#[rstest] +#[case::leading_zero_two_digits("00", &["NUMIDENT(00)"])] +#[case::leading_zero_three_digits("007", &["NUMIDENT(007)"])] +#[case::leading_zero_ten("010", &["NUMIDENT(010)"])] +#[case::leading_zero_negative("-022", &["NUMIDENT(-022)"])] +#[case::leading_zero_float("007.5", &["NUMIDENT(007.5)"])] +#[case::leading_zero_float_negative("-00.5", &["NUMIDENT(-00.5)"])] +fn lexer_leading_zero_is_not_a_number(#[case] input: &str, #[case] expected: &[&str]) { + run_lexer_test(input, expected); +} + // ============================================================================= // Floats // ============================================================================= diff --git a/crates/kaish-kernel/tests/numeric_operand_tests.rs b/crates/kaish-kernel/tests/numeric_operand_tests.rs new file mode 100644 index 00000000..ac638a40 --- /dev/null +++ b/crates/kaish-kernel/tests/numeric_operand_tests.rs @@ -0,0 +1,73 @@ +//! Numeric operand coercion: kaish numbers are JSON numbers. +//! +//! `value_to_num` (`[[ ]]`/`test` numeric ops) and `value_to_exit_code` +//! (`return`/`exit`) both parse a string operand as `i64` then `f64`. JSON +//! has no `inf`/`nan`, and an i64-shaped string that only overflows must +//! name the 64-bit limit rather than round through `f64` or say a generic +//! "numeric argument required". +#![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:?}"), + } +} + +/// `"1e309"` overflows to `f64::INFINITY` and used to compare as a real +/// number; `inf`/`nan` already refused (they are not a `.`/`e`/`E` spelling +/// `value_to_num` recognizes, so they never reach the `f64` parse at all — +/// they fail as a non-numeric string). All three must refuse now, and none +/// of them by way of the unrelated NaN-comparison diagnostic. +#[tokio::test] +async fn non_finite_operands_are_refused_not_compared() { + for source in [r#"[[ "1e309" -gt 1 ]]"#, "[[ inf -gt 1 ]]", "[[ nan -eq nan ]]"] { + let text = err_of(source).await; + assert!( + !text.contains("NaN comparison"), + "{source:?} must not reach the NaN-comparison diagnostic: {text:?}" + ); + } +} + +/// A float spelling that overflows names the 64-bit range, the same way an +/// integer-shaped overflow does. +#[tokio::test] +async fn overflowing_float_spelling_names_the_range() { + let text = err_of(r#"[[ "1e309" -gt 1 ]]"#).await; + assert!(text.contains("64-bit float range"), "must name the range: {text:?}"); +} + +/// Ordinary float spellings are unaffected. +#[tokio::test] +async fn ordinary_float_operands_still_compare() { + let (code, _, err) = run(r#"[[ "1.5" -gt 1 ]]"#).await; + assert_eq!(code, 0, "must still compare: {err:?}"); + let (code, _, err) = run(r#"[[ "1e3" -eq 1000 ]]"#).await; + assert_eq!(code, 0, "must still compare: {err:?}"); +} + +/// `return`/`exit` coerce their operand the same way `[[ ]]` does — an +/// i64-shaped string that only overflows must name the 64-bit limit, not +/// say a generic "numeric argument required" as if the text were not a +/// number at all. +#[tokio::test] +async fn return_of_an_overflowing_string_names_the_limit() { + let text = err_of(r#"x="9223372036854775808"; f() { return $x; }; f"#).await; + assert!(text.contains("64-bit"), "must name the limit: {text:?}"); +} diff --git a/crates/kaish-kernel/tests/plan_builtin_tests.rs b/crates/kaish-kernel/tests/plan_builtin_tests.rs index e7148e22..e577e7d5 100644 --- a/crates/kaish-kernel/tests/plan_builtin_tests.rs +++ b/crates/kaish-kernel/tests/plan_builtin_tests.rs @@ -179,3 +179,115 @@ async fn an_empty_plan_stays_empty_under_the_kernel_json_rule() { assert_eq!(code, 0, "an empty statement is not an error"); assert_eq!(out, "", "an empty success stays empty under --json"); } + +/// A numeral argv word must round-trip its exact source text: `Plan.rendered` +/// and `PlannedCommand::args` are lexed into a typed `Int`/`Float` and were +/// re-serialized from the *value*, not the source. `i64`/`f64` have no +/// negative zero (`-0` → `0`) or memory of leading zeros (`007` → `7`) or +/// non-canonical trailing fraction digits (`1.0` → `1`), so every one of +/// these silently rewrote the word it was given. +/// +/// `xargs -0 rm -f` is the case that matters: `-0` planned (and executed) as +/// a bare `0`, turning `xargs`'s idiomatic null-delimiter flag into a +/// positional argument with no error. +#[tokio::test] +async fn noncanonical_numeric_argv_words_round_trip_in_rendered() { + let cases: &[(&str, &str)] = &[ + ("echo -0", "echo -0"), + ("echo -00", "echo -00"), + ("echo -0.0", "echo -0.0"), + ("echo -0.00", "echo -0.00"), + ("echo 00", "echo 00"), + ("echo 007", "echo 007"), + ("echo 010", "echo 010"), + ("echo 0.10", "echo 0.10"), + ("echo 1.0", "echo 1.0"), + ("xargs -0 rm -f", "xargs -0 rm -f"), + ]; + for (source, expected) in cases { + let doc = plan_json(&format!("plan '{source}' --json")).await; + assert_eq!( + doc["statements"][0]["plan"]["rendered"], *expected, + "rendered must reproduce the source word exactly for {source:?}: {doc}" + ); + } +} + +/// The same fidelity, checked on the structured `args[].plain` field — +/// `Plan.rendered` is a flat string a classifier might not re-split, but +/// `PlannedCommand::args` is what a hook is meant to read argument-by-argument. +#[tokio::test] +async fn noncanonical_numeric_argv_words_round_trip_in_args_plain() { + let doc = plan_json("plan 'xargs -0 rm -f' --json").await; + let args: Vec<&str> = doc["statements"][0]["plan"]["commands"][0]["args"] + .as_array() + .expect("args") + .iter() + .map(|a| a["plain"].as_str().unwrap_or_default()) + .collect(); + assert_eq!( + args, + vec!["-0", "rm", "-f"], + "xargs -0 must survive as its own argv word, not become a bare 0: {doc}" + ); +} + +/// Canonical numerals — the common case — must stay exactly as correct as +/// they were before: this class of fix must not touch a numeral whose +/// `Display` already reproduces the source. +#[tokio::test] +async fn canonical_numeric_argv_words_are_unaffected() { + let cases: &[(&str, &str)] = &[ + ("echo -1", "echo -1"), + ("echo -5", "echo -5"), + ("echo -0.5", "echo -0.5"), + ("echo +0", "echo +0"), + ("echo +1", "echo +1"), + ]; + for (source, expected) in cases { + let doc = plan_json(&format!("plan '{source}' --json")).await; + assert_eq!( + doc["statements"][0]["plan"]["rendered"], *expected, + "a canonical numeral must round-trip too: {source:?}: {doc}" + ); + } +} + +/// A leading-zero numeral (`007`, `010`) is not a valid JSON number (RFC +/// 8259's `int = zero / (digit1-9 *DIGIT)` excludes it), and kaish's own +/// `fromjson` already refuses it (`fromjson '007'` is a parse error) — the +/// lexer used to disagree, typing it `Int(7)`. Nobody writing `007` expects +/// the number 7, so it types as a string instead. `typeof` observes the +/// TYPE directly, which a rendered-text check cannot: `007` already +/// rendered as `007` once the source-text fix landed, whether it was typed +/// `Int` or `String` underneath — this test is the one that would fail if +/// the type were still `Int` even though the text looked right. +#[tokio::test] +async fn leading_zero_numeral_types_as_string_not_number() { + for c in ["00", "007", "010", "-022", "007.5", "-00.5"] { + let (code, out, err) = run(&format!("typeof {c}")).await; + assert_eq!(code, 0, "typeof should succeed for {c:?}: {err}"); + assert_eq!( + out.trim(), + "string", + "{c:?} has a leading zero, not a valid JSON number, so it must type as a string: got {out:?}" + ); + } +} + +/// A numeral without a leading-zero problem — including the `-0`/`0.10` +/// class Ruling 1 keeps typed but re-spells at render time — must still +/// type as `number`. Ruling 2 narrows what counts as a numeral; it must not +/// widen what counts as a string. +#[tokio::test] +async fn non_leading_zero_numeral_still_types_as_number() { + for c in ["0", "7", "123", "-1", "3.14", "-0", "-0.0", "0.10", "1.0"] { + let (code, out, err) = run(&format!("typeof {c}")).await; + assert_eq!(code, 0, "typeof should succeed for {c:?}: {err}"); + assert_eq!( + out.trim(), + "number", + "{c:?} is a valid JSON number and must keep typing as one: got {out:?}" + ); + } +} diff --git a/crates/kaish-kernel/tests/shell_compat_tests.rs b/crates/kaish-kernel/tests/shell_compat_tests.rs index 99a177ed..dca97359 100644 --- a/crates/kaish-kernel/tests/shell_compat_tests.rs +++ b/crates/kaish-kernel/tests/shell_compat_tests.rs @@ -774,11 +774,11 @@ shell_compat! { eq: "ok", } -shell_compat! { - name: numeric_eq_leading_zero_string, - script: r#"[[ "01" -eq "1" ]] && echo ok || echo nope"#, - eq: "ok", -} +// bash reads "01" as octal 01 (= 1) and calls the two sides equal; kaish +// refuses a leading-zero numeral in a number position instead, which the +// `shell_compat!` macro cannot express (it `.expect()`s the kaish side to +// succeed). The refusal is pinned in `leading_zero_tests.rs`, +// `numeric_comparison_refuses_a_leading_zero_rather_than_reading_decimal`. shell_compat! { name: numeric_ne_quoted_strings, @@ -808,12 +808,6 @@ shell_compat! { eq: "agree", } -shell_compat! { - name: numeric_eq_handles_what_string_eq_does_not, - script: r#"X="01"; [[ "$X" -eq 1 ]] && echo numeric || echo lex"#, - eq: "numeric", -} - shell_compat! { name: compound_short_circuit_or, script: r#"[[ -d / || $(cat /nonexistent_file) == "x" ]] && echo "yes" || echo "no""#, diff --git a/crates/kaish-kernel/tests/test_builtin_tests.rs b/crates/kaish-kernel/tests/test_builtin_tests.rs index 9e908af5..159df0ef 100644 --- a/crates/kaish-kernel/tests/test_builtin_tests.rs +++ b/crates/kaish-kernel/tests/test_builtin_tests.rs @@ -83,7 +83,18 @@ async fn numeric_integer() { assert_eq!(code_of("test 3 -lt 5").await, 0); assert_eq!(code_of("test 5 -ge 5").await, 0); assert_eq!(code_of("test 4 -le 5").await, 0); - assert_eq!(code_of("test 08 -eq 8").await, 0, "leading zero is decimal, not octal"); +} + +/// Deliberate inversion of the old pin above (leading zero used to read as +/// decimal). The number-position rule itself lives in `leading_zero_tests.rs`. +#[tokio::test] +async fn numeric_leading_zero_operand_is_refused() { + let dir = tempdir().unwrap(); + let kernel = kernel_at(dir.path()); + let result = kernel.execute("test 08 -eq 8").await.expect("kernel execute"); + assert_eq!(result.code, 2, "leading-zero operand is refused, not read as decimal"); + assert!(result.err.contains("(leading zero)"), "must name the cause: {:?}", result.err); + assert!(result.err.contains("write `8`"), "must name the fix: {:?}", result.err); } /// Negative numbers as operands (second position is the killer spot — a `-5` diff --git a/crates/kaish-repl/tests/numeral_argv_fidelity_tests.rs b/crates/kaish-repl/tests/numeral_argv_fidelity_tests.rs new file mode 100644 index 00000000..8afbb773 --- /dev/null +++ b/crates/kaish-repl/tests/numeral_argv_fidelity_tests.rs @@ -0,0 +1,40 @@ +//! An external process must receive the exact numeral word that was typed. +//! +//! `-0`, `0.10`, and `1.0` are valid JSON numbers whose own `Display` cannot +//! reproduce them, so the kernel carries their source text alongside the typed +//! value. `exec` builds its argv at its own edge rather than through +//! `build_args_flat`, so the two spellings of one command can drift apart. +//! +//! This spawns the real binary because `exec` replaces the calling process: +//! run in-process, it would execve over the test harness, and the remaining +//! tests in that binary would silently never run. +//! +//! Test-fixture code: unwrap/expect on known-good setup is the idiom here. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use std::process::Command; + +fn stdout_of(source: &str) -> String { + let output = Command::new(env!("CARGO_BIN_EXE_kaish")) + .arg("-c") + .arg(source) + .output() + .expect("run kaish -c"); + String::from_utf8_lossy(&output.stdout).trim().to_string() +} + +const WORDS: &str = "-0 007 010 0.10 1.0"; + +#[test] +fn an_external_command_receives_the_source_words() { + assert_eq!(stdout_of(&format!("/bin/echo {WORDS}")), WORDS); +} + +#[test] +fn exec_hands_over_the_same_argv_as_the_direct_spelling() { + assert_eq!( + stdout_of(&format!("exec /bin/echo {WORDS}")), + stdout_of(&format!("/bin/echo {WORDS}")), + "exec must not disagree with the direct spelling" + ); +} diff --git a/crates/kaish-types/src/tool.rs b/crates/kaish-types/src/tool.rs index 8a4ad64f..d7f59817 100644 --- a/crates/kaish-types/src/tool.rs +++ b/crates/kaish-types/src/tool.rs @@ -471,8 +471,25 @@ impl ToolSchema { pub struct ToolArgs { /// Positional arguments in order. pub positional: Vec, + /// Verbatim source text for a `positional` entry whose typed `Display` + /// would not reproduce it — `-0`, `0.10`, `1.0`. Keyed by index into + /// `positional`, and empty for the common case. + /// + /// Most builtins read a positional straight off `positional`, never off + /// the clap-parsed struct, and a plain `Value::Int` cannot carry the + /// source text once typed. A builtin echoing a positional's text verbatim + /// should call [`positional_text`](Self::positional_text). + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub positional_raw: BTreeMap, /// Named arguments by key. pub named: BTreeMap, + /// Same idea as [`positional_raw`](Self::positional_raw), keyed by the + /// named key. A named value is normally read off the clap-parsed struct, + /// so `to_argv`/`to_argv_excluding` consult this when rendering + /// `--key=value` and the source text reaches the clap field that way. + /// Only populated for a single, non-repeated value. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub named_raw: BTreeMap, /// Boolean flags (e.g., -l, --force). pub flags: HashSet, /// Every word after the tool name, in source order, post-expansion — @@ -487,6 +504,10 @@ pub struct ToolArgs { /// Render it to a clap argv with [`ToolArgs::words_argv`]. #[serde(default, skip_serializing_if = "Option::is_none")] pub words: Option>, + /// Same idea as `positional_raw`, but for `words` — keyed by index into + /// `words`. Only ever populated when `words` is `Some`. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub words_raw: BTreeMap, } impl ToolArgs { @@ -495,18 +516,41 @@ impl ToolArgs { Self::default() } + /// The display text for positional `index`: its + /// [`positional_raw`](Self::positional_raw) entry when there is one, + /// otherwise `value_to_argv_token` on the typed value. `None` when + /// `index` is out of range. + /// + /// A builtin printing a positional's text as-is should call this rather + /// than stringify `positional[index]` itself. A builtin needing the typed + /// value should keep reading `positional[index]`, which is unaffected. + pub fn positional_text(&self, index: usize) -> Option { + if let Some(raw) = self.positional_raw.get(&index) { + return Some(raw.clone()); + } + self.positional.get(index).map(value_to_argv_token) + } + /// Render [`words`](Self::words) into argv tokens for a verbatim tool's /// own parser. Empty when the tool is not verbatim. /// /// A [`Value::Bytes`] word renders as an inert placeholder token, as /// [`to_argv`](Self::to_argv) does for a binary positional; the real bytes - /// stay at the matching index in `words`. + /// stay at the matching index in `words`. A non-canonical numeral (`-0`, + /// `0.10`) renders its `words_raw` entry instead of `value`'s `Display`, + /// the same substitution `positional_text` makes for `positional`. pub fn words_argv(&self) -> Vec { self.words .as_deref() .unwrap_or_default() .iter() - .map(value_to_argv_token) + .enumerate() + .map(|(i, value)| { + self.words_raw + .get(&i) + .cloned() + .unwrap_or_else(|| value_to_argv_token(value)) + }) .collect() } @@ -694,6 +738,10 @@ impl ToolArgs { if exclude.contains(&key.as_str()) { continue; } + if let Some(raw) = self.named_raw.get(key) { + argv.push(format!("{}={}", flag_token(key), raw)); + continue; + } for rendered in render_named_value(key, value)? { argv.push(format!("{}={}", flag_token(key), rendered)); } @@ -703,8 +751,13 @@ impl ToolArgs { // they begin with `-` (e.g. `echo -- -n` should print `-n`). if !self.positional.is_empty() { argv.push("--".to_string()); - for value in &self.positional { - argv.push(value_to_argv_token(value)); + for (i, value) in self.positional.iter().enumerate() { + argv.push( + self.positional_raw + .get(&i) + .cloned() + .unwrap_or_else(|| value_to_argv_token(value)), + ); } } @@ -972,6 +1025,57 @@ mod to_argv_tests { assert_eq!(args.to_argv().unwrap(), vec!["--", "hello", "world"]); } + // `-0` is correctly typed as `Value::Int(0)`, but that value's `Display` + // cannot get back to `-0`. `*_raw` holds the text `Value` alone lost. + + #[test] + fn positional_raw_overrides_the_typed_value_in_to_argv() { + let mut args = ToolArgs::new(); + args.positional.push(Value::Int(0)); + args.positional_raw.insert(0, "-0".to_string()); + assert_eq!(args.to_argv().unwrap(), vec!["--", "-0"]); + } + + #[test] + fn positional_text_prefers_raw_and_falls_back_to_the_typed_value() { + let mut args = ToolArgs::new(); + args.positional.push(Value::Int(0)); + args.positional.push(Value::Int(5)); + args.positional_raw.insert(0, "-0".to_string()); + assert_eq!(args.positional_text(0).as_deref(), Some("-0")); + assert_eq!(args.positional_text(1).as_deref(), Some("5")); + assert_eq!(args.positional_text(2), None, "out of range"); + } + + #[test] + fn named_raw_overrides_the_typed_value_in_to_argv() { + let mut args = ToolArgs::new(); + args.named.insert("count".into(), Value::Float(0.10)); + args.named_raw.insert("count".into(), "0.10".to_string()); + assert_eq!(args.to_argv().unwrap(), vec!["--count=0.10"]); + } + + #[test] + fn words_raw_overrides_the_typed_value_in_words_argv() { + let mut args = ToolArgs::new(); + args.words = Some(vec![Value::String("echo".into()), Value::Float(1.0)]); + args.words_raw.insert(1, "1.0".to_string()); + assert_eq!(args.words_argv(), vec!["echo", "1.0"]); + } + + #[test] + fn a_canonical_numeral_is_unaffected_by_the_raw_fields() { + let mut args = ToolArgs::new(); + args.positional.push(Value::Int(5)); + args.named.insert("count".into(), Value::Int(3)); + args.words = Some(vec![Value::Int(7)]); + assert!(args.positional_raw.is_empty()); + assert!(args.named_raw.is_empty()); + assert!(args.words_raw.is_empty()); + assert_eq!(args.to_argv().unwrap(), vec!["--count=3", "--", "5"]); + assert_eq!(args.words_argv(), vec!["7"]); + } + #[test] fn single_char_flags_emit_short_form() { let mut args = ToolArgs::new(); diff --git a/docs/LANGUAGE.md b/docs/LANGUAGE.md index fd5c9c55..0a7f325e 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -27,6 +27,63 @@ echo "${NAME} more text" **Why lowercase only?** `true` and `false` are the boolean literals. `TRUE`, `Yes`, `yes`, `on`, and `1` are ordinary values — `x=TRUE` binds the string `"TRUE"` and `x=1` binds the number `1`, neither a boolean. Check with `typeof` when it matters. +### A bare number follows JSON rules + +```sh +echo 007 # the string 007 — a leading zero is not a JSON number +chmod 0644 f # the string 0644 — a mode keeps every digit you typed +echo -0 # the number 0 — valid JSON; -0 and 0 are the same number +echo "-0" # the string -0 — quote it to keep those two characters +break 7 # a loop count is a number — `break 007` is an error +``` + +`007`, `010`, and `00` are strings: nobody writes `007` expecting the number +7, and `fromjson '007'` already refused it as invalid JSON — the bare word +now agrees. `-0`, `0.10`, and `1.0` are numbers, and kaish prints back the +word you typed (`-0`, not `0`) wherever that word crosses into argv or a +plan. Once the number moves — through a variable, `--json`, arithmetic — it +is a plain typed number again and prints its canonical form: `x=-0; echo +$x` prints `0`. Quote a number (`"-0"`) to keep it a string on purpose. + +Where kaish needs a number, a leading zero is an error, and the error names +the number to write. That covers a `break`/`continue` count, arithmetic, a +numeric comparison, and a list index: + +```sh +break 007 # error — write `break 7` +echo $((010 + 1)) # error — kaish reads no octal; write `10` +[[ 010 -eq 10 ]] # error — write `10` +xs=[10 20] +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. + +A bare integer must also fit in 64 bits (`-9223372036854775808` to +`9223372036854775807`); a longer numeral is an error naming the limit, and +quoting it keeps the text: `echo 9223372036854775808` errors, `echo +"9223372036854775808"` prints the digits. + +Arithmetic refuses the numeral however it arrives, so a variable holding the +text is refused the same way the literal is: + +```sh +x=010 +echo $((x)) # error — write `10` +``` + +A record key is text, so a leading zero is a key like any other and reads +back under the name it was stored under: + +```sh +r={"007":9} +echo ${r[007]} # 9 +``` + ### Inline environment prefix — `NAME=value command` One or more assignments placed *before* a command scope those variables to that