Allow omitting arguments in formula functions (#555)

quadratic-core/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "quadratic-core"
-version = "0.1.9"
+version = "0.1.10"
 authors = ["Andrew Farkas <andrew.farkas@quadratic.to>"]
 edition = "2021"
 description = "Infinite data grid with Python, JavaScript, and SQL built-in"

quadratic-core/src/formulas/ast.rs

@@ -20,6 +20,7 @@ pub type AstNode = Spanned<AstNodeContents>;
 
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub enum AstNodeContents {
+    Empty,
     FunctionCall {
         func: Spanned<String>,
         args: Vec<AstNode>,
@@ -33,6 +34,7 @@ pub enum AstNodeContents {
 impl fmt::Display for AstNodeContents {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
+            AstNodeContents::Empty => write!(f, ""),
             AstNodeContents::FunctionCall { func, args } => {
                 write!(f, "{func}(")?;
                 if let Some(first) = args.first() {
@@ -59,6 +61,7 @@ impl fmt::Display for AstNodeContents {
 impl AstNodeContents {
     fn type_string(&self) -> &'static str {
         match self {
+            AstNodeContents::Empty => "empty expression",
             AstNodeContents::FunctionCall { func, .. } => match func.inner.as_str() {
                 "=" | "==" | "<>" | "!=" | "<" | ">" | "<=" | ">=" => "comparison",
                 s if s.chars().all(|c| c.is_alphanumeric() || c == '_') => "function call",
@@ -112,6 +115,8 @@ impl AstNode {
 
     async fn eval_inner<'ctx: 'a, 'a>(&'a self, ctx: &'a mut Ctx<'ctx>) -> FormulaResult {
         let value = match &self.inner {
+            AstNodeContents::Empty => BasicValue::Blank.into(),
+
             // Cell range
             AstNodeContents::FunctionCall { func, args } if func.inner == ":" => {
                 if args.len() != 2 {

quadratic-core/src/formulas/functions/macros.rs

@@ -308,7 +308,7 @@ macro_rules! formula_fn_arg {
 
     // Optional argument
     (@assign($ctx:ident, $args:ident); $arg_name:ident: Option< $($arg_type:tt)*) => {
-        let $arg_name = match $args.take_next() {
+        let $arg_name = match $args.take_next_optional() {
             // $($arg_type)* will include an extra `>` at the end, and that's ok.
             Some(arg_value) => Some(formula_fn_convert_arg!(arg_value, Value -> $($arg_type)*)),
             None => None,
@@ -344,7 +344,7 @@ macro_rules! formula_fn_arg {
         // If the argument is present, store it in `$args_to_zip_map` and store
         // the index in `$arg_name`.
         let $arg_name: Option<usize>;
-        match $args.take_next() {
+        match $args.take_next_optional() {
             Some(arg_value) => {
                 $arg_name = Some($args_to_zip_map.len());
                 $args_to_zip_map.push(arg_value);

quadratic-core/src/formulas/functions/mod.rs

@@ -74,15 +74,15 @@ impl FormulaFnArgs {
         }
     }
     /// Takes the next argument, or returns `None` if there is none.
-    pub fn take_next(&mut self) -> Option<Spanned<Value>> {
+    pub fn take_next_optional(&mut self) -> Option<Spanned<Value>> {
         if !self.values.is_empty() {
             self.args_popped += 1;
         }
         self.values.pop_front()
     }
     /// Takes the next argument, or returns an error if there is none.
     pub fn take_next_required(&mut self, arg_name: &'static str) -> FormulaResult<Spanned<Value>> {
-        self.take_next().ok_or_else(|| {
+        self.take_next_optional().ok_or_else(|| {
             FormulaErrorMsg::MissingRequiredArgument {
                 func_name: self.func_name,
                 arg_name,

quadratic-core/src/formulas/functions/statistics.rs

@@ -106,7 +106,7 @@ mod tests {
     fn test_formula_average() {
         let form = parse_formula("AVERAGE(3, B1:D3)", pos![nAn1]).unwrap();
 
-        let mut g = FnGrid(|pos| {
+        let g = &mut FnGrid(|pos| {
             if (1..=3).contains(&pos.x) && (1..=3).contains(&pos.y) {
                 Some((pos.x * 3 + pos.y).to_string()) // 4 .. 12
             } else {
@@ -116,15 +116,27 @@ mod tests {
 
         assert_eq!(
             "7.5".to_string(),
-            form.eval_blocking(&mut g, pos![nAn1]).unwrap().to_string(),
+            form.eval_blocking(g, pos![nAn1]).unwrap().to_string(),
         );
 
         assert_eq!(
             "17",
-            eval_to_string(&mut g, "AVERAGE({\"_\", \"a\"}, 12, -3.5, 42.5)"),
+            eval_to_string(g, "AVERAGE({\"_\", \"a\"}, 12, -3.5, 42.5)"),
+        );
+        assert_eq!("5.5", eval_to_string(g, "AVERAGE(1..10)"));
+        assert_eq!("5", eval_to_string(g, "AVERAGE(0..10)"));
+
+        // Test that null arguments count as zero.
+        assert_eq!("1", eval_to_string(g, "AVERAGE(3,,)"));
+        assert_eq!("1", eval_to_string(g, "AVERAGE(,3,)"));
+        assert_eq!("1", eval_to_string(g, "AVERAGE(,,3)"));
+        assert_eq!("0", eval_to_string(g, "AVERAGE(,)"));
+
+        // Test with no arguments
+        assert_eq!(
+            FormulaErrorMsg::DivideByZero,
+            eval_to_err(g, "AVERAGE()").msg,
         );
-        assert_eq!("5.5", eval_to_string(&mut g, "AVERAGE(1..10)"));
-        assert_eq!("5", eval_to_string(&mut g, "AVERAGE(0..10)"));
     }
 
     #[test]
@@ -196,6 +208,7 @@ mod tests {
     #[test]
     fn test_countblank() {
         let g = &mut BlankGrid;
+        assert_eq!("1", eval_to_string(g, "COUNTBLANK(\"\", \"a\", 0, 1)"));
         assert_eq!("2", eval_to_string(g, "COUNTBLANK(B3, \"\", \"a\", 0, 1)"));
     }
 

quadratic-core/src/formulas/params.rs

@@ -50,23 +50,20 @@ pub(super) fn arg_completion_string(args: &[Param]) -> &'static str {
     let mut i = 0;
     let mut depth = 0;
     let mut is_first = true;
-    let mut is_after_first_required_argument = false;
+    // Assume that all required arguments come before any optional ones.
     for arg in args {
         i += 1;
         if is_first {
             is_first = false;
         } else {
-            if arg.is_optional() && is_after_first_required_argument {
+            if arg.is_optional() {
                 ret.push_str(&format!("${{{i}:, "));
                 i += 1;
                 depth += 1;
             } else {
                 ret.push_str(", ");
             }
         }
-        if arg.is_required() {
-            is_after_first_required_argument = true;
-        }
         ret.push_str(&format!("${{{i}:{}}}", arg.usage_string()));
     }
     for _ in 0..depth {

quadratic-core/src/formulas/parser/rules/combinators.rs

@@ -99,6 +99,8 @@ pub struct List<R> {
 
     /// User-friendly name for separator.
     pub(super) sep_name: &'static str,
+    /// Whether to allow a trailing separator.
+    pub(super) allow_trailing_sep: bool,
 }
 impl<R: fmt::Display> fmt::Display for List<R> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -121,13 +123,19 @@ impl<R: Copy + SyntaxRule> SyntaxRule for List<R> {
         let mut items = vec![];
         p.parse(self.start)?;
         loop {
-            // End the list or consume an item.
-            match parse_one_of!(p, [self.inner.map(Some), self.end.map(|_| None)])? {
+            let result = if self.allow_trailing_sep || items.is_empty() {
+                // End the list or consume an item.
+                parse_one_of!(p, [self.end.map(|_| None), self.inner.map(Some)])?
+            } else {
+                // Consume an item.
+                parse_one_of!(p, [self.inner.map(Some)])?
+            };
+            match result {
                 Some(item) => items.push(item), // There is an item.
                 None => break,                  // End of list; empty list, or trailing separator.
             }
             // End the list or consume a separator.
-            match parse_one_of!(p, [self.sep.map(Some), self.end.map(|_| None)])? {
+            match parse_one_of!(p, [self.end.map(|_| None), self.sep.map(Some)])? {
                 Some(_) => continue, // There is a separator.
                 None => break,       // End of list, no trailing separator.
             }

quadratic-core/src/formulas/parser/rules/expression.rs

@@ -117,9 +117,10 @@ impl SyntaxRule for ExpressionWithPrecedence {
                 Token::LBracket => false,
                 Token::LBrace => true,
 
-                Token::RParen | Token::RBracket | Token::RBrace => false,
-
-                Token::ArgSep | Token::RowSep => false,
+                //  These match because an expression in a list or array or
+                //  functional call is allowed to be empty.
+                Token::RParen | Token::RBracket | Token::RBrace => true,
+                Token::ArgSep | Token::RowSep => true,
 
                 Token::Eql | Token::Neq | Token::Lt | Token::Gt | Token::Lte | Token::Gte => false,
 
@@ -162,7 +163,7 @@ impl SyntaxRule for ExpressionWithPrecedence {
                     ArrayLiteral.map(Some),
                     CellReference.map(Some),
                     ParenExpression.map(Some),
-                    Epsilon.map(|_| None),
+                    EmptyExpression.map(Some),
                 ],
             )
             .transpose()
@@ -329,6 +330,7 @@ impl SyntaxRule for FunctionCall {
             start: Token::FunctionCall,
             end: Token::RParen,
             sep_name: "comma",
+            allow_trailing_sep: false,
         })?;
         let args = spanned_args.inner;
 
@@ -406,3 +408,28 @@ impl SyntaxRule for ArrayLiteral {
         })
     }
 }
+
+/// Matches an empty expression that is followed by something that should
+/// normally follow an expression (such as a comma or semicolon or right paren).
+#[derive(Debug, Copy, Clone)]
+pub struct EmptyExpression;
+impl_display!(for EmptyExpression, "empty expression");
+impl SyntaxRule for EmptyExpression {
+    type Output = ast::AstNode;
+
+    fn prefix_matches(&self, mut p: Parser<'_>) -> bool {
+        match p.next() {
+            None => true,
+            Some(Token::ArgSep | Token::RowSep) => true,
+            Some(Token::RBrace | Token::RBracket | Token::RParen) => true,
+            _ => false,
+        }
+    }
+
+    fn consume_match(&self, p: &mut Parser<'_>) -> FormulaResult<Self::Output> {
+        Ok(Spanned {
+            span: Span::empty(p.cursor.unwrap_or(0) as u32),
+            inner: ast::AstNodeContents::Empty,
+        })
+    }
+}

quadratic-core/src/formulas/tests.rs

@@ -140,6 +140,20 @@ fn test_formula_range_operator() {
     );
 }
 
+#[test]
+fn test_formula_blank_array_parsing() {
+    let g = &mut BlankGrid;
+    const B: BasicValue = BasicValue::Blank;
+    assert_eq!(Value::from(array![B]), eval(g, "{}").unwrap());
+    assert_eq!(Value::from(array![B; B]), eval(g, "{;}").unwrap());
+    assert_eq!(Value::from(array![B, B]), eval(g, "{,}").unwrap());
+    assert_eq!(Value::from(array![B, B; B, B]), eval(g, "{,;,}").unwrap());
+    assert_eq!(
+        FormulaErrorMsg::NonRectangularArray,
+        eval_to_err(g, "{;,;}").msg,
+    );
+}
+
 #[test]
 fn test_formula_array_op() {
     let mut g = FnGrid(|pos| Some((pos.x * 10 + pos.y).to_string()));
@@ -210,12 +224,12 @@ fn test_array_parsing() {
         eval(&mut NoGrid, "{1; 3, 4}").unwrap_err().msg,
     );
 
-    // Empty array
-    assert!(eval(&mut NoGrid, "{}").is_err());
-    assert!(eval(&mut NoGrid, "{ }").is_err());
+    // Blank values
+    assert_eq!("{}", eval_to_string(&mut NoGrid, "{}"));
+    assert_eq!("{}", eval_to_string(&mut NoGrid, "{ }"));
 
     // Empty row
-    assert!(eval(&mut NoGrid, "{ ; }").is_err());
+    assert_eq!("{; }", eval_to_string(&mut NoGrid, "{ ; }"));
 }
 
 #[test]
@@ -232,6 +246,34 @@ fn test_hyphen_after_cell_ref() {
     assert_eq!("25", eval_to_string(&mut g, "Z1-5"));
 }
 
+#[test]
+fn test_formula_omit_required_argument() {
+    let g = &mut NoGrid;
+    assert!(eval_to_string(g, "ATAN2(,1)").starts_with("1.57"));
+    assert_eq!("0", eval_to_string(g, "ATAN2(1,)"));
+    assert_eq!(
+        FormulaErrorMsg::MissingRequiredArgument {
+            func_name: "ATAN2",
+            arg_name: "x"
+        },
+        eval_to_err(g, "ATAN2()").msg,
+    );
+    assert_eq!(
+        FormulaErrorMsg::MissingRequiredArgument {
+            func_name: "ATAN2",
+            arg_name: "y"
+        },
+        eval_to_err(g, "ATAN2(1)").msg,
+    );
+}
+
+#[test]
+fn test_formula_blank_to_string() {
+    let g = &mut NoGrid;
+    assert_eq!("", eval_to_string(g, "IF(1=1,,)"));
+}
+
+#[test]
 fn test_find_cell_references() {
     use CellRefCoord::{Absolute, Relative};
 

quadratic-core/src/formulas/values.rs

@@ -216,7 +216,6 @@ impl Spanned<Value> {
             v.into_non_error_value()
                 .map(|inner| Spanned { span, inner })
         })
-
     }
 }
 
@@ -392,7 +391,7 @@ pub enum BasicValue {
 impl fmt::Display for BasicValue {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            BasicValue::Blank => write!(f, "[blank]"),
+            BasicValue::Blank => write!(f, ""),
             BasicValue::String(s) => write!(f, "{s}"),
             BasicValue::Number(n) => write!(f, "{n}"),
             BasicValue::Bool(true) => write!(f, "TRUE"),