Detect duplicated parameters in function definitions.

This commit is contained in:
Stephen Chung 2020-03-27 16:46:08 +08:00
parent 337a96394f
commit 796690f506
4 changed files with 48 additions and 9 deletions

View File

@ -32,7 +32,7 @@ pub type FnAny = dyn Fn(&mut FnCallArgs, Position) -> Result<Dynamic, EvalAltRes
type IteratorFn = dyn Fn(&Dynamic) -> Box<dyn Iterator<Item = Dynamic>>; type IteratorFn = dyn Fn(&Dynamic) -> Box<dyn Iterator<Item = Dynamic>>;
pub(crate) const MAX_CALL_STACK_DEPTH: usize = 50; pub(crate) const MAX_CALL_STACK_DEPTH: usize = 64;
pub(crate) const KEYWORD_PRINT: &str = "print"; pub(crate) const KEYWORD_PRINT: &str = "print";
pub(crate) const KEYWORD_DEBUG: &str = "debug"; pub(crate) const KEYWORD_DEBUG: &str = "debug";
pub(crate) const KEYWORD_DUMP_AST: &str = "dump_ast"; pub(crate) const KEYWORD_DUMP_AST: &str = "dump_ast";
@ -242,10 +242,6 @@ impl Engine<'_> {
pos: Position, pos: Position,
level: usize, level: usize,
) -> Result<Dynamic, EvalAltResult> { ) -> Result<Dynamic, EvalAltResult> {
if level >= self.max_call_stack_depth {
return Err(EvalAltResult::ErrorStackOverflow(pos));
}
// First search in script-defined functions (can override built-in) // First search in script-defined functions (can override built-in)
if let Some(fn_def) = self.fn_lib.get_function(fn_name, args.len()) { if let Some(fn_def) = self.fn_lib.get_function(fn_name, args.len()) {
let mut scope = Scope::new(); let mut scope = Scope::new();

View File

@ -82,6 +82,9 @@ pub enum ParseErrorType {
/// A function definition is missing the parameters list. Wrapped value is the function name. /// A function definition is missing the parameters list. Wrapped value is the function name.
#[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_function"))]
FnMissingParams(String), FnMissingParams(String),
/// A function definition has duplicated parameters. Wrapped values are the function name and parameter name.
#[cfg(not(feature = "no_function"))]
FnDuplicateParam(String, String),
/// A function definition is missing the body. Wrapped value is the function name. /// A function definition is missing the body. Wrapped value is the function name.
#[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_function"))]
FnMissingBody(String), FnMissingBody(String),
@ -146,6 +149,8 @@ impl ParseError {
#[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_function"))]
ParseErrorType::FnMissingParams(_) => "Expecting parameters in function declaration", ParseErrorType::FnMissingParams(_) => "Expecting parameters in function declaration",
#[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_function"))]
ParseErrorType::FnDuplicateParam(_,_) => "Duplicated parameters in function declaration",
#[cfg(not(feature = "no_function"))]
ParseErrorType::FnMissingBody(_) => "Expecting body statement block for function declaration", ParseErrorType::FnMissingBody(_) => "Expecting body statement block for function declaration",
#[cfg(not(feature = "no_function"))] #[cfg(not(feature = "no_function"))]
ParseErrorType::WrongFnDefinition => "Function definitions must be at global level and cannot be inside a block or another function", ParseErrorType::WrongFnDefinition => "Function definitions must be at global level and cannot be inside a block or another function",
@ -187,6 +192,11 @@ impl fmt::Display for ParseError {
write!(f, "Expecting body statement block for function '{}'", s)? write!(f, "Expecting body statement block for function '{}'", s)?
} }
#[cfg(not(feature = "no_function"))]
ParseErrorType::FnDuplicateParam(ref s, ref arg) => {
write!(f, "Duplicated parameter '{}' for function '{}'", arg, s)?
}
ParseErrorType::MissingRightParen(ref s) | ParseErrorType::MissingRightBrace(ref s) => { ParseErrorType::MissingRightParen(ref s) | ParseErrorType::MissingRightBrace(ref s) => {
write!(f, "{} for {}", self.desc(), s)? write!(f, "{} for {}", self.desc(), s)?
} }

View File

@ -2287,8 +2287,8 @@ fn parse_fn<'a>(
.next() .next()
.ok_or_else(|| PERR::MissingRightParen(end_err.to_string()).into_err_eof())? .ok_or_else(|| PERR::MissingRightParen(end_err.to_string()).into_err_eof())?
{ {
(Token::Identifier(s), _) => { (Token::Identifier(s), pos) => {
params.push(s); params.push((s, pos));
} }
(_, pos) => return Err(PERR::MissingRightParen(end_err).into_err(pos)), (_, pos) => return Err(PERR::MissingRightParen(end_err).into_err(pos)),
} }
@ -2305,6 +2305,22 @@ fn parse_fn<'a>(
} }
} }
// Check for duplicating parameters
params
.iter()
.enumerate()
.try_for_each(|(i, (p1, _))| {
params
.iter()
.skip(i + 1)
.find(|(p2, _)| p2 == p1)
.map_or_else(|| Ok(()), |(p2, pos)| Err((p2, *pos)))
})
.map_err(|(p, pos)| {
PERR::FnDuplicateParam(name.to_string(), p.to_string()).into_err(pos)
})?;
// Parse function body
let body = match input.peek() { let body = match input.peek() {
Some((Token::LeftBrace, _)) => parse_block(input, false, allow_stmt_expr)?, Some((Token::LeftBrace, _)) => parse_block(input, false, allow_stmt_expr)?,
Some((_, pos)) => return Err(PERR::FnMissingBody(name).into_err(*pos)), Some((_, pos)) => return Err(PERR::FnMissingBody(name).into_err(*pos)),
@ -2313,7 +2329,7 @@ fn parse_fn<'a>(
Ok(FnDef { Ok(FnDef {
name, name,
params, params: params.into_iter().map(|(p, _)| p).collect(),
body, body,
pos, pos,
}) })

View File

@ -1,5 +1,22 @@
#![cfg(not(feature = "no_function"))] #![cfg(not(feature = "no_function"))]
use rhai::{Engine, EvalAltResult, INT}; use rhai::{Engine, EvalAltResult, ParseError, ParseErrorType, INT};
#[test]
fn test_fn() -> Result<(), EvalAltResult> {
let mut engine = Engine::new();
// Expect duplicated parameters error
match engine
.compile("fn hello(x, x) { x }")
.expect_err("should be error")
.error_type()
{
ParseErrorType::FnDuplicateParam(f, p) if f == "hello" && p == "x" => (),
_ => assert!(false, "wrong error"),
}
Ok(())
}
#[test] #[test]
fn test_call_fn() -> Result<(), EvalAltResult> { fn test_call_fn() -> Result<(), EvalAltResult> {