This commit is contained in:
Brandon Thomas
2026-10-24 02:10:36 -04:00
5 changed files with 194 additions and 0 deletions
+85
View File
@@ -1588,6 +1588,91 @@ fn offset_extreme_arguments_return_ref() {
}
}
#[test]
fn analysis_toolpak_value_params_reject_multi_cell_references() {
// Excel 365 (observed): EOMONTH, WORKDAY, COUPDAYS, BIN2DEC... give #VALUE! for a reference to more
// than one cell, however the reference is formed, and lift array constants and computed arrays.
let mut t = T::new();
for (a, v) in [
("A1", "45000"),
("A2", "45031"),
("A3", "45061"),
("B1", "1"),
("B2", "2"),
("B3", "3"),
("C2", "7"),
("F1", "1"),
("F2", "2"),
("F3", "11"),
] {
t.set(a, v);
}
t.wb.names.push(gridcraft_model::DefinedName {
name: "Dates".into(),
scope: None,
formula: "Sheet1!$A$1:$A$3".into(),
comment: String::new(),
hidden: false,
});
let value = Value::Error(CellError::Value);
for f in [
"=EOMONTH(A1:A3,1)",
"=EOMONTH(A1,B1:B3)",
"=EDATE(A1:A3,1)",
"=SUMPRODUCT(EOMONTH(A1:A3,0))",
"=EOMONTH(INDEX(A1:A3,0,1),0)",
"=EOMONTH(OFFSET(A1,0,0,3,1),0)",
"=EOMONTH(IF(TRUE,A1:A3),0)",
"=EOMONTH(CHOOSE(1,A1:A3),0)",
"=LET(x,A1:A3,EOMONTH(x,0))",
"=EOMONTH(Dates,0)",
"=EOMONTH((A1,A2),0)",
"=EOMONTH(Z:Z,0)",
"=EOMONTH(A1:INDEX(A1:A3,3),0)",
"=WORKDAY(A1:A3,1)",
"=NETWORKDAYS(A1,A3:A4)",
"=YEARFRAC(A1,A3,B1:B2)",
"=WEEKNUM(A1,B1:B3)",
"=COUPDAYS(A1:A3,46000,2,0)",
"=DISC(A1,46000,98,100,B1:B3)",
"=BIN2DEC(B1:B3)",
"=ISEVEN(B1:B3)",
] {
t.set("H5", f);
assert_eq!((t.get("H5"), t.get("H6")), (value.clone(), Value::Empty), "{f}");
t.set("H5", "");
}
// One-cell references, intersections, @ and the range parameters are fine.
for (f, want) in [
("=EOMONTH(A1:A1,0)", 45016.0),
("=EOMONTH(INDEX(A1:A3,2),0)", 45046.0),
("=EOMONTH(OFFSET(A1,0,0,1,1),0)", 45016.0),
("=EOMONTH(A1:A3 A2:C2,0)", 45046.0),
("=EOMONTH(@A1:A3,0)", 45046.0),
("=WORKDAY(A1,1,A1:A3)", 45001.0),
("=XNPV(0,B1:B3,A1:A3)", 6.0),
] {
t.set("H2", f);
assert_eq!(t.num("H2"), want, "{f}");
t.set("H2", "");
}
// Arrays that aren't references lift; so does the WORKDAY.INTL/NETWORKDAYS.INTL weekend.
for (f, below, right) in [
("=EOMONTH(A1:A3+0,0)", Value::Number(45046.0), Value::Empty),
("=EOMONTH(A1,{1,2})", Value::Empty, Value::Number(45077.0)),
("=EOMONTH(TRANSPOSE(A1:A3),0)", Value::Empty, Value::Number(45046.0)),
("=MAP(A1:A3,LAMBDA(x,EOMONTH(x,0)))", Value::Number(45046.0), Value::Empty),
("=NETWORKDAYS.INTL(A1,A3+30,F1:F3)", Value::Number(66.0), Value::Empty),
("=ISOWEEKNUM(A1:A3)", Value::Number(15.0), Value::Empty),
// The rejected reference adds no shape: a 2x1 array of #VALUE!.
("=EOMONTH(A1:A3,{1;2})", value.clone(), Value::Empty),
] {
t.set("H5", f);
assert_eq!((t.get("H6"), t.get("I5")), (below, right), "{f}");
t.set("H5", "");
}
}
/// A formula reading one further down (or on a later sheet) is calculated on demand, inside the
/// formula that reads it. Chains of thousands of such formulas, common in financial models, used
/// to overflow the stack (or give `#CIRC!` past 2,000 levels); they now calculate in full.
+18
View File
@@ -30,6 +30,7 @@ macro_rules! f {
category: $crate::Category::$cat,
volatile: true,
scalar_args: $scal,
value_args: &[],
signature: $sig,
description: $desc,
imp: {
@@ -48,6 +49,7 @@ macro_rules! f {
category: $crate::Category::$cat,
volatile: false,
scalar_args: $scal,
value_args: &[],
signature: $sig,
description: $desc,
imp: {
@@ -156,6 +158,11 @@ pub struct FnSpec {
/// (broadcast like Excel's dynamic arrays). The last entry repeats for variadic tails; an
/// empty slice means no lifting.
pub scalar_args: &'static [bool],
/// Parameter positions that take a single value and reject a reference to more than one
/// cell with `#VALUE!` (the Analysis ToolPak functions: EOMONTH, WORKDAY, COUPDAYS, BIN2DEC…),
/// where other scalar parameters lift the range. Arrays that aren't references still lift.
/// Same layout as `scalar_args`; filled at registration from [`lift::value_args`].
pub value_args: &'static [bool],
pub signature: &'static str,
pub description: &'static str,
pub imp: FnImpl,
@@ -176,6 +183,14 @@ impl FnSpec {
}
}
/// Whether parameter `i` takes a single value, so a multi-cell reference there is `#VALUE!`.
pub fn is_value_arg(&self, i: usize) -> bool {
match self.value_args.get(i) {
Some(b) => *b,
None => self.value_args.last().copied().unwrap_or(false),
}
}
/// Whether a call with `n` arguments is valid (Excel allows at most 255).
pub fn accepts(&self, n: usize) -> bool {
(self.min_args..=self.max_args).contains(&n) && n <= 255
@@ -204,6 +219,9 @@ fn registry() -> &'static (Vec<FnSpec>, HashMap<&'static str, usize>) {
] {
v.extend(list);
}
for s in &mut v {
s.value_args = lift::value_args(s.name);
}
let mut map = HashMap::with_capacity(v.len());
for (i, s) in v.iter().enumerate() {
map.entry(s.name).or_insert(i);
+87
View File
@@ -6,7 +6,61 @@ use gridcraft_core::{Array, CellError, Value};
use crate::util::MAX_CELLS;
use crate::{Arg, Ctx, FnSpec};
/// Value-parameter masks (see [`FnSpec::value_args`]), observed in Excel 365: a reference to more
/// than one cell at these positions gives `#VALUE!` in dynamic-array and legacy formulas alike (no
/// implicit intersection), while array constants and computed arrays lift. Range parameters
/// (holidays, schedules, cash flows, coefficients) and the WORKDAY.INTL/NETWORKDAYS.INTL weekend
/// (which lifts a range) are `false`.
pub(crate) const VALUE_ARGS: &[(&str, &[bool])] = &[
// Date and time.
("EOMONTH EDATE YEARFRAC WEEKNUM", &[true]),
("WORKDAY NETWORKDAYS", &[true, true, false]),
("WORKDAY.INTL NETWORKDAYS.INTL", &[true, true, false, false]),
// Financial.
(
"ACCRINT ACCRINTM AMORDEGRC AMORLINC COUPDAYBS COUPDAYS COUPDAYSNC COUPNCD COUPNUM COUPPCD CUMIPMT CUMPRINC DISC \
DOLLARDE DOLLARFR DURATION EFFECT INTRATE MDURATION NOMINAL ODDFPRICE ODDFYIELD ODDLPRICE ODDLYIELD PRICE PRICEDISC \
PRICEMAT RECEIVED TBILLEQ TBILLPRICE TBILLYIELD YIELD YIELDDISC YIELDMAT",
&[true],
),
("FVSCHEDULE XNPV", &[true, false]),
("XIRR", &[false, false, true]),
// Math and information.
("ISEVEN ISODD QUOTIENT MROUND FACTDOUBLE SQRTPI RANDBETWEEN", &[true]),
("SERIESSUM", &[true, true, true, false]),
// Engineering (IMSUM and IMPRODUCT aggregate, so they aren't here).
(
"BIN2DEC BIN2HEX BIN2OCT DEC2BIN DEC2HEX DEC2OCT HEX2BIN HEX2DEC HEX2OCT OCT2BIN OCT2DEC OCT2HEX CONVERT DELTA GESTEP \
ERF ERF.PRECISE ERFC ERFC.PRECISE BESSELI BESSELJ BESSELK BESSELY COMPLEX IMABS IMAGINARY IMARGUMENT IMCONJUGATE \
IMCOS IMCOSH IMCOT IMCSC IMCSCH IMDIV IMEXP IMLN IMLOG10 IMLOG2 IMPOWER IMREAL IMSEC IMSECH IMSIN IMSINH IMSQRT IMSUB \
IMTAN",
&[true],
),
];
/// The value-parameter mask of `name` (empty for functions without value parameters).
pub(crate) fn value_args(name: &str) -> &'static [bool] {
VALUE_ARGS.iter().find(|(names, _)| names.split_ascii_whitespace().any(|n| n == name)).map_or(&[], |(_, mask)| mask)
}
/// A reference to more than one cell at a value parameter becomes a scalar `#VALUE!`, so it adds
/// no shape: `EOMONTH(A1:A3, {1;2})` is a 2x1 array of `#VALUE!`, as in Excel.
fn reject_multi_cell_refs(spec: &FnSpec, args: &[Arg]) -> Option<Vec<Arg>> {
let multi = |i: usize, a: &Arg| a.from_ref && spec.is_value_arg(i) && matches!(&a.value, Value::Array(arr) if arr.data.len() > 1);
if !args.iter().enumerate().any(|(i, a)| multi(i, a)) {
return None;
}
Some(
args.iter()
.enumerate()
.map(|(i, a)| if multi(i, a) { Arg { value: Value::Error(CellError::Value), from_ref: false } } else { a.clone() })
.collect(),
)
}
pub(crate) fn call_lifted(spec: &FnSpec, args: &[Arg], ctx: &mut dyn Ctx) -> Value {
let rejected = if spec.value_args.is_empty() { None } else { reject_multi_cell_refs(spec, args) };
let args = rejected.as_deref().unwrap_or(args);
let mut rows = 0usize;
let mut cols = 0usize;
let mut any = false;
@@ -94,4 +148,37 @@ mod tests {
);
is_err(huge, CellError::Num);
}
#[test]
fn value_params_reject_multi_cell_references() {
let s = FnSpec { value_args: &[true, false], ..spec() };
let mut ctx = TestCtx::default();
// A multi-cell reference is #VALUE!; a one-cell reference and a computed array are fine.
is_err(crate::call(&s, &[rf(col(&[1.0, 2.0, 3.0])), n(1.0)], &mut ctx), CellError::Value);
assert_eq!(crate::call(&s, &[rf(col(&[4.0])), n(1.0)], &mut ctx), nv(5.0));
assert_eq!(rows_of(&crate::call(&s, &[av(col(&[1.0, 2.0])), n(1.0)], &mut ctx)), vec![vec![nv(2.0)], vec![nv(3.0)]]);
// The rejected reference adds no shape: the other argument still lifts.
let v = Value::Error(CellError::Value);
assert_eq!(rows_of(&crate::call(&s, &[rf(col(&[1.0, 2.0, 3.0])), av(col(&[1.0, 2.0]))], &mut ctx)), vec![vec![v.clone()], vec![v]]);
// A position that isn't a value parameter still lifts the reference.
assert_eq!(rows_of(&crate::call(&s, &[n(1.0), rf(row(&[1.0, 2.0]))], &mut ctx)), vec![vec![nv(2.0), nv(3.0)]]);
}
#[test]
fn value_arg_table_names_registered_scalar_positions() {
for (names, mask) in super::VALUE_ARGS {
assert!(!mask.is_empty());
for name in names.split_ascii_whitespace() {
let Some(s) = crate::lookup(name) else { panic!("{name} isn't registered") };
assert_eq!(s.value_args, *mask, "{name}");
for i in 0..s.max_args.min(12) {
assert!(!s.is_value_arg(i) || s.is_scalar_arg(i), "{name} argument {i} is a value parameter but doesn't lift");
}
}
}
assert!(crate::lookup("WORKDAY").is_some_and(|s| s.is_value_arg(1) && !s.is_value_arg(2)));
assert!(crate::lookup("WORKDAY.INTL").is_some_and(|s| !s.is_value_arg(2) && s.is_scalar_arg(2)));
assert!(crate::lookup("SUM").is_some_and(|s| s.value_args.is_empty()));
assert!(crate::lookup("ISOWEEKNUM").is_some_and(|s| s.value_args.is_empty()));
}
}