diff --git a/crates/core/src/eval/functions/statistical/max/mod.rs b/crates/core/src/eval/functions/statistical/max/mod.rs index 87e1ad249..2366cd3ca 100644 --- a/crates/core/src/eval/functions/statistical/max/mod.rs +++ b/crates/core/src/eval/functions/statistical/max/mod.rs @@ -13,10 +13,19 @@ use crate::types::{ErrorKind, Value}; /// booleans contribute a number in array context. Captured in Google /// Sheets; the rows land separately, since they fail until this code exists. /// -/// Everything else numberless — blanks, dates, zoned instants — is unprobed -/// and keeps the `#REF!` MAX has always given it. `array_had_content` is set -/// by text and booleans alone, so it exempts exactly what was captured and -/// makes no claim past it. +/// An array of nothing but *blanks* — what `=MAX(A1:A3)` over an untouched +/// column materializes as — is 0 as well. That is captured across seven range +/// shapes, each with a populated control; the shapes, the controls and where +/// the rows live are set out on [`stat_helpers::is_blank_only_array`], which +/// is what decides the case. Deciding it there rather than through +/// `array_had_content` is deliberate: a blank sitting next to something else +/// numberless changes nothing. +/// +/// This change moves the blank-only array and nothing else. `array_had_content` +/// is still set by text and booleans alone, so it exempts exactly what those +/// captures cover and makes no claim past them. +/// +/// [`stat_helpers::is_blank_only_array`]: super::stat_helpers::is_blank_only_array pub fn max_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -81,11 +90,18 @@ pub fn max_fn(args: &[Value]) -> Value { } // An array holding text or booleans answers 0 (`=MAX({"a","b"})` and // `=MAX({TRUE,FALSE})` are both 0). `array_had_content` is set by exactly - // those two variants and nothing else, so every other numberless array — - // blanks, dates, zoned instants — keeps the long-standing #REF!. Those are - // unprobed; the flag exempts what the capture covers and makes no claim - // beyond it. + // those two variants and nothing else, so it exempts what that capture + // covers and makes no claim beyond it. if had_array && !array_had_content && result.is_none() { + // An array of nothing but blanks is a further exemption, and it is + // captured: `=MAX(A1:A3)` over empty cells is 0, the same answer MIN, + // MAXA and MINA give it — see `is_blank_only_array` for every range + // shape that was probed and where the rows live. The check is on the + // arguments as a whole, so a blank mixed with anything else is + // untouched by this rule. + if super::stat_helpers::is_blank_only_array(args) { + return Value::Number(0.0); + } return Value::Error(ErrorKind::Ref); } Value::Number(result.unwrap_or(0.0)) @@ -104,7 +120,9 @@ pub fn max_fn(args: &[Value]) -> Value { /// It is deliberately not a catch-all: every other non-numeric variant, most /// notably `Date`, leaves it alone and so keeps the `#REF!` MAX has always /// answered. Listing the variants rather than falling through also stops a -/// future `Value` kind inheriting content-hood by accident. +/// future `Value` kind inheriting content-hood by accident. An all-blank array +/// no longer ends at `#REF!` either, but it gets there without this flag — see +/// the blank-only check in `max_fn`. fn max_array_into( elems: &[Value], result: &mut Option, diff --git a/crates/core/src/eval/functions/statistical/max/tests/edge.rs b/crates/core/src/eval/functions/statistical/max/tests/edge.rs index 739698faf..abb4abfca 100644 --- a/crates/core/src/eval/functions/statistical/max/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/max/tests/edge.rs @@ -42,19 +42,43 @@ fn max_non_empty_array_without_numbers_returns_zero() { } #[test] -fn max_array_of_only_blanks_is_unchanged_at_ref_error() { - // No captured row covers an all-blank array, so MAX keeps the #REF! it - // has always given. Pinned here because narrowing the rule for text and - // booleans must not disturb this case. +fn max_array_of_only_blanks_is_zero() { + // `=MAX(A1:A3)` over empty cells is 0 in Google Sheets — the same answer + // MIN, MAXA and MINA give it. MAX used to be alone at #REF! here. + // + // That answer is captured across seven range shapes, each with a populated + // control, but **none of those rows are in this repo yet**: they land in a + // separate fixtures-only PR (see `stat_helpers::is_blank_only_array` for + // the shapes and the branch). Read from this repo alone, this test pins + // the behaviour, not the Sheets answer. assert_eq!( max_fn(&[Value::Array(vec![Value::Empty, Value::Empty, Value::Empty])]), - Value::Error(ErrorKind::Ref) + Value::Number(0.0) ); + // Same through the nested-row shape a vertical range materializes as. assert_eq!( max_fn(&[Value::Array(vec![ Value::Array(vec![Value::Empty]), Value::Array(vec![Value::Empty]), ])]), + Value::Number(0.0) + ); +} + +#[test] +fn max_blank_beside_something_else_numberless_is_still_ref_error() { + // The blank-only rule is decided over the arguments as a whole, not by a + // per-element flag, so a blank cannot on its own pull an array that holds + // something else into the blank-only 0. Pinned with a date element because + // that is where MAX leaves such an array today; if that ever moves it has + // to move deliberately, not as fallout from this rule. + assert_eq!( + max_fn(&[Value::Array(vec![Value::Empty, Value::Date(43831.0)])]), + Value::Error(ErrorKind::Ref) + ); + // An empty array argument stays #REF! even alongside an all-blank one. + assert_eq!( + max_fn(&[Value::Array(vec![Value::Empty]), Value::Array(vec![])]), Value::Error(ErrorKind::Ref) ); } diff --git a/crates/core/src/eval/functions/statistical/maxa/mod.rs b/crates/core/src/eval/functions/statistical/maxa/mod.rs index 43d10a279..7d299d715 100644 --- a/crates/core/src/eval/functions/statistical/maxa/mod.rs +++ b/crates/core/src/eval/functions/statistical/maxa/mod.rs @@ -6,6 +6,7 @@ use crate::types::{ErrorKind, Value}; /// - Text in direct args → `#VALUE!`. /// - Empty → skip. /// - Empty array argument → `#REF!`. +/// - Array of nothing but blanks → 0. /// - No args → `#N/A`. pub fn maxa_fn(args: &[Value]) -> Value { if args.is_empty() { @@ -50,6 +51,13 @@ pub fn maxa_fn(args: &[Value]) -> Value { match result { Some(n) => Value::Number(n), None if skipped_sparkline => Value::Number(0.0), + // An array of nothing but blanks is 0, not #N/A: `=MAXA(A1:A3)` over + // empty cells answers the same 0 that MAX, MIN and MINA give it — see + // `is_blank_only_array` for every range shape that was probed, the + // controls that prove the ranges resolved, and where the rows live. A + // blank argument with no array in sight keeps the #N/A below — that + // shape is unprobed. + None if super::stat_helpers::is_blank_only_array(args) => Value::Number(0.0), None => Value::Error(ErrorKind::NA), } } diff --git a/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs b/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs index a1634bcef..f3346f6bc 100644 --- a/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs @@ -53,6 +53,38 @@ fn maxa_empty_array_is_ref_error() { ); } +#[test] +fn maxa_array_of_only_blanks_is_zero() { + // `=MAXA(A1:A3)` over empty cells is 0 in Google Sheets — the same answer + // MAX, MIN and MINA give it. MAXA used to answer #N/A here. + // + // Captured across seven range shapes, each with a populated control, but + // **none of those rows are in this repo yet** — they land in a separate + // fixtures-only PR (see `stat_helpers::is_blank_only_array` for the shapes + // and the branch). Read from this repo alone, this test pins the + // behaviour, not the Sheets answer. + assert_eq!( + maxa_fn(&[Value::Array(vec![Value::Empty, Value::Empty, Value::Empty])]), + Value::Number(0.0) + ); + // Same through the nested-row shape a vertical range materializes as. + assert_eq!( + maxa_fn(&[Value::Array(vec![ + Value::Array(vec![Value::Empty]), + Value::Array(vec![Value::Empty]), + ])]), + Value::Number(0.0) + ); +} + +#[test] +fn maxa_blank_without_an_array_is_still_na() { + // The rule is confined to the range form. A bare blank argument is + // unprobed, so it keeps the #N/A MAXA has always given it. + assert_eq!(maxa_fn(&[Value::Empty]), Value::Error(ErrorKind::NA)); + assert_eq!(maxa_fn(&[Value::Empty, Value::Empty]), Value::Error(ErrorKind::NA)); +} + #[test] fn maxa_text_only_array_is_zero() { // `=MAXA({"a","b"})` is 0 — text counts as zero rather than being diff --git a/crates/core/src/eval/functions/statistical/min/mod.rs b/crates/core/src/eval/functions/statistical/min/mod.rs index 6122d9fab..c29fc6094 100644 --- a/crates/core/src/eval/functions/statistical/min/mod.rs +++ b/crates/core/src/eval/functions/statistical/min/mod.rs @@ -15,8 +15,15 @@ use crate::types::{ErrorKind, Value}; /// lands with the others. /// /// MIN needs no code for the second rule — it already falls through to 0. -/// An array holding only *blanks* is a third case, unprobed, left at the 0 -/// MIN has always given it. +/// An array holding only *blanks* — what `=MIN(A1:A3)` over an untouched +/// column materializes as — is 0 too, and is now captured rather than assumed: +/// seven range shapes, each with a populated control, are laid out on +/// [`stat_helpers::is_blank_only_array`] along with the note that those rows +/// are not in this repo yet. MIN was the one of the four already giving that +/// answer, so it needs no code for this rule either — the predicate is not +/// called from here at all; MAX, MAXA and MINA were brought to it. +/// +/// [`stat_helpers::is_blank_only_array`]: super::stat_helpers::is_blank_only_array pub fn min_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); diff --git a/crates/core/src/eval/functions/statistical/min/tests/edge.rs b/crates/core/src/eval/functions/statistical/min/tests/edge.rs index 9852387be..53eedeea4 100644 --- a/crates/core/src/eval/functions/statistical/min/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/min/tests/edge.rs @@ -56,10 +56,15 @@ fn min_non_empty_array_without_numbers_still_returns_zero() { } #[test] -fn min_array_of_only_blanks_is_unchanged_at_zero() { - // No captured row covers an all-blank array, so MIN keeps the 0 it has - // always given. Pinned here so a future change to it has to be deliberate - // rather than a side effect of the empty-array rule above. +fn min_array_of_only_blanks_is_zero() { + // `=MIN(A1:A3)` over empty cells is 0 in Google Sheets. MIN was already + // there; MAX, MAXA and MINA were brought to the same answer. + // + // Captured across seven range shapes, each with a populated control, but + // **none of those rows are in this repo yet** — they land in a separate + // fixtures-only PR (see `stat_helpers::is_blank_only_array` for the shapes + // and the branch). Read from this repo alone, this test pins the + // behaviour, not the Sheets answer. assert_eq!( min_fn(&[Value::Array(vec![Value::Empty, Value::Empty, Value::Empty])]), Value::Number(0.0) diff --git a/crates/core/src/eval/functions/statistical/mina/mod.rs b/crates/core/src/eval/functions/statistical/mina/mod.rs index 63449e413..594cea4cb 100644 --- a/crates/core/src/eval/functions/statistical/mina/mod.rs +++ b/crates/core/src/eval/functions/statistical/mina/mod.rs @@ -6,6 +6,7 @@ use crate::types::{ErrorKind, Value}; /// - Text in direct args → `#VALUE!`. /// - Empty → skip. /// - Empty array argument → `#REF!`. +/// - Array of nothing but blanks → 0. /// - No args → `#N/A`. pub fn mina_fn(args: &[Value]) -> Value { if args.is_empty() { @@ -50,6 +51,13 @@ pub fn mina_fn(args: &[Value]) -> Value { match result { Some(n) => Value::Number(n), None if skipped_sparkline => Value::Number(0.0), + // An array of nothing but blanks is 0, not #N/A: `=MINA(A1:A3)` over + // empty cells answers the same 0 that MIN, MAX and MAXA give it — see + // `is_blank_only_array` for every range shape that was probed, the + // controls that prove the ranges resolved, and where the rows live. A + // blank argument with no array in sight keeps the #N/A below — that + // shape is unprobed. + None if super::stat_helpers::is_blank_only_array(args) => Value::Number(0.0), None => Value::Error(ErrorKind::NA), } } diff --git a/crates/core/src/eval/functions/statistical/mina/tests/edge.rs b/crates/core/src/eval/functions/statistical/mina/tests/edge.rs index 09b256883..aeb5d8d0a 100644 --- a/crates/core/src/eval/functions/statistical/mina/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/mina/tests/edge.rs @@ -53,6 +53,38 @@ fn mina_empty_array_is_ref_error() { ); } +#[test] +fn mina_array_of_only_blanks_is_zero() { + // `=MINA(A1:A3)` over empty cells is 0 in Google Sheets — the same answer + // MAX, MIN and MAXA give it. MINA used to answer #N/A here. + // + // Captured across seven range shapes, each with a populated control, but + // **none of those rows are in this repo yet** — they land in a separate + // fixtures-only PR (see `stat_helpers::is_blank_only_array` for the shapes + // and the branch). Read from this repo alone, this test pins the + // behaviour, not the Sheets answer. + assert_eq!( + mina_fn(&[Value::Array(vec![Value::Empty, Value::Empty, Value::Empty])]), + Value::Number(0.0) + ); + // Same through the nested-row shape a vertical range materializes as. + assert_eq!( + mina_fn(&[Value::Array(vec![ + Value::Array(vec![Value::Empty]), + Value::Array(vec![Value::Empty]), + ])]), + Value::Number(0.0) + ); +} + +#[test] +fn mina_blank_without_an_array_is_still_na() { + // The rule is confined to the range form. A bare blank argument is + // unprobed, so it keeps the #N/A MINA has always given it. + assert_eq!(mina_fn(&[Value::Empty]), Value::Error(ErrorKind::NA)); + assert_eq!(mina_fn(&[Value::Empty, Value::Empty]), Value::Error(ErrorKind::NA)); +} + #[test] fn mina_text_only_array_is_zero() { // `=MINA({"a","b"})` is 0 — text counts as zero rather than being diff --git a/crates/core/src/eval/functions/statistical/stat_helpers.rs b/crates/core/src/eval/functions/statistical/stat_helpers.rs index c8d3040e4..bbb5e136d 100644 --- a/crates/core/src/eval/functions/statistical/stat_helpers.rs +++ b/crates/core/src/eval/functions/statistical/stat_helpers.rs @@ -67,6 +67,78 @@ pub fn zoned_extreme(args: &[Value], want_min: bool) -> Option { best.map(|z| Value::Zoned(Box::new(z))) } +/// True when every argument is blank *and* at least one of them arrived as an +/// array — the shape a range over empty cells materializes as, which is what +/// `=MAX(A1:A3)` over an untouched column evaluates. Google Sheets answers the +/// number 0 here for `MAX`, `MIN`, `MAXA` and `MINA` alike. +/// +/// # What was captured +/// +/// The rule is broader than any one range shape, so more than one shape was +/// probed. All four functions were run over a sheet of empty cells in each of +/// these arrangements, and every one answered 0: +/// +/// - `=FN(Data!A1:A1)` — a single-cell range +/// - `=FN(Data!A1:A3)` — a single column +/// - `=FN(Data!A1:B1)` — a single row across columns +/// - `=FN(Data!A1:B2)` — two-dimensional +/// - `=FN(Data!A1:A100)` — reaching far past the used area +/// - `=FN(Data!A1,Data!A1:A3)` — a blank scalar, then a blank range +/// - `=FN(Data!A1:A3,Data!A1)` — the same two reversed +/// +/// Each shape carried a populated control — the identical formula with one +/// cell holding `7` — and every control returned `7`, so the range really did +/// resolve rather than silently failing to. Blankness was asserted rather than +/// assumed (`=COUNTA(Data!A1:B100)` → 0, `=COUNTBLANK(Data!A1:A100)` → 100), +/// and the answer was read back through a real cell (`=Data!R1` → `0`, type +/// **number**), so it is a plain zero and not a date-formatted one. +/// +/// # What is not in this repo +/// +/// None of those rows are here yet. They live on the conformance-fixtures +/// pipeline branch `feat/stat-range-probe` and land in a separate +/// fixtures-only PR, both because three of the four fail until this code +/// exists and because CI rejects a PR that mixes fixture TSVs with code. +/// Nothing under `tests/fixtures/google_sheets/` covers a blank-only array +/// today — the nearest rows are `=MAX({})` → `#REF!` and the sparkline +/// `Data!K1:K1` rows, and neither is this case. A reviewer working from this +/// repo alone can check the unit tests and this predicate; the Sheets answer +/// itself has to be taken from that branch. +/// +/// Requiring an array confines the rule to that range form. A bare blank +/// argument with no array in sight (`=MAXA(A1)` on an empty cell) is unprobed +/// and is left exactly where it was. +/// +/// Every `Value` variant is spelled out rather than swept up by a catch-all, +/// so nothing else — `Date` most of all — can reach the blank-only answer, and +/// a variant added later is a compile error here rather than a silent 0. +pub fn is_blank_only_array(args: &[Value]) -> bool { + fn walk(v: &Value, saw_array: &mut bool) -> bool { + match v { + Value::Empty => true, + Value::Array(elems) => { + *saw_array = true; + // An *empty* array argument is #REF! at every call site before + // this runs. Reporting it as not-blank keeps it that way even + // if it ever reaches here nested inside another array. + !elems.is_empty() && elems.iter().all(|e| walk(e, saw_array)) + } + Value::Number(_) + | Value::Text(_) + | Value::Bool(_) + | Value::Date(_) + | Value::Zoned(_) + | Value::Sparkline(_) + | Value::Error(_) + | Value::ErrorMsg(_, _) => false, + } + } + + let mut saw_array = false; + let all_blank = args.iter().all(|a| walk(a, &mut saw_array)); + all_blank && saw_array +} + /// Collect numeric values from args, flattening arrays. /// Numbers and Dates are included. Bool/Text/Empty are ignored. /// Used for range/array contexts where GS skips non-numerics.