From 473e40e8a43e50d975d2e4f56f38b74c0be333df Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Sat, 7 Mar 2020 10:16:20 +0800 Subject: [PATCH] Catch more invalid LHS for assignments. --- src/engine.rs | 62 ++++++++------ src/parser.rs | 226 +++++++++++++++++++++++--------------------------- 2 files changed, 138 insertions(+), 150 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index 53cb3ddc..c0ccad18 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -199,7 +199,7 @@ impl Engine<'_> { } // xxx.lhs[idx_expr] - Expr::Index(lhs, idx_expr) => { + Expr::Index(lhs, idx_expr, idx_pos) => { let idx = self.eval_index_value(scope, idx_expr)?; let (lhs_value, _) = match lhs.as_ref() { @@ -213,11 +213,12 @@ impl Engine<'_> { expr => return Err(EvalAltResult::ErrorDotExpr(expr.position())), }; - Self::get_indexed_value(lhs_value, idx, idx_expr.position()).map(|(v, _)| v) + Self::get_indexed_value(lhs_value, idx, idx_expr.position(), *idx_pos) + .map(|(v, _)| v) } // xxx.lhs.rhs - Expr::Dot(lhs, rhs) => match lhs.as_ref() { + Expr::Dot(lhs, rhs, _) => match lhs.as_ref() { // xxx.id.rhs Expr::Identifier(id, pos) => { let get_fn_name = format!("get${}", id); @@ -226,7 +227,7 @@ impl Engine<'_> { .and_then(|mut v| self.get_dot_val_helper(scope, v.as_mut(), rhs)) } // xxx.lhs[idx_expr].rhs - Expr::Index(lhs, idx_expr) => { + Expr::Index(lhs, idx_expr, idx_pos) => { let idx = self.eval_index_value(scope, idx_expr)?; let (lhs_value, _) = match lhs.as_ref() { @@ -240,7 +241,7 @@ impl Engine<'_> { expr => return Err(EvalAltResult::ErrorDotExpr(expr.position())), }; - Self::get_indexed_value(lhs_value, idx, idx_expr.position()).and_then( + Self::get_indexed_value(lhs_value, idx, idx_expr.position(), *idx_pos).and_then( |(mut value, _)| self.get_dot_val_helper(scope, value.as_mut(), rhs), ) } @@ -282,7 +283,8 @@ impl Engine<'_> { fn get_indexed_value( val: Dynamic, idx: i64, - pos: Position, + val_pos: Position, + idx_pos: Position, ) -> Result<(Dynamic, IndexSourceType), EvalAltResult> { if val.is::() { // val_array[idx] @@ -292,9 +294,9 @@ impl Engine<'_> { arr.get(idx as usize) .cloned() .map(|v| (v, IndexSourceType::Array)) - .ok_or_else(|| EvalAltResult::ErrorArrayBounds(arr.len(), idx, pos)) + .ok_or_else(|| EvalAltResult::ErrorArrayBounds(arr.len(), idx, val_pos)) } else { - Err(EvalAltResult::ErrorArrayBounds(arr.len(), idx, pos)) + Err(EvalAltResult::ErrorArrayBounds(arr.len(), idx, val_pos)) } } else if val.is::() { // val_string[idx] @@ -304,17 +306,19 @@ impl Engine<'_> { s.chars() .nth(idx as usize) .map(|ch| (ch.into_dynamic(), IndexSourceType::String)) - .ok_or_else(|| EvalAltResult::ErrorStringBounds(s.chars().count(), idx, pos)) + .ok_or_else(|| { + EvalAltResult::ErrorStringBounds(s.chars().count(), idx, val_pos) + }) } else { Err(EvalAltResult::ErrorStringBounds( s.chars().count(), idx, - pos, + val_pos, )) } } else { // Error - cannot be indexed - Err(EvalAltResult::ErrorIndexingType(pos)) + Err(EvalAltResult::ErrorIndexingType(idx_pos)) } } @@ -324,6 +328,7 @@ impl Engine<'_> { scope: &mut Scope, lhs: &'a Expr, idx_expr: &Expr, + idx_pos: Position, ) -> Result<(IndexSourceType, Option<(&'a str, usize)>, usize, Dynamic), EvalAltResult> { let idx = self.eval_index_value(scope, idx_expr)?; @@ -332,7 +337,7 @@ impl Engine<'_> { Expr::Identifier(id, _) => Self::search_scope( scope, &id, - |val| Self::get_indexed_value(val, idx, idx_expr.position()), + |val| Self::get_indexed_value(val, idx, idx_expr.position(), idx_pos), lhs.position(), ) .map(|(src_idx, (val, src_type))| { @@ -340,8 +345,13 @@ impl Engine<'_> { }), // (expr)[idx_expr] - expr => Self::get_indexed_value(self.eval_expr(scope, expr)?, idx, idx_expr.position()) - .map(|(val, _)| (IndexSourceType::Expression, None, idx as usize, val)), + expr => Self::get_indexed_value( + self.eval_expr(scope, expr)?, + idx, + idx_expr.position(), + idx_pos, + ) + .map(|(val, _)| (IndexSourceType::Expression, None, idx as usize, val)), } } @@ -412,9 +422,9 @@ impl Engine<'_> { } // lhs[idx_expr].??? - Expr::Index(lhs, idx_expr) => { + Expr::Index(lhs, idx_expr, idx_pos) => { let (src_type, src, idx, mut target) = - self.eval_index_expr(scope, lhs, idx_expr)?; + self.eval_index_expr(scope, lhs, idx_expr, *idx_pos)?; let value = self.get_dot_val_helper(scope, target.as_mut(), dot_rhs); // In case the expression mutated `target`, we need to reassign it because @@ -457,7 +467,7 @@ impl Engine<'_> { } // xxx.lhs.rhs - Expr::Dot(lhs, rhs) => match lhs.as_ref() { + Expr::Dot(lhs, rhs, _) => match lhs.as_ref() { Expr::Identifier(id, pos) => { let get_fn_name = format!("get${}", id); @@ -502,9 +512,9 @@ impl Engine<'_> { } // lhs[idx_expr].??? - Expr::Index(lhs, idx_expr) => { + Expr::Index(lhs, idx_expr, idx_pos) => { let (src_type, src, idx, mut target) = - self.eval_index_expr(scope, lhs, idx_expr)?; + self.eval_index_expr(scope, lhs, idx_expr, *idx_pos)?; let value = self.set_dot_val_helper(target.as_mut(), dot_rhs, source_val); // In case the expression mutated `target`, we need to reassign it because @@ -534,12 +544,12 @@ impl Engine<'_> { Expr::Identifier(id, pos) => { Self::search_scope(scope, id, Ok, *pos).map(|(_, val)| val) } - Expr::Index(lhs, idx_expr) => self - .eval_index_expr(scope, lhs, idx_expr) + Expr::Index(lhs, idx_expr, idx_pos) => self + .eval_index_expr(scope, lhs, idx_expr, *idx_pos) .map(|(_, _, _, x)| x), // lhs = rhs - Expr::Assignment(lhs, rhs) => { + Expr::Assignment(lhs, rhs, _) => { let rhs_val = self.eval_expr(scope, rhs)?; match lhs.as_ref() { @@ -554,9 +564,9 @@ impl Engine<'_> { } // idx_lhs[idx_expr] = rhs - Expr::Index(idx_lhs, idx_expr) => { + Expr::Index(idx_lhs, idx_expr, idx_pos) => { let (src_type, src, idx, _) = - self.eval_index_expr(scope, idx_lhs, idx_expr)?; + self.eval_index_expr(scope, idx_lhs, idx_expr, *idx_pos)?; if let Some((id, src_idx)) = src { Ok(Self::update_indexed_variable_in_scope( @@ -570,7 +580,7 @@ impl Engine<'_> { } // dot_lhs.dot_rhs = rhs - Expr::Dot(dot_lhs, dot_rhs) => { + Expr::Dot(dot_lhs, dot_rhs, _) => { self.set_dot_val(scope, dot_lhs, dot_rhs, rhs_val) } @@ -579,7 +589,7 @@ impl Engine<'_> { } } - Expr::Dot(lhs, rhs) => self.get_dot_val(scope, lhs, rhs), + Expr::Dot(lhs, rhs, _) => self.get_dot_val(scope, lhs, rhs), Expr::Array(contents, _) => { let mut arr = Vec::new(); diff --git a/src/parser.rs b/src/parser.rs index 18052ee9..d01ecdce 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -130,9 +130,9 @@ pub enum Expr { CharConstant(char, Position), StringConstant(String, Position), FunctionCall(String, Vec, Option, Position), - Assignment(Box, Box), - Dot(Box, Box), - Index(Box, Box), + Assignment(Box, Box, Position), + Dot(Box, Box, Position), + Index(Box, Box, Position), Array(Vec, Position), And(Box, Box), Or(Box, Box), @@ -155,9 +155,9 @@ impl Expr { | Expr::False(pos) | Expr::Unit(pos) => *pos, - Expr::Index(e, _) - | Expr::Assignment(e, _) - | Expr::Dot(e, _) + Expr::Index(e, _, _) + | Expr::Assignment(e, _, _) + | Expr::Dot(e, _, _) | Expr::And(e, _) | Expr::Or(e, _) => e.position(), } @@ -1103,11 +1103,12 @@ fn parse_call_expr<'a>( fn parse_index_expr<'a>( lhs: Box, input: &mut Peekable>, + pos: Position, ) -> Result { parse_expr(input).and_then(|idx_expr| match input.peek() { Some(&(Token::RightBracket, _)) => { input.next(); - return Ok(Expr::Index(lhs, Box::new(idx_expr))); + return Ok(Expr::Index(lhs, Box::new(idx_expr), pos)); } Some(&(_, pos)) => { return Err(ParseError::new( @@ -1134,9 +1135,9 @@ fn parse_ident_expr<'a>( input.next(); parse_call_expr(id, input, begin) } - Some(&(Token::LeftBracket, _)) => { + Some(&(Token::LeftBracket, pos)) => { input.next(); - parse_index_expr(Box::new(Expr::Identifier(id, begin)), input) + parse_index_expr(Box::new(Expr::Identifier(id, begin)), input, pos) } Some(_) => Ok(Expr::Identifier(id, begin)), None => Ok(Expr::Identifier(id, Position::eof())), @@ -1225,9 +1226,9 @@ fn parse_primary<'a>(input: &mut Peekable>) -> Result(input: &mut Peekable>) -> Result Result { - match lhs { - // Only assignments to a variable, and index erxpression and a dot expression is valid LHS - Expr::Identifier(_, _) | Expr::Index(_, _) | Expr::Dot(_, _) => { - Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs))) +fn parse_assignment(lhs: Expr, rhs: Expr, pos: Position) -> Result { + fn all_identifiers(expr: &Expr) -> (bool, Position) { + match expr { + // variable + Expr::Identifier(_, pos) => (true, *pos), + // indexing + Expr::Index(lhs, _, idx_pos) => match lhs.as_ref() { + // variable[x] + &Expr::Identifier(_, pos) => (true, pos), + // all other indexing is invalid + _ => (false, *idx_pos), + }, + // variable.prop.prop.prop... + Expr::Dot(lhs, rhs, _) => match lhs.as_ref() { + // variable.prop + &Expr::Identifier(_, pos) => { + let r = all_identifiers(rhs); + (r.0, if r.0 { pos } else { r.1 }) + } + // all other property access is invalid + _ => (false, lhs.position()), + }, + // everything else is invalid + _ => (false, expr.position()), } + } - // All other LHS cannot be assigned to - _ => Err(ParseError::new( - PERR::AssignmentToInvalidLHS, - lhs.position(), - )), + let r = all_identifiers(&lhs); + + if r.0 { + Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)) + } else { + Err(ParseError::new(PERR::AssignmentToInvalidLHS, r.1)) } } @@ -1322,32 +1344,24 @@ fn parse_binary_op<'a>( } Token::Divide => Expr::FunctionCall("/".into(), vec![current_lhs, rhs], None, pos), - Token::Equals => parse_assignment(current_lhs, rhs)?, + Token::Equals => parse_assignment(current_lhs, rhs, pos)?, Token::PlusAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "+".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("+".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::MinusAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "-".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("-".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } - Token::Period => Expr::Dot(Box::new(current_lhs), Box::new(rhs)), + Token::Period => Expr::Dot(Box::new(current_lhs), Box::new(rhs), pos), // Comparison operators default to false when passed invalid operands Token::EqualsTo => Expr::FunctionCall( @@ -1392,63 +1406,43 @@ fn parse_binary_op<'a>( Token::XOr => Expr::FunctionCall("^".into(), vec![current_lhs, rhs], None, pos), Token::OrAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "|".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("|".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::AndAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "&".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("&".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::XOrAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "^".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("^".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::MultiplyAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "*".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("*".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::DivideAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "/".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("/".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::Pipe => Expr::FunctionCall("|".into(), vec![current_lhs, rhs], None, pos), Token::LeftShift => { @@ -1459,27 +1453,19 @@ fn parse_binary_op<'a>( } Token::LeftShiftAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "<<".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("<<".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::RightShiftAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - ">>".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall(">>".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::Ampersand => { Expr::FunctionCall("&".into(), vec![current_lhs, rhs], None, pos) @@ -1487,28 +1473,20 @@ fn parse_binary_op<'a>( Token::Modulo => Expr::FunctionCall("%".into(), vec![current_lhs, rhs], None, pos), Token::ModuloAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "%".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("%".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } Token::PowerOf => Expr::FunctionCall("~".into(), vec![current_lhs, rhs], None, pos), Token::PowerOfAssign => { let lhs_copy = current_lhs.clone(); - Expr::Assignment( - Box::new(current_lhs), - Box::new(Expr::FunctionCall( - "~".into(), - vec![lhs_copy, rhs], - None, - pos, - )), - ) + parse_assignment( + current_lhs, + Expr::FunctionCall("~".into(), vec![lhs_copy, rhs], None, pos), + pos, + )? } token => { return Err(ParseError::new( @@ -1605,8 +1583,8 @@ fn parse_var<'a>(input: &mut Peekable>) -> Result { input.next(); - let initializer = parse_expr(input)?; - Ok(Stmt::Let(name, Some(Box::new(initializer)), pos)) + let init_value = parse_expr(input)?; + Ok(Stmt::Let(name, Some(Box::new(init_value)), pos)) } _ => Ok(Stmt::Let(name, None, pos)), }