Fix parser requiring extra semicolons in blocks

fire/src/lib.rs

@@ -538,7 +538,7 @@ mod tests {
                     { { { {
                         return "boo";
                     } } } }
-                };
+                }
 
                 "different string"
             }

xparser/src/constructs.rs

@@ -29,15 +29,15 @@ use ast::Value;
 use ast::{Ast, TypeContent};
 use location::Location;
 use location::SpanTuple;
-use nom::sequence::tuple;
 use nom::Err::Error as NomError;
 use nom::Slice;
 use nom_locate::position;
 use symbol::Symbol;
 
 use nom::{
     branch::alt, bytes::complete::take, character::complete::multispace0, combinator::opt,
-    multi::many0, sequence::delimited, sequence::pair, sequence::preceded, sequence::terminated,
+    multi::many0, multi::many1, sequence::delimited, sequence::pair, sequence::preceded,
+    sequence::terminated, sequence::tuple,
 };
 
 // FIXME: Refactor, rework, remove, depreciate
@@ -58,7 +58,7 @@ pub fn many_exprs(mut input: ParseInput) -> ParseResult<ParseInput, Vec<Ast>> {
         if input.is_empty() {
             return Ok((input, exprs));
         }
-        let (new_input, expr) = expr_semicolon(input)?;
+        let (new_input, expr) = expr_maybe_semi(input)?;
         input = new_input;
         exprs.push(expr);
     }
@@ -67,13 +67,25 @@ pub fn many_exprs(mut input: ParseInput) -> ParseResult<ParseInput, Vec<Ast>> {
 /// Parse an instruction and maybe the semicolon that follows.
 ///
 /// expr_semicolon = expr [ ';' ]
-pub fn expr_semicolon(input: ParseInput) -> ParseResult<ParseInput, Ast> {
+pub fn expr_maybe_semi(input: ParseInput) -> ParseResult<ParseInput, Ast> {
     let (input, expr) = expr(input)?;
+    let input = next(input);
     let (input, _) = opt(tokens::semicolon)(input)?;
 
     Ok((input, expr))
 }
 
+/// Parse an instruction and the semicolon that follows.
+///
+/// expr_semicolon = expr [ ';' ]
+pub fn expr_semicolon(input: ParseInput) -> ParseResult<ParseInput, Ast> {
+    let (input, expr) = expr(input)?;
+    let input = next(input);
+    let (input, _) = tokens::semicolon(input)?;
+
+    Ok((input, expr))
+}
+
 // expr = cmp ( '<' cmp | '>' cmp | '<=' cmp | '>=' cmp | '==' cmp | '!=' cmp)*
 pub fn expr(input: ParseInput) -> ParseResult<ParseInput, Ast> {
     let input = next(input);
@@ -777,23 +789,97 @@ fn return_type(input: ParseInput) -> ParseResult<ParseInput, Option<Type>> {
     }
 }
 
-/// block = '{' next inner_block
+/// block = '{' ([expr ';' | loop | if | function])* expr? '}'
 pub fn block(input: ParseInput) -> ParseResult<ParseInput, Ast> {
     let (input, start_loc) = position(input)?;
     let (input, _) = tokens::left_curly_bracket(input)?;
     let input = next(input);
-    let (input, block) = inner_block(input)?;
+
+    // Check if the block is immediately closed and early return
+    if let Ok((input, _)) = tokens::right_curly_bracket(input) {
+        let (input, end_loc) = position(input)?;
+
+        return Ok((
+            input,
+            Ast {
+                location: pos_to_loc(input, start_loc, end_loc),
+                node: Node::Block {
+                    stmts: vec![],
+                    last_is_expr: false,
+                },
+            },
+        ));
+    }
+
+    let stmt = |input| {
+        let input = next(input);
+        let (input, start_loc) = position(input)?;
+
+        let (input, stmt) = if let Ok((input, _)) = tokens::if_tok(input) {
+            unit_if(input, start_loc.into())?
+        } else if let Ok((input, kind)) =
+            alt((tokens::func_tok, tokens::test_tok, tokens::mock_tok))(input)
+        {
+            unit_func(input, kind, start_loc.into())?
+        } else if tokens::left_curly_bracket(input).is_ok() {
+            // TODO: This does not work to have a block as the last expression of another block
+            // `stmt` will parse it and return it as a statement
+            // we need to do something smarted like lifting the last statement to an expression if there was no semicolon
+
+            // we don't update `input` here so that we can just call `block`
+            block(input)?
+        } else {
+            expr_semicolon(input)?
+        };
+
+        let (input, _) = opt(tokens::semicolon)(input)?;
+
+        Ok((input, stmt))
+    };
+
+    let (input, stmts) = opt(many1(stmt))(input)?;
+    let (input, last_expr) = opt(expr)(input)?;
+    let input = next(input);
+    let (input, _) = tokens::right_curly_bracket(input)?;
+
+    let last_is_expr = last_expr.is_some();
+
     let (input, end_loc) = position(input)?;
+    let mut stmts = stmts.unwrap_or_default();
+
+    if let Some(expr) = last_expr {
+        stmts.push(expr)
+    };
 
     Ok((
         input,
         Ast {
             location: pos_to_loc(input, start_loc, end_loc),
-            node: block,
+            node: Node::Block {
+                stmts,
+                last_is_expr,
+            },
         },
     ))
 }
 
+// /// block = '{' next inner_block
+// pub fn block(input: ParseInput) -> ParseResult<ParseInput, Ast> {
+//     let (input, start_loc) = position(input)?;
+//     let (input, _) = tokens::left_curly_bracket(input)?;
+//     let input = next(input);
+//     let (input, block) = inner_block(input)?;
+//     let (input, end_loc) = position(input)?;
+
+//     Ok((
+//         input,
+//         Ast {
+//             location: pos_to_loc(input, start_loc, end_loc),
+//             node: block,
+//         },
+//     ))
+// }
+
 /// inner_block = '}'
 ///             | expr '}'                  (* The only case where block is an expr *)
 ///             | expr ';' next inner_block
@@ -819,6 +905,23 @@ fn inner_block(input: ParseInput) -> ParseResult<ParseInput, Node> {
         ));
     }
 
+    // the lines from here on out until the end of the function are problematic. when we parse
+    // the following:
+    //
+    // ```text
+    // func foo() -> ReturnValue {
+    //     if condition {}
+    //
+    //     return_value
+    // }
+    // ```
+    //
+    // then the parser will fail, as we indicate that all expresssions or statements of a block must be preceded
+    // by a semicolon. this isn't great. the code which will work with the current parser thus adds an extra
+    // semicolon at the end of the `if` block. plus, this function keeps calling itself recursively and adding
+    // at the beginning of the `stmts` vector, instead of accumulating statements.
+    // Issue #395
+
     let (input, block) = preceded(tokens::semicolon, preceded(nom_next, inner_block))(input)?;
     let (mut stmts, last_is_expr) = match block {
         Node::Block {
@@ -2295,4 +2398,51 @@ mod tests {
             TypeKind::FunctionLike(..)
         ));
     }
+
+    #[test]
+    fn block_in_block_does_not_require_extra_semicolon_395() {
+        let input = span!("func foo() { if true { 15 } else { 14 } }");
+
+        assert!(expr(input).is_ok());
+    }
+
+    #[test]
+    fn block_in_block_does_not_require_extra_semicolon_395_2() {
+        let input = span!("func foo()  { { { where x = 14; } } }");
+
+        assert!(expr(input).is_ok());
+    }
+
+    #[test]
+    fn early_return395() {
+        let input = span!("func halloween(b: bool) -> int { if b { return 14 }; 15 }");
+
+        assert!(expr(input).is_ok());
+    }
+
+    #[test]
+    fn inner_block_with_semi395() {
+        let input = span!(
+            "func halloween() -> int { func inner() -> int { { { { { return 14; } } } } }; 15 }"
+        );
+
+        assert!(expr(input).is_ok())
+    }
+
+    #[test]
+    fn inner_nested_function395() {
+        let input = span! {r#"
+            func halloween() -> string {
+                func inner() -> string {
+                    { { { {
+                        return "boo";
+                    } } } }
+                };
+
+                "different string"
+            }
+        "#};
+
+        assert!(expr(input).is_ok())
+    }
 }