diff --git a/crates/functions/src/criteria.rs b/crates/functions/src/criteria.rs index 20410c8..2b4c488 100644 --- a/crates/functions/src/criteria.rs +++ b/crates/functions/src/criteria.rs @@ -5,6 +5,10 @@ use std::cmp::Ordering; use gridcraft_core::parse::parse_number_text; use gridcraft_core::{CellError, Value, compare_numbers, compare_text}; +/// Longest text criterion COUNTIF and its relatives accept, in UTF-16 code units as Excel +/// counts them (😀 is two units); a longer one is `#VALUE!`. +const MAX_CRITERION_UTF16_UNITS: usize = 255; + #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum Op { Eq, @@ -22,6 +26,15 @@ enum Operand { Error(CellError), /// Text (lower-cased chars kept for wildcard matching). Text(String), + /// A criterion cut to its first 255 UTF-16 units in the middle of a surrogate pair: the + /// complete characters before the cut, followed (in Excel) by a lone high surrogate. + /// + /// No cell text holds a lone surrogate, so `=` matches nothing and `<>` everything, as + /// Excel gives 0 for SUMIFS with a criterion of 128 emoji. For `<`, `<=`, `>` and `>=` the + /// criterion sorts just above the kept text: a cell matches `>`/`>=` when it sorts above + /// that text, and `<`/`<=` otherwise. This is an approximation (Excel wasn't checked) and + /// only matters for comparison criteria over 255 units. + CutText(String), /// Nothing after the operator (`"="`, `"<>"`) or an empty criterion. Blank, } @@ -48,25 +61,52 @@ impl Criterion { } } - fn parse_text(t: &str) -> Criterion { - let (op, rest) = if let Some(r) = t.strip_prefix(">=") { - (Op::Ge, r) - } else if let Some(r) = t.strip_prefix("<=") { - (Op::Le, r) - } else if let Some(r) = t.strip_prefix("<>") { - (Op::Ne, r) - } else if let Some(r) = t.strip_prefix('=') { - (Op::Eq, r) - } else if let Some(r) = t.strip_prefix('>') { - (Op::Gt, r) - } else if let Some(r) = t.strip_prefix('<') { - (Op::Lt, r) - } else { - if t.is_empty() { - return Criterion { op: Op::Eq, operand: Operand::Blank, bare_empty: true }; + /// Parses the criteria argument of COUNTIF, SUMIF, AVERAGEIF and the single-pair *IFS + /// functions: like `parse`, but text longer than 255 UTF-16 code units is `#VALUE!`, as in + /// Excel (checked on Excel 2016 and Microsoft 365: `"a"×254 & "😀"` is 255 characters but + /// 256 units, and is `#VALUE!`). + pub(crate) fn parse_arg(v: &Value) -> Result { + match v { + Value::Array(a) => Criterion::parse_arg(a.data.first().unwrap_or(&Value::Empty)), + // A UTF-8 string has at least as many bytes as UTF-16 units, so short text skips the count. + Value::Text(t) if t.len() > MAX_CRITERION_UTF16_UNITS && t.encode_utf16().nth(MAX_CRITERION_UTF16_UNITS).is_some() => { + Err(CellError::Value) } - (Op::Eq, t) - }; + other => Ok(Criterion::parse(other)), + } + } + + /// Parses a criteria argument of an *IFS function with two or more criteria pairs. Excel + /// doesn't reject a longer text criterion there: it matches its first 255 UTF-16 code units + /// (checked on Excel 2016 and Microsoft 365 with COUNTIFS, SUMIFS, AVERAGEIFS, MAXIFS and + /// MINIFS). A cut through a surrogate pair matches nothing; see `Operand::CutText`. + pub(crate) fn parse_arg_truncated(v: &Value) -> Criterion { + match v { + Value::Array(a) => Criterion::parse_arg_truncated(a.data.first().unwrap_or(&Value::Empty)), + Value::Text(t) if t.len() > MAX_CRITERION_UTF16_UNITS => { + let mut units = 0; + for (i, c) in t.char_indices() { + if units + c.len_utf16() > MAX_CRITERION_UTF16_UNITS { + if units == MAX_CRITERION_UTF16_UNITS { + return Criterion::parse_text(&t[..i]); + } + // The cut falls between the two halves of `c`. + let (op, rest) = split_op(&t[..i]); + return Criterion { op, operand: Operand::CutText(rest.to_string()), bare_empty: false }; + } + units += c.len_utf16(); + } + Criterion::parse_text(t) + } + other => Criterion::parse(other), + } + } + + fn parse_text(t: &str) -> Criterion { + if t.is_empty() { + return Criterion { op: Op::Eq, operand: Operand::Blank, bare_empty: true }; + } + let (op, rest) = split_op(t); let operand = if rest.is_empty() { Operand::Blank } else if let Some(n) = parse_number_text(rest) { @@ -140,10 +180,35 @@ impl Criterion { _ => false, }, }, + Operand::CutText(kept) => match self.op { + Op::Eq => false, + Op::Ne => true, + Op::Lt | Op::Le => matches!(&v, Value::Text(t) if compare_text(t, kept) != Ordering::Greater), + Op::Gt | Op::Ge => matches!(&v, Value::Text(t) if compare_text(t, kept) == Ordering::Greater), + }, } } } +/// Splits a leading comparison operator (`=` when there is none) off a text criterion. +fn split_op(t: &str) -> (Op, &str) { + if let Some(r) = t.strip_prefix(">=") { + (Op::Ge, r) + } else if let Some(r) = t.strip_prefix("<=") { + (Op::Le, r) + } else if let Some(r) = t.strip_prefix("<>") { + (Op::Ne, r) + } else if let Some(r) = t.strip_prefix('=') { + (Op::Eq, r) + } else if let Some(r) = t.strip_prefix('>') { + (Op::Gt, r) + } else if let Some(r) = t.strip_prefix('<') { + (Op::Lt, r) + } else { + (Op::Eq, t) + } +} + fn cmp_op(op: Op, o: Ordering) -> bool { match op { Op::Eq => o == Ordering::Equal, diff --git a/crates/functions/src/math.rs b/crates/functions/src/math.rs index 6410918..576302a 100644 --- a/crates/functions/src/math.rs +++ b/crates/functions/src/math.rs @@ -958,6 +958,9 @@ fn criteria_mask(a: &[Arg], first_pair: usize, rows: usize, cols: usize) -> R R R { fn countif(a: &[Arg], _c: &mut dyn Ctx) -> R { let range = as_array(&crate::util::arg(a, 0)?.value); - let c = Criterion::parse(&crate::util::arg(a, 1)?.value.scalar()); + let c = Criterion::parse_arg(&crate::util::arg(a, 1)?.value.scalar())?; num_val(range.iter().filter(|v| c.matches(v)).count() as f64) } @@ -1034,7 +1038,7 @@ fn countif(a: &[Arg], _c: &mut dyn Ctx) -> R { /// the evaluator does that resizing (it has the references), so cells line up one to one here. fn if_numbers(a: &[Arg]) -> R> { let range = as_array(&crate::util::arg(a, 0)?.value); - let c = Criterion::parse(&crate::util::arg(a, 1)?.value.scalar()); + let c = Criterion::parse_arg(&crate::util::arg(a, 1)?.value.scalar())?; let sum_range = if has(a, 2) { Some(as_array(&crate::util::arg(a, 2)?.value)) } else { None }; let mut out = Vec::new(); for r in 0..range.rows { @@ -1658,4 +1662,77 @@ mod tests { is_err(ev("COUNTIFS", vec![rf(qty.clone()), t(">1"), rf(qty)]), CellError::Value); is_err(ev("SUMIF", vec![rf(col(&[1.0, 2.0])), t(">0"), rf(arr(vec![vec![Value::Error(NA)], vec![nv(1.0)]]))]), NA); } + + #[test] + fn criteria_over_255_chars_are_value_errors_with_one_pair_and_truncated_with_more() { + let ok = "x".repeat(255); + let long = "x".repeat(256); + let cells = arr(vec![vec![tv(&ok)], vec![tv(&long)]]); + let qty = col(&[1.0, 2.0]); + close(ev("COUNTIF", vec![rf(cells.clone()), t(&ok)]), 1.0); + is_err(ev("COUNTIF", vec![rf(cells.clone()), t(&long)]), CellError::Value); + close(ev("SUMIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&ok)]), 1.0); + is_err(ev("SUMIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&long)]), CellError::Value); + is_err(ev("COUNTIFS", vec![rf(cells.clone()), t(&long)]), CellError::Value); + // With two or more pairs Excel matches the first 255 characters: the 255-character cell + // (quantity 1), not the 256-character one (quantity 2). Checked on Excel 2016 and 365. + let over = format!("{ok}y"); + close(ev("COUNTIFS", vec![rf(cells.clone()), t(&long), rf(qty.clone()), t(">0")]), 1.0); + close(ev("SUMIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&long), rf(qty.clone()), t(">0")]), 1.0); + close(ev("SUMIFS", vec![rf(qty.clone()), rf(qty.clone()), t(">0"), rf(cells.clone()), t(&over)]), 1.0); + close(ev("AVERAGEIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&long), rf(qty.clone()), t(">0")]), 1.0); + close(ev("MAXIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&"x".repeat(300)), rf(qty), t(">0")]), 1.0); + } + + #[test] + fn criteria_over_255_utf16_units_follow_excel() { + // The tester's sheet, identical on Excel 2016 and Microsoft 365: A1:A6 hold these + // strings, B is the row number, and each row uses its own string as the criterion. + let e = "😀"; + let strings = + [e.repeat(127), e.repeat(128), format!("{}{e}", "a".repeat(254)), format!("{}{e}", "a".repeat(253)), e.repeat(200), "я".repeat(256)]; + let cells = arr(strings.iter().map(|s| vec![tv(s)]).collect()); + let rows = col(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0]); + // COUNTIF, SUMIF, COUNTIFS with one pair, SUMIFS with a second pair `B,">0"`. + let expected: [Option<[f64; 4]>; 6] = [Some([1.0, 1.0, 1.0, 1.0]), None, None, Some([1.0, 4.0, 1.0, 4.0]), None, None]; + for (s, want) in strings.iter().zip(expected) { + let countif = ev("COUNTIF", vec![rf(cells.clone()), t(s)]); + let sumif = ev("SUMIF", vec![rf(cells.clone()), t(s), rf(rows.clone())]); + let countifs = ev("COUNTIFS", vec![rf(cells.clone()), t(s)]); + let sumifs = ev("SUMIFS", vec![rf(rows.clone()), rf(cells.clone()), t(s), rf(rows.clone()), t(">0")]); + match want { + Some([a, b, c, d]) => { + close(countif, a); + close(sumif, b); + close(countifs, c); + close(sumifs, d); + } + // Over 255 units: `#VALUE!` with one pair; cut to 255 units with two, which + // matches no cell (for emoji it splits a surrogate pair). + None => { + is_err(countif, CellError::Value); + is_err(sumif, CellError::Value); + is_err(countifs, CellError::Value); + close(sumifs, 0.0); + } + } + } + } + + #[test] + fn criteria_cut_inside_a_surrogate_pair() { + let e = "😀"; + let cells = arr(vec![vec![tv(&e.repeat(127))], vec![tv(&e.repeat(128))], vec![tv("zzz")], vec![Value::Empty]]); + let rows = col(&[1.0, 2.0, 3.0, 4.0]); + let sumifs = |crit: String| ev("SUMIFS", vec![rf(rows.clone()), rf(cells.clone()), t(&crit), rf(rows.clone()), t(">0")]); + // "=" + 😀×128 is 257 units: the cut after 255 keeps "=" and 127 whole emoji. + close(sumifs(format!("={}", e.repeat(128))), 1.0); + // With a two-unit prefix the cut keeps 126 emoji and half of the 127th. Such a criterion + // equals no cell text: "=" matches nothing and "<>" everything, blanks included. + close(sumifs(format!("=a{}", e.repeat(128))), 0.0); + close(sumifs(format!("<>{}", e.repeat(128))), 10.0); + // Comparisons treat it as sorting just above the 126 kept emoji. + close(sumifs(format!(">={}", e.repeat(128))), 1.0 + 2.0); + close(sumifs(format!("<={}", e.repeat(128))), 3.0); + } }