engine: refuse formulas with the wrong argument count on entry

A formula calling a built-in function with too few or too many
arguments (=DATEDIF(a,b), =SUM(), =DATE(2024,1)) was stored and showed
#VALUE! with no hint about what was wrong. Entry now refuses it with
the function's signature, e.g. "DATEDIF has too few arguments. Use
DATEDIF(start_date, end_date, unit)", and leaves the cell unchanged.

Only registered functions are checked: special forms the evaluator
handles itself (IF, LET, LAMBDA...), LAMBDA names and unknown functions
are skipped. Paste and Find & Replace keep turning input that fails
entry into text. DATEDIF's description now lists its units.

Fixes #363

Claude-Session: https://claude.ai/code/session_01BGBBGBC2gRpQ7GDuwQ6WPt
This commit is contained in:
Shirish Veerabattini 2026-10-11 13:29:14 -05:00
parent 26489863b0
commit 56f784473d
3 changed files with 62 additions and 1 deletions

View File

@ -150,6 +150,7 @@ pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gri
}
let expr = parsed.map_err(|e| EngineError::Other(format!("There's a problem with this formula: {e}")))?;
let expr = wb.name_call_case(expr);
check_arg_counts(&expr)?;
let mut cell = Cell::formula(Formula::from_expr(expr));
cell.style = style;
// Formulas whose result is a date/time get a format from functions like TODAY().
@ -190,6 +191,27 @@ pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gri
Ok(Some(Cell { value: parsed.value, formula: None, style }))
}
/// Like Excel, a formula that calls a built-in function with too few or too many arguments is
/// refused when it's entered, rather than stored to show `#VALUE!`. Special forms the evaluator
/// handles itself (IF, LET, LAMBDA…) and defined LAMBDA names aren't in the registry: skipped.
fn check_arg_counts(expr: &gridcraft_formula::Expr) -> Result<()> {
let mut bad = None;
expr.walk(&mut |e| {
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)
{
let few = args.len() < spec.min_args;
bad = Some(format!("{} has too {} arguments. Use {}", spec.name, if few { "few" } else { "many" }, spec.signature));
}
});
match bad {
Some(msg) => Err(EngineError::Other(msg)),
None => Ok(()),
}
}
fn cell_set(s: &mut Session, p: &Json) -> Result<Json> {
let sheet = target_sheet(s, p)?;
let at = match cell_param(p, "cell") {

View File

@ -202,6 +202,45 @@ fn enter_values_and_formulas() {
assert_eq!(crate::display::cell_text(&d.wb, sh, CellRef::parse("B1").unwrap()), "12%");
}
#[test]
fn formulas_with_the_wrong_argument_count_are_refused_on_entry() {
// #363: =DATEDIF(start, end) was stored and showed #VALUE!. Like Excel, entry refuses it and
// the message gives the signature, so the missing unit argument is visible.
let mut s = s();
s.execute("cell.set", json!({"cell": "A1", "input": "kept"})).unwrap();
for (input, word) in [
("=DATEDIF(DATE(2024,1,1),DATE(2024,3,15))", "few"),
("=1+DATE(2024,1)", "few"),
("=SUM()", "few"),
("=IFERROR(1,LEFT(\"a\",1,2,3))", "many"),
("=datedif(1,2,\"D\",4)", "many"),
] {
let e = s.execute("cell.set", json!({"cell": "A1", "input": input})).unwrap_err().to_string();
assert!(e.contains(&format!("too {word} arguments")), "{input}: {e}");
assert_eq!(v(&s, "A1"), Value::text("kept"), "{input} must not replace the cell");
}
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
// functions (#NAME?) still enter.
s.execute("formulas.defineName", json!({"name": "TWICE", "refersTo": "=LAMBDA(x,x*2)"})).unwrap();
for (cell, input) in [
("B1", "=DATEDIF(DATE(2024,1,1),DATE(2024,3,15),\"D\")"),
("B2", "=IF(TRUE,1)"),
("B3", "=LET(a,1,b,2,a+b)"),
("B4", "=TWICE(4)"),
("B5", "=NOSUCHFUNCTION(1,2,3)"),
("B6", "=ROUND(2.5,)"),
] {
s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap_or_else(|e| panic!("{input}: {e}"));
}
assert_eq!(v(&s, "B1"), Value::Number(74.0));
assert_eq!(v(&s, "B4"), Value::Number(8.0));
// Pasted text that isn't a valid formula stays text, as it does for a parse error.
s.execute("edit.paste", json!({"at": "C1", "text": "=DATEDIF(1,2)\n"})).unwrap();
assert_eq!(v(&s, "C1"), Value::text("=DATEDIF(1,2)"));
}
#[test]
fn undo_redo() {
let mut s = s();

View File

@ -496,7 +496,7 @@ pub(crate) fn specs() -> Vec<FnSpec> {
f!("ISOWEEKNUM", 1, 1, DateTime, S, "ISOWEEKNUM(date)", "Returns the ISO 8601 week number of a date.", isoweeknum),
f!("EDATE", 2, 2, DateTime, S, "EDATE(start_date, months)", "Returns the date a number of months before or after a date.", edate),
f!("EOMONTH", 2, 2, DateTime, S, "EOMONTH(start_date, months)", "Returns the last day of the month a number of months away.", eomonth),
f!("DATEDIF", 3, 3, DateTime, S, "DATEDIF(start_date, end_date, unit)", "Counts whole years, months or days between two dates.", datedif),
f!("DATEDIF", 3, 3, DateTime, S, "DATEDIF(start_date, end_date, unit)", "Years, months or days apart. unit: Y, M, D, MD, YM or YD.", datedif),
f!("DATEVALUE", 1, 1, DateTime, S, "DATEVALUE(date_text)", "Converts a date written as text into a date serial number.", datevalue),
f!("TIMEVALUE", 1, 1, DateTime, S, "TIMEVALUE(time_text)", "Converts a time written as text into a fraction of a day.", timevalue),
f!("DAYS", 2, 2, DateTime, S, "DAYS(end_date, start_date)", "Returns the number of days between two dates.", days),