Properly detect invalid assignment LHS at compile time.

This commit is contained in:
Stephen Chung 2020-03-07 17:32:15 +08:00
parent a8be700b9f
commit d055638e83
4 changed files with 90 additions and 59 deletions

View File

@ -17,6 +17,8 @@ pub type FnCallArgs<'a> = Vec<&'a mut Variant>;
const KEYWORD_PRINT: &'static str = "print";
const KEYWORD_DEBUG: &'static str = "debug";
const KEYWORD_TYPE_OF: &'static str = "type_of";
const FUNC_GETTER: &'static str = "get$";
const FUNC_SETTER: &'static str = "set$";
#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
enum IndexSourceType {
@ -153,6 +155,12 @@ impl Engine<'_> {
} else if let Some(val) = def_value {
// Return default value
Ok(val.clone())
} else if spec.name.starts_with(FUNC_GETTER) || spec.name.starts_with(FUNC_SETTER) {
// Getter or setter
Err(EvalAltResult::ErrorDotExpr(
"- invalid property access".to_string(),
pos,
))
} else {
let types_list = args
.iter()
@ -193,7 +201,7 @@ impl Engine<'_> {
// xxx.id
Expr::Identifier(id, pos) => {
let get_fn_name = format!("get${}", id);
let get_fn_name = format!("{}{}", FUNC_GETTER, id);
self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)
}
@ -204,16 +212,18 @@ impl Engine<'_> {
let (lhs_value, _) = match lhs.as_ref() {
Expr::Identifier(id, pos) => {
let get_fn_name = format!("get${}", id);
let get_fn_name = format!("{}{}", FUNC_GETTER, id);
(
self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)?,
*pos,
)
}
expr => return Err(EvalAltResult::ErrorDotExpr(expr.position())),
expr => {
return Err(EvalAltResult::ErrorDotExpr("".to_string(), expr.position()))
}
};
Self::get_indexed_value(lhs_value, idx, idx_expr.position(), *idx_pos)
self.get_indexed_value(lhs_value, idx, idx_expr.position(), *idx_pos)
.map(|(v, _)| v)
}
@ -221,7 +231,7 @@ impl Engine<'_> {
Expr::Dot(lhs, rhs, _) => match lhs.as_ref() {
// xxx.id.rhs
Expr::Identifier(id, pos) => {
let get_fn_name = format!("get${}", id);
let get_fn_name = format!("{}{}", FUNC_GETTER, id);
self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)
.and_then(|mut v| self.get_dot_val_helper(scope, v.as_mut(), rhs))
@ -232,25 +242,34 @@ impl Engine<'_> {
let (lhs_value, _) = match lhs.as_ref() {
Expr::Identifier(id, pos) => {
let get_fn_name = format!("get${}", id);
let get_fn_name = format!("{}{}", FUNC_GETTER, id);
(
self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)?,
*pos,
)
}
expr => return Err(EvalAltResult::ErrorDotExpr(expr.position())),
expr => {
return Err(EvalAltResult::ErrorDotExpr(
"".to_string(),
expr.position(),
))
}
};
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),
)
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)
})
}
// Syntax error
_ => Err(EvalAltResult::ErrorDotExpr(lhs.position())),
_ => Err(EvalAltResult::ErrorDotExpr("".to_string(), lhs.position())),
},
// Syntax error
_ => Err(EvalAltResult::ErrorDotExpr(dot_rhs.position())),
_ => Err(EvalAltResult::ErrorDotExpr(
"".to_string(),
dot_rhs.position(),
)),
}
}
@ -281,6 +300,7 @@ impl Engine<'_> {
/// Get the value at the indexed position of a base type
fn get_indexed_value(
&self,
val: Dynamic,
idx: i64,
val_pos: Position,
@ -318,7 +338,10 @@ impl Engine<'_> {
}
} else {
// Error - cannot be indexed
Err(EvalAltResult::ErrorIndexingType(idx_pos))
Err(EvalAltResult::ErrorIndexingType(
self.map_type_name(val.type_name()).to_string(),
idx_pos,
))
}
}
@ -337,7 +360,7 @@ impl Engine<'_> {
Expr::Identifier(id, _) => Self::search_scope(
scope,
&id,
|val| Self::get_indexed_value(val, idx, idx_expr.position(), idx_pos),
|val| self.get_indexed_value(val, idx, idx_expr.position(), idx_pos),
lhs.position(),
)
.map(|(src_idx, (val, src_type))| {
@ -345,13 +368,12 @@ impl Engine<'_> {
}),
// (expr)[idx_expr]
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)),
expr => {
let val = self.eval_expr(scope, expr)?;
self.get_indexed_value(val, idx, idx_expr.position(), idx_pos)
.map(|(value, _)| (IndexSourceType::Expression, None, idx as usize, value))
}
}
}
@ -456,7 +478,7 @@ impl Engine<'_> {
match dot_rhs {
// xxx.id
Expr::Identifier(id, pos) => {
let set_fn_name = format!("set${}", id);
let set_fn_name = format!("{}{}", FUNC_SETTER, id);
self.call_fn_raw(
&set_fn_name,
@ -469,7 +491,7 @@ impl Engine<'_> {
// xxx.lhs.rhs
Expr::Dot(lhs, rhs, _) => match lhs.as_ref() {
Expr::Identifier(id, pos) => {
let get_fn_name = format!("get${}", id);
let get_fn_name = format!("{}{}", FUNC_GETTER, id);
self.call_fn_raw(&get_fn_name, vec![this_ptr], None, *pos)
.and_then(|mut v| {
@ -477,16 +499,22 @@ impl Engine<'_> {
.map(|_| v) // Discard Ok return value
})
.and_then(|mut v| {
let set_fn_name = format!("set${}", id);
let set_fn_name = format!("{}{}", FUNC_SETTER, id);
self.call_fn_raw(&set_fn_name, vec![this_ptr, v.as_mut()], None, *pos)
})
}
_ => Err(EvalAltResult::ErrorDotExpr(lhs.position())),
_ => Err(EvalAltResult::ErrorDotExpr(
"for assignment".to_string(),
lhs.position(),
)),
},
// Syntax error
_ => Err(EvalAltResult::ErrorDotExpr(dot_rhs.position())),
_ => Err(EvalAltResult::ErrorDotExpr(
"for assignment".to_string(),
dot_rhs.position(),
)),
}
}
@ -530,7 +558,10 @@ impl Engine<'_> {
}
// Syntax error
_ => Err(EvalAltResult::ErrorDotExpr(dot_lhs.position())),
_ => Err(EvalAltResult::ErrorDotExpr(
"for assignment".to_string(),
dot_lhs.position(),
)),
}
}

View File

@ -116,7 +116,7 @@ impl Error for ParseError {
ParseErrorType::FnMissingName => "Expecting name in function declaration",
ParseErrorType::FnMissingParams(_) => "Expecting parameters in function declaration",
ParseErrorType::WrongFnDefinition => "Function definitions must be at top level and cannot be inside a block or another function",
ParseErrorType::AssignmentToInvalidLHS => "Assignment to an unsupported left-hand side expression"
ParseErrorType::AssignmentToInvalidLHS => "Cannot assign to this expression because it will only be changing a copy of the value"
}
}

View File

@ -1275,39 +1275,36 @@ fn parse_unary<'a>(input: &mut Peekable<TokenIterator<'a>>) -> Result<Expr, Pars
}
fn parse_assignment(lhs: Expr, rhs: Expr, pos: Position) -> Result<Expr, ParseError> {
fn all_identifiers(expr: &Expr) -> (bool, Position) {
fn all_dots(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),
Expr::Dot(dot_lhs, dot_rhs, _) => match dot_lhs.as_ref() {
Expr::Identifier(_, _) => all_dots(dot_rhs),
_ => (false, dot_lhs.position()),
},
// 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()),
}
}
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))
match &lhs {
Expr::Identifier(_, _) => return Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)),
Expr::Index(idx_lhs, _, _) => match idx_lhs.as_ref() {
Expr::Identifier(_, _) => {
return Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos))
}
_ => (),
},
Expr::Dot(_, _, _) => match all_dots(&lhs) {
(true, _) => return Ok(Expr::Assignment(Box::new(lhs), Box::new(rhs), pos)),
(false, pos) => return Err(ParseError::new(PERR::AssignmentToInvalidLHS, pos)),
},
_ => (),
}
return Err(ParseError::new(
PERR::AssignmentToInvalidLHS,
lhs.position(),
));
}
fn parse_op_assignment(

View File

@ -26,7 +26,7 @@ pub enum EvalAltResult {
/// Wrapped values are the current number of characters in the string and the index number.
ErrorStringBounds(usize, i64, Position),
/// Trying to index into a type that is not an array and not a string.
ErrorIndexingType(Position),
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.
@ -43,7 +43,7 @@ pub enum EvalAltResult {
/// Error reading from a script file. Wrapped value is the path of the script file.
ErrorReadingScriptFile(String, std::io::Error),
/// Inappropriate member access.
ErrorDotExpr(Position),
ErrorDotExpr(String, Position),
/// Arithmetic error encountered. Wrapped value is the error message.
ErrorArithmetic(String, Position),
/// Run-time error encountered. Wrapped value is the error message.
@ -65,7 +65,9 @@ impl Error for EvalAltResult {
}
Self::ErrorBooleanArgMismatch(_, _) => "Boolean operator expects boolean operands",
Self::ErrorIndexExpr(_) => "Indexing into an array or string expects an integer index",
Self::ErrorIndexingType(_) => "Indexing can only be performed on an array or a string",
Self::ErrorIndexingType(_, _) => {
"Indexing can only be performed on an array or a string"
}
Self::ErrorArrayBounds(_, index, _) if *index < 0 => {
"Array access expects non-negative index"
}
@ -84,7 +86,7 @@ impl Error for EvalAltResult {
}
Self::ErrorMismatchOutputType(_, _) => "Output type is incorrect",
Self::ErrorReadingScriptFile(_, _) => "Cannot read from script file",
Self::ErrorDotExpr(_) => "Malformed dot expression",
Self::ErrorDotExpr(_, _) => "Malformed dot expression",
Self::ErrorArithmetic(_, _) => "Arithmetic error",
Self::ErrorRuntime(_, _) => "Runtime error",
Self::LoopBreak => "[Not Error] Breaks out of loop",
@ -104,13 +106,14 @@ impl std::fmt::Display for EvalAltResult {
match self {
Self::ErrorFunctionNotFound(s, pos) => write!(f, "{}: '{}' ({})", desc, s, pos),
Self::ErrorVariableNotFound(s, pos) => write!(f, "{}: '{}' ({})", desc, s, pos),
Self::ErrorIndexingType(pos) => write!(f, "{} ({})", desc, pos),
Self::ErrorIndexingType(_, pos) => write!(f, "{} ({})", desc, pos),
Self::ErrorIndexExpr(pos) => write!(f, "{} ({})", desc, pos),
Self::ErrorIfGuard(pos) => write!(f, "{} ({})", desc, pos),
Self::ErrorFor(pos) => write!(f, "{} ({})", desc, pos),
Self::ErrorAssignmentToUnknownLHS(pos) => write!(f, "{} ({})", desc, pos),
Self::ErrorMismatchOutputType(s, pos) => write!(f, "{}: {} ({})", desc, s, pos),
Self::ErrorDotExpr(pos) => write!(f, "{} ({})", desc, pos),
Self::ErrorDotExpr(s, pos) if !s.is_empty() => write!(f, "{} {} ({})", desc, s, pos),
Self::ErrorDotExpr(_, pos) => write!(f, "{} ({})", desc, pos),
Self::ErrorArithmetic(s, pos) => write!(f, "{}: {} ({})", desc, s, pos),
Self::ErrorRuntime(s, pos) if s.is_empty() => write!(f, "{} ({})", desc, pos),
Self::ErrorRuntime(s, pos) => write!(f, "{}: {} ({})", desc, s, pos),