From dbcdee1c339c38121ecbccacda203ba933455177 Mon Sep 17 00:00:00 2001
From: fef <owo@fef.moe>
Date: Thu, 28 Jul 2022 20:53:59 +0200
Subject: [PATCH] ast: unify function param and array parsing

---
 src/ast/mod.rs  | 76 ++++++++++++++++++++++++++++++-------------------
 src/ast/tree.rs | 12 ++++++--
 2 files changed, 56 insertions(+), 32 deletions(-)

diff --git a/src/ast/mod.rs b/src/ast/mod.rs
index b04a5ab..2b87810 100644
--- a/src/ast/mod.rs
+++ b/src/ast/mod.rs
@@ -452,7 +452,11 @@ impl Parser {
             token::Kind::StringLiteral => Ok(tree::Node::String(token.raw)),
             token::Kind::TrueKeyword => Ok(tree::Node::Bool(true)),
             token::Kind::FalseKeyword => Ok(tree::Node::Bool(false)),
-            token::Kind::OBracket => self.parse_array(),
+            token::Kind::OBracket => {
+                let elements =
+                    self.parse_delimited_list(token::Kind::Comma, token::Kind::CBracket, true)?;
+                Ok(tree::Node::Array(elements))
+            }
             _ => self.syntax_error(format!("Unexpected token {}", token.kind), &token),
         }
     }
@@ -472,7 +476,8 @@ impl Parser {
                 token::Kind::OParen => {
                     // function call
                     self.lexer.next();
-                    let params = self.parse_param_list()?;
+                    let params =
+                        self.parse_delimited_list(token::Kind::Comma, token::Kind::CParen, false)?;
                     self.parse_primary_expr_rest(tree::Node::CallExpr {
                         func: Box::new(start),
                         params,
@@ -494,43 +499,54 @@ impl Parser {
         }
     }
 
-    /// ```notrust
-    /// ArrayLiteral
-    ///  : "[" ArrayElements "]"
-    ///
-    /// ArrayElements
-    ///  : Expression [ "," [ ArrayElements ] ]
-    /// ```
-    fn parse_array(&mut self) -> Result<tree::Node, Error> {
-        let mut elements = Vec::new();
-        while let Some(result) = self.lexer.peek() {
-            if result?.kind == token::Kind::CBracket {
+    /// Parse a terminated, delimited list of expressions.  This is used for
+    /// parameter lists in function calls and elements in array literals.
+    fn parse_delimited_list(
+        &mut self,
+        delimiter: token::Kind,
+        terminator: token::Kind,
+        allow_trailing_delimiter: bool,
+    ) -> Result<Vec<tree::Node>, Error> {
+        let mut list = Vec::new();
+
+        // In the simplest case, we immediately see the terminator.
+        // That means we are already finished and return an empty list.
+        if let Some(Ok(token)) = self.lexer.peek() {
+            if token.kind == terminator {
                 self.lexer.next();
-                break;
-            } else {
-                elements.push(self.parse_expr(&[token::Kind::Comma, token::Kind::CBracket])?);
+                return Ok(list);
             }
         }
-        Ok(tree::Node::Array(elements))
-    }
 
-    fn parse_param_list(&mut self) -> Result<Vec<tree::Node>, Error> {
-        let mut params = Vec::new();
-        while let Some(result) = self.lexer.peek() {
-            match result?.kind {
-                token::Kind::CParen => {
-                    self.lexer.next();
-                    break;
-                }
-                _ => {
-                    params.push(self.parse_expr(&[token::Kind::Comma, token::Kind::CParen])?);
-                    if self.lexer.current().unwrap().kind == token::Kind::CParen {
+        // now we know the list must contain at least one item
+        while self.lexer.peek().is_some() {
+            list.push(self.parse_expr(&[delimiter, terminator])?);
+
+            let current = self.lexer.current().unwrap();
+            if current.kind == terminator {
+                // this is the end of the list, we are finished
+                break;
+            } else if current.kind == delimiter {
+                // depending on whether trailing delimiters are allowed,
+                // this might still be the end of the list
+                if let Some(Ok(token)) = self.lexer.peek() {
+                    if token.kind == terminator && allow_trailing_delimiter {
+                        // so we saw a trailing delimiter followed by the
+                        // terminator *and* trailing delimiters are allowed;
+                        // this means we are finished here
+                        self.lexer.next();
                         break;
                     }
                 }
+            } else {
+                // this should never happen since parse_expr() always returns
+                // with the current token kind being one of the ones specified
+                // (otherwise it would return a syntax error, in which case we
+                // wouldn't even reach this entire if block in the first place)
+                panic!("parse_expr() ended with an illegal token");
             }
         }
-        Ok(params)
+        Ok(list)
     }
 
     /// Ensure that the `scope` stack contains a certain scope.
diff --git a/src/ast/tree.rs b/src/ast/tree.rs
index 4c80c43..1665d7a 100644
--- a/src/ast/tree.rs
+++ b/src/ast/tree.rs
@@ -120,7 +120,11 @@ impl Node {
                     p.visit(cb, depth);
                 }
             }
-            Node::IfStmt { condition, then_block, else_block} => {
+            Node::IfStmt {
+                condition,
+                then_block,
+                else_block,
+            } => {
                 condition.visit(cb, depth);
                 then_block.visit(cb, depth);
                 if let Some(eb) = else_block {
@@ -174,7 +178,11 @@ impl fmt::Display for Node {
                 Node::Array(_) => "<array>",
                 Node::ArrayExpr { array, index } => "<array-access>",
                 Node::CallExpr { func, params } => "<call>",
-                Node::IfStmt { condition, then_block, else_block } => "<if>",
+                Node::IfStmt {
+                    condition,
+                    then_block,
+                    else_block,
+                } => "<if>",
                 Node::UnaryExpr { op, node } => op.raw(),
                 Node::BinaryExpr { op, lhs, rhs } => op.raw(),
                 Node::TypeExpr(_) => "type",