From 0946b67ed73783fdf12fb06af3251407744cba0e Mon Sep 17 00:00:00 2001 From: A Tobey Date: Mon, 31 Aug 2026 17:50:39 -0400 Subject: [PATCH 1/3] printf refuses an operand it cannot read as a number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md names `printf '%d' 0xff` printing `0` as the shape to refuse, and printf was printing it. The conversions read a value through `s.parse().unwrap_or(0)` and `*f as i64`, so `0xff`, `abc` and `1e3` all answered `0`, `1.5` answered `1`, and `%c` of 4294967296 truncated through `as u32` into NUL. `007` was the worst of them: it answered `7`, a plausible number, while every other number position in kaish had just been taught that a leading zero is text. The rule printf now reads by is JSON's, the same one `fromjson` uses, so the answer does not depend on which builtin is asking: `1e3` is a number, `0xff` and `007` are not. Each refusal names the spelling that works — `255` or `$(( 0xff ))` for a based numeral, `8#7` or `7` for a leading zero, with the sign carried into both suggestions so a fix cannot flip the value. The seam worth explaining is `FormatArg`. printf and awk share the format engine, and they disagree about this on purpose: in POSIX awk a non-numeric string IS 0, and that is awk being correct, not awk being unfixed. So the trait's numeric conversions became fallible and each caller answers for itself — awk's impl is always `Ok`, printf's refuses. Two tests pin awk's coercion as the control that the refusal did not leak across the trait. A MISSING operand still converts as 0, which POSIX requires and which is not the fallback being removed here; only a value that is present and unreadable refuses. The refusal happens before any output, so a caller never reads a half-written line as a whole answer. This diverges from GNU for `0xff`, which GNU reads as 255 — deliberate, and the same divergence `$(( ))` already documents; for `abc` GNU also errors. Gates: clippy -D warnings, cargo test --all (204 suites, 6501 passed; +16 new, no existing test changed), insta --check, no-default-features, wasi build, rustdoc -D warnings. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 3 + crates/kaish-kernel/src/tools/builtin/awk.rs | 14 +- .../src/tools/builtin/format_string.rs | 167 ++++++++++-------- .../kaish-kernel/src/tools/builtin/printf.rs | 157 +++++++++++++--- .../tests/printf_numeric_refusal_tests.rs | 167 ++++++++++++++++++ docs/LANGUAGE.md | 7 + 6 files changed, 417 insertions(+), 98 deletions(-) create mode 100644 crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5892e360..c99e7718 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,9 @@ breaking entries are marked **BREAKING**. existing implementations keep compiling; not breaking. ### Fixed +- **`printf` refuses an operand it cannot read as a number** — `printf '%d' 0xff` + printed `0`; it now names `255` and `$(( 0xff ))`. `007`, `abc`, a fraction and + a 64-bit overflow refuse too. A missing operand is still `0`; awk is unchanged. - **`${path:-default}` inside `$(( ))` follows the ordinary `:-` contract** — unset, null, an empty string, a missing key, and an out-of-bounds index select the default; a shape error stays loud instead of quietly running the fallback. diff --git a/crates/kaish-kernel/src/tools/builtin/awk.rs b/crates/kaish-kernel/src/tools/builtin/awk.rs index cf1e53fb..357bc11c 100644 --- a/crates/kaish-kernel/src/tools/builtin/awk.rs +++ b/crates/kaish-kernel/src/tools/builtin/awk.rs @@ -1718,13 +1718,16 @@ impl std::fmt::Display for AwkValue { } } +// awk never refuses a conversion: in POSIX awk a non-numeric string IS 0, and +// `to_number` is that rule. These Results are always `Ok` on purpose — the +// printf refusal must not reach a language whose own spec answers 0. impl super::format_string::FormatArg for AwkValue { fn as_format_string(&self) -> String { self.to_string() } - fn as_format_int(&self) -> i64 { self.to_number() as i64 } - fn as_format_float(&self) -> f64 { self.to_number() } - fn as_format_char(&self) -> Option { + fn as_format_int(&self) -> Result { Ok(self.to_number() as i64) } + fn as_format_float(&self) -> Result { Ok(self.to_number()) } + fn as_format_char(&self) -> Result, String> { let n = self.to_number() as u32; - char::from_u32(n) + Ok(char::from_u32(n)) } } @@ -2841,7 +2844,8 @@ impl AwkRuntime { } fn sprintf(&self, format: &str, args: &[AwkValue]) -> Result { - Ok(super::format_string::format_string(format, args)) + // Always `Ok` in practice: `AwkValue`'s conversions never refuse. + super::format_string::format_string(format, args) } } diff --git a/crates/kaish-kernel/src/tools/builtin/format_string.rs b/crates/kaish-kernel/src/tools/builtin/format_string.rs index b7a073e1..fb6396d5 100644 --- a/crates/kaish-kernel/src/tools/builtin/format_string.rs +++ b/crates/kaish-kernel/src/tools/builtin/format_string.rs @@ -7,11 +7,20 @@ use unicode_width::UnicodeWidthStr; /// Trait for values that can be formatted by printf-style specifiers. +/// +/// The numeric conversions are fallible because the two callers disagree +/// about what a non-numeric value means. `printf` refuses it and names the +/// fix; awk answers 0, which is POSIX awk's own coercion rule and correct +/// there. The trait carries the question, not the answer. pub trait FormatArg { fn as_format_string(&self) -> String; - fn as_format_int(&self) -> i64; - fn as_format_float(&self) -> f64; - fn as_format_char(&self) -> Option; + /// This value as an integer, or the reason it is not one. + fn as_format_int(&self) -> Result; + /// This value as a float, or the reason it is not one. + fn as_format_float(&self) -> Result; + /// This value as a character, `None` when there is none to print, or + /// the reason it is not one. + fn as_format_char(&self) -> Result, String>; } /// Parsed format specifier: `%[flags][width][.precision]conversion`. @@ -35,13 +44,15 @@ struct FormatSpec { /// Backslash escapes: `\n`, `\t`, `\r`, `\\`, `\0`, `\NNN` (octal) /// /// This is a single pass: each conversion consumes one argument in order, and -/// missing arguments fall back to defaults (`""`, `0`, `0.0`). awk's `sprintf` +/// a MISSING argument falls back to a default (`""`, `0`, `0.0`), which POSIX +/// requires. A present argument that a numeric conversion cannot read is an +/// error instead, and the caller decides what that means. awk's `sprintf` /// uses this directly. POSIX `printf` reuses the format until all operands are /// consumed — see [`format_string_cycling`]. -pub fn format_string(format: &str, args: &[A]) -> String { +pub fn format_string(format: &str, args: &[A]) -> Result { let mut output = String::new(); - let _ = format_pass(format, args, &mut output); - output + format_pass(format, args, &mut output)?; + Ok(output) } /// POSIX `printf` cycling: reuse the format string until all operands are @@ -51,31 +62,35 @@ pub fn format_string(format: &str, args: &[A]) -> String { /// specifiers. A format with no conversions is printed exactly once (extra /// operands are ignored, matching bash) — this also guards against an infinite /// loop. The final pass may run short on operands; the missing ones default. -pub fn format_string_cycling(format: &str, args: &[A]) -> String { +pub fn format_string_cycling(format: &str, args: &[A]) -> Result { let mut output = String::new(); // The first pass always runs, so a zero-operand call still prints the // literal text and an all-default conversion line. - let (per_pass, stop) = format_pass(format, args, &mut output); + let (per_pass, stop) = format_pass(format, args, &mut output)?; if stop || per_pass == 0 { - return output; + return Ok(output); } let mut start = per_pass; while start < args.len() { let end = (start + per_pass).min(args.len()); - let (_, stop) = format_pass(format, &args[start..end], &mut output); + let (_, stop) = format_pass(format, &args[start..end], &mut output)?; if stop { break; } start = end; } - output + Ok(output) } /// Run one formatting pass, appending to `output`. Returns the number of /// conversion specifiers applied (i.e. operand slots consumed this pass) and /// whether output should stop entirely (a `\c` was reached, in the format /// literal or via a `%b` argument). -fn format_pass(format: &str, args: &[A], output: &mut String) -> (usize, bool) { +fn format_pass( + format: &str, + args: &[A], + output: &mut String, +) -> Result<(usize, bool), String> { let mut arg_index = 0; let mut chars = format.chars().peekable(); @@ -84,10 +99,10 @@ fn format_pass(format: &str, args: &[A], output: &mut String) -> ( match parse_specifier(&mut chars) { Some(spec) => { let arg = args.get(arg_index); - let stop = apply_specifier(&spec, arg, output); + let stop = apply_specifier(&spec, arg, output)?; arg_index += 1; if stop { - return (arg_index, true); + return Ok((arg_index, true)); } } None => { @@ -98,7 +113,7 @@ fn format_pass(format: &str, args: &[A], output: &mut String) -> ( } else if c == '\\' { // `\c` in the format literal stops all output (GNU printf). if chars.peek() == Some(&'c') { - return (arg_index, true); + return Ok((arg_index, true)); } parse_backslash_escape(&mut chars, output); } else { @@ -106,7 +121,7 @@ fn format_pass(format: &str, args: &[A], output: &mut String) -> ( } } - (arg_index, false) + Ok((arg_index, false)) } /// Parse and emit a backslash escape sequence, starting after the `\`. @@ -279,7 +294,11 @@ fn parse_specifier(chars: &mut std::iter::Peekable>) -> Opti /// /// Returns `true` if output should stop entirely (a `%b` argument contained /// `\c`), so the caller can abandon the rest of the format and any cycling. -fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mut String) -> bool { +fn apply_specifier( + spec: &FormatSpec, + arg: Option<&A>, + output: &mut String, +) -> Result { match spec.conversion { 's' => { let val = arg.map(|a| a.as_format_string()).unwrap_or_default(); @@ -294,19 +313,19 @@ fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mu apply_string_padding(spec, &val, output); } 'd' | 'i' => { - let val = arg.map(|a| a.as_format_int()).unwrap_or(0); + let val = arg.map(|a| a.as_format_int()).transpose()?.unwrap_or(0); apply_int_format(spec, val, output, IntBase::Decimal); } 'u' => { // Unsigned decimal: reinterpret the i64 bits as u64. - let val = arg.map(|a| a.as_format_int()).unwrap_or(0); + let val = arg.map(|a| a.as_format_int()).transpose()?.unwrap_or(0); let unsigned = val as u64; let raw = format!("{unsigned}"); let with_sign = apply_sign_and_prefix(spec, false, false, &raw); apply_padded(spec, false, &with_sign, spec.precision, false, output); } 'f' => { - let val = arg.map(|a| a.as_format_float()).unwrap_or(0.0); + let val = arg.map(|a| a.as_format_float()).transpose()?.unwrap_or(0.0); let precision = spec.precision.unwrap_or(6); let formatted = format!("{:.prec$}", val, prec = precision); let negative = val.is_sign_negative() && val != 0.0 || formatted.starts_with('-'); @@ -315,7 +334,7 @@ fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mu apply_padded(spec, false, &with_sign, None, false, output); } 'e' => { - let val = arg.map(|a| a.as_format_float()).unwrap_or(0.0); + let val = arg.map(|a| a.as_format_float()).transpose()?.unwrap_or(0.0); let precision = spec.precision.unwrap_or(6); let formatted = format_scientific(val, precision, false); let negative = val.is_sign_negative(); @@ -324,7 +343,7 @@ fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mu apply_padded(spec, false, &with_sign, None, false, output); } 'E' => { - let val = arg.map(|a| a.as_format_float()).unwrap_or(0.0); + let val = arg.map(|a| a.as_format_float()).transpose()?.unwrap_or(0.0); let precision = spec.precision.unwrap_or(6); let formatted = format_scientific(val, precision, true); let negative = val.is_sign_negative(); @@ -333,7 +352,7 @@ fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mu apply_padded(spec, false, &with_sign, None, false, output); } 'g' => { - let val = arg.map(|a| a.as_format_float()).unwrap_or(0.0); + let val = arg.map(|a| a.as_format_float()).transpose()?.unwrap_or(0.0); let precision = spec.precision.unwrap_or(6).max(1); let formatted = format_g(val, precision, false); let negative = val.is_sign_negative(); @@ -342,7 +361,7 @@ fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mu apply_padded(spec, false, &with_sign, None, false, output); } 'G' => { - let val = arg.map(|a| a.as_format_float()).unwrap_or(0.0); + let val = arg.map(|a| a.as_format_float()).transpose()?.unwrap_or(0.0); let precision = spec.precision.unwrap_or(6).max(1); let formatted = format_g(val, precision, true); let negative = val.is_sign_negative(); @@ -351,20 +370,20 @@ fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mu apply_padded(spec, false, &with_sign, None, false, output); } 'x' => { - let val = arg.map(|a| a.as_format_int()).unwrap_or(0); + let val = arg.map(|a| a.as_format_int()).transpose()?.unwrap_or(0); apply_int_format(spec, val, output, IntBase::LowerHex); } 'X' => { - let val = arg.map(|a| a.as_format_int()).unwrap_or(0); + let val = arg.map(|a| a.as_format_int()).transpose()?.unwrap_or(0); apply_int_format(spec, val, output, IntBase::UpperHex); } 'o' => { - let val = arg.map(|a| a.as_format_int()).unwrap_or(0); + let val = arg.map(|a| a.as_format_int()).transpose()?.unwrap_or(0); apply_int_format(spec, val, output, IntBase::Octal); } 'c' => { // %c honors width and the left-align flag (`printf '%5c' x` → ` x`). - if let Some(ch) = arg.and_then(|a| a.as_format_char()) { + if let Some(ch) = arg.map(|a| a.as_format_char()).transpose()?.flatten() { apply_string_padding(spec, &ch.to_string(), output); } } @@ -374,7 +393,7 @@ fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mu let raw = arg.map(|a| a.as_format_string()).unwrap_or_default(); let (val, stop) = interpret_backslash_escapes(&raw); apply_string_padding(spec, &val, output); - return stop; + return Ok(stop); } other => { // Unknown conversion — output literally @@ -382,7 +401,7 @@ fn apply_specifier(spec: &FormatSpec, arg: Option<&A>, output: &mu output.push(other); } } - false + Ok(false) } enum IntBase { @@ -655,93 +674,95 @@ mod tests { TestVal::Float(f) => f.to_string(), } } - fn as_format_int(&self) -> i64 { - match self { + // This fixture exercises the format PARSER, not either caller's + // number rule, so it keeps the permissive coercion awk uses. + fn as_format_int(&self) -> Result { + Ok(match self { TestVal::Int(i) => *i, TestVal::Float(f) => *f as i64, TestVal::Str(s) => s.parse().unwrap_or(0), - } + }) } - fn as_format_float(&self) -> f64 { - match self { + fn as_format_float(&self) -> Result { + Ok(match self { TestVal::Float(f) => *f, TestVal::Int(i) => *i as f64, TestVal::Str(s) => s.parse().unwrap_or(0.0), - } + }) } - fn as_format_char(&self) -> Option { - match self { + fn as_format_char(&self) -> Result, String> { + Ok(match self { TestVal::Str(s) => s.chars().next(), TestVal::Int(i) => char::from_u32(*i as u32), _ => None, - } + }) } } #[test] fn test_bare_specifiers() { let args = vec![TestVal::Str("hello".into()), TestVal::Int(42)]; - assert_eq!(format_string("%s %d", &args), "hello 42"); + assert_eq!(format_string("%s %d", &args).unwrap(), "hello 42"); } #[test] fn test_left_align() { let args = vec![TestVal::Str("hi".into())]; - assert_eq!(format_string("%-10s|", &args), "hi |"); + assert_eq!(format_string("%-10s|", &args).unwrap(), "hi |"); } #[test] fn test_right_align() { let args = vec![TestVal::Str("hi".into())]; - assert_eq!(format_string("%10s|", &args), " hi|"); + assert_eq!(format_string("%10s|", &args).unwrap(), " hi|"); } #[test] fn test_zero_pad_int() { let args = vec![TestVal::Int(42)]; - assert_eq!(format_string("%08d", &args), "00000042"); + assert_eq!(format_string("%08d", &args).unwrap(), "00000042"); } #[test] fn test_zero_pad_hex() { let args = vec![TestVal::Int(255)]; - assert_eq!(format_string("%08x", &args), "000000ff"); + assert_eq!(format_string("%08x", &args).unwrap(), "000000ff"); } #[test] fn test_precision_float() { let args = vec![TestVal::Float(3.14159)]; - assert_eq!(format_string("%.2f", &args), "3.14"); + assert_eq!(format_string("%.2f", &args).unwrap(), "3.14"); } #[test] fn test_width_and_precision_float() { let args = vec![TestVal::Float(3.14)]; - assert_eq!(format_string("%10.2f", &args), " 3.14"); + assert_eq!(format_string("%10.2f", &args).unwrap(), " 3.14"); } #[test] fn test_percent_escape() { let args: Vec = vec![]; - assert_eq!(format_string("100%%", &args), "100%"); + assert_eq!(format_string("100%%", &args).unwrap(), "100%"); } #[test] fn test_backslash_escapes() { let args: Vec = vec![]; - assert_eq!(format_string("a\\nb\\tc", &args), "a\nb\tc"); + assert_eq!(format_string("a\\nb\\tc", &args).unwrap(), "a\nb\tc"); } #[test] fn test_width_int() { let args = vec![TestVal::Int(42)]; - assert_eq!(format_string("%6d", &args), " 42"); + assert_eq!(format_string("%6d", &args).unwrap(), " 42"); } #[test] fn test_left_align_int() { let args = vec![TestVal::Int(42)]; - assert_eq!(format_string("%-6d|", &args), "42 |"); + assert_eq!(format_string("%-6d|", &args).unwrap(), "42 |"); } #[test] @@ -752,7 +773,7 @@ mod tests { TestVal::Str("b".into()), TestVal::Str("c".into()), ]; - assert_eq!(format_string_cycling("%s\\n", &args), "a\nb\nc\n"); + assert_eq!(format_string_cycling("%s\\n", &args).unwrap(), "a\nb\nc\n"); } #[test] @@ -764,7 +785,7 @@ mod tests { TestVal::Str("c".into()), TestVal::Str("d".into()), ]; - assert_eq!(format_string_cycling("%s-%s ", &args), "a-b c-d "); + assert_eq!(format_string_cycling("%s-%s ", &args).unwrap(), "a-b c-d "); } #[test] @@ -775,7 +796,7 @@ mod tests { TestVal::Str("b".into()), TestVal::Str("c".into()), ]; - assert_eq!(format_string_cycling("%s-%s ", &args), "a-b c- "); + assert_eq!(format_string_cycling("%s-%s ", &args).unwrap(), "a-b c- "); } #[test] @@ -783,14 +804,14 @@ mod tests { // No conversions: print the format exactly once, ignore extra operands. // (Also guards against an infinite loop.) let args = vec![TestVal::Str("a".into()), TestVal::Str("b".into())]; - assert_eq!(format_string_cycling("hello\\n", &args), "hello\n"); + assert_eq!(format_string_cycling("hello\\n", &args).unwrap(), "hello\n"); } #[test] fn test_cycling_single_pass_when_args_match() { let args = vec![TestVal::Str("Alice".into()), TestVal::Int(30)]; assert_eq!( - format_string_cycling("%s is %d", &args), + format_string_cycling("%s is %d", &args).unwrap(), "Alice is 30" ); } @@ -798,7 +819,7 @@ mod tests { #[test] fn test_cycling_no_args_still_runs_once() { let args: Vec = vec![]; - assert_eq!(format_string_cycling("%s\\n", &args), "\n"); + assert_eq!(format_string_cycling("%s\\n", &args).unwrap(), "\n"); } // --- new tests covering previously broken cases -------------------------- @@ -806,80 +827,80 @@ mod tests { #[test] fn test_plus_flag() { let args = vec![TestVal::Int(5)]; - assert_eq!(format_string("%+d", &args), "+5"); + assert_eq!(format_string("%+d", &args).unwrap(), "+5"); } #[test] fn test_space_flag() { let args = vec![TestVal::Int(5)]; - assert_eq!(format_string("% d", &args), " 5"); + assert_eq!(format_string("% d", &args).unwrap(), " 5"); } #[test] fn test_hash_hex() { let args = vec![TestVal::Int(255)]; - assert_eq!(format_string("%#x", &args), "0xff"); + assert_eq!(format_string("%#x", &args).unwrap(), "0xff"); } #[test] fn test_hash_octal() { let args = vec![TestVal::Int(8)]; - assert_eq!(format_string("%#o", &args), "010"); + assert_eq!(format_string("%#o", &args).unwrap(), "010"); } #[test] fn test_precision_string() { let args = vec![TestVal::Str("abcdef".into())]; - assert_eq!(format_string("%.3s", &args), "abc"); + assert_eq!(format_string("%.3s", &args).unwrap(), "abc"); } #[test] fn test_precision_decimal() { let args = vec![TestVal::Int(5)]; - assert_eq!(format_string("%.3d", &args), "005"); + assert_eq!(format_string("%.3d", &args).unwrap(), "005"); } #[test] fn test_precision_overrides_zero_flag() { // POSIX: precision for integers suppresses the 0 flag. let args = vec![TestVal::Int(5)]; - assert_eq!(format_string("%05.3d", &args), " 005"); + assert_eq!(format_string("%05.3d", &args).unwrap(), " 005"); } #[test] fn test_u_conversion() { let args = vec![TestVal::Int(5)]; - assert_eq!(format_string("%u", &args), "5"); + assert_eq!(format_string("%u", &args).unwrap(), "5"); } #[test] fn test_big_e_conversion() { let args = vec![TestVal::Float(1000.0)]; - assert_eq!(format_string("%E", &args), "1.000000E+03"); + assert_eq!(format_string("%E", &args).unwrap(), "1.000000E+03"); } #[test] fn test_little_e_conversion() { let args = vec![TestVal::Float(1000.0)]; - assert_eq!(format_string("%e", &args), "1.000000e+03"); + assert_eq!(format_string("%e", &args).unwrap(), "1.000000e+03"); } #[test] fn test_big_g_large() { let args = vec![TestVal::Float(1234567.0)]; - assert_eq!(format_string("%G", &args), "1.23457E+06"); + assert_eq!(format_string("%G", &args).unwrap(), "1.23457E+06"); } #[test] fn test_little_g_large() { let args = vec![TestVal::Float(1234567.0)]; - assert_eq!(format_string("%g", &args), "1.23457e+06"); + assert_eq!(format_string("%g", &args).unwrap(), "1.23457e+06"); } #[test] fn test_b_tab() { let args = vec![TestVal::Str("\\t".into())]; - assert_eq!(format_string("%b", &args), "\t"); + assert_eq!(format_string("%b", &args).unwrap(), "\t"); } #[test] @@ -887,14 +908,14 @@ mod tests { // \0NNN: leading 0 is the first of up to 3 octal digits (≤2 more), like // GNU/bash/dash. \0101 → octal 010 (BS) + literal '1', NOT 'A'. let args: Vec = vec![]; - assert_eq!(format_string("\\0101", &args), "\u{8}1"); - assert_eq!(format_string("\\012", &args), "\n"); + assert_eq!(format_string("\\0101", &args).unwrap(), "\u{8}1"); + assert_eq!(format_string("\\012", &args).unwrap(), "\n"); } #[test] fn test_octal_escape_no_zero_prefix() { // \101 in format → 'A' let args: Vec = vec![]; - assert_eq!(format_string("\\101", &args), "A"); + assert_eq!(format_string("\\101", &args).unwrap(), "A"); } } diff --git a/crates/kaish-kernel/src/tools/builtin/printf.rs b/crates/kaish-kernel/src/tools/builtin/printf.rs index 2fbe3f2d..d1ed6131 100644 --- a/crates/kaish-kernel/src/tools/builtin/printf.rs +++ b/crates/kaish-kernel/src/tools/builtin/printf.rs @@ -18,7 +18,9 @@ struct PrintfArgs { #[command(flatten)] global: GlobalFlags, - /// Format string followed by arguments substituted into it. + /// Format string followed by arguments substituted into it. A numeric + /// conversion (%d, %f, %x, %o, %c) needs a number: `0xff` and `007` are + /// errors naming the spelling that works, and a missing argument is 0. format_args: Vec, } @@ -37,34 +39,143 @@ impl FormatArg for Value { } } - fn as_format_int(&self) -> i64 { + fn as_format_int(&self) -> Result { match self { - Value::Int(i) => *i, - Value::Float(f) => *f as i64, - Value::String(s) => s.parse().unwrap_or(0), - Value::Bool(b) => i64::from(*b), - _ => 0, + Value::Int(i) => Ok(*i), + Value::Bool(b) => Ok(i64::from(*b)), + Value::Float(f) => float_as_int(*f, &f.to_string()), + Value::String(s) => match string_as_number(s)? { + Number::Int(i) => Ok(i), + Number::Float(f) => float_as_int(f, s), + }, + other => Err(not_a_number(other)), } } - fn as_format_float(&self) -> f64 { + fn as_format_float(&self) -> Result { match self { - Value::Float(f) => *f, - Value::Int(i) => *i as f64, - Value::String(s) => s.parse().unwrap_or(0.0), - _ => 0.0, + Value::Float(f) => Ok(*f), + Value::Int(i) => Ok(*i as f64), + Value::Bool(b) => Ok(f64::from(u8::from(*b))), + Value::String(s) => match string_as_number(s)? { + Number::Int(i) => Ok(i as f64), + Number::Float(f) => Ok(f), + }, + other => Err(not_a_number(other)), } } - fn as_format_char(&self) -> Option { + fn as_format_char(&self) -> Result, String> { match self { - Value::String(s) => s.chars().next(), - Value::Int(i) => char::from_u32(*i as u32), - _ => None, + Value::String(s) => Ok(s.chars().next()), + Value::Int(i) => { + let code = u32::try_from(*i) + .ok() + .and_then(char::from_u32) + .ok_or_else(|| format!("`{i}` is not a character code"))?; + Ok(Some(code)) + } + Value::Null => Ok(None), + other => Err(not_a_number(other)), } } } +/// A number printf read out of an operand. +enum Number { + Int(i64), + Float(f64), +} + +/// Read an operand's text as a number, or say why it is not one. +/// +/// The rule is JSON's, the same one `fromjson` reads, so `1e3` is a number +/// while `0xff` and `007` are not. Each refusal names the spelling that +/// works, because a model that reads `$(( 0xff ))` gets it right next turn. +fn string_as_number(s: &str) -> Result { + // Checked before the JSON parse: `"007".parse::()` succeeds and + // would answer 7 for text kaish reads as text everywhere else. + if crate::lexer::is_leading_zero_numeral(s) { + let decimal = crate::arithmetic::leading_zero_decimal(s) + .unwrap_or_else(|| "0".to_string()); + let sign = if s.starts_with('-') { "-" } else { "" }; + let digits = s.trim_start_matches('-').trim_start_matches('0'); + let digits = if digits.is_empty() { "0" } else { digits }; + return Err(format!( + "`{s}` has a leading zero — kaish reads no octal; \ + write `{sign}8#{digits}` for octal or `{decimal}` for decimal" + )); + } + if let Ok(n) = s.parse::() { + return Ok(Number::Int(n)); + } + match serde_json::from_str::(s) { + Ok(n) => match n.as_i64() { + Some(i) => Ok(Number::Int(i)), + // A JSON number too wide or too precise for i64 — `1e3` lands + // here, and so does `1e19`, which `float_as_int` then refuses. + None => n + .as_f64() + .map(Number::Float) + .ok_or_else(|| format!("`{s}` is outside the 64-bit range")), + }, + Err(_) => Err(base_aware_refusal(s)), + } +} + +/// Name the fix for text that is shaped like a number in another base. +/// +/// `$(( ))` is where kaish reads a base, so that is what the message points +/// at rather than leaving the reader to guess. +fn base_aware_refusal(s: &str) -> String { + let unsigned = s.strip_prefix('-').unwrap_or(s); + let based = unsigned.strip_prefix("0x").or_else(|| unsigned.strip_prefix("0X")); + if let Some(digits) = based + && let Ok(v) = i64::from_str_radix(digits, 16) + { + let sign = if s.starts_with('-') { "-" } else { "" }; + return format!( + "`{s}` is not a number; write `{sign}{v}`, or `$(( {s} ))` to read the base" + ); + } + if unsigned.contains('#') { + return format!("`{s}` is not a number; `$(( {s} ))` reads a based numeral"); + } + format!("`{s}` is not a number") +} + +/// Convert a float to an integer, or say why it will not convert. +fn float_as_int(f: f64, text: &str) -> Result { + if !f.is_finite() { + return Err(format!("`{text}` is not a finite number")); + } + if f.fract() != 0.0 { + return Err(format!( + "`{text}` is not a whole number; an integer conversion needs one" + )); + } + // The bounds are compared in f64 because `i64::MAX as f64` rounds up: + // testing `f <= i64::MAX as f64` would admit 2^63 itself. + if f < -(2f64.powi(63)) || f >= 2f64.powi(63) { + return Err(format!("`{text}` is outside the 64-bit range")); + } + Ok(f as i64) +} + +/// Name a value that has no numeric reading at all. +fn not_a_number(value: &Value) -> String { + let kind = match value { + Value::Null => "null", + Value::Json(serde_json::Value::Array(_)) => "a list", + Value::Json(serde_json::Value::Object(_)) => "a record", + Value::Json(_) => "a JSON value", + Value::Bytes(_) => "binary data", + // The scalar arms are handled by the callers above. + _ => "this value", + }; + format!("{kind} is not a number") +} + #[async_trait] impl Tool for Printf { fn name(&self) -> &str { @@ -127,7 +238,13 @@ impl Tool for Printf { } } // POSIX printf reuses the format until all operands are consumed. - let output = format_string::format_string_cycling(&format, &format_args); + // A numeric conversion that cannot read its operand refuses here + // rather than printing 0 — nothing partial is emitted, so a caller + // never reads half a line as a whole answer. + let output = match format_string::format_string_cycling(&format, &format_args) { + Ok(text) => text, + Err(e) => return ExecResult::failure(1, format!("printf: {e}")), + }; ExecResult::with_output(OutputData::text(output)) } @@ -136,9 +253,9 @@ impl Tool for Printf { /// FormatArg impl for references (used by printf which collects &Value) impl FormatArg for &Value { fn as_format_string(&self) -> String { (*self).as_format_string() } - fn as_format_int(&self) -> i64 { (*self).as_format_int() } - fn as_format_float(&self) -> f64 { (*self).as_format_float() } - fn as_format_char(&self) -> Option { (*self).as_format_char() } + fn as_format_int(&self) -> Result { (*self).as_format_int() } + fn as_format_float(&self) -> Result { (*self).as_format_float() } + fn as_format_char(&self) -> Result, String> { (*self).as_format_char() } } #[cfg(test)] diff --git a/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs b/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs new file mode 100644 index 00000000..132aeee0 --- /dev/null +++ b/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs @@ -0,0 +1,167 @@ +//! Kernel-routed tests for `printf`'s numeric conversions. +//! +//! `printf '%d' 0xff` printing `0` is the silent-fallback shape AGENTS.md +//! names as the one to refuse. These pin the refusals and, just as +//! importantly, the values that must keep converting. +//! +//! `awk` shares the format engine but not the rule: in POSIX awk a +//! non-numeric string IS 0, so the awk tests below are the control that the +//! refusal did not leak across the trait. + +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use kaish_kernel::{Kernel, KernelConfig}; + +fn kernel() -> Kernel { + Kernel::new(KernelConfig::transient().with_skip_validation(true)).expect("kernel") +} + +/// Run a script, expect a refusal, and hand back the diagnostic. +async fn refused(script: &str) -> String { + let k = kernel(); + let r = k.execute(script).await.expect("execute"); + assert_ne!(r.code, 0, "{script:?} must refuse, got out={:?}", r.text_out()); + assert!( + r.text_out().is_empty(), + "{script:?} must print nothing when it refuses, got {:?}", + r.text_out() + ); + r.err +} + +/// Run a script that must succeed, and hand back stdout. +async fn ok(script: &str) -> String { + let k = kernel(); + let r = k.execute(script).await.expect("execute"); + assert_eq!(r.code, 0, "{script:?} must succeed: {}", r.err); + r.text_out().to_string() +} + +// --------------------------------------------------------------------------- +// Refusals: a value that is not a number does not become one +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn hex_spelling_names_the_base_reader_and_the_decimal() { + // The case AGENTS.md names. `0xff` is a string here: JSON has no hex. + let msg = refused("printf '%d' 0xff").await; + assert!(msg.contains("0xff"), "must quote the value: {msg}"); + assert!(msg.contains("$(( 0xff ))"), "must name the base reader: {msg}"); + assert!(msg.contains("255"), "must name the decimal: {msg}"); +} + +#[tokio::test] +async fn a_leading_zero_names_octal_and_decimal() { + // 0.17.0's rule: where kaish needs a number, a leading zero is an error. + // This is the case that used to answer plausibly (7), not visibly (0). + let msg = refused("printf '%d' 007").await; + assert!(msg.contains("007"), "must quote the value: {msg}"); + assert!(msg.contains("8#7"), "must name the octal spelling: {msg}"); + assert!(msg.contains("`7`"), "must name the decimal spelling: {msg}"); +} + +#[tokio::test] +async fn a_negative_leading_zero_keeps_its_sign_in_the_fix() { + // The sign must survive into both suggestions, or the fix changes the + // value — the same rule the arithmetic refusal follows. + let msg = refused("printf '%d' -- -007").await; + assert!(msg.contains("-8#7"), "octal fix must keep the sign: {msg}"); + assert!(msg.contains("`-7`"), "decimal fix must keep the sign: {msg}"); +} + +#[tokio::test] +async fn a_non_numeric_string_is_refused_not_zeroed() { + let msg = refused("printf '%d' abc").await; + assert!(msg.contains("abc"), "must quote the value: {msg}"); +} + +#[tokio::test] +async fn a_fractional_value_is_refused_for_an_integer_conversion() { + let msg = refused("printf '%d' 1.5").await; + assert!(msg.contains("1.5"), "must quote the value: {msg}"); +} + +#[tokio::test] +async fn a_float_past_the_integer_range_names_the_limit() { + // `*f as i64` saturated silently. 1e19 is past i64::MAX. + let msg = refused("printf '%d' 1e19").await; + assert!(msg.contains("64-bit"), "must name the limit: {msg}"); +} + +#[tokio::test] +async fn a_non_numeric_string_is_refused_for_a_float_conversion_too() { + let msg = refused("printf '%f' abc").await; + assert!(msg.contains("abc"), "must quote the value: {msg}"); +} + +#[tokio::test] +async fn a_character_conversion_past_the_range_is_refused() { + // `*i as u32` truncated a wide integer into some other character. + let msg = refused("printf '%c' 4294967296").await; + assert!(msg.contains("4294967296"), "must quote the value: {msg}"); +} + +// --------------------------------------------------------------------------- +// Controls: what must keep working. A refusal that swallows these is worse +// than the bug it replaced. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn plain_integers_still_convert() { + assert_eq!(ok("printf '%d' 42").await, "42"); + assert_eq!(ok("printf '%d' -- -5").await, "-5"); + assert_eq!(ok("printf '%d' 0").await, "0"); +} + +#[tokio::test] +async fn a_missing_operand_is_still_zero() { + // POSIX: an absent operand converts as 0, and that is NOT the silent + // fallback being removed here. Only a value that IS present and is not + // a number now refuses. + assert_eq!(ok("printf '%d'").await, "0"); + assert_eq!(ok("printf '%d-%d' 7").await, "7-0"); +} + +#[tokio::test] +async fn exponent_notation_is_a_json_number_and_converts() { + // `fromjson` reads 1e3; printf reads what fromjson reads. + assert_eq!(ok("printf '%d' 1e3").await, "1000"); +} + +#[tokio::test] +async fn a_boolean_still_converts() { + assert_eq!(ok("printf '%d' true").await, "1"); + assert_eq!(ok("printf '%d' false").await, "0"); +} + +#[tokio::test] +async fn float_conversions_are_unaffected() { + assert_eq!(ok("printf '%.1f' 1.5").await, "1.5"); + assert_eq!(ok("printf '%.1f' 2").await, "2.0"); +} + +#[tokio::test] +async fn a_string_conversion_takes_any_text() { + // Only the NUMERIC conversions gained a rule. %s still prints anything. + assert_eq!(ok("printf '%s' 0xff").await, "0xff"); + assert_eq!(ok("printf '%s' 007").await, "007"); + assert_eq!(ok("printf '%s' abc").await, "abc"); +} + +// --------------------------------------------------------------------------- +// The awk control: same format engine, different and correct rule +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn awk_still_coerces_a_non_numeric_string_to_zero() { + // POSIX awk: a non-numeric string is 0. This is awk being right, not + // awk being unfixed — if this test starts failing, the printf refusal + // leaked through `FormatArg` into a language that does not want it. + assert_eq!(ok(r#"awk 'BEGIN { printf "%d\n", "abc" }'"#).await, "0\n"); +} + +#[tokio::test] +async fn awk_still_reads_a_leading_zero_as_decimal() { + // awk has no octal rule for this either; 007 is the number 7. + assert_eq!(ok(r#"awk 'BEGIN { printf "%d\n", "007" }'"#).await, "7\n"); +} diff --git a/docs/LANGUAGE.md b/docs/LANGUAGE.md index f46898d1..a559403f 100644 --- a/docs/LANGUAGE.md +++ b/docs/LANGUAGE.md @@ -64,6 +64,13 @@ instead: `$((8#10))` is 8 and `$((10#$x))` reads text with a leading zero as decimal (see "Arithmetic"); `printf "%o"` and `printf "%x"` format the other way. +`printf` reads a number by the same rules. `printf '%d' 255` and `printf '%d' +1e3` print `255` and `1000`; `printf '%d' 0xff` and `printf '%d' 007` are +errors naming the spelling that works, because a conversion that cannot read +its operand refuses instead of printing `0`. An operand that is missing +entirely still converts as `0`, which POSIX requires: `printf '%d'` prints +`0`. `%s` takes any text. + 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 From 0e9ceff320f6b463dbfbc1c692c05fcdd8c9c025 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Mon, 31 Aug 2026 17:58:14 -0400 Subject: [PATCH 2/3] An empty operand refuses in its own words MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `printf '%d' "$x"` with `x` unset is the common way an empty operand reaches a number position, and the general refusal answered "`` is not a number" — it quoted nothing, so it named nothing the reader could act on. Refusing is the right answer and is not in question here: the arithmetic rewrite already made an unset or empty operand an error rather than 0, and printf reading it as 0 would put the two number positions back into disagreement. Only the wording changes. Also pins the two integer bounds, which the range guard's comparison against 2^63 exists to get right: `i64::MIN` is representable in f64 and must not be refused, and `i64::MAX` must survive the i64 parse path rather than being rounded up into the refusal. Co-Authored-By: Claude Opus 5 --- .../kaish-kernel/src/tools/builtin/printf.rs | 7 ++++++ .../tests/printf_numeric_refusal_tests.rs | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/crates/kaish-kernel/src/tools/builtin/printf.rs b/crates/kaish-kernel/src/tools/builtin/printf.rs index d1ed6131..922014f6 100644 --- a/crates/kaish-kernel/src/tools/builtin/printf.rs +++ b/crates/kaish-kernel/src/tools/builtin/printf.rs @@ -93,6 +93,13 @@ enum Number { /// while `0xff` and `007` are not. Each refusal names the spelling that /// works, because a model that reads `$(( 0xff ))` gets it right next turn. fn string_as_number(s: &str) -> Result { + // An empty operand is the common shape of an unset variable reaching a + // number position. It refuses like any other non-number — the same call + // arithmetic makes — but says so in its own words, because `` is not a + // number`` names nothing the reader can act on. + if s.is_empty() { + return Err("an empty operand is not a number".to_string()); + } // Checked before the JSON parse: `"007".parse::()` succeeds and // would answer 7 for text kaish reads as text everywhere else. if crate::lexer::is_leading_zero_numeral(s) { diff --git a/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs b/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs index 132aeee0..5e78b5c6 100644 --- a/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs +++ b/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs @@ -94,6 +94,29 @@ async fn a_non_numeric_string_is_refused_for_a_float_conversion_too() { assert!(msg.contains("abc"), "must quote the value: {msg}"); } +#[tokio::test] +async fn an_empty_operand_is_refused_in_its_own_words() { + // The common shape of an unset variable reaching a number position. + // Arithmetic already calls an empty operand an error; printf agrees, + // and says which operand problem it is rather than quoting nothing. + let msg = refused(r#"printf '%d' """#).await; + assert!(msg.contains("empty"), "must name the emptiness: {msg}"); +} + +#[tokio::test] +async fn an_empty_variable_is_refused_the_same_way() { + let msg = refused(r#"x=""; printf '%d' "$x""#).await; + assert!(msg.contains("empty"), "must name the emptiness: {msg}"); +} + +#[tokio::test] +async fn the_integer_bounds_convert_exactly() { + // i64::MIN is representable in f64 and must not be refused by the + // range guard; i64::MAX must survive the i64 parse path. + assert_eq!(ok("printf '%d' -- -9223372036854775808").await, "-9223372036854775808"); + assert_eq!(ok("printf '%d' 9223372036854775807").await, "9223372036854775807"); +} + #[tokio::test] async fn a_character_conversion_past_the_range_is_refused() { // `*i as u32` truncated a wide integer into some other character. From af6769d0cb4e2260854d42da87663c3f64882ca9 Mon Sep 17 00:00:00 2001 From: A Tobey Date: Mon, 31 Aug 2026 18:14:46 -0400 Subject: [PATCH 3/3] An integer-shaped overflow refuses before a float can round it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found the refusal had left one silent answer standing, at the one boundary where rounding hides it. `printf '%d' '-9223372036854775809'` printed `-9223372036854775808` and exited 0: the operand is not an i64, so it fell through to the JSON reader, which has no arbitrary precision and read it as f64, where it rounds to exactly `i64::MIN`. The range guard then accepted it, correctly, because `i64::MIN` IS in range. Every step was locally right and the answer was wrong by one, which is the shape this whole conversion exists to refuse. The kernel already had the guard: `value_to_num` refuses an integer-shaped string that failed its i64 parse before any float sees it, because overflow is the only way that parse can fail for that shape. printf now makes the same call through the same predicate, so `is_i64_overflow_shape` gains a third caller and the three number positions name the 64-bit limit with one sentence. The positive side already refused, but by a longer route; it now gives the canonical message too. The same review found four messages that named the wrong fix or none. A leading `+` reached neither the leading-zero nor the overflow check, so `+007` answered 7 where `007` refuses — the sign is now split off first, with `-` carried into every suggestion and `+` dropped, since `+7` and `7` are the same number but `-7` is not. `007.5` was offered `8#7.5`, which is not a numeral in any base; a fractional value now gets only its decimal spelling. `0b101` and `0o17` said only "is not a number" where the arithmetic lexer names `2#101` and `8#17`, and now say the same thing it does. `1e999` said "is not a number" when the problem is magnitude; it names the range, while the word `inf` still does not, having no numeral in it. Tests: the two bounds cases were passing through the typed-integer arm rather than the reader under test, and every refusal case had an empty buffer at the moment it refused, so none of them could have caught a partial write. Both are pinned properly now — a literal and a good operand before the bad one, and a refusal in a second cycling pass — along with the negative boundary, the `+` spellings, the base spellings, and a missing-operand default for every numeric conversion rather than `%d` alone. Gates: clippy -D warnings, cargo test --all (204 suites, 6514 passed), insta --check, no-default-features, wasi build, rustdoc -D warnings. Co-Authored-By: Claude Opus 5 --- crates/kaish-kernel/src/interpreter.rs | 2 +- crates/kaish-kernel/src/interpreter/eval.rs | 7 +- .../kaish-kernel/src/tools/builtin/printf.rs | 114 +++++++++++++----- .../tests/printf_numeric_refusal_tests.rs | 109 +++++++++++++++++ 4 files changed, 201 insertions(+), 31 deletions(-) diff --git a/crates/kaish-kernel/src/interpreter.rs b/crates/kaish-kernel/src/interpreter.rs index 78051726..da4b5d13 100644 --- a/crates/kaish-kernel/src/interpreter.rs +++ b/crates/kaish-kernel/src/interpreter.rs @@ -44,4 +44,4 @@ pub use result::{apply_output_format, hex_dump, json_to_value, json_to_value_no_ pub use scope::{PathError, Scope}; // Crate-internal: the reduced sync evaluator (scheduler/pipeline.rs) reuses the // resolver error-message shape without widening the public API. -pub(crate) use eval::format_path; +pub(crate) use eval::{format_path, is_i64_overflow_shape}; diff --git a/crates/kaish-kernel/src/interpreter/eval.rs b/crates/kaish-kernel/src/interpreter/eval.rs index 9f7d4324..24e5a27b 100644 --- a/crates/kaish-kernel/src/interpreter/eval.rs +++ b/crates/kaish-kernel/src/interpreter/eval.rs @@ -641,9 +641,10 @@ 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 { +/// parse can fail exclusively by overflow. Shared by `value_to_exit_code`, +/// `value_to_num` and printf's operand reader so all three name the same +/// 64-bit limit the same way. +pub(crate) 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()) } diff --git a/crates/kaish-kernel/src/tools/builtin/printf.rs b/crates/kaish-kernel/src/tools/builtin/printf.rs index 922014f6..a4238e64 100644 --- a/crates/kaish-kernel/src/tools/builtin/printf.rs +++ b/crates/kaish-kernel/src/tools/builtin/printf.rs @@ -100,52 +100,112 @@ fn string_as_number(s: &str) -> Result { if s.is_empty() { return Err("an empty operand is not a number".to_string()); } - // Checked before the JSON parse: `"007".parse::()` succeeds and - // would answer 7 for text kaish reads as text everywhere else. - if crate::lexer::is_leading_zero_numeral(s) { - let decimal = crate::arithmetic::leading_zero_decimal(s) - .unwrap_or_else(|| "0".to_string()); - let sign = if s.starts_with('-') { "-" } else { "" }; - let digits = s.trim_start_matches('-').trim_start_matches('0'); - let digits = if digits.is_empty() { "0" } else { digits }; - return Err(format!( - "`{s}` has a leading zero — kaish reads no octal; \ - write `{sign}8#{digits}` for octal or `{decimal}` for decimal" - )); + + // printf's operand grammar takes an explicit sign, so split it off before + // the shape checks: `is_leading_zero_numeral` and `is_i64_overflow_shape` + // both know `-` and neither knows `+`, and a `+007` that slipped past the + // leading-zero rule would answer 7 where `007` refuses. A `+` is dropped + // from the suggestions because `+7` and `7` are the same number; a `-` is + // carried into every one of them, because dropping it changes the value. + let (sign, magnitude) = match s.strip_prefix('+') { + Some(rest) => ("", rest), + None => match s.strip_prefix('-') { + Some(rest) => ("-", rest), + None => ("", s), + }, + }; + if magnitude.is_empty() { + return Err(format!("`{s}` is not a number")); + } + + // Checked before any parse: `"007".parse::()` succeeds and would + // answer 7 for text kaish reads as text everywhere else. + if crate::lexer::is_leading_zero_numeral(magnitude) { + return Err(leading_zero_refusal(s, sign, magnitude)); } if let Ok(n) = s.parse::() { return Ok(Number::Int(n)); } + // An integer-shaped operand can only have failed that parse by + // overflowing. It must refuse HERE: `serde_json` would read it as f64, + // and `-9223372036854775809` rounds to exactly `i64::MIN`, which the + // range guard then accepts — a silent wrong answer, the very shape this + // whole conversion exists to refuse. `value_to_num` guards the same way. + if crate::interpreter::is_i64_overflow_shape(magnitude) { + return Err(format!("`{s}` {}", crate::lexer::INTEGER_OUT_OF_RANGE)); + } + + // A JSON number is the rule, the same one `fromjson` reads. match serde_json::from_str::(s) { Ok(n) => match n.as_i64() { Some(i) => Ok(Number::Int(i)), - // A JSON number too wide or too precise for i64 — `1e3` lands - // here, and so does `1e19`, which `float_as_int` then refuses. - None => n - .as_f64() - .map(Number::Float) - .ok_or_else(|| format!("`{s}` is outside the 64-bit range")), + // A float spelling: `1e3` lands here, and so does `1e19`, which + // `float_as_int` then refuses. The integer-too-wide case cannot + // reach this arm — the overflow-shape guard above took it. + None => match n.as_f64() { + Some(f) => Ok(Number::Float(f)), + None => Err(format!("`{s}` is outside the 64-bit range")), + }, }, - Err(_) => Err(base_aware_refusal(s)), + // serde_json refuses a magnitude that overflows f64 (`1e999`). That + // is a range problem, not unreadable text, so it says so — but only + // for something actually shaped like a number, or the word `inf` + // would borrow the message. + Err(_) => { + if magnitude.starts_with(|c: char| c.is_ascii_digit()) + && s.parse::().is_ok_and(|f| !f.is_finite()) + { + return Err(format!("`{s}` is outside the 64-bit range")); + } + Err(base_aware_refusal(s, sign, magnitude)) + } + } +} + +/// Name the octal and decimal spellings for a numeral with a leading zero. +fn leading_zero_refusal(s: &str, sign: &str, magnitude: &str) -> String { + let trimmed = magnitude.trim_start_matches('0'); + let decimal = if trimmed.is_empty() { "0" } else { trimmed }; + // `8#7.5` is not a numeral in any base, so a fractional value is offered + // only its decimal spelling. Octal is a whole-number question. + if magnitude.contains('.') { + return format!( + "`{s}` has a leading zero — kaish reads no octal; write `{sign}{decimal}`" + ); } + format!( + "`{s}` has a leading zero — kaish reads no octal; \ + write `{sign}8#{decimal}` for octal or `{sign}{decimal}` for decimal" + ) } -/// Name the fix for text that is shaped like a number in another base. +/// Name the fix for text shaped like a number in another base. /// /// `$(( ))` is where kaish reads a base, so that is what the message points -/// at rather than leaving the reader to guess. -fn base_aware_refusal(s: &str) -> String { - let unsigned = s.strip_prefix('-').unwrap_or(s); - let based = unsigned.strip_prefix("0x").or_else(|| unsigned.strip_prefix("0X")); - if let Some(digits) = based +/// at rather than leaving the reader to guess. `0b`/`0o` are not kaish +/// spellings at all, so those name `2#`/`8#` the way the arithmetic lexer +/// does rather than pointing at a `$(( ))` that would refuse them too. +fn base_aware_refusal(s: &str, sign: &str, magnitude: &str) -> String { + if let Some(digits) = magnitude + .strip_prefix("0x") + .or_else(|| magnitude.strip_prefix("0X")) && let Ok(v) = i64::from_str_radix(digits, 16) { - let sign = if s.starts_with('-') { "-" } else { "" }; return format!( "`{s}` is not a number; write `{sign}{v}`, or `$(( {s} ))` to read the base" ); } - if unsigned.contains('#') { + let radix_prefix = magnitude.get(..2).map(str::to_ascii_lowercase); + if let Some(prefix) = radix_prefix.as_deref() + && matches!(prefix, "0b" | "0o") + { + let digits = &magnitude[2..]; + let (base, word) = if prefix == "0b" { (2, "binary") } else { (8, "octal") }; + return format!( + "`{s}` is not a kaish base spelling; write `{sign}{base}#{digits}` for {word}" + ); + } + if magnitude.contains('#') { return format!("`{s}` is not a number; `$(( {s} ))` reads a based numeral"); } format!("`{s}` is not a number") diff --git a/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs b/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs index 5e78b5c6..8dbac474 100644 --- a/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs +++ b/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs @@ -117,6 +117,115 @@ async fn the_integer_bounds_convert_exactly() { assert_eq!(ok("printf '%d' 9223372036854775807").await, "9223372036854775807"); } +#[tokio::test] +async fn a_string_operand_one_past_the_negative_bound_refuses() { + // The regression this guard exists for. `-9223372036854775809` is not + // an i64, and reading it as f64 rounds it to EXACTLY i64::MIN, which + // the range check then accepts — a silent wrong answer wearing a + // plausible face. It must refuse before any float ever sees it. + let msg = refused(r#"printf '%d' '-9223372036854775809'"#).await; + assert!(msg.contains("64-bit"), "must name the limit: {msg}"); + + // The control: one step inside the bound still converts, so the guard + // cannot pass by refusing the whole neighborhood. + assert_eq!( + ok(r#"printf '%d' '-9223372036854775808'"#).await, + "-9223372036854775808" + ); +} + +#[tokio::test] +async fn a_string_operand_past_the_positive_bound_refuses() { + let msg = refused(r#"printf '%d' '9223372036854775808'"#).await; + assert!(msg.contains("64-bit"), "must name the limit: {msg}"); + assert_eq!( + ok(r#"printf '%d' '9223372036854775807'"#).await, + "9223372036854775807" + ); +} + +#[tokio::test] +async fn a_quoted_string_operand_takes_the_json_reading() { + // Unquoted, `1e3` and `42` arrive already typed, so these are the cases + // that actually exercise the string reader. + assert_eq!(ok(r#"printf '%d' '1e3'"#).await, "1000"); + assert_eq!(ok(r#"printf '%d' '42'"#).await, "42"); + assert_eq!(ok(r#"printf '%.1f' '1.5'"#).await, "1.5"); +} + +#[tokio::test] +async fn an_explicit_plus_is_read_and_does_not_dodge_the_rules() { + // `is_leading_zero_numeral` knows `-` and not `+`, so `+007` could slip + // past the leading-zero rule and answer 7. + assert_eq!(ok(r#"printf '%d' '+7'"#).await, "7"); + let msg = refused(r#"printf '%d' '+007'"#).await; + assert!(msg.contains("8#7"), "must still name the octal spelling: {msg}"); +} + +#[tokio::test] +async fn the_non_kaish_base_spellings_name_the_kaish_ones() { + let binary = refused("printf '%d' 0b101").await; + assert!(binary.contains("2#101"), "must name the binary spelling: {binary}"); + let octal = refused("printf '%d' 0o17").await; + assert!(octal.contains("8#17"), "must name the octal spelling: {octal}"); +} + +#[tokio::test] +async fn a_fractional_leading_zero_is_not_offered_an_octal_fix() { + // `8#7.5` is not a numeral in any base — offering it would be advice + // that fails when followed. + let msg = refused(r#"printf '%d' '007.5'"#).await; + assert!(msg.contains("7.5"), "must name the decimal: {msg}"); + assert!(!msg.contains("8#7.5"), "must not invent a fractional octal: {msg}"); +} + +#[tokio::test] +async fn a_magnitude_past_f64_names_the_range_but_a_word_does_not() { + let big = refused(r#"printf '%d' '1e999'"#).await; + assert!(big.contains("64-bit"), "must name the range: {big}"); + // `"inf".parse::()` also yields a non-finite float; it must not + // borrow the range message, because `inf` is not a numeral at all. + let word = refused("printf '%d' inf").await; + assert!(!word.contains("64-bit"), "a word is not a range problem: {word}"); +} + +// --------------------------------------------------------------------------- +// Nothing is printed before a refusal is discovered +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn a_refusal_discards_text_already_formatted_before_it() { + // Every other refusal test has an empty buffer at the moment it + // refuses, so none of them can catch a partial write. These do: the + // literal `x`, and a good operand, are already formatted when the bad + // operand is reached. + let msg = refused("printf 'x%d' abc").await; + assert!(msg.contains("abc"), "{msg}"); + let msg = refused("printf '%d-%d' 1 abc").await; + assert!(msg.contains("abc"), "{msg}"); +} + +#[tokio::test] +async fn a_refusal_in_a_later_cycling_pass_discards_the_earlier_passes() { + // printf reuses the format until the operands run out, so the first + // pass has already written `1` when the second pass refuses. + let msg = refused("printf '%d\\n' 1 abc").await; + assert!(msg.contains("abc"), "{msg}"); +} + +// --------------------------------------------------------------------------- +// Missing operands default for every numeric conversion, not just %d +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn every_numeric_conversion_defaults_a_missing_operand() { + assert_eq!(ok("printf '%x'").await, "0"); + assert_eq!(ok("printf '%o'").await, "0"); + assert_eq!(ok("printf '%.1f'").await, "0.0"); + // %c with nothing to print emits nothing rather than a NUL. + assert_eq!(ok("printf '%c'").await, ""); +} + #[tokio::test] async fn a_character_conversion_past_the_range_is_refused() { // `*i as u32` truncated a wide integer into some other character.