From c4db140def45761f7e3963bb2789303409e41d10 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Mon, 24 Aug 2026 07:14:42 -0400 Subject: [PATCH 01/27] fix: preserve source text for non-canonical numeral argv words MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amy's report: `find -print0 | xargs -0 rm -f` planned AND executed as `xargs 0 rm -f` — the `-0` null-delimiter flag silently became a bare `0`. `-0` lexes as `Token::Int(-0)`; `i64` has no negative zero, so the sign was gone the moment the literal was parsed, long before any renderer touched it. Mapped the class against the shipped 0.16.0 binary before touching code: leading zeros (`007`, `010`, `00`, `-00`), negative zero for both int and float (`-0`, `-0.0`, `-0.00`), and non-canonical trailing fraction digits (`0.10`, `1.0`, and `0.0` itself — Rust's `f64::to_string()` drops a bare `.0`). Canonical numerals (`-1`, `-5`, `-0.5`, `+0`) were already fine and needed to stay that way. Confirmed this is not only a `Plan.rendered` cosmetic issue: running `/bin/echo -0 007 -0.0` for real against the unfixed tree printed `0 7 -0` — `kernel.rs::build_args_flat`, the real external-command argv builder, loses the same fidelity as the renderer, because both re-serialize from the typed `Value` and the source text is already gone by then. Wrote failing tests first: `plan_builtin_tests.rs` asserts `rendered`/`args[].plain` round-trip for the whole class (plus a canonical-numeral regression guard), and a new `external_command_argv_preserves_noncanonical_numeral_source_text` in `external_command_tests.rs` spawns real `/bin/echo` and checks the actual argv, not just the plan. Both reproduced the bug against current main before any fix landed. Fix keeps numbers typed — `kaish-types::Value` is untouched — and adds a raw-text side channel at the AST layer only for the non-canonical case. `lexer.rs` gains `Token::NumericLiteral`, synthesized as the LAST step of `tokenize_impl:: preserve_numeric_source_text`, after the fusion passes: those match `Int`/`Float` directly for colon- and glob-fusion decisions, so a numeral must still look ordinary while fusion runs. `ast/types.rs` gains the parallel `Expr::NumericLiteral { value, raw }`. Every downstream consumer of a positional/named/wordassign literal now prefers `raw` over `value.to_string()`: `ast/plan.rs::render_expr` (the plan/classifier path), `kernel.rs::format_expr` and `build_args_flat` (the real execve argv), and the dispatch.rs test-mirror kept in sync with it. `eval_expr`/`eval_expr_async` unwrap straight to the typed `value`, so arithmetic, comparisons, and `--json` never see this variant at all — the canonical case (everything that already round-tripped) never even builds the new token, paying one string comparison per numeral. Scope decision: this closes the bug report's own case — Plan rendering and real external-command execution. It does NOT close a same-class gap found along the way: a BUILTIN tool's argv (`echo -0` without an absolute path) still prints `0`, because `kernel.rs::bind_tool_args` pushes a typed `Value` into `ToolArgs::positional` and the text-sink formatting that would need `raw` happens later, inside each builder's own clap-argv assembly. Closing that needs either a raw-text side channel on `ToolArgs` itself or accepting that a non-canonical literal reaches a builtin as `Value::String` instead of `Value::Int`/`Float` — a real tradeoff, not a mechanical extension of this fix, so it's left as a follow-up rather than decided here. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 7 ++ crates/kaish-kernel/src/ast/plan.rs | 6 ++ crates/kaish-kernel/src/ast/sexpr.rs | 3 + crates/kaish-kernel/src/ast/types.rs | 13 ++++ crates/kaish-kernel/src/dispatch.rs | 5 ++ crates/kaish-kernel/src/interpreter/eval.rs | 5 ++ crates/kaish-kernel/src/kernel.rs | 25 ++++++ crates/kaish-kernel/src/lexer.rs | 76 ++++++++++++++++++- crates/kaish-kernel/src/parser.rs | 18 +++++ crates/kaish-kernel/src/validator/walker.rs | 1 + .../tests/external_command_tests.rs | 21 +++++ crates/kaish-kernel/tests/lexer_tests.rs | 8 ++ .../kaish-kernel/tests/plan_builtin_tests.rs | 73 ++++++++++++++++++ 13 files changed, 258 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c549c17d..37037ef2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ breaking entries are marked **BREAKING**. ## [Unreleased] +### Fixed + +- **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. + ## [0.16.0] - 2026-08-23 ### Added diff --git a/crates/kaish-kernel/src/ast/plan.rs b/crates/kaish-kernel/src/ast/plan.rs index a2d4c009..49005638 100644 --- a/crates/kaish-kernel/src/ast/plan.rs +++ b/crates/kaish-kernel/src/ast/plan.rs @@ -501,6 +501,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 @@ -860,6 +861,11 @@ pub(crate) fn render_expr(expr: &Expr) -> String { format!("${{{}:-{}}}", render_varpath(path), render_parts(default)) } Expr::Arithmetic(e) => format!("$(({e}))"), + // The whole reason this variant exists: a numeral whose `Display` + // would not reproduce its own source (`-0`, `007`, `1.0`) renders + // as the verbatim text it was written as, not `value`'s canonical + // form. + 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..69f2bc34 100644 --- a/crates/kaish-kernel/src/ast/types.rs +++ b/crates/kaish-kernel/src/ast/types.rs @@ -393,6 +393,19 @@ 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 (`Int`/`Float`) whose own `Display` does not reproduce the + /// source text it was written as — `-0` (negative zero has no distinct + /// `i64` spelling), `007` (a leading zero), `0.10`/`1.0` (a non-canonical + /// trailing fraction digit). `value` is the typed value: arithmetic, + /// comparisons, `set x = 007`, and `--json` all still see a real + /// `Int`/`Float`. `raw` is the exact source text: argv/plan rendering and + /// real external-command argv use it instead of `value`'s `Display`, so + /// `xargs -0 rm -f` keeps its `-0`. + /// + /// A canonical numeral (`-1`, `42`, `3.14`) never reaches this variant — + /// it parses as the plain `Literal(Value::Int/Float)` it always did, + /// unchanged. See `lexer::Token::NumericLiteral`, which this mirrors. + 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..5893b4ec 100644 --- a/crates/kaish-kernel/src/dispatch.rs +++ b/crates/kaish-kernel/src/dispatch.rs @@ -316,6 +316,11 @@ 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()), + // A numeral whose source text does not round-trip + // through its typed `Display` (`-0`, `007`, `1.0`) — + // kept in sync with kernel.rs::build_args_flat, which + // pushes `raw` directly 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..661c4082 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -181,6 +181,11 @@ 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 ever needs `value` — arithmetic and + // comparisons see the real `Int`/`Float`. `raw` exists for + // argv/plan text-sink positions, which read the `Expr` directly + // rather than going through `eval`. + 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 } => { diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index c881c24a..6aa7a462 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -3479,6 +3479,9 @@ 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(), + // A numeral whose own `Display` would drop a leading zero or a + // negative zero: 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 +3917,17 @@ impl Kernel { continue; } } + // A numeral whose typed `Display` would not reproduce its + // source (`-0`, `007`, `1.0`) goes to the external + // process as the exact word it was written as — the same + // rule `ast::plan::render_expr` applies, so the argv that + // executes matches the argv a plan showed. Skips + // `eval_expr_async`/text-sink entirely: there is no + // `Value` that could reproduce `raw` anyway. + 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 +3943,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 +3956,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 +4067,9 @@ impl Kernel { Ok(Value::Bool(!is_truthy(&value))) } Expr::Literal(value) => Ok(value.clone()), + // Typed evaluation only ever needs `value`; `raw` is for + // argv/plan text-sink positions 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) { diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 115ff047..d76d3ef4 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); @@ -207,6 +208,15 @@ impl fmt::Display for LexerError { /// from `content` only for an interpolated body containing `$((…))`, which /// `content` carries in the rewritten `${__ARITH:…}` form — a kernel-internal /// spelling that must never reach a plan. +/// 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`, below). +#[derive(Debug, Clone, PartialEq)] +pub struct NumericLiteralData { + pub value: Value, + pub raw: String, +} + #[derive(Debug, Clone, PartialEq)] pub struct HereDocData { pub content: String, @@ -640,6 +650,25 @@ 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`; an + /// `i64`/`f64` has no distinct negative-zero spelling once parsed back + /// out for `Int`, and `f64::to_string` drops the trailing `.0` for + /// `Float`), a leading zero (`007`, `010`), or a non-canonical trailing + /// fraction digit (`0.10`, `1.0`). Carries both the typed value (so + /// arithmetic, comparisons, and `--json` still see a real `Int`/`Float`) + /// and the verbatim source text (so argv/plan rendering can reproduce + /// exactly what was typed). + /// + /// Never produced directly by logos — `tokenize_impl`'s + /// `preserve_numeric_source_text` pass synthesizes it from a plain + /// `Int`/`Float` token as the LAST step, after the fusion passes run, so + /// `is_colon_mergeable`/`is_glob_mergeable` (which match `Int`/`Float` + /// directly) see the ordinary token during fusion and this variant only + /// ever reaches the parser. The common case (`-1`, `42`, `3.14`) is + /// untouched and pays nothing. + NumericLiteral(NumericLiteralData), + // ═══════════════════════════════════════════════════════════════════ // Invalid patterns (caught before valid tokens for better errors) // ═══════════════════════════════════════════════════════════════════ @@ -830,7 +859,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(_) @@ -1305,6 +1336,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 +1424,7 @@ impl Token { | Token::Arithmetic(_) | Token::Int(_) | Token::Float(_) + | Token::NumericLiteral(_) | Token::True | Token::False | Token::VarRef(_) @@ -3501,12 +3534,49 @@ 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, )) } +/// Replace a plain `Int`/`Float` token with `NumericLiteral` when the source +/// text it was lexed from does not round-trip through its own `Display` — +/// see `Token::NumericLiteral` for the exact cases (negative zero, leading +/// zeros, non-canonical trailing fraction digits). The common case pays one +/// string comparison and stays a plain `Int`/`Float`. +/// +/// Runs as the LAST step of `tokenize_impl`, after every fusion pass — those +/// passes (`is_colon_mergeable`, `is_glob_mergeable`) match `Int`/`Float` +/// directly, so a numeral must still present its ordinary shape while fusion +/// decisions are made. Spans are already original-source coordinates by this +/// point, 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 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 diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index 1dcd18b0..05896807 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -3096,6 +3096,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) @@ -3604,6 +3605,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/validator/walker.rs b/crates/kaish-kernel/src/validator/walker.rs index 6351d6f1..b2c27877 100644 --- a/crates/kaish-kernel/src/validator/walker.rs +++ b/crates/kaish-kernel/src/validator/walker.rs @@ -538,6 +538,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 { 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/lexer_tests.rs b/crates/kaish-kernel/tests/lexer_tests.rs index 9f386514..67d536a2 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), diff --git a/crates/kaish-kernel/tests/plan_builtin_tests.rs b/crates/kaish-kernel/tests/plan_builtin_tests.rs index e7148e22..06008587 100644 --- a/crates/kaish-kernel/tests/plan_builtin_tests.rs +++ b/crates/kaish-kernel/tests/plan_builtin_tests.rs @@ -179,3 +179,76 @@ 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}" + ); + } +} From b9434a93788113bb36dfc0e9b96c51be39dbc458 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Mon, 24 Aug 2026 07:24:58 -0400 Subject: [PATCH 02/27] lexer: NumericLiteralData was standing inside HereDocData's docs Clippy caught this and the test suite could not: doc_lazy_continuation fired on three lines that read as a broken bullet list. The cause was worse than the symptom. NumericLiteralData had been inserted between HereDocData's 22-line doc comment and HereDocData itself, so rustdoc attached the whole here-doc explanation -- literal, strip_tabs, body_start_offset, delimiter, source_body -- to the new numeric struct, and left HereDocData with no documentation at all. Nothing about behavior changed, which is exactly why no test moved. The gate that found it was `cargo clippy --all-targets -- -D warnings`, and it found it as a formatting complaint, not as the documentation loss it actually was. Moved the struct below HereDocData so each type owns its own doc, and turned its "same shape as HereDocData, below" into "above" to match. Co-Authored-By: Claude Opus 5 --- crates/kaish-kernel/src/lexer.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index d76d3ef4..7def76d8 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -208,15 +208,6 @@ impl fmt::Display for LexerError { /// from `content` only for an interpolated body containing `$((…))`, which /// `content` carries in the rewritten `${__ARITH:…}` form — a kernel-internal /// spelling that must never reach a plan. -/// 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`, below). -#[derive(Debug, Clone, PartialEq)] -pub struct NumericLiteralData { - pub value: Value, - pub raw: String, -} - #[derive(Debug, Clone, PartialEq)] pub struct HereDocData { pub content: String, @@ -227,6 +218,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. From faf999238d81cfb17757143f920389d76c538386 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Mon, 24 Aug 2026 08:15:32 -0400 Subject: [PATCH 03/27] lexer: a leading zero is not a number, it is a string Amy's ruling, after checking the JSON grammar: RFC 8259 defines int = zero / (digit1-9 *DIGIT), which excludes a leading zero followed by another digit. kaish's own fromjson already enforces this (fromjson '007' is a parse error), but the lexer disagreed and typed the same text as Int(7) everywhere else - two parts of one shell answering "is 007 a number" differently. The lexer was the one that was wrong. The user-facing reason to lead with is not the spec: nobody writing 007 expects the number 7. The spec citation is the engineering argument for why the lexer, not fromjson, was the party to fix, and belongs here rather than in help text or the changelog. has_invalid_leading_zero (lexer.rs) checks the numeral's own integer part - more than one digit, leading '0' - which also covers a leading-zero float's integer part (007.5) since JSON's int grammar is shared by the float production. A lone 0 (0, -0, 0.5) is the zero alternative and stays a number; -0, 0.10, and 1.0 remain valid JSON numbers and are untouched by this change (that class is Ruling 1's territory: they stay typed and get their source text back at render time). The check runs inside the same LAST-step pass Ruling 1 already added (preserve_numeric_source_text), ahead of the raw-text substitution: a leading zero reclassifies to Token::NumberIdent - the same bareword-string shape a digit run with a trailing letter (019dda1c) already gets - so every existing consumer of that token already knows what to do with it. No new Token or Expr variant, no new match arms anywhere downstream. Verified against the shipped binary, not by reasoning: x=007; echo $((x+1)) still evaluates to 8 (arithmetic's own string-to-int coercion tolerates leading zeros, unlike JSON); test 08 -eq 8 still passes (test's numeric comparison coerces the string the same way); fromjson '007' still errors. Ran the full test_builtin_tests, lexer_pipeline_tests, and lexer_idiom_tests suites specifically for this - the two Bug-4 fusion tests (a:007, 007*) are unaffected because a leading zero fused into a larger word never reaches this pass as a standalone Int/Float token in the first place. Updated the one stale comment found asserting "pure digit sequences still lex as Int" (lexer_tests.rs) - true only of the NumberIdent regex itself (it requires an alpha character), not of the numeral's final classification once this pass runs. No test asserted 007-as- Int as intended behavior; the only close call (test 08 -eq 8) is carried by test's own tolerant string coercion, unaffected by what type the literal started as. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 ++ crates/kaish-kernel/src/ast/plan.rs | 7 +-- crates/kaish-kernel/src/ast/types.rs | 15 ++++-- crates/kaish-kernel/src/dispatch.rs | 8 +-- crates/kaish-kernel/src/lexer.rs | 53 ++++++++++++++----- crates/kaish-kernel/tests/lexer_tests.rs | 28 +++++++++- .../kaish-kernel/tests/plan_builtin_tests.rs | 39 ++++++++++++++ 7 files changed, 129 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37037ef2..ae0cc0d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ breaking entries are marked **BREAKING**. `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. + ## [0.16.0] - 2026-08-23 ### Added diff --git a/crates/kaish-kernel/src/ast/plan.rs b/crates/kaish-kernel/src/ast/plan.rs index 49005638..69059bcb 100644 --- a/crates/kaish-kernel/src/ast/plan.rs +++ b/crates/kaish-kernel/src/ast/plan.rs @@ -862,9 +862,10 @@ pub(crate) fn render_expr(expr: &Expr) -> String { } Expr::Arithmetic(e) => format!("$(({e}))"), // The whole reason this variant exists: a numeral whose `Display` - // would not reproduce its own source (`-0`, `007`, `1.0`) renders - // as the verbatim text it was written as, not `value`'s canonical - // form. + // would not reproduce its own source (`-0`, `1.0`) renders as the + // verbatim text it was written as, not `value`'s canonical form. A + // leading zero (`007`) is a different case: `Expr::Literal(String)` + // by the time it gets here, handled above like any other bareword. Expr::NumericLiteral { raw, .. } => raw.clone(), Expr::Command(cmd) => render_command(cmd), Expr::LastExitCode => "$?".to_string(), diff --git a/crates/kaish-kernel/src/ast/types.rs b/crates/kaish-kernel/src/ast/types.rs index 69f2bc34..fcd187c5 100644 --- a/crates/kaish-kernel/src/ast/types.rs +++ b/crates/kaish-kernel/src/ast/types.rs @@ -395,13 +395,18 @@ pub enum Expr { RecordLiteral(Vec), /// A numeral (`Int`/`Float`) whose own `Display` does not reproduce the /// source text it was written as — `-0` (negative zero has no distinct - /// `i64` spelling), `007` (a leading zero), `0.10`/`1.0` (a non-canonical - /// trailing fraction digit). `value` is the typed value: arithmetic, - /// comparisons, `set x = 007`, and `--json` all still see a real - /// `Int`/`Float`. `raw` is the exact source text: argv/plan rendering and - /// real external-command argv use it instead of `value`'s `Display`, so + /// `i64` spelling), `0.10`/`1.0` (a non-canonical trailing fraction + /// digit). `value` is the typed value: arithmetic, comparisons, + /// `set x = -0`, and `--json` all still see a real `Int`/`Float`. `raw` + /// is the exact source text: argv/plan rendering and real + /// external-command argv use it instead of `value`'s `Display`, so /// `xargs -0 rm -f` keeps its `-0`. /// + /// A leading zero (`007`) is a DIFFERENT case and never reaches this + /// variant — not a valid JSON number, so it parses as + /// `Literal(Value::String("007"))` instead, the same as any other + /// bareword. + /// /// A canonical numeral (`-1`, `42`, `3.14`) never reaches this variant — /// it parses as the plain `Literal(Value::Int/Float)` it always did, /// unchanged. See `lexer::Token::NumericLiteral`, which this mirrors. diff --git a/crates/kaish-kernel/src/dispatch.rs b/crates/kaish-kernel/src/dispatch.rs index 5893b4ec..73612b72 100644 --- a/crates/kaish-kernel/src/dispatch.rs +++ b/crates/kaish-kernel/src/dispatch.rs @@ -317,9 +317,11 @@ impl BackendDispatcher { Expr::Literal(Value::Int(i)) => argv.push(i.to_string()), Expr::Literal(Value::Float(f)) => argv.push(f.to_string()), // A numeral whose source text does not round-trip - // through its typed `Display` (`-0`, `007`, `1.0`) — - // kept in sync with kernel.rs::build_args_flat, which - // pushes `raw` directly for the same reason. + // through its typed `Display` (`-0`, `1.0`) — kept in + // sync with kernel.rs::build_args_flat, which pushes + // `raw` directly for the same reason. A leading zero + // (`007`) is a plain `Literal(String)` by now, handled + // above like any other bareword. Expr::NumericLiteral { raw, .. } => argv.push(raw.clone()), Expr::VarRef(path) => { if let Ok(v) = ctx.scope.resolve_path(path) { diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 7def76d8..0d9204b0 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -654,11 +654,16 @@ pub enum Token { /// source text it was lexed from — a negative zero (`-0`, `-0.0`; an /// `i64`/`f64` has no distinct negative-zero spelling once parsed back /// out for `Int`, and `f64::to_string` drops the trailing `.0` for - /// `Float`), a leading zero (`007`, `010`), or a non-canonical trailing - /// fraction digit (`0.10`, `1.0`). Carries both the typed value (so - /// arithmetic, comparisons, and `--json` still see a real `Int`/`Float`) - /// and the verbatim source text (so argv/plan rendering can reproduce - /// exactly what was typed). + /// `Float`), or a non-canonical trailing fraction digit (`0.10`, + /// `1.0`). Carries both the typed value (so arithmetic, comparisons, + /// and `--json` still see a real `Int`/`Float`) and the verbatim + /// source text (so argv/plan rendering can reproduce exactly what was + /// typed). + /// + /// A leading zero (`007`, `010`) is a DIFFERENT case, reclassified to + /// `NumberIdent` instead — see `has_invalid_leading_zero`. It is not a + /// valid JSON number, and kaish's own `fromjson` already refuses it, so + /// it is not a number here either: a string, not a mistyped `Int`. /// /// Never produced directly by logos — `tokenize_impl`'s /// `preserve_numeric_source_text` pass synthesizes it from a plain @@ -3543,17 +3548,38 @@ fn tokenize_impl( )) } -/// Replace a plain `Int`/`Float` token with `NumericLiteral` when the source -/// text it was lexed from does not round-trip through its own `Display` — -/// see `Token::NumericLiteral` for the exact cases (negative zero, leading -/// zeros, non-canonical trailing fraction digits). The common case pays one -/// string comparison and stays a plain `Int`/`Float`. +/// True when a numeral's own 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 (since JSON's `int` production is +/// shared by the float grammar) the integer part of `007.5`. A lone `0` +/// (`0`, `-0`, `0.5`) is the `zero` alternative and is fine. +/// +/// `fromjson` already refuses these (`fromjson '007'` is a parse error); +/// before this pass the lexer disagreed and typed them as `Int(7)`. Nobody +/// writing `007` expects the number 7, so the lexer now agrees: not a +/// number, a string. +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') +} + +/// Reclassify a plain `Int`/`Float` token from its own source text — see +/// `has_invalid_leading_zero` (a leading zero makes it a string, not a +/// number: `Token::NumberIdent`, the same shape a digit run with a trailing +/// letter already gets) and `Token::NumericLiteral`'s doc comment (a numeral +/// whose source text does not round-trip through its own typed `Display`: +/// negative zero, a non-canonical trailing fraction digit). The common case +/// (`-1`, `42`, `3.14`) pays one string comparison and stays a plain +/// `Int`/`Float`. /// /// Runs as the LAST step of `tokenize_impl`, after every fusion pass — those /// passes (`is_colon_mergeable`, `is_glob_mergeable`) match `Int`/`Float` /// directly, so a numeral must still present its ordinary shape while fusion -/// decisions are made. Spans are already original-source coordinates by this -/// point, so `source[span]` is the exact word the author typed. +/// decisions are made — a leading zero fused into a larger word (`a:007`, +/// `007*`) is not a standalone numeral and never reaches this pass at all. +/// Spans are already original-source coordinates by this point, so +/// `source[span]` is the exact word the author typed. fn preserve_numeric_source_text(tokens: Vec>, source: &str) -> Vec> { tokens .into_iter() @@ -3566,6 +3592,9 @@ fn preserve_numeric_source_text(tokens: Vec>, source: &str) -> Ve 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; } diff --git a/crates/kaish-kernel/tests/lexer_tests.rs b/crates/kaish-kernel/tests/lexer_tests.rs index 67d536a2..6e9c3c10 100644 --- a/crates/kaish-kernel/tests/lexer_tests.rs +++ b/crates/kaish-kernel/tests/lexer_tests.rs @@ -244,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)"])] @@ -288,6 +292,26 @@ fn lexer_integers(#[case] input: &str, #[case] expected: &[&str]) { run_lexer_test(input, expected); } +// 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/plan_builtin_tests.rs b/crates/kaish-kernel/tests/plan_builtin_tests.rs index 06008587..e577e7d5 100644 --- a/crates/kaish-kernel/tests/plan_builtin_tests.rs +++ b/crates/kaish-kernel/tests/plan_builtin_tests.rs @@ -252,3 +252,42 @@ async fn canonical_numeric_argv_words_are_unaffected() { ); } } + +/// 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:?}" + ); + } +} From 39dcf8d900110ef0bf3d6055496c67403ecc5c58 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Mon, 24 Aug 2026 08:16:22 -0400 Subject: [PATCH 04/27] kernel: ToolArgs gets its own raw-text field for builtin argv Amy's ruling on the residual the first fix flagged and declined to decide: echo -0 still printed 0 even after external-command argv and plan rendering were both fixed, because echo is a builtin, not a spawned process, and never goes near build_args_flat at all. "Give ToolArgs its own raw-text field" - a first-class fact earns a first-class field, not an overload of the existing typed Value. Traced why echo was still wrong: value_to_argv_token (kaish-types) looked like the single choke point, but it turned out to be a validation-only sink for positionals - echo, like most builtins with positional args, reads args.positional directly as a typed Value (never the clap-parsed field) and stringifies it itself via value_to_text_sink. A plain Value::Int(0) has no way back to "-0" no matter which stringifier touches it; the fix has to intercept before that Expr::NumericLiteral{value, raw} node collapses to a bare Value at all, and it has to reach every place bind_tool_args hands a typed value on to something that will eventually print it. Design: ToolArgs::positional_raw and words_raw (index-keyed) plus named_raw (key-keyed) - sparse maps, not parallel Vecs, so a push/insert site that doesn't know about raw text just doesn't touch them and nothing breaks. bind_tool_args populates them in all three arg-binding modes (Verbatim, raw_argv, typed) wherever a non-canonical Expr::NumericLiteral reaches a positional or named slot; the typed Value pushed alongside is unchanged, so test's numeric -eq/-gt still gets a real Int. Named/WordAssign compose their "key=value" text immediately in bind_tool_args rather than deferring it, so those get the raw substitution inline instead of through a side-channel. to_argv/to_argv_excluding/words_argv (kaish-types) prefer the raw field when rendering, so a named value's fix reaches its usual home too: the clap-parsed struct, which is what most builtins read for a NAMED value (the inverse of the positional case). Found and fixed one correctness trap while wiring this up: the map_positionals reindexing pass at the end of bind_tool_args drains and redistributes tool_args.positional (backend/MCP tools with computed positional-to-named mapping), which would have silently misattributed positional_raw entries to the wrong index or the wrong named key after the reshuffle - a second bug of exactly the kind this whole fix exists to close, caught before it shipped by re-deriving positional_raw and named_raw alongside the redistribution instead of leaving the old index-keyed map stale. echo.rs is the concrete fix: it now checks positional_raw before falling back to value_to_text_sink. Also fixed, because they read tool_args.positional through the identical value_to_string pattern: function-call and script positional parameters ($1, $2, ...) - an unrelated bug in the same shape, found by grep while tracing every consumer of tool_args.positional, not something Amy asked for by name. Verified end to end against the built binary: echo -0 007 010 0.10 1.0 022 now prints exactly the table Amy's ruling asked for (-0 007 010 0.10 1.0 022), matching what /bin/echo already printed via the external-command fix. function f { echo $1 }; f -0 also prints -0. Explicitly out of scope, left as a residual: a repeatable flag's accumulated value (push_repeatable_value's Json(Array(Array(...))) shape) is not itself a bare numeral literal at the point it accumulates, so a non-canonical numeral inside a repeated --flag=value was not threaded through - a rare intersection, not touched here. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 + crates/kaish-kernel/src/kernel.rs | 205 ++++++++++++++---- crates/kaish-kernel/src/tools/builtin/echo.rs | 10 +- .../tests/builtin_fidelity_tests.rs | 27 +++ crates/kaish-types/src/tool.rs | 126 ++++++++++- 5 files changed, 327 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae0cc0d3..13057680 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ breaking entries are marked **BREAKING**. not a JSON number, and `fromjson` already refused it. `-0`, `0.10`, and `1.0` are unaffected; they stay numbers. +- **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. + ## [0.16.0] - 2026-08-23 ### Added diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index 6aa7a462..a55c203b 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -3918,10 +3918,12 @@ impl Kernel { } } // A numeral whose typed `Display` would not reproduce its - // source (`-0`, `007`, `1.0`) goes to the external - // process as the exact word it was written as — the same - // rule `ast::plan::render_expr` applies, so the argv that - // executes matches the argv a plan showed. Skips + // source (`-0`, `1.0`) goes to the external process as + // the exact word it was written as — the same rule + // `ast::plan::render_expr` applies, so the argv that + // executes matches the argv a plan showed. (A leading + // zero like `007` is a plain string by now, handled by + // the ordinary `Value::String` path below.) Skips // `eval_expr_async`/text-sink entirely: there is no // `Value` that could reproduce `raw` anyway. if let Expr::NumericLiteral { raw, .. } = expr { @@ -4681,9 +4683,20 @@ impl Kernel { let saved = scope.save_positional(); // Set up new positional parameters ($0 = function name, $1, $2, ... = args) + // A non-canonical numeral (`-0`, `0.10`) uses its own source + // text: `$1` is a text sink like any argv word, and `value_to_string` + // on a plain `Value::Int`/`Float` can't reproduce it — 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); @@ -5148,9 +5161,18 @@ impl Kernel { } // Set up positional parameters ($0 = script name, $1, $2, ... = args) + // Same non-canonical-numeral fidelity as the function-call site + // above — 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(); isolated_scope.set_positional(name, positional_args); @@ -6637,6 +6659,13 @@ 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? { + // A non-canonical numeral (`-0`, `0.10`) keeps + // its source text alongside the typed push — + // see `ToolArgs::words_raw`. Recorded at the + // index the push below lands at. + if let Expr::NumericLiteral { raw, .. } = expr { + tool_args.words_raw.insert(words.len(), raw.clone()); + } words.push(apply_tilde_expansion(value, home.as_deref())); } } @@ -6671,12 +6700,21 @@ 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. A non-canonical + // numeral (`-0`, `0.10`) uses its own source text instead + // of re-stringifying the typed value, same reasoning as + // `kernel.rs::build_args_flat`'s `Arg::Named` arm — the + // whole word is composed here, so there is no later + // render step a `words_raw` entry could reach. + 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 } => { @@ -6684,11 +6722,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 => { @@ -6733,6 +6775,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); } } @@ -6743,6 +6790,16 @@ pub(crate) async fn bind_tool_args( ) })?; let value = apply_tilde_expansion(value, home.as_deref()); + // A non-canonical numeral (`-0`, `0.10`) keeps its + // source text alongside the typed push — see + // `ToolArgs::positional_raw`. `test`'s numeric + // operators still get the real `value`; a text + // consumer gets `raw` instead. + if let Expr::NumericLiteral { raw, .. } = expr { + tool_args + .positional_raw + .insert(tool_args.positional.len(), raw.clone()); + } tool_args.positional.push(value); } } @@ -6759,12 +6816,18 @@ 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. A non-canonical + // numeral uses its own source text instead, same as the + // Verbatim binder's `Arg::Named` arm above. + 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}"))); @@ -6776,11 +6839,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}"))); @@ -6861,6 +6928,19 @@ pub(crate) async fn bind_tool_args( } if let Some(value) = source.eval(expr).await? { let value = apply_tilde_expansion(value, home.as_deref()); + // A non-canonical numeral (`-0`, `0.10`) keeps its + // source text alongside the typed push — see + // `ToolArgs::positional_raw`. This is the path + // `echo` reads: it takes `args.positional` directly + // (never the clap-parsed field, a validation-only + // sink — see `value_to_argv_token`'s doc comment), + // so a plain `Value::Int`/`Float` here could never + // have reproduced `-0` on its own. + if let Expr::NumericLiteral { raw, .. } = expr { + tool_args + .positional_raw + .insert(tool_args.positional.len(), raw.clone()); + } tool_args.positional.push(value); } } @@ -6872,11 +6952,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}"))); @@ -6931,6 +7015,13 @@ pub(crate) async fn bind_tool_args( } // Value::Bool(false): absent == false, nothing to insert. } else { + // A non-canonical numeral (`-0`, `0.10`) keeps its + // source text — see `ToolArgs::named_raw`. A named + // value is normally read off the clap-parsed field, + // built from `to_argv()`, so this reaches it there. + if let Expr::NumericLiteral { raw, .. } = value { + tool_args.named_raw.insert(key.clone(), raw.clone()); + } tool_args.named.insert(key.clone(), val); } } @@ -6951,18 +7042,27 @@ 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]`. A non-canonical numeral uses + // its own source text instead of re-stringifying the + // typed value, same as 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}"))); } } @@ -7210,7 +7310,18 @@ pub(crate) async fn bind_tool_args( tool_args.positional.len() }; + // `positional_raw` is keyed by index into `positional`, and this + // block redistributes/reindexes `positional` — a value moving to + // `named` or to a new position in `remaining` must carry its raw + // text (if any) along, or `positional_raw` would point at the + // wrong entry afterward, mislabeling some OTHER positional's text + // as this one's. `old_raw` is keyed by the pre-drain index; both + // destinations below insert under the index/key 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 { @@ -7223,10 +7334,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, @@ -7234,8 +7351,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/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/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-types/src/tool.rs b/crates/kaish-types/src/tool.rs index 8a4ad64f..3c96d0d9 100644 --- a/crates/kaish-types/src/tool.rs +++ b/crates/kaish-types/src/tool.rs @@ -471,8 +471,34 @@ 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 — a negative zero or a non-canonical trailing + /// fraction digit (`-0`, `0.10`, `1.0`; see `lexer::Token::NumericLiteral` + /// in kaish-kernel). Keyed by index into `positional`; absent at every + /// index holding an ordinary value, so this stays empty for the common + /// case and costs nothing there. + /// + /// This exists because most builtins read a positional's real value + /// straight off `positional`, never off the clap-parsed struct (see + /// `value_to_argv_token`'s doc comment) — a plain `Value::Int`/`Float` + /// genuinely cannot carry back the source text once typed. A builtin + /// that echoes a positional's text verbatim (`echo`) should check this + /// first: `args.positional_raw.get(&i)`. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub positional_raw: BTreeMap, /// Named arguments by key. pub named: BTreeMap, + /// Verbatim source text for a `named` entry whose typed `Display` would + /// not reproduce it — same idea as `positional_raw`, keyed by the same + /// key. Unlike a positional, a named value is normally read off the + /// clap-parsed struct (`parsed.count`, …) rather than `named` directly, + /// so `to_argv`/`to_argv_excluding` consult this when rendering + /// `--key=value`, and the fix reaches the clap field the usual way. + /// Only ever populated for a single (non-repeated) value — a repeatable + /// flag's accumulated `Value::Json(Array(Array(...)))` is not a bare + /// numeral literal in the first place. + #[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 +513,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 +525,44 @@ impl ToolArgs { Self::default() } + /// The display text for positional `index`: its verbatim source text + /// from `positional_raw` when the literal there was a non-canonical + /// numeral, otherwise `value_to_argv_token` on the typed value. `None` + /// when `index` is out of range. + /// + /// A builtin that prints a positional's text as-is (`echo`) should call + /// this instead of stringifying `positional[index]` itself — that is + /// exactly the gap this method closes. A builtin that needs the typed + /// value (arithmetic, a numeric comparison) should keep reading + /// `positional[index]` directly; `value` there is unaffected by this + /// field and stays a real `Int`/`Float`. + 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 +750,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 +763,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 +1037,59 @@ mod to_argv_tests { assert_eq!(args.to_argv().unwrap(), vec!["--", "hello", "world"]); } + // ─────────────────── positional_raw / named_raw / words_raw ─────────────────── + // A non-canonical numeral (`-0`) is still typed as `Value::Int(0)` — that's + // the mathematically correct value — but its own `Display` cannot get back + // to `-0`. `*_raw` is where the source text that `Value` alone lost lives. + + #[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(); From 0306daf64aabbcd88e3f1a3ca78c85e531b3464a Mon Sep 17 00:00:00 2001 From: A Tobey Date: Mon, 24 Aug 2026 08:19:23 -0400 Subject: [PATCH 05/27] docs: teach the seam between JSON number rules and quoting Amy: agents already know shell and already know JSON; the help text should let them find the seam between the two quickly instead of explaining kaish's whole type system to get there. Written after the leading-zero and ToolArgs-raw-text rulings landed, since the two now read as similar but are not: one decides what TYPE a bare word gets, the other decides what TEXT a typed word renders back as, and a reader who conflates them will reach for quoting to fix the wrong one. New "Numbers" section, placed right after "Variables" in both docs/LANGUAGE.md and the kaish-help Syntax fragments (which single- source content/en/syntax.md and `help numbers`) - the same spot a reader already goes to learn what a bare word means. Three lines carry the whole rule, per house style: 007 is a string (no user action needed, kaish already agrees with the reader's expectation), -0 is a number that reprints as -0 in argv/plan but collapses to canonical 0 once it moves through a variable or arithmetic, and "-0" is how you keep the string on purpose. Led with expectation, not RFC 8259 - the spec citation stays in the Ruling 2 commit, where it is the engineering argument for the fix, not the user's reason to care. Every claim in the example was checked against the built binary before writing it down, including the one most likely to be assumed rather than tested: x=-0; echo $x prints 0, not -0 - a variable copy is a plain typed value with no memory of the literal it came from, so the fidelity the first two commits added does not extend past the first read. content/en/syntax.md is generated, not hand-edited - added the section to fragments.rs's registry (between the "variables" and "expansion" keys, so it renders in that position) and ran `cargo run -p kaish-help --example regen_syntax`. Ran the drift test (syntax_md_matches_fragments) and the LANGUAGE.md coverage test (language_md_still_covers_the_syntax_surface) after regenerating; both pass unchanged, and `help numbers` resolves the new section through the existing key-lookup mechanism with no additional plumbing. No CHANGELOG entry: this documents behavior the two Ruling commits already logged, and the project's Keep a Changelog convention does not carry a Documentation category for content updates. Co-Authored-By: Claude Opus 5 --- crates/kaish-help/content/en/syntax.md | 15 +++++++++++++++ crates/kaish-help/src/fragments.rs | 16 ++++++++++++++++ docs/LANGUAGE.md | 16 ++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/crates/kaish-help/content/en/syntax.md b/crates/kaish-help/content/en/syntax.md index 06fbbb65..c9cfb34f 100644 --- a/crates/kaish-help/content/en/syntax.md +++ b/crates/kaish-help/content/en/syntax.md @@ -10,6 +10,21 @@ 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 +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 +``` + +`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. + ## Expansion ```sh diff --git a/crates/kaish-help/src/fragments.rs b/crates/kaish-help/src/fragments.rs index bbadeeb3..d324eb8f 100644 --- a/crates/kaish-help/src/fragments.rs +++ b/crates/kaish-help/src/fragments.rs @@ -303,6 +303,22 @@ 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 +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 +``` + +`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."#, ), syntax_section( "expansion", diff --git a/docs/LANGUAGE.md b/docs/LANGUAGE.md index 2873d12d..1dbf0e56 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -27,6 +27,22 @@ 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 +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 +``` + +`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. + ### Inline environment prefix — `NAME=value command` One or more assignments placed *before* a command scope those variables to that From 4fe53eb05d02bceca7f3c1de677ee6597808883c Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 25 Aug 2026 18:32:57 -0400 Subject: [PATCH 06/27] Where kaish needs a number, a leading zero is an error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lexer change alone left the rule half-taught. `007` became text in argv, which is what bash does and what the mode operand `chmod 0644` needs, but the positions that require a real number each answered differently: `break 007` and `xs[007]=v` failed with a shape mismatch against the whole statement alternative set, `${xs[007]}` silently resolved to index 7, and `$((010 + 1))` silently answered 11 where bash answers 9. Three spellings of the same numeral, three different answers, two of them silent. Amy's ruling: a leading zero means text; where kaish needs a number, a leading zero is an error. That splits cleanly, because the ambiguity is only ever in the position, never in the word: echo 007, chmod 0644 text, exactly as typed break 007, $((010)) error naming the leading zero and the fix ${xs[007]} on a list error naming the fix, ${xs[7]} ${r[007]} on a record the "007" key, which is what it says That last one was a bug the rule fixes rather than creates. A record stored under "007" could not be read back by the name it was stored under, because the subscript normalized to 7 first. The write side gets the read side's classification (`NumberIdent` in `lvalue_subscript_parser`), so `r[007]=v` and `${r[007]}` finally name the same key. `break`/`continue` are diagnosed after the grammar has already failed, on the #413 pattern, so no passing program reaches the new message. Arithmetic refuses rather than reading decimal: silently disagreeing with bash by two is worse than stopping. Redirect targets read the source text the way `plan_redirect_target` already does. `--plan` reported `-0` while the run created a file named `0` — a plan that describes a write that never happens, which is exactly what #414 asked plan consumers to trust. Co-Authored-By: Claude Opus 5 --- crates/kaish-kernel/src/arithmetic.rs | 11 ++ crates/kaish-kernel/src/interpreter/scope.rs | 10 ++ crates/kaish-kernel/src/lexer.rs | 30 ++++ crates/kaish-kernel/src/parser.rs | 56 +++++- crates/kaish-kernel/src/scheduler/pipeline.rs | 8 + .../kaish-kernel/tests/leading_zero_tests.rs | 170 ++++++++++++++++++ 6 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 crates/kaish-kernel/tests/leading_zero_tests.rs diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 0bb84425..fd68daf6 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -311,6 +311,17 @@ impl<'a> ArithParser<'a> { } } let num_str = &self.input[start..self.pos]; + // kaish has no octal, and bash does: `$((010 + 1))` is 9 there and + // would be 11 here. Answering a different number than the shell the + // author learned is the worst outcome, so refuse the numeral. + 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" + ); + } num_str.parse().context("invalid number in arithmetic expression") } diff --git a/crates/kaish-kernel/src/interpreter/scope.rs b/crates/kaish-kernel/src/interpreter/scope.rs index 224c251f..d26a2b50 100644 --- a/crates/kaish-kernel/src/interpreter/scope.rs +++ b/crates/kaish-kernel/src/interpreter/scope.rs @@ -118,6 +118,16 @@ fn classify_index(json: &serde_json::Value, i: i64, path: &str) -> Result Result { match json { serde_json::Value::Object(_) => Ok(Step::Key(key.to_string())), + serde_json::Value::Array(_) if crate::lexer::is_leading_zero_numeral(key) => { + // The author almost certainly meant an index. Name the leading + // zero, or the message reads as a type confusion they never made. + let index = key.trim_start_matches('-').trim_start_matches('0'); + let index = if index.is_empty() { "0" } else { index }; + Err(PathError::Shape(format!( + "${{{path}[{key}]}}: `{key}` is text (leading zero) and a list is indexed by \ + number — write ${{{path}[{index}]}}" + ))) + } 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/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 0d9204b0..97fef7e7 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -3564,6 +3564,36 @@ fn has_invalid_leading_zero(raw: &str) -> bool { 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`], the +/// same shape `9lives` gets, because a leading zero makes a word text (see +/// [`has_invalid_leading_zero`]). +/// +/// Callers use this where kaish needs a number and got text, so the error can +/// name the leading zero instead of reporting a shape mismatch: `break 007`, +/// `$((010 + 1))`, a subscript on a list. A word that is not otherwise a +/// numeral (`007abc`) is ordinary text and 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 — see /// `has_invalid_leading_zero` (a leading zero makes it a string, not a /// number: `Token::NumberIdent`, the same shape a digit run with a trailing diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index 52717ce9..d825c651 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -472,8 +472,12 @@ 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]`. A leading zero makes the word text, so + // `[007]` falls through to the bareword key below — on a record it finds + // the "007" key, and on a list it raises the loud error in `scope.rs`. + if !lexer::is_leading_zero_numeral(inner) + && let Ok(i) = inner.parse::() + { return VarSegment::Index(i); } // Bareword literal key: `[name]`, `[content-type]`. @@ -1289,6 +1293,13 @@ fn parse_tokens( if let Err(specific) = validate_heredoc_bodies(&tokens) { return specific; } + // `break 007` / `continue 007`: the count grammar matches `Token::Int` + // and a leading zero no longer produces one, so chumsky reports a + // shape mismatch against the whole statement alternative set. Name + // the leading zero instead. + if let Err(specific) = validate_leading_zero_counts(&tokens) { + 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 @@ -1608,6 +1619,10 @@ where select! { Token::SingleString(s) => VarSegment::Key(s) }, select! { Token::Int(n) => VarSegment::Index(n) }, select! { Token::Ident(s) => parse_subscript(&s) }, + // A leading-zero numeral lexes as `NumberIdent`, so 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) }, )); just(Token::LBracket) @@ -3546,6 +3561,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, @@ -3674,6 +3690,42 @@ 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`/`continue` take a loop count, and a count is a number. A leading +/// zero makes the word text ([`lexer::is_leading_zero_numeral`]), so `break +/// 007` no longer matches the count grammar and 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 for a leading-zero +/// numeral standing directly after `break`/`continue`, so it replaces a +/// confusing message for a statement kaish rejects either way. It never makes +/// a passing program fail. +fn validate_leading_zero_counts(tokens: &[(Token, Span)]) -> Result<(), Vec> { + for pair in tokens.windows(2) { + let keyword = match &pair[0].0 { + Token::Break => "break", + Token::Continue => "continue", + _ => continue, + }; + let Token::NumberIdent(word) = &pair[1].0 else { + continue; + }; + if !lexer::is_leading_zero_numeral(word) { + continue; + } + let count = word.trim_start_matches('-').trim_start_matches('0'); + let count = if count.is_empty() { "1" } else { count }; + return Err(vec![ParseError { + span: pair[1].1, + message: format!( + "`{keyword}` takes a loop count and `{word}` is text (leading zero) — write \ + `{keyword} {count}`" + ), + }]); + } + Ok(()) +} + fn validate_glued_args( tokens: &[(Token, Span)], from_offset: usize, 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/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs new file mode 100644 index 00000000..3c590683 --- /dev/null +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -0,0 +1,170 @@ +//! 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"); +} + +/// 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"); +} + +#[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::>() + ); + } +} From 8bc548a53d38a0fa996916ca4b80c07915c1a1f0 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 25 Aug 2026 18:42:58 -0400 Subject: [PATCH 07/27] docs: teach the second half of the leading-zero rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Numbers section taught what a leading zero MEANS and stopped there, which was the whole rule when a leading zero only ever produced a string. Now that the number-demanded positions refuse it, a reader who learns only the first half meets an error the docs never mentioned. Added the positions and their fixes to both surfaces, and led with the case that decides it for a bash reader: $((010 + 1)) is an error, not 9 and not 11. Naming both wrong answers is the point — a reader who knows bash expects 9 and would otherwise assume kaish silently agrees. Named the deliberate conversions in the same breath, since "no octal" without an escape hatch reads as a missing feature: printf "%o"/"%x" format one, xxd dumps bytes. Nothing parses a base yet. The record-key example is in the docs because it is the one place the rule GIVES something back: ${r[007]} now reads the "007" key, which the old normalize-to-7 read could not reach at all. fragments.rs is the source for content/en/syntax.md — edited the fragment and ran `cargo run -p kaish-help --example regen_syntax`. LANGUAGE.md is hand-maintained and got the longer treatment. Every example was run against the built binary before it was written down. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 12 ++++++++++ crates/kaish-help/content/en/syntax.md | 14 ++++++++--- crates/kaish-help/src/fragments.rs | 16 +++++++++---- docs/LANGUAGE.md | 33 +++++++++++++++++++++++--- 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2f6bb91..1b91f730 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,18 @@ breaking entries are marked **BREAKING**. 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))`, and a list index name the number to write. kaish reads no + octal, so arithmetic refuses instead of answering 11 where bash answers 9. + +- **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 name the + same text key. + +- **`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 diff --git a/crates/kaish-help/content/en/syntax.md b/crates/kaish-help/content/en/syntax.md index c9cfb34f..83a7d61b 100644 --- a/crates/kaish-help/content/en/syntax.md +++ b/crates/kaish-help/content/en/syntax.md @@ -13,9 +13,11 @@ ENABLED=true # boolean (only true/false) ## Numbers ```sh -echo 007 # the string 007 — a leading zero is not a JSON number -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 +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 @@ -25,6 +27,12 @@ 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, 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. + ## Expansion ```sh diff --git a/crates/kaish-help/src/fragments.rs b/crates/kaish-help/src/fragments.rs index d324eb8f..e241f537 100644 --- a/crates/kaish-help/src/fragments.rs +++ b/crates/kaish-help/src/fragments.rs @@ -308,9 +308,11 @@ ENABLED=true # boolean (only true/false) "numbers", "Numbers", r#"```sh -echo 007 # the string 007 — a leading zero is not a JSON number -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 +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 @@ -318,7 +320,13 @@ 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."#, +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, 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."#, ), syntax_section( "expansion", diff --git a/docs/LANGUAGE.md b/docs/LANGUAGE.md index 1dbf0e56..333727e6 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -30,9 +30,11 @@ echo "${NAME} more text" ### A bare number follows JSON rules ```sh -echo 007 # the string 007 — a leading zero is not a JSON number -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 +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 @@ -43,6 +45,31 @@ 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, and +a list index: + +```sh +break 007 # error — write `break 7` +echo $((010 + 1)) # error — kaish reads no octal; 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 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 From 48b675578ff1cd53675772369236be92c9f7f4dc Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 25 Aug 2026 18:49:18 -0400 Subject: [PATCH 08/27] A slice bound is a number position too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Probing read-and-write agreement across every subscript spelling turned up the one the rule had missed, and it was the silent kind: a slice bound parsed `007` as 7, so `${xs[007:2]}` sliced from 7, inverted the range, and returned an empty list. Exit 0, no message. An empty result is the one wrong answer a caller cannot tell from a correct one. Both bounds refuse a leading zero now and fall through to a bareword key, which the container reports against. `without_leading_zeros` splits on `:` so the suggested fix names the whole subscript rather than one half of it: `${xs[007:2]}` says write `${xs[7:2]}`. The four ordinary spellings — `[0:2]`, `[:2]`, `[1:]`, `[-2:]` — are pinned by their own test, because refusing one spelling of a grammar is an easy way to break the rest of it. Co-Authored-By: Claude Opus 5 --- crates/kaish-kernel/src/interpreter/scope.rs | 30 +++++++++++++++--- crates/kaish-kernel/src/parser.rs | 6 ++++ .../kaish-kernel/tests/leading_zero_tests.rs | 31 +++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/crates/kaish-kernel/src/interpreter/scope.rs b/crates/kaish-kernel/src/interpreter/scope.rs index d26a2b50..88087e2a 100644 --- a/crates/kaish-kernel/src/interpreter/scope.rs +++ b/crates/kaish-kernel/src/interpreter/scope.rs @@ -112,20 +112,42 @@ 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 crate::lexer::is_leading_zero_numeral(key) => { + serde_json::Value::Array(_) if let Some(fix) = without_leading_zeros(key) => { // The author almost certainly meant an index. Name the leading // zero, or the message reads as a type confusion they never made. - let index = key.trim_start_matches('-').trim_start_matches('0'); - let index = if index.is_empty() { "0" } else { index }; Err(PathError::Shape(format!( "${{{path}[{key}]}}: `{key}` is text (leading zero) and a list is indexed by \ - number — write ${{{path}[{index}]}}" + number — write ${{{path}[{fix}]}}" ))) } serde_json::Value::Array(_) => Err(PathError::Shape(format!( diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index d825c651..2fc6e611 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -461,9 +461,15 @@ 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, so a leading zero is text here + // too — without this `[007:2]` sliced from 7 and silently returned an + // empty list. Refusing the bound 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) } diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index 3c590683..7aadc8bc 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -127,6 +127,37 @@ async fn a_record_key_that_is_a_leading_zero_numeral_round_trips() { 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:?}"); + } +} + #[tokio::test] async fn an_ordinary_index_is_untouched() { let (code, out, err) = run("xs=[10 20 30]; echo ${xs[1]}").await; From 47b24acb85d0cab4bc60d5dc7a34fc24aefd3351 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Tue, 25 Aug 2026 19:04:34 -0400 Subject: [PATCH 09/27] The count message was answering for errors it never judged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the first two commits found the new `break`/`continue` message was the worst of the four defects it turned up, and it was mine rather than the bug's: it scanned the whole token stream with no gate, so it could answer a completely unrelated error. if true; then echo hi; done <- the real error break 007 <- my message answered this instead That is the line #413 drew — a post-failure validator may reword a diagnosis and must never author one — and this one was weaker than the `validate_glued_args` it was modeled on. Two gates, because one was not enough: the grammar's own error must sit on that numeral, AND `break` must be in statement position. `echo break 007` fails on the numeral too, so position in the stream alone still blamed an `echo` command for a loop count. Three more from the same review. Arithmetic only refused the literal. `x=010; echo $((x))` answered 10 where bash answers 8 — the same silent divergence the rule exists to stop, arriving by another road. Variables and positionals refuse it now. Read and write still disagreed for `[-0]` and `[1.0]`. Neither is a leading-zero numeral; they are numerals whose source text does not round-trip, and they reach the subscript through the same token. Both sides classify from the raw text now and produce the same message. `break -022` suggested `break 22`. A suggestion that drops the sign is worse than no suggestion, since it names a statement that is valid and different. What the review flagged that I did not change: env assignments, `for` words and `case` subjects drop the source text. Those are variable bindings, they behave exactly as `x=-0` does, and `help numbers` already documents that limit. Left alone deliberately. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 11 ++- crates/kaish-kernel/src/arithmetic.rs | 31 ++++++- crates/kaish-kernel/src/parser.rs | 57 +++++++++++-- .../kaish-kernel/tests/leading_zero_tests.rs | 83 +++++++++++++++++++ docs/LANGUAGE.md | 8 ++ 5 files changed, 178 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b91f730..94089147 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,9 +44,16 @@ breaking entries are marked **BREAKING**. `$((010 + 1))`, and a list index name the number to write. kaish reads no octal, so arithmetic refuses instead of answering 11 where bash answers 9. +- **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 name the - same text key. + 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 diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index fd68daf6..70e52ec2 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -35,6 +35,21 @@ pub fn eval_arithmetic(expr: &str, scope: &Scope) -> Result { } /// Simple recursive descent parser for arithmetic expressions. +/// The decimal a leading-zero numeral was probably meant to be — `010` becomes +/// `10`, `-007` becomes `-7`. `None` when the text is not one. +/// +/// Arithmetic refuses these rather than reading them: kaish has no octal and +/// bash does, so `010` is 8 there and would be 10 here. The suggestion keeps +/// the sign, because `-007` is not fixed by writing `7`. +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 })) +} + struct ArithParser<'a> { input: &'a str, pos: usize, @@ -346,6 +361,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) }); @@ -438,7 +459,15 @@ 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; reading it back into arithmetic + // is a number position like any other, and parsing it decimal + // would answer 10 for `010` where bash answers 8. + 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/parser.rs b/crates/kaish-kernel/src/parser.rs index 2fc6e611..cdfaabb5 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -1303,7 +1303,8 @@ fn parse_tokens( // and a leading zero no longer produces one, so chumsky reports a // shape mismatch against the whole statement alternative set. Name // the leading zero instead. - if let Err(specific) = validate_leading_zero_counts(&tokens) { + 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 @@ -1629,6 +1630,11 @@ where // `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 a numeral whose source text does not round-trip: + // `r[-0]=v` and `r[1.0]=v` were parse errors while both read fine. + // Classifying from the raw text is what makes the two sides agree — + // it is the same string the read path hands `parse_subscript`. + select! { Token::NumericLiteral(d) => parse_subscript(&d.raw) }, )); just(Token::LBracket) @@ -3702,25 +3708,58 @@ fn glue_candidate_units(tokens: &[(Token, Span)]) -> Vec { /// against every statement alternative — a long message that never mentions /// the zero. /// -/// Runs only after the grammar has already failed, and only for a leading-zero -/// numeral standing directly after `break`/`continue`, so it replaces a -/// confusing message for a statement kaish rejects either way. It never makes -/// a passing program fail. -fn validate_leading_zero_counts(tokens: &[(Token, Span)]) -> Result<(), Vec> { - for pair in tokens.windows(2) { +/// 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; } - let count = word.trim_start_matches('-').trim_start_matches('0'); - let count = if count.is_empty() { "1" } else { count }; + // 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 }); return Err(vec![ParseError { span: pair[1].1, message: format!( diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index 7aadc8bc..ae2a350f 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -85,6 +85,56 @@ async fn an_ordinary_loop_count_still_parses() { assert_eq!(out, "done"); } +/// 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:?}"); +} + /// 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. @@ -158,6 +208,39 @@ async fn every_ordinary_slice_spelling_still_works() { } } +/// 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; diff --git a/docs/LANGUAGE.md b/docs/LANGUAGE.md index 333727e6..e720163e 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -62,6 +62,14 @@ 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. +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: From 3c097804ffdf51652ca7c135e47943d57ed1a55b Mon Sep 17 00:00:00 2001 From: A Tobey Date: Wed, 26 Aug 2026 20:13:05 -0400 Subject: [PATCH 10/27] Cut the leading-zero comments to the house length Amy on the review: "The comments seem to repeat a lot of what is in or should be in LANGUAGE.md." They did. The rule was restated at nine kernel.rs call sites, each one re-explaining that a non-canonical numeral keeps its source text and that 007 is a different case. LANGUAGE.md already teaches both under "A bare number follows JSON rules", so the rule is stated once on Token::NumericLiteral and Expr::NumericLiteral, which point there, and the call sites say only what is true at that site. The bash-octal reasoning stays in arithmetic.rs -- refusing to answer a third number is why that arm exists, and nothing else on the line says so. 378 comment lines against 772 of code, now 283. --- crates/kaish-kernel/src/arithmetic.rs | 14 ++- crates/kaish-kernel/src/ast/plan.rs | 7 +- crates/kaish-kernel/src/ast/types.rs | 25 ++---- crates/kaish-kernel/src/dispatch.rs | 8 +- crates/kaish-kernel/src/interpreter/eval.rs | 6 +- crates/kaish-kernel/src/interpreter/scope.rs | 7 +- crates/kaish-kernel/src/kernel.rs | 91 +++++++------------- crates/kaish-kernel/src/lexer.rs | 79 ++++++----------- crates/kaish-kernel/src/parser.rs | 38 ++++---- crates/kaish-types/src/tool.rs | 52 ++++------- 10 files changed, 116 insertions(+), 211 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 70e52ec2..a28090a7 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -38,9 +38,7 @@ pub fn eval_arithmetic(expr: &str, scope: &Scope) -> 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. /// -/// Arithmetic refuses these rather than reading them: kaish has no octal and -/// bash does, so `010` is 8 there and would be 10 here. The suggestion keeps -/// the sign, because `-007` is not fixed by writing `7`. +/// The suggestion keeps the sign: `-007` is not fixed by writing `7`. fn leading_zero_decimal(text: &str) -> Option { if !crate::lexer::is_leading_zero_numeral(text) { return None; @@ -326,9 +324,8 @@ impl<'a> ArithParser<'a> { } } let num_str = &self.input[start..self.pos]; - // kaish has no octal, and bash does: `$((010 + 1))` is 9 there and - // would be 11 here. Answering a different number than the shell the - // author learned is the worst outcome, so refuse the numeral. + // 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 }; @@ -459,9 +456,8 @@ impl<'a> ArithParser<'a> { match value { Value::Int(n) => Ok(*n), Value::String(s) => { - // `x=007` stores the string; reading it back into arithmetic - // is a number position like any other, and parsing it decimal - // would answer 10 for `010` where bash answers 8. + // `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 \ diff --git a/crates/kaish-kernel/src/ast/plan.rs b/crates/kaish-kernel/src/ast/plan.rs index 27378193..30c17bc2 100644 --- a/crates/kaish-kernel/src/ast/plan.rs +++ b/crates/kaish-kernel/src/ast/plan.rs @@ -897,11 +897,8 @@ pub(crate) fn render_expr(expr: &Expr) -> String { format!("${{{}:-{}}}", render_varpath(path), render_parts(default)) } Expr::Arithmetic(e) => format!("$(({e}))"), - // The whole reason this variant exists: a numeral whose `Display` - // would not reproduce its own source (`-0`, `1.0`) renders as the - // verbatim text it was written as, not `value`'s canonical form. A - // leading zero (`007`) is a different case: `Expr::Literal(String)` - // by the time it gets here, handled above like any other bareword. + // 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(), diff --git a/crates/kaish-kernel/src/ast/types.rs b/crates/kaish-kernel/src/ast/types.rs index fcd187c5..87f5de9e 100644 --- a/crates/kaish-kernel/src/ast/types.rs +++ b/crates/kaish-kernel/src/ast/types.rs @@ -393,23 +393,16 @@ 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 (`Int`/`Float`) whose own `Display` does not reproduce the - /// source text it was written as — `-0` (negative zero has no distinct - /// `i64` spelling), `0.10`/`1.0` (a non-canonical trailing fraction - /// digit). `value` is the typed value: arithmetic, comparisons, - /// `set x = -0`, and `--json` all still see a real `Int`/`Float`. `raw` - /// is the exact source text: argv/plan rendering and real - /// external-command argv use it instead of `value`'s `Display`, so - /// `xargs -0 rm -f` keeps its `-0`. + /// 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 leading zero (`007`) is a DIFFERENT case and never reaches this - /// variant — not a valid JSON number, so it parses as - /// `Literal(Value::String("007"))` instead, the same as any other - /// bareword. - /// - /// A canonical numeral (`-1`, `42`, `3.14`) never reaches this variant — - /// it parses as the plain `Literal(Value::Int/Float)` it always did, - /// unchanged. See `lexer::Token::NumericLiteral`, which this mirrors. + /// 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 }, } diff --git a/crates/kaish-kernel/src/dispatch.rs b/crates/kaish-kernel/src/dispatch.rs index 73612b72..e1cf3a0a 100644 --- a/crates/kaish-kernel/src/dispatch.rs +++ b/crates/kaish-kernel/src/dispatch.rs @@ -316,12 +316,8 @@ 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()), - // A numeral whose source text does not round-trip - // through its typed `Display` (`-0`, `1.0`) — kept in - // sync with kernel.rs::build_args_flat, which pushes - // `raw` directly for the same reason. A leading zero - // (`007`) is a plain `Literal(String)` by now, handled - // above like any other bareword. + // 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) { diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index 661c4082..ecda11ba 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -181,10 +181,8 @@ 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 ever needs `value` — arithmetic and - // comparisons see the real `Int`/`Float`. `raw` exists for - // argv/plan text-sink positions, which read the `Expr` directly - // rather than going through `eval`. + // 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), diff --git a/crates/kaish-kernel/src/interpreter/scope.rs b/crates/kaish-kernel/src/interpreter/scope.rs index 88087e2a..a1abe73f 100644 --- a/crates/kaish-kernel/src/interpreter/scope.rs +++ b/crates/kaish-kernel/src/interpreter/scope.rs @@ -116,8 +116,7 @@ fn classify_index(json: &serde_json::Value, i: i64, path: &str) -> Result Option { let mut changed = false; let fixed = key @@ -143,8 +142,8 @@ fn classify_key(json: &serde_json::Value, key: &str, path: &str) -> Result Ok(Step::Key(key.to_string())), serde_json::Value::Array(_) if let Some(fix) = without_leading_zeros(key) => { - // The author almost certainly meant an index. Name the leading - // zero, or the message reads as a type confusion they never made. + // 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}]}}" diff --git a/crates/kaish-kernel/src/kernel.rs b/crates/kaish-kernel/src/kernel.rs index 6b4e35ea..7a313903 100644 --- a/crates/kaish-kernel/src/kernel.rs +++ b/crates/kaish-kernel/src/kernel.rs @@ -3479,8 +3479,7 @@ 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(), - // A numeral whose own `Display` would drop a leading zero or a - // negative zero: show the source text, not the typed value. + // Show the source text, not the typed value. Expr::NumericLiteral { raw, .. } => raw.clone(), Expr::VarRef(path) => { let mut name = String::new(); @@ -3917,15 +3916,9 @@ impl Kernel { continue; } } - // A numeral whose typed `Display` would not reproduce its - // source (`-0`, `1.0`) goes to the external process as - // the exact word it was written as — the same rule - // `ast::plan::render_expr` applies, so the argv that - // executes matches the argv a plan showed. (A leading - // zero like `007` is a plain string by now, handled by - // the ordinary `Value::String` path below.) Skips - // `eval_expr_async`/text-sink entirely: there is no - // `Value` that could reproduce `raw` anyway. + // 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; @@ -4069,8 +4062,8 @@ impl Kernel { Ok(Value::Bool(!is_truthy(&value))) } Expr::Literal(value) => Ok(value.clone()), - // Typed evaluation only ever needs `value`; `raw` is for - // argv/plan text-sink positions that read the `Expr` directly. + // 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; @@ -4683,10 +4676,8 @@ impl Kernel { let saved = scope.save_positional(); // Set up new positional parameters ($0 = function name, $1, $2, ... = args) - // A non-canonical numeral (`-0`, `0.10`) uses its own source - // text: `$1` is a text sink like any argv word, and `value_to_string` - // on a plain `Value::Int`/`Float` can't reproduce it — see - // `ToolArgs::positional_raw`. + // `$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() .enumerate() @@ -5161,8 +5152,7 @@ impl Kernel { } // Set up positional parameters ($0 = script name, $1, $2, ... = args) - // Same non-canonical-numeral fidelity as the function-call site - // above — see `ToolArgs::positional_raw`. + // Same source-text fidelity as the function-call site above. let positional_args: Vec = tool_args.positional .iter() .enumerate() @@ -6248,10 +6238,8 @@ 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? { - // A non-canonical numeral (`-0`, `0.10`) keeps - // its source text alongside the typed push — - // see `ToolArgs::words_raw`. Recorded at the - // index the push below lands at. + // 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()); } @@ -6289,12 +6277,9 @@ 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. A non-canonical - // numeral (`-0`, `0.10`) uses its own source text instead - // of re-stringifying the typed value, same reasoning as - // `kernel.rs::build_args_flat`'s `Arg::Named` arm — the - // whole word is composed here, so there is no later - // render step a `words_raw` entry could reach. + // 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 { @@ -6379,11 +6364,8 @@ pub(crate) async fn bind_tool_args( ) })?; let value = apply_tilde_expansion(value, home.as_deref()); - // A non-canonical numeral (`-0`, `0.10`) keeps its - // source text alongside the typed push — see - // `ToolArgs::positional_raw`. `test`'s numeric - // operators still get the real `value`; a text - // consumer gets `raw` instead. + // `test`'s numeric operators still get the real + // `value`; a text consumer gets `raw`. if let Expr::NumericLiteral { raw, .. } = expr { tool_args .positional_raw @@ -6405,9 +6387,8 @@ 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. A non-canonical - // numeral uses its own source text instead, same as the - // Verbatim binder's `Arg::Named` arm above. + // 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 { @@ -6517,14 +6498,9 @@ pub(crate) async fn bind_tool_args( } if let Some(value) = source.eval(expr).await? { let value = apply_tilde_expansion(value, home.as_deref()); - // A non-canonical numeral (`-0`, `0.10`) keeps its - // source text alongside the typed push — see - // `ToolArgs::positional_raw`. This is the path - // `echo` reads: it takes `args.positional` directly - // (never the clap-parsed field, a validation-only - // sink — see `value_to_argv_token`'s doc comment), - // so a plain `Value::Int`/`Float` here could never - // have reproduced `-0` on its own. + // 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 @@ -6604,10 +6580,9 @@ pub(crate) async fn bind_tool_args( } // Value::Bool(false): absent == false, nothing to insert. } else { - // A non-canonical numeral (`-0`, `0.10`) keeps its - // source text — see `ToolArgs::named_raw`. A named - // value is normally read off the clap-parsed field, - // built from `to_argv()`, so this reaches it there. + // 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()); } @@ -6640,9 +6615,8 @@ pub(crate) async fn bind_tool_args( // 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]`. A non-canonical numeral uses - // its own source text instead of re-stringifying the - // typed value, same as every other composed-string arm. + // `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 { @@ -6899,14 +6873,11 @@ pub(crate) async fn bind_tool_args( tool_args.positional.len() }; - // `positional_raw` is keyed by index into `positional`, and this - // block redistributes/reindexes `positional` — a value moving to - // `named` or to a new position in `remaining` must carry its raw - // text (if any) along, or `positional_raw` would point at the - // wrong entry afterward, mislabeling some OTHER positional's text - // as this one's. `old_raw` is keyed by the pre-drain index; both - // destinations below insert under the index/key the value actually - // lands at. + // 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 = diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 97fef7e7..a33ef675 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -650,28 +650,19 @@ 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`; an - /// `i64`/`f64` has no distinct negative-zero spelling once parsed back - /// out for `Int`, and `f64::to_string` drops the trailing `.0` for - /// `Float`), or a non-canonical trailing fraction digit (`0.10`, - /// `1.0`). Carries both the typed value (so arithmetic, comparisons, - /// and `--json` still see a real `Int`/`Float`) and the verbatim - /// source text (so argv/plan rendering can reproduce exactly what was - /// typed). + /// 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`, `010`) is a DIFFERENT case, reclassified to - /// `NumberIdent` instead — see `has_invalid_leading_zero`. It is not a - /// valid JSON number, and kaish's own `fromjson` already refuses it, so - /// it is not a number here either: a string, not a mistyped `Int`. + /// A leading zero (`007`) is a different case, reclassified to + /// `NumberIdent` — see [`has_invalid_leading_zero`]. /// - /// Never produced directly by logos — `tokenize_impl`'s - /// `preserve_numeric_source_text` pass synthesizes it from a plain - /// `Int`/`Float` token as the LAST step, after the fusion passes run, so - /// `is_colon_mergeable`/`is_glob_mergeable` (which match `Int`/`Float` - /// directly) see the ordinary token during fusion and this variant only - /// ever reaches the parser. The common case (`-1`, `42`, `3.14`) is - /// untouched and pays nothing. + /// 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), // ═══════════════════════════════════════════════════════════════════ @@ -3548,16 +3539,12 @@ fn tokenize_impl( )) } -/// True when a numeral's own 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 (since JSON's `int` production is -/// shared by the float grammar) the integer part of `007.5`. A lone `0` -/// (`0`, `-0`, `0.5`) is the `zero` alternative and is fine. +/// 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` already refuses these (`fromjson '007'` is a parse error); -/// before this pass the lexer disagreed and typed them as `Int(7)`. Nobody -/// writing `007` expects the number 7, so the lexer now agrees: not a -/// number, a string. +/// `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); @@ -3565,15 +3552,12 @@ fn has_invalid_leading_zero(raw: &str) -> bool { } /// True when a word is a numeral in every respect except its leading zero — -/// `007`, `010`, `-022`, `007.5`. These lex as [`Token::NumberIdent`], the -/// same shape `9lives` gets, because a leading zero makes a word text (see -/// [`has_invalid_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 instead of reporting a shape mismatch: `break 007`, -/// `$((010 + 1))`, a subscript on a list. A word that is not otherwise a -/// numeral (`007abc`) is ordinary text and answers false — there is no -/// number the author might have meant. +/// 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('.'); @@ -3594,22 +3578,15 @@ pub(crate) fn is_leading_zero_numeral(word: &str) -> bool { has_invalid_leading_zero(word) } -/// Reclassify a plain `Int`/`Float` token from its own source text — see -/// `has_invalid_leading_zero` (a leading zero makes it a string, not a -/// number: `Token::NumberIdent`, the same shape a digit run with a trailing -/// letter already gets) and `Token::NumericLiteral`'s doc comment (a numeral -/// whose source text does not round-trip through its own typed `Display`: -/// negative zero, a non-canonical trailing fraction digit). The common case -/// (`-1`, `42`, `3.14`) pays one string comparison and stays a plain -/// `Int`/`Float`. +/// 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 — those -/// passes (`is_colon_mergeable`, `is_glob_mergeable`) match `Int`/`Float` -/// directly, so a numeral must still present its ordinary shape while fusion -/// decisions are made — a leading zero fused into a larger word (`a:007`, -/// `007*`) is not a standalone numeral and never reaches this pass at all. -/// Spans are already original-source coordinates by this point, so -/// `source[span]` is the exact word the author typed. +/// Runs as the LAST step of `tokenize_impl`, after every fusion pass: +/// `is_colon_mergeable` and `is_glob_mergeable` match `Int`/`Float` directly, +/// 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() diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index cdfaabb5..f933484a 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -461,10 +461,9 @@ 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, so a leading zero is text here - // too — without this `[007:2]` sliced from 7 and silently returned an - // empty list. Refusing the bound falls through to a bareword key, - // which `scope.rs` reports against the container. + // 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) @@ -478,9 +477,8 @@ fn parse_subscript(inner: &str) -> VarSegment { return VarSegment::Slice(start, end); } } - // Integer index: `[0]`, `[-1]`. A leading zero makes the word text, so - // `[007]` falls through to the bareword key below — on a record it finds - // the "007" key, and on a list it raises the loud error in `scope.rs`. + // 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::() { @@ -1299,10 +1297,8 @@ fn parse_tokens( if let Err(specific) = validate_heredoc_bodies(&tokens) { return specific; } - // `break 007` / `continue 007`: the count grammar matches `Token::Int` - // and a leading zero no longer produces one, so chumsky reports a - // shape mismatch against the whole statement alternative set. Name - // the leading zero instead. + // 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; @@ -1626,14 +1622,12 @@ where select! { Token::SingleString(s) => VarSegment::Key(s) }, select! { Token::Int(n) => VarSegment::Index(n) }, select! { Token::Ident(s) => parse_subscript(&s) }, - // A leading-zero numeral lexes as `NumberIdent`, so without this arm - // `r[007]=v` was a parse error while `${r[007]}` read fine. Both are - // the same text key now. + // 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 a numeral whose source text does not round-trip: - // `r[-0]=v` and `r[1.0]=v` were parse errors while both read fine. - // Classifying from the raw text is what makes the two sides agree — - // it is the same string the read path hands `parse_subscript`. + // 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) }, )); @@ -3702,11 +3696,9 @@ 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`/`continue` take a loop count, and a count is a number. A leading -/// zero makes the word text ([`lexer::is_leading_zero_numeral`]), so `break -/// 007` no longer matches the count grammar and chumsky reports the miss -/// against every statement alternative — a long message that never mentions -/// the zero. +/// `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 diff --git a/crates/kaish-types/src/tool.rs b/crates/kaish-types/src/tool.rs index 3c96d0d9..d7f59817 100644 --- a/crates/kaish-types/src/tool.rs +++ b/crates/kaish-types/src/tool.rs @@ -472,31 +472,22 @@ pub struct ToolArgs { /// Positional arguments in order. pub positional: Vec, /// Verbatim source text for a `positional` entry whose typed `Display` - /// would not reproduce it — a negative zero or a non-canonical trailing - /// fraction digit (`-0`, `0.10`, `1.0`; see `lexer::Token::NumericLiteral` - /// in kaish-kernel). Keyed by index into `positional`; absent at every - /// index holding an ordinary value, so this stays empty for the common - /// case and costs nothing there. + /// would not reproduce it — `-0`, `0.10`, `1.0`. Keyed by index into + /// `positional`, and empty for the common case. /// - /// This exists because most builtins read a positional's real value - /// straight off `positional`, never off the clap-parsed struct (see - /// `value_to_argv_token`'s doc comment) — a plain `Value::Int`/`Float` - /// genuinely cannot carry back the source text once typed. A builtin - /// that echoes a positional's text verbatim (`echo`) should check this - /// first: `args.positional_raw.get(&i)`. + /// 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, - /// Verbatim source text for a `named` entry whose typed `Display` would - /// not reproduce it — same idea as `positional_raw`, keyed by the same - /// key. Unlike a positional, a named value is normally read off the - /// clap-parsed struct (`parsed.count`, …) rather than `named` directly, + /// 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 fix reaches the clap field the usual way. - /// Only ever populated for a single (non-repeated) value — a repeatable - /// flag's accumulated `Value::Json(Array(Array(...)))` is not a bare - /// numeral literal in the first place. + /// `--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). @@ -525,17 +516,14 @@ impl ToolArgs { Self::default() } - /// The display text for positional `index`: its verbatim source text - /// from `positional_raw` when the literal there was a non-canonical - /// numeral, otherwise `value_to_argv_token` on the typed value. `None` - /// when `index` is out of range. + /// 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 that prints a positional's text as-is (`echo`) should call - /// this instead of stringifying `positional[index]` itself — that is - /// exactly the gap this method closes. A builtin that needs the typed - /// value (arithmetic, a numeric comparison) should keep reading - /// `positional[index]` directly; `value` there is unaffected by this - /// field and stays a real `Int`/`Float`. + /// 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()); @@ -1037,10 +1025,8 @@ mod to_argv_tests { assert_eq!(args.to_argv().unwrap(), vec!["--", "hello", "world"]); } - // ─────────────────── positional_raw / named_raw / words_raw ─────────────────── - // A non-canonical numeral (`-0`) is still typed as `Value::Int(0)` — that's - // the mathematically correct value — but its own `Display` cannot get back - // to `-0`. `*_raw` is where the source text that `Value` alone lost lives. + // `-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() { From 192e72363c0ccb037d9c42e2c71c6e75f401ef3a Mon Sep 17 00:00:00 2001 From: A Tobey Date: Wed, 26 Aug 2026 20:16:23 -0400 Subject: [PATCH 11/27] Name a private item, do not link it Same slip as the one on #412: the trim linked has_invalid_leading_zero from Token::NumericLiteral's public docs, and that fn is private. Caught by RUSTDOCFLAGS=-D warnings, which cargo doc alone does not apply. --- crates/kaish-kernel/src/lexer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index a33ef675..af979b35 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -658,7 +658,7 @@ pub enum Token { /// 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`]. + /// `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 From 2db38b52eff72600b00d4af809cd5b4b241489fc Mon Sep 17 00:00:00 2001 From: A Tobey Date: Wed, 26 Aug 2026 20:37:59 -0400 Subject: [PATCH 12/27] A negative-zero loop count still parses kaibo's review of this branch found it: the count grammar matches Token::Int, and making -0 carry its source text moved it to Token::NumericLiteral, so `break -0` and `continue -0` became parse errors. Confirmed against a main build -- exit 0 there, exit 2 here -- and `break -1` and `break 0` were never affected, so it was exactly the -0 spelling that this branch calls a valid number in LANGUAGE.md. The count now accepts a NumericLiteral carrying an Int. The test fails without the grammar change. --- crates/kaish-kernel/src/parser.rs | 22 ++++++++++++++----- .../kaish-kernel/tests/leading_zero_tests.rs | 16 ++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index f933484a..8753fc27 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -1387,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 diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index ae2a350f..d802439b 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -85,6 +85,22 @@ async fn an_ordinary_loop_count_still_parses() { 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 From 22719d151dbda34655040b3c0e433b6b41fd643a Mon Sep 17 00:00:00 2001 From: A Tobey Date: Wed, 26 Aug 2026 20:40:45 -0400 Subject: [PATCH 13/27] exec hands over the same argv as the direct spelling kaibo's review found exec building its argv at its own edge rather than through build_args_flat, so it never got the source-text rule. Verified: `/bin/echo -0 007 0.10` printed `-0 007 0.10` while `exec /bin/echo -0 007 0.10` printed `0 007 0.1` -- two spellings of one command disagreeing, inside the branch whose whole point is that argv matches what was typed. The test spawns the real binary. Written in-process first, it execve'd over the test harness: 11 of 48 tests ran, the harness became /bin/echo, and the run still exited 0. kaish-repl already has the spawn pattern for frontend behavior, so the test lives there. Also corrected a comment this branch's trim left overstated: is_glob_mergeable matches Int, not Int and Float. --- crates/kaish-kernel/src/lexer.rs | 5 ++- crates/kaish-kernel/src/tools/builtin/exec.rs | 8 +++- .../tests/numeral_argv_fidelity_tests.rs | 40 +++++++++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 crates/kaish-repl/tests/numeral_argv_fidelity_tests.rs diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index af979b35..bab6bc5d 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -3583,8 +3583,9 @@ pub(crate) fn is_leading_zero_numeral(word: &str) -> bool { /// (`-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` and `is_glob_mergeable` match `Int`/`Float` directly, -/// so a numeral must still present its ordinary shape while fusion decides. +/// `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> { 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-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" + ); +} From 6d758ed8439224cddce2ee3468ccb3a644a18591 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 07:58:40 -0400 Subject: [PATCH 14/27] A numeric comparison is a number position too `[[ 010 -eq 10 ]]` and `test 010 -eq 10` answered true: value_to_num string-parsed "010" as decimal 10. Arithmetic already refuses this numeral (bash reads it as octal 8; kaish reads no octal), so the comparison was quietly giving a third answer nothing else in the language would give. value_to_num backs both `[[ ]]` and the test builtin, so one guard covers both. It now checks arithmetic::leading_zero_decimal before parsing and refuses with the same "(leading zero)" / "no octal" / "write `N`" wording arithmetic already uses, so a numeral is refused the same way wherever it reaches a number position. leading_zero_decimal moved to pub(crate) so eval.rs could reach it without duplicating the suggestion logic. Docs and help gained the comparison as a fourth listed number position, alongside break/continue, arithmetic, and a list index. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 5 +- crates/kaish-help/content/en/syntax.md | 8 +-- crates/kaish-help/src/fragments.rs | 8 +-- crates/kaish-kernel/src/arithmetic.rs | 2 +- crates/kaish-kernel/src/interpreter/eval.rs | 10 ++++ .../kaish-kernel/tests/leading_zero_tests.rs | 59 +++++++++++++++++++ docs/LANGUAGE.md | 5 +- 7 files changed, 84 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94089147..102fd37a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,8 +41,9 @@ breaking entries are marked **BREAKING**. `1.0` are unaffected; they stay numbers. - **Where kaish needs a number, a leading zero is an error** — `break 007`, - `$((010 + 1))`, and a list index name the number to write. kaish reads no - octal, so arithmetic refuses instead of answering 11 where bash answers 9. + `$((010 + 1))`, `[[ 010 -eq 10 ]]`, and a list index name the number to + write. kaish reads no octal, so arithmetic refuses instead of answering 11 + where bash answers 9. - **Arithmetic refuses a leading zero however it arrives** — `$((010 + 1))` and `x=010; $((x))` both name the decimal to write. Reading the text as diff --git a/crates/kaish-help/content/en/syntax.md b/crates/kaish-help/content/en/syntax.md index 83a7d61b..565aa36a 100644 --- a/crates/kaish-help/content/en/syntax.md +++ b/crates/kaish-help/content/en/syntax.md @@ -28,10 +28,10 @@ or a plan. Once the number moves through a variable, arithmetic, or 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, 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. +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. ## Expansion diff --git a/crates/kaish-help/src/fragments.rs b/crates/kaish-help/src/fragments.rs index e241f537..80b3f751 100644 --- a/crates/kaish-help/src/fragments.rs +++ b/crates/kaish-help/src/fragments.rs @@ -323,10 +323,10 @@ or a plan. Once the number moves through a variable, arithmetic, or 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, 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."#, +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."#, ), syntax_section( "expansion", diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index a28090a7..2a91769a 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -39,7 +39,7 @@ pub fn eval_arithmetic(expr: &str, scope: &Scope) -> Result { /// `10`, `-007` becomes `-7`. `None` when the text is not one. /// /// The suggestion keeps the sign: `-007` is not fixed by writing `7`. -fn leading_zero_decimal(text: &str) -> Option { +pub(crate) fn leading_zero_decimal(text: &str) -> Option { if !crate::lexer::is_leading_zero_numeral(text) { return None; } diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index ecda11ba..57c90273 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -1194,6 +1194,16 @@ fn value_to_num(value: &Value) -> EvalResult { 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::() { diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index d802439b..cc82b77f 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -298,3 +298,62 @@ async fn a_redirect_target_writes_the_file_the_plan_names() { ); } } + +// ── 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", + ] { + 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`"), + "{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")); +} diff --git a/docs/LANGUAGE.md b/docs/LANGUAGE.md index e720163e..27781946 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -46,12 +46,13 @@ 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, and -a list index: +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]} ``` From 4273c3fb6c0c6760fa5669d1ffe609cea11965a8 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 07:59:15 -0400 Subject: [PATCH 15/27] An integer past 64 bits names the limit and the fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `echo 9223372036854775808` failed the whole statement with "lexer error: invalid number" — true, but it offers nothing to do about it. The word is a valid JSON number; i64 is kaish's limit, not JSON's, and the regex behind lex_int/parse_int admits only `-?[0-9]+`, so overflow is the only way that parse ever fails there. Added LexerError::IntegerOutOfRange, mapped from the same parse failure lex_int and parse_int already handled, with a Display that names the range and the fix: quote the numeral to keep it as text. lex_float/parse_float are untouched — they still say "invalid number", since a float overflow is a different, still-unnamed case. Docs and help gained the 64-bit limit alongside the other JSON-number rules for a bare numeral. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 4 ++-- crates/kaish-help/content/en/syntax.md | 4 ++++ crates/kaish-help/src/fragments.rs | 6 +++++- crates/kaish-kernel/src/lexer.rs | 14 ++++++++++++-- docs/LANGUAGE.md | 5 +++++ 5 files changed, 28 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 102fd37a..553242b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,8 +42,8 @@ breaking entries are marked **BREAKING**. - **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, so arithmetic refuses instead of answering 11 - where bash answers 9. + write; kaish reads no octal. An integer past 64 bits names the limit and + the fix instead of "invalid number". - **Arithmetic refuses a leading zero however it arrives** — `$((010 + 1))` and `x=010; $((x))` both name the decimal to write. Reading the text as diff --git a/crates/kaish-help/content/en/syntax.md b/crates/kaish-help/content/en/syntax.md index 565aa36a..24484401 100644 --- a/crates/kaish-help/content/en/syntax.md +++ b/crates/kaish-help/content/en/syntax.md @@ -33,6 +33,10 @@ 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 80b3f751..293d4203 100644 --- a/crates/kaish-help/src/fragments.rs +++ b/crates/kaish-help/src/fragments.rs @@ -326,7 +326,11 @@ 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."#, +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/lexer.rs b/crates/kaish-kernel/src/lexer.rs index bab6bc5d..8517c23f 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -88,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). @@ -144,6 +149,11 @@ impl fmt::Display for LexerError { } LexerError::InvalidEscape => write!(f, "invalid escape sequence"), LexerError::InvalidNumber => write!(f, "invalid number"), + LexerError::IntegerOutOfRange => write!( + f, + "does not fit in a 64-bit integer (-9223372036854775808..9223372036854775807); \ + quote it to keep the text" + ), 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), @@ -1082,7 +1092,7 @@ 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) + lex.slice().parse().map_err(|_| LexerError::IntegerOutOfRange) } /// Lex a float literal. @@ -3749,7 +3759,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/docs/LANGUAGE.md b/docs/LANGUAGE.md index 27781946..53d4eb4d 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -63,6 +63,11 @@ 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: From d732e276e4f90ed89de08126c3949ad5ff4eb765 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:18:44 -0400 Subject: [PATCH 16/27] Retire two shell_compat pins the refusal made unrunnable numeric_eq_leading_zero_string and numeric_eq_handles_what_string_eq_does_not pinned bash's octal reading of "01" as equal to 1. kaish now refuses a leading-zero numeral in a number position, so the kaish side of each pin panics on `.expect("kaish execute")` inside the shell_compat! macro, which has no way to express a deliberate Err from the kaish side. Deleted both pins and left a comment pointing at the refusal's real pin: leading_zero_tests.rs, numeric_comparison_refuses_a_leading_zero_rather_ than_reading_decimal. Extended that test's source list with the quoted spellings the two deleted pins used, and its accepted fix-substring chain with `write \`1\`` so the quoted form stays covered. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/tests/leading_zero_tests.rs | 7 ++++++- crates/kaish-kernel/tests/shell_compat_tests.rs | 16 +++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index cc82b77f..b9387890 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -312,12 +312,17 @@ async fn numeric_comparison_refuses_a_leading_zero_rather_than_reading_decimal() "[[ 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 `10`") + || text.contains("write `100`") + || text.contains("write `-7`") + || text.contains("write `1`"), "{source:?} must name the fix: {text:?}" ); } 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""#, From 0c9a38ea559c6034a732654d87851511b9afc6a0 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:20:26 -0400 Subject: [PATCH 17/27] A leading-zero numeral is text before it is an overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit echo 09223372036854775808 failed with the 64-bit message instead of printing itself as text. lex_int parses during tokenization and only afterward does preserve_numeric_source_text read the source span and reclassify a leading-zero Int into NumberIdent — so an overflowing leading-zero numeral hit the i64 parse failure first and never reached the reclassification pass. lex_int now checks has_invalid_leading_zero on its own slice before parsing. A leading-zero numeral returns a placeholder Ok(0): the value is discarded regardless, since preserve_numeric_source_text rebuilds the token from the source text, not the parsed int. The rule stays: leading zero decides the word is text before overflow ever gets a vote. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/lexer.rs | 11 ++++++++++- crates/kaish-kernel/tests/leading_zero_tests.rs | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 8517c23f..9bf4bb33 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -1092,7 +1092,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::IntegerOutOfRange) + 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. diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index b9387890..5c40a853 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -362,3 +362,19 @@ async fn an_integer_past_64_bits_names_the_limit_and_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:?}"); +} From 0997086d5b9b8868f28584d90839698270f195c8 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:22:29 -0400 Subject: [PATCH 18/27] Arithmetic overflow names the same 64-bit limit the lexer does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit echo \$((9223372036854775808)) said only "invalid number in arithmetic expression" — arithmetic::parse_number parses its own numerals separately from the lexer and had never been told the lexer's newer, more specific IntegerOutOfRange wording. Pulled the lexer's message into a shared pub(crate) const, lexer::INTEGER_OUT_OF_RANGE, and pointed both call sites at it: the LexerError::IntegerOutOfRange Display arm, and parse_number's overflow context (the `[0-9]+` scan above it admits only digits, so overflow is its only failure mode). One string, so the two callers cannot drift apart again. Addition overflow keeps its own distinct wording. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 4 +++- crates/kaish-kernel/src/lexer.rs | 12 +++++++----- crates/kaish-kernel/tests/leading_zero_tests.rs | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 2a91769a..120c0b07 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -334,7 +334,9 @@ impl<'a> ArithParser<'a> { `{trimmed}` for the decimal value" ); } - num_str.parse().context("invalid number in arithmetic expression") + // 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 { diff --git a/crates/kaish-kernel/src/lexer.rs b/crates/kaish-kernel/src/lexer.rs index 9bf4bb33..d178ca5c 100644 --- a/crates/kaish-kernel/src/lexer.rs +++ b/crates/kaish-kernel/src/lexer.rs @@ -135,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 { @@ -149,11 +155,7 @@ impl fmt::Display for LexerError { } LexerError::InvalidEscape => write!(f, "invalid escape sequence"), LexerError::InvalidNumber => write!(f, "invalid number"), - LexerError::IntegerOutOfRange => write!( - f, - "does not fit in a 64-bit integer (-9223372036854775808..9223372036854775807); \ - quote it to keep the text" - ), + 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), diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index 5c40a853..349dc561 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -378,3 +378,17 @@ async fn a_leading_zero_numeral_past_64_bits_is_still_text() { 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:?}"); +} From cae09fda49b69488956b5f1b74e6e117d155d17d Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:23:15 -0400 Subject: [PATCH 19/27] Pin -0's argv-vs-variable split with a test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit x=-0; echo \$x prints 0 was documented in docs/LANGUAGE.md and in a comment in exec.rs, but no test pinned it, and no test pinned the argv-vs-variable split behind it: -0 is a valid JSON number (only the past-one-digit form is a leading-zero refusal), so argv keeps the typed word (echo -0 -> -0) while a variable canonicalizes it (x=-0; echo $x -> 0) the same way any other typed number does when it moves off argv. This item is coverage only — the behavior it pins was already correct; no code changed. Co-Authored-By: Claude Fable 5 --- .../kaish-kernel/tests/leading_zero_tests.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index 349dc561..51c7c527 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -392,3 +392,21 @@ async fn arithmetic_overflow_literal_names_the_64_bit_limit() { let text = err_of("echo $((9223372036854775807 + 1))").await; assert!(text.contains("overflow"), "addition overflow must still say overflow: {text:?}"); } + +// ── `-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:?}"); +} From d3a02db3aa48ea990079b378e283fa47ba23cf27 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:24:56 -0400 Subject: [PATCH 20/27] break's leading-zero recovery no longer suggests a fraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit break 007.5 said `write \`break 7.5\`` — the zero-trim keeps the fraction, but the count grammar takes only a whole number, so the suggested fix was itself a parse error. The recovery now checks the trimmed value for a `.` before suggesting it. A fraction gets its own wording naming the integer part instead ("takes a whole-number loop count ... write a whole number such as `break 7`"); a genuine integer (`break 007`) is unaffected and keeps the original "write `break 7`" message. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/parser.rs | 19 ++++++++++++++----- .../kaish-kernel/tests/leading_zero_tests.rs | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/crates/kaish-kernel/src/parser.rs b/crates/kaish-kernel/src/parser.rs index 8753fc27..82ea1a75 100644 --- a/crates/kaish-kernel/src/parser.rs +++ b/crates/kaish-kernel/src/parser.rs @@ -3762,13 +3762,22 @@ fn validate_leading_zero_counts( 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 }); - return Err(vec![ParseError { - span: pair[1].1, - message: format!( + // 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(()) } diff --git a/crates/kaish-kernel/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index 51c7c527..85a05a45 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -151,6 +151,20 @@ async fn the_suggested_count_keeps_its_sign() { 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. From a691a551ec9613dbef61ea44ace62a56445e13d0 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:25:22 -0400 Subject: [PATCH 21/27] Split an over-length changelog bullet in two The "Where kaish needs a number" bullet ran 51 words against the project's <=40 rule (repeated feedback, per CLAUDE.md). Split it: the number-position rule stays its own bullet (35 words), and the 64-bit overflow message gets its own (23 words) rather than a trailing sentence riding along. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 553242b9..81ccbef7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,8 +42,10 @@ breaking entries are marked **BREAKING**. - **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". + 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 From 5a8ee0f5af6979f8b5724d2b970c63dd58c92708 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:28:44 -0400 Subject: [PATCH 22/27] value_to_num no longer rounds an overflowing string through f64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit x="9223372036854775808"; [[ \$x -eq 9223372036854775807 ]] answered true: value_to_num's String arm tried i64, and on any failure — overflow included — fell straight to f64. Both operands round to 2^63 in f64, so two numbers that are not equal compared equal. The f64 fallback now only fires when the trimmed text actually looks like a float (contains `.`, `e`, or `E`). An all-digit string (optional leading `-`) that fails the i64 parse is refused instead, with the same 64-bit wording the lexer and \$(( )) now share (lexer::INTEGER_OUT_OF_ RANGE, wired in the arithmetic-overflow commit earlier in this branch). Added a lexer_tests.rs case pinning that i64::MAX + 1 and i64::MIN - 1 lex as IntegerOutOfRange (the variant already existed; it had no direct unit test). Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/interpreter/eval.rs | 22 ++++++++++++++++--- .../kaish-kernel/tests/leading_zero_tests.rs | 20 +++++++++++++++++ crates/kaish-kernel/tests/lexer_tests.rs | 9 ++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index 57c90273..9a5681db 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -1205,9 +1205,25 @@ fn value_to_num(value: &Value) -> EvalResult { }); } 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::() + { + return Ok(Num::Float(f)); + } + let all_digits = t.strip_prefix('-').unwrap_or(t); + if !all_digits.is_empty() && all_digits.bytes().all(|b| b.is_ascii_digit()) { + 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/tests/leading_zero_tests.rs b/crates/kaish-kernel/tests/leading_zero_tests.rs index 85a05a45..d0ecf587 100644 --- a/crates/kaish-kernel/tests/leading_zero_tests.rs +++ b/crates/kaish-kernel/tests/leading_zero_tests.rs @@ -407,6 +407,26 @@ async fn arithmetic_overflow_literal_names_the_64_bit_limit() { 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_ diff --git a/crates/kaish-kernel/tests/lexer_tests.rs b/crates/kaish-kernel/tests/lexer_tests.rs index 6e9c3c10..b588386d 100644 --- a/crates/kaish-kernel/tests/lexer_tests.rs +++ b/crates/kaish-kernel/tests/lexer_tests.rs @@ -292,6 +292,15 @@ 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 From fca9175375fa867810166e5c5a935070bad43010 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:36:44 -0400 Subject: [PATCH 23/27] test's leading-zero operand is refused, not read as decimal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit numeric_integer pinned the OLD rule: "test 08 -eq 8" -> exit 0, "leading zero is decimal, not octal". The branch inverts that rule on purpose — where kaish needs a number, a leading zero is an error naming the decimal to write, and test 08 -eq 8 is now the same refusal as [[ 010 -eq 10 ]] and $((010)). cargo test --all caught the stale pin as a real failure (exit 2, not 0) once the inversion landed. Moved the assertion into its own test, numeric_leading_zero_operand_ is_refused, asserting exit 2 and that the test: err text names the cause ("(leading zero)") and the fix ("write `8`"). The other eight numeric_integer assertions are untouched. The number-position rule itself is pinned in leading_zero_tests.rs; this test only pins that the test builtin reports the same refusal with its own test: prefix and exit 2. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/tests/test_builtin_tests.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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` From dabcfa5d63e76e49ef7700b117172d370df7aee5 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:54:31 -0400 Subject: [PATCH 24/27] A numeric operand is a JSON number: no inf, no nan, and overflow names the limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [[ "1e309" -gt 1 ]] answered true: value_to_num's f64 fallback parses "1e309" to f64::INFINITY without an Err, so the earlier .{./e/E} gate let an out-of-range float spelling through while a bare inf or nan already refused (they contain no ./e/E, so they never reach the f64 parse — they fail as a non-numeric string instead). Deliberate divergence, stated once here: kaish numbers are JSON numbers, and JSON has neither infinity nor NaN, so both must refuse, not compare. value_to_num now checks f.is_finite() after a successful f64 parse and refuses a non-finite result by name ("outside the 64-bit float range"), consistent with the leading-zero and i64-overflow refusals it already carries. return/exit's value_to_exit_code had the same overflow gap from the other direction: an i64-shaped string that only overflowed said the generic "numeric argument required", identical to what a non-numeric string says. Pulled the digit-shape check into a shared helper, is_i64_overflow_shape, and pointed both value_to_num and value_to_exit_code at lexer::INTEGER_OUT_OF_RANGE for that case, so an overflowing return/exit argument names the same 64-bit limit $(( )) and [[ ]] do. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/interpreter/eval.rs | 26 ++++++- .../tests/numeric_operand_tests.rs | 73 +++++++++++++++++++ 2 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 crates/kaish-kernel/tests/numeric_operand_tests.rs diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index 9a5681db..064b1b2d 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -617,7 +617,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(_) => { @@ -626,6 +630,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 @@ -1216,10 +1228,18 @@ fn value_to_num(value: &Value) -> EvalResult { 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)); } - let all_digits = t.strip_prefix('-').unwrap_or(t); - if !all_digits.is_empty() && all_digits.bytes().all(|b| b.is_ascii_digit()) { + if is_i64_overflow_shape(t) { Err(EvalError::TypeError { expected: "a number", got: format!("`{t}`, which {}", crate::lexer::INTEGER_OUT_OF_RANGE), 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:?}"); +} From eb443d20b6341df862c615b65ac74bab24aa6f54 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:56:29 -0400 Subject: [PATCH 25/27] Two doc comments sat on the wrong item arithmetic.rs: "Simple recursive descent parser for arithmetic expressions." opened leading_zero_decimal's doc comment, describing ArithParser two items below it, which had no doc of its own. Moved the sentence onto struct ArithParser; leading_zero_decimal keeps only the prose that actually describes it. eval.rs: "Coerce a value to a number for arithmetic test ops... parsed as i64 then f64" sat on enum Num instead of value_to_num, the function it describes, and the rule it stated was already stale (i64-then-f64 predates the leading-zero refusal, the finite check, and the overflow naming added earlier on this branch). Gave enum Num a one-line doc of its own and moved a restated, current rule onto value_to_num: leading zero refuses; then i64; then f64 only for a float spelling and only when finite; an all-digit i64 overflow names the 64-bit limit instead of falling through to f64. Neither misattachment could be caught by rustdoc; both were found by reading the file next to the code they claim to describe. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/arithmetic.rs | 2 +- crates/kaish-kernel/src/interpreter/eval.rs | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/kaish-kernel/src/arithmetic.rs b/crates/kaish-kernel/src/arithmetic.rs index 120c0b07..7f8b1b3d 100644 --- a/crates/kaish-kernel/src/arithmetic.rs +++ b/crates/kaish-kernel/src/arithmetic.rs @@ -34,7 +34,6 @@ pub fn eval_arithmetic(expr: &str, scope: &Scope) -> Result { Ok(result) } -/// Simple recursive descent parser for arithmetic expressions. /// The decimal a leading-zero numeral was probably meant to be — `010` becomes /// `10`, `-007` becomes `-7`. `None` when the text is not one. /// @@ -48,6 +47,7 @@ pub(crate) fn leading_zero_decimal(text: &str) -> Option { Some(format!("{sign}{}", if digits.is_empty() { "0" } else { digits })) } +/// Simple recursive descent parser for arithmetic expressions. struct ArithParser<'a> { input: &'a str, pos: usize, diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index 064b1b2d..acc2fa77 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -1191,15 +1191,19 @@ 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)), From 0ac6f7f6016fb201795f1e9c3bda82660ca7d5f2 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 08:57:22 -0400 Subject: [PATCH 26/27] A third misattached doc comment, found sweeping the same file While moving the value_to_num doc for the "Two doc comments" commit, found a third one nearby: "Convert a Value to its string representation for interpolation." opened value_to_exit_code's doc block, describing value_to_string, a function ~190 lines further down that had no doc comment of its own. Moved the sentence onto value_to_string and left value_to_exit_code with only the prose that describes it. Not one of the two items asked for on this round; fixed alongside them because it is the identical bug class, in the same file, found while reading it for the requested fix. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/interpreter/eval.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index acc2fa77..adb5e425 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -603,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" @@ -793,6 +792,7 @@ pub fn scalar_test_operand_error(op_symbol: &str, value: &Value) -> Option String { match value { Value::Null => "null".to_string(), From 6b2bdc9dc45abdf4dd6227d64a745fa40373e110 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Thu, 27 Aug 2026 09:33:02 -0400 Subject: [PATCH 27/27] The validator reads a numeral by its value, not its spelling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit seq 1 0.0 10 slipped past validation and only got caught by the builtin's own runtime check. seq's validate() reads a positional's typed Value, but validation builds its ToolArgs from AST Expr nodes through expr_to_placeholder, which matched only Expr::Literal. A numeral whose source text doesn't round-trip through its canonical Display — 0.0, -0, 0.00, 007 — lexes as Expr::NumericLiteral, not Literal, the same split kernel.rs's runtime binder already reads (Expr::NumericLiteral { value, .. } => Ok(value.clone()) there). expr_to_placeholder fell through to its string placeholder for every one of these, so seq's Value::Float(0.0) match never fired and the zero increment reached execute() instead of validate(). Fixed at the single conversion point, expr_to_placeholder in crates/kaish-kernel/src/validator/walker.rs: added an Expr::NumericLiteral arm returning the wrapped value, mirroring the runtime binder. Swept every other Tool::validate override (grep, sed, jq, diff, test, scatter, push, read, env, export, unset) for a similar numeric match; seq's zero-increment check is the only validate() site in the tree that reads a number, so it is also the only one this bug could hide behind. Added seq_zero_increment_is_caught_at_every_spelling next to the existing seq-increment validation test in kernel_error_tests.rs, pinning 0.0, -0, and 0.00 all raising SeqZeroIncrement at validation time, the same as the canonical seq 1 0 10. Co-Authored-By: Claude Fable 5 --- crates/kaish-kernel/src/validator/walker.rs | 7 ++++++ .../kaish-kernel/tests/kernel_error_tests.rs | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/crates/kaish-kernel/src/validator/walker.rs b/crates/kaish-kernel/src/validator/walker.rs index 5ec145b2..7b3eff59 100644 --- a/crates/kaish-kernel/src/validator/walker.rs +++ b/crates/kaish-kernel/src/validator/walker.rs @@ -1178,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/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.