Disallow statement expressions in if and while guards to reduce code confusion.

src/engine.rs

@@ -1159,7 +1159,7 @@ impl Engine<'_> {
             Stmt::IfElse(guard, if_body, else_body) => self
                 .eval_expr(scope, guard)?
                 .downcast::<bool>()
-                .map_err(|_| EvalAltResult::ErrorIfGuard(guard.position()))
+                .map_err(|_| EvalAltResult::ErrorLogicGuard(guard.position()))
                 .and_then(|guard_val| {
                     if *guard_val {
                         self.eval_stmt(scope, if_body)
@@ -1186,7 +1186,7 @@ impl Engine<'_> {
                             return Ok(().into_dynamic());
                         }
                     }
-                    Err(_) => return Err(EvalAltResult::ErrorIfGuard(guard.position())),
+                    Err(_) => return Err(EvalAltResult::ErrorLogicGuard(guard.position())),
                 }
             },
 

src/error.rs

@@ -47,7 +47,7 @@ pub enum ParseErrorType {
     /// Error in the script text. Wrapped value is the error message.
     BadInput(String),
     /// The script ends prematurely.
-    InputPastEndOfFile,
+    UnexpectedEOF,
     /// An unknown operator is encountered. Wrapped value is the operator.
     UnknownOperator(String),
     /// An open `(` is missing the corresponding closing `)`.
@@ -72,6 +72,8 @@ pub enum ParseErrorType {
     ForbiddenConstantExpr(String),
     /// Missing a variable name after the `let`, `const` or `for` keywords.
     VariableExpected,
+    /// Missing an expression.
+    ExprExpected(String),
     /// A `for` statement is missing the `in` keyword.
     MissingIn,
     /// Defining a function `fn` in an appropriate place (e.g. inside another function).
@@ -119,7 +121,7 @@ impl ParseError {
     pub(crate) fn desc(&self) -> &str {
         match self.0 {
             ParseErrorType::BadInput(ref p) => p,
-            ParseErrorType::InputPastEndOfFile => "Script is incomplete",
+            ParseErrorType::UnexpectedEOF => "Script is incomplete",
             ParseErrorType::UnknownOperator(_) => "Unknown operator",
             ParseErrorType::MissingRightParen(_) => "Expecting ')'",
             ParseErrorType::MissingLeftBrace => "Expecting '{'",
@@ -134,6 +136,7 @@ impl ParseError {
             ParseErrorType::ForbiddenConstantExpr(_) => "Expecting a constant",
             ParseErrorType::MissingIn => "Expecting 'in'",
             ParseErrorType::VariableExpected => "Expecting name of a variable",
+            ParseErrorType::ExprExpected(_) => "Expecting an expression",
             #[cfg(not(feature = "no_function"))]
             ParseErrorType::FnMissingName => "Expecting name in function declaration",
             #[cfg(not(feature = "no_function"))]
@@ -168,6 +171,8 @@ impl fmt::Display for ParseError {
                 write!(f, "{}", if s.is_empty() { self.desc() } else { s })?
             }
 
+            ParseErrorType::ExprExpected(ref s) => write!(f, "Expecting {} expression", s)?,
+
             #[cfg(not(feature = "no_function"))]
             ParseErrorType::FnMissingParams(ref s) => {
                 write!(f, "Expecting parameters for function '{}'", s)?

src/parser.rs

@@ -1601,23 +1601,21 @@ fn parse_array_literal<'a>(
 
 /// Parse a primary expression.
 fn parse_primary<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<Expr, ParseError> {
-    // { - block statement as expression
+    let token = match input
-    match input.peek() {
+        .peek()
-        Some((Token::LeftBrace, pos)) => {
+        .ok_or_else(|| ParseError::new(PERR::UnexpectedEOF, Position::eof()))?
+    {
+        // { - block statement as expression
+        (Token::LeftBrace, pos) => {
             let pos = *pos;
             return parse_block(input, false).map(|block| Expr::Stmt(Box::new(block), pos));
         }
-        _ => (),
+        _ => input.next().expect("should be a token"),
-    }
+    };
-
-    let token = input.next();
 
     let mut can_be_indexed = false;
 
-    #[allow(unused_mut)]
+    let mut root_expr = match token {
-    let mut root_expr = match token
-        .ok_or_else(|| ParseError::new(PERR::InputPastEndOfFile, Position::eof()))?
-    {
         #[cfg(not(feature = "no_float"))]
         (Token::FloatConstant(x), pos) => Ok(Expr::FloatConstant(x, pos)),
 
@@ -1666,7 +1664,7 @@ fn parse_primary<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<Expr, Pa
 fn parse_unary<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<Expr, ParseError> {
     match input
         .peek()
-        .ok_or_else(|| ParseError::new(PERR::InputPastEndOfFile, Position::eof()))?
+        .ok_or_else(|| ParseError::new(PERR::UnexpectedEOF, Position::eof()))?
     {
         // -expr
         (Token::UnaryMinus, pos) => {
@@ -1978,6 +1976,22 @@ fn parse_expr<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<Expr, Parse
     parse_binary_op(input, 1, lhs)
 }
 
+/// Make sure that the expression is not a statement expression (i.e. wrapped in {})
+fn ensure_not_statement_expr<'a>(
+    input: &mut Peekable<TokenIterator<'a>>,
+    type_name: &str,
+) -> Result<(), ParseError> {
+    match input
+        .peek()
+        .ok_or_else(|| ParseError(PERR::ExprExpected(type_name.to_string()), Position::eof()))?
+    {
+        // Disallow statement expressions
+        (Token::LeftBrace, pos) => Err(ParseError(PERR::ExprExpected(type_name.to_string()), *pos)),
+        // No need to check for others at this time - leave it for the expr parser
+        _ => Ok(()),
+    }
+}
+
 /// Parse an if statement.
 fn parse_if<'a>(
     input: &mut Peekable<TokenIterator<'a>>,
@@ -1986,19 +2000,20 @@ fn parse_if<'a>(
     // if ...
     input.next();
 
-    // if guard { body }
+    // if guard { if_body }
+    ensure_not_statement_expr(input, "a boolean")?;
     let guard = parse_expr(input)?;
     let if_body = parse_block(input, breakable)?;
 
-    // if guard { body } else ...
+    // if guard { if_body } else ...
     let else_body = if matches!(input.peek(), Some((Token::Else, _))) {
         input.next();
 
         Some(Box::new(if matches!(input.peek(), Some((Token::If, _))) {
-            // if guard { body } else if ...
+            // if guard { if_body } else if ...
             parse_if(input, breakable)?
         } else {
-            // if guard { body } else { else-body }
+            // if guard { if_body } else { else-body }
             parse_block(input, breakable)?
         }))
     } else {
@@ -2014,6 +2029,7 @@ fn parse_while<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<Stmt, Pars
     input.next();
 
     // while guard { body }
+    ensure_not_statement_expr(input, "a boolean")?;
     let guard = parse_expr(input)?;
     let body = parse_block(input, true)?;
 
@@ -2061,6 +2077,7 @@ fn parse_for<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<Stmt, ParseE
     }
 
     // for name in expr { body }
+    ensure_not_statement_expr(input, "a boolean")?;
     let expr = parse_expr(input)?;
     let body = parse_block(input, true)?;
 
@@ -2189,10 +2206,12 @@ fn parse_stmt<'a>(
     input: &mut Peekable<TokenIterator<'a>>,
     breakable: bool,
 ) -> Result<Stmt, ParseError> {
-    match input
+    let token = match input.peek() {
-        .peek()
+        Some(token) => token,
-        .ok_or_else(|| ParseError::new(PERR::InputPastEndOfFile, Position::eof()))?
+        None => return Ok(Stmt::Noop(Position::eof())),
-    {
+    };
+
+    match token {
         // Semicolon - empty statement
         (Token::SemiColon, pos) => Ok(Stmt::Noop(*pos)),
 
@@ -2244,10 +2263,7 @@ fn parse_stmt<'a>(
 /// Parse a function definition.
 #[cfg(not(feature = "no_function"))]
 fn parse_fn<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<FnDef, ParseError> {
-    let pos = input
+    let pos = input.next().expect("should be fn").1;
-        .next()
-        .ok_or_else(|| ParseError::new(PERR::InputPastEndOfFile, Position::eof()))?
-        .1;
 
     let name = match input
         .next()

src/result.rs

@@ -40,8 +40,8 @@ pub enum EvalAltResult {
     ErrorIndexingType(String, Position),
     /// Trying to index into an array or string with an index that is not `i64`.
     ErrorIndexExpr(Position),
-    /// The guard expression in an `if` statement does not return a boolean value.
+    /// The guard expression in an `if` or `while` statement does not return a boolean value.
-    ErrorIfGuard(Position),
+    ErrorLogicGuard(Position),
     /// The `for` statement encounters a type that is not an iterator.
     ErrorFor(Position),
     /// Usage of an unknown variable. Wrapped value is the name of the variable.
@@ -93,7 +93,7 @@ impl EvalAltResult {
             }
             Self::ErrorStringBounds(0, _, _) => "Indexing of empty string",
             Self::ErrorStringBounds(_, _, _) => "String index out of bounds",
-            Self::ErrorIfGuard(_) => "If guard expects boolean expression",
+            Self::ErrorLogicGuard(_) => "Boolean expression expected",
             Self::ErrorFor(_) => "For loop expects array or range",
             Self::ErrorVariableNotFound(_, _) => "Variable not found",
             Self::ErrorAssignmentToUnknownLHS(_) => {
@@ -123,7 +123,7 @@ impl fmt::Display for EvalAltResult {
             Self::ErrorVariableNotFound(s, pos) => write!(f, "{}: '{}' ({})", desc, s, pos),
             Self::ErrorIndexingType(_, pos) => write!(f, "{} ({})", desc, pos),
             Self::ErrorIndexExpr(pos) => write!(f, "{} ({})", desc, pos),
-            Self::ErrorIfGuard(pos) => write!(f, "{} ({})", desc, pos),
+            Self::ErrorLogicGuard(pos) => write!(f, "{} ({})", desc, pos),
             Self::ErrorFor(pos) => write!(f, "{} ({})", desc, pos),
             Self::ErrorAssignmentToUnknownLHS(pos) => write!(f, "{} ({})", desc, pos),
             Self::ErrorAssignmentToConstant(s, pos) => write!(f, "{}: '{}' ({})", desc, s, pos),
@@ -222,7 +222,7 @@ impl EvalAltResult {
             | Self::ErrorStringBounds(_, _, pos)
             | Self::ErrorIndexingType(_, pos)
             | Self::ErrorIndexExpr(pos)
-            | Self::ErrorIfGuard(pos)
+            | Self::ErrorLogicGuard(pos)
             | Self::ErrorFor(pos)
             | Self::ErrorVariableNotFound(_, pos)
             | Self::ErrorAssignmentToUnknownLHS(pos)
@@ -250,7 +250,7 @@ impl EvalAltResult {
             | Self::ErrorStringBounds(_, _, ref mut pos)
             | Self::ErrorIndexingType(_, ref mut pos)
             | Self::ErrorIndexExpr(ref mut pos)
-            | Self::ErrorIfGuard(ref mut pos)
+            | Self::ErrorLogicGuard(ref mut pos)
             | Self::ErrorFor(ref mut pos)
             | Self::ErrorVariableNotFound(_, ref mut pos)
             | Self::ErrorAssignmentToUnknownLHS(ref mut pos)