From 993c1086607ad19c491d9b43dd860e9fce6110d7 Mon Sep 17 00:00:00 2001 From: Shirish Veerabattini Date: Sun, 11 Oct 2026 15:37:09 -0500 Subject: [PATCH] functions: share the argument-count rule between call and entry FnSpec::accepts holds the arity check (including Excel's 255-argument limit) that functions::call and the entry check both use, so entry also refuses 256 arguments to a variadic function. The evaluator's own check in calc is unchanged. Claude-Session: https://claude.ai/code/session_01BGBBGBC2gRpQ7GDuwQ6WPt --- crates/engine/src/cmd/edit.rs | 2 +- crates/engine/src/tests.rs | 6 ++++++ crates/functions/src/lib.rs | 7 ++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 9f48a87..b487947 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -200,7 +200,7 @@ fn check_arg_counts(expr: &gridcraft_formula::Expr) -> Result<()> { if bad.is_none() && let gridcraft_formula::Expr::Call(name, args) = e && let Some(spec) = gridcraft_functions::lookup(name) - && (args.len() < spec.min_args || args.len() > spec.max_args) + && !spec.accepts(args.len()) { let few = args.len() < spec.min_args; bad = Some(format!("{} has too {} arguments. Use {}", spec.name, if few { "few" } else { "many" }, spec.signature)); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6e6c9d4..620dfcb 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -219,6 +219,12 @@ fn formulas_with_the_wrong_argument_count_are_refused_on_entry() { assert!(e.contains(&format!("too {word} arguments")), "{input}: {e}"); assert_eq!(v(&s, "A1"), Value::text("kept"), "{input} must not replace the cell"); } + // Excel's 255-argument limit applies to variadic functions too. + let many = format!("=SUM({})", vec!["1"; 256].join(",")); + let e = s.execute("cell.set", json!({"cell": "A1", "input": many})).unwrap_err().to_string(); + assert!(e.contains("too many arguments"), "{e}"); + s.execute("cell.set", json!({"cell": "A2", "input": format!("=SUM({})", vec!["1"; 255].join(","))})).unwrap(); + assert_eq!(v(&s, "A2"), Value::Number(255.0)); let e = s.execute("cell.set", json!({"cell": "A1", "input": "=DATEDIF(1,2)"})).unwrap_err().to_string(); assert!(e.contains("DATEDIF(start_date, end_date, unit)"), "{e}"); // Correct counts, optional arguments left empty, special forms, LAMBDA names and unknown diff --git a/crates/functions/src/lib.rs b/crates/functions/src/lib.rs index 1e4415e..282b230 100644 --- a/crates/functions/src/lib.rs +++ b/crates/functions/src/lib.rs @@ -160,6 +160,11 @@ impl FnSpec { None => self.scalar_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 + } } /// Variadic marker for `max_args`. @@ -215,7 +220,7 @@ pub fn all() -> &'static [FnSpec] { /// Calls a function with array lifting applied. Wrong arity gives `#VALUE!`. pub fn call(spec: &FnSpec, args: &[Arg], ctx: &mut dyn Ctx) -> Value { - if args.len() < spec.min_args || args.len() > spec.max_args || args.len() > 255 { + if !spec.accepts(args.len()) { return Value::Error(CellError::Value); } lift::call_lifted(spec, args, ctx)