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/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/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..a4238e64 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,210 @@ 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 { + // 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()); + } + + // 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 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")), + }, + }, + // 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 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. `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) + { + return format!( + "`{s}` is not a number; write `{sign}{v}`, or `$(( {s} ))` to read the base" + ); + } + 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") +} + +/// 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 +305,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 +320,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..8dbac474 --- /dev/null +++ b/crates/kaish-kernel/tests/printf_numeric_refusal_tests.rs @@ -0,0 +1,299 @@ +//! 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 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_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. + 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