Catch more invalid LHS for assignments.

This commit is contained in:
Stephen Chung 2020-03-07 10:16:20 +08:00
parent 024133ae2d
commit 473e40e8a4
2 changed files with 138 additions and 150 deletions

View File

@ -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::<Array>() {
// 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::<String>() {
// 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,7 +345,12 @@ impl Engine<'_> {
}),
// (expr)[idx_expr]
expr => Self::get_indexed_value(self.eval_expr(scope, expr)?, idx, idx_expr.position())
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();

View File

@ -130,9 +130,9 @@ pub enum Expr {
CharConstant(char, Position),
StringConstant(String, Position),
FunctionCall(String, Vec<Expr>, Option<Dynamic>, Position),
Assignment(Box<Expr>, Box<Expr>),
Dot(Box<Expr>, Box<Expr>),
Index(Box<Expr>, Box<Expr>),
Assignment(Box<Expr>, Box<Expr>, Position),
Dot(Box<Expr>, Box<Expr>, Position),
Index(Box<Expr>, Box<Expr>, Position),
Array(Vec<Expr>, Position),
And(Box<Expr>, Box<Expr>),
Or(Box<Expr>, Box<Expr>),
@ -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<Expr>,
input: &mut Peekable<TokenIterator<'a>>,
pos: Position,
) -> Result<Expr, ParseError> {
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<TokenIterator<'a>>) -> Result<Expr, Pa
}
// Tail processing all possible indexing
while let Some(&(Token::LeftBracket, _)) = input.peek() {
while let Some(&(Token::LeftBracket, pos)) = input.peek() {
input.next();
root_expr = parse_index_expr(Box::new(root_expr), input)?;
root_expr = parse_index_expr(Box::new(root_expr), input, pos)?;
}
Ok(root_expr)
@ -1263,18 +1264,39 @@ fn parse_unary<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<Expr, Pars
}
}
fn parse_assignment(lhs: Expr, rhs: Expr) -> Result<Expr, ParseError> {
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<Expr, ParseError> {
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,
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,
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,
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,
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,
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,
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,
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,
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,
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,
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,
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<TokenIterator<'a>>) -> Result<Stmt, ParseE
match input.peek() {
Some(&(Token::Equals, _)) => {
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)),
}