From f3c0609377de54df7f3bce45da038dace96d01d6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Thu, 7 May 2020 12:25:09 +0800 Subject: [PATCH] Catch more assignment errors at parse time. --- src/error.rs | 14 ++++++------ src/parser.rs | 56 +++++++++++++++++++++++++++++++++++++--------- tests/constants.rs | 6 ++--- 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/error.rs b/src/error.rs index b08e5975..eed98271 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,7 +5,7 @@ use crate::token::Position; use crate::stdlib::{boxed::Box, char, error::Error, fmt, string::String}; /// Error when tokenizing the script text. -#[derive(Debug, Eq, PartialEq, Hash, Clone)] +#[derive(Debug, Eq, PartialEq, Clone, Hash)] pub enum LexError { /// An unexpected character is encountered when tokenizing the script text. UnexpectedChar(char), @@ -44,7 +44,7 @@ impl fmt::Display for LexError { /// Some errors never appear when certain features are turned on. /// They still exist so that the application can turn features on and off without going through /// massive code changes to remove/add back enum variants in match statements. -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, Eq, PartialEq, Clone, Hash)] pub enum ParseErrorType { /// Error in the script text. Wrapped value is the error message. BadInput(String), @@ -98,8 +98,8 @@ pub enum ParseErrorType { /// /// Never appears under the `no_function` feature. FnMissingBody(String), - /// Assignment to an inappropriate LHS (left-hand-side) expression. - AssignmentToInvalidLHS, + /// Assignment to a copy of a value. + AssignmentToCopy, /// Assignment to an a constant variable. AssignmentToConstant(String), /// Break statement not inside a loop. @@ -114,7 +114,7 @@ impl ParseErrorType { } /// Error when parsing a script. -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, Hash)] pub struct ParseError(pub(crate) ParseErrorType, pub(crate) Position); impl ParseError { @@ -147,8 +147,8 @@ impl ParseError { ParseErrorType::FnDuplicatedParam(_,_) => "Duplicated parameters in function declaration", ParseErrorType::FnMissingBody(_) => "Expecting body statement block for function declaration", ParseErrorType::WrongFnDefinition => "Function definitions must be at global level and cannot be inside a block or another function", - ParseErrorType::AssignmentToInvalidLHS => "Cannot assign to this expression", - ParseErrorType::AssignmentToConstant(_) => "Cannot assign to a constant variable.", + ParseErrorType::AssignmentToCopy => "Only a copy of the value is change with this assignment", + ParseErrorType::AssignmentToConstant(_) => "Cannot assign to a constant value.", ParseErrorType::LoopBreak => "Break statement should only be used inside a loop" } } diff --git a/src/parser.rs b/src/parser.rs index 35b2d2e8..953c12a0 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -152,11 +152,11 @@ impl AST { pub fn clear_functions(&mut self) { #[cfg(feature = "sync")] { - self.1 = Arc::new(FunctionsLib::new()); + self.1 = Arc::new(Default::default()); } #[cfg(not(feature = "sync"))] { - self.1 = Rc::new(FunctionsLib::new()); + self.1 = Rc::new(Default::default()); } } @@ -1212,15 +1212,47 @@ fn parse_unary<'a>( } } -fn parse_assignment_stmt<'a>( - input: &mut Peekable>, +fn make_assignment_stmt<'a>( stack: &mut Stack, lhs: Expr, - allow_stmt_expr: bool, + rhs: Expr, + pos: Position, ) -> Result> { - let pos = eat_token(input, Token::Equals); - let rhs = parse_expr(input, stack, allow_stmt_expr)?; - Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)) + match &lhs { + Expr::Variable(_, _, None, _) => Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)), + Expr::Variable(name, _, Some(index), var_pos) => { + match stack[(stack.len() - index.get())].1 { + ScopeEntryType::Normal => Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)), + // Constant values cannot be assigned to + ScopeEntryType::Constant => { + Err(PERR::AssignmentToConstant(name.to_string()).into_err(*var_pos)) + } + ScopeEntryType::Module => unreachable!(), + } + } + Expr::Index(idx_lhs, _, _) | Expr::Dot(idx_lhs, _, _) => match idx_lhs.as_ref() { + Expr::Variable(_, _, None, _) => { + Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)) + } + Expr::Variable(name, _, Some(index), var_pos) => { + match stack[(stack.len() - index.get())].1 { + ScopeEntryType::Normal => { + Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)) + } + // Constant values cannot be assigned to + ScopeEntryType::Constant => { + Err(PERR::AssignmentToConstant(name.to_string()).into_err(*var_pos)) + } + ScopeEntryType::Module => unreachable!(), + } + } + _ => Err(PERR::AssignmentToCopy.into_err(idx_lhs.position())), + }, + expr if expr.is_constant() => { + Err(PERR::AssignmentToConstant("".into()).into_err(lhs.position())) + } + _ => Err(PERR::AssignmentToCopy.into_err(lhs.position())), + } } /// Parse an operator-assignment expression. @@ -1231,7 +1263,11 @@ fn parse_op_assignment_stmt<'a>( allow_stmt_expr: bool, ) -> Result> { let (op, pos) = match *input.peek().unwrap() { - (Token::Equals, _) => return parse_assignment_stmt(input, stack, lhs, allow_stmt_expr), + (Token::Equals, _) => { + let pos = eat_token(input, Token::Equals); + let rhs = parse_expr(input, stack, allow_stmt_expr)?; + return make_assignment_stmt(stack, lhs, rhs, pos); + } (Token::PlusAssign, pos) => ("+", pos), (Token::MinusAssign, pos) => ("-", pos), (Token::MultiplyAssign, pos) => ("*", pos), @@ -1254,7 +1290,7 @@ fn parse_op_assignment_stmt<'a>( // lhs op= rhs -> lhs = op(lhs, rhs) let args = vec![lhs_copy, rhs]; let rhs_expr = Expr::FnCall(Box::new(op.into()), None, Box::new(args), None, pos); - Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs_expr), pos)) + make_assignment_stmt(stack, lhs, rhs_expr, pos) } /// Make a dot expression. diff --git a/tests/constants.rs b/tests/constants.rs index 55752d28..d5e0820a 100644 --- a/tests/constants.rs +++ b/tests/constants.rs @@ -1,4 +1,4 @@ -use rhai::{Engine, EvalAltResult, INT}; +use rhai::{Engine, EvalAltResult, ParseErrorType, INT}; #[test] fn test_constant() -> Result<(), Box> { @@ -8,13 +8,13 @@ fn test_constant() -> Result<(), Box> { assert!(matches!( *engine.eval::("const x = 123; x = 42;").expect_err("expects error"), - EvalAltResult::ErrorAssignmentToConstant(var, _) if var == "x" + EvalAltResult::ErrorParsing(err) if err.error_type() == &ParseErrorType::AssignmentToConstant("x".to_string()) )); #[cfg(not(feature = "no_index"))] assert!(matches!( *engine.eval::("const x = [1, 2, 3, 4, 5]; x[2] = 42;").expect_err("expects error"), - EvalAltResult::ErrorAssignmentToConstant(var, _) if var == "x" + EvalAltResult::ErrorParsing(err) if err.error_type() == &ParseErrorType::AssignmentToConstant("x".to_string()) )); Ok(())