From cb351fe6b1dda644a4ec023dc850cdfe83732503 Mon Sep 17 00:00:00 2001 From: René Kijewski Date: Thu, 27 Jan 2022 10:33:13 +0100 Subject: Unify handling of calls (#614) Instead of having `Expr::VarCall`, `Expr::PathCall` and `Expr::MethodCall`, this PR unifies the handling of calls by removing the former three variants, and introducing `Expr::Call`. --- askama_shared/src/generator.rs | 110 +++++++---------------- askama_shared/src/parser.rs | 192 +++++++++++++++++++++++++++-------------- 2 files changed, 157 insertions(+), 145 deletions(-) (limited to 'askama_shared/src') diff --git a/askama_shared/src/generator.rs b/askama_shared/src/generator.rs index 53729c6..fabe277 100644 --- a/askama_shared/src/generator.rs +++ b/askama_shared/src/generator.rs @@ -603,7 +603,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { // If `iter` is a call then we assume it's something that returns // an iterator. If not then the user can explicitly add the needed // call without issues. - Expr::MethodCall(..) | Expr::PathCall(..) | Expr::Index(..) => { + Expr::Call(..) | Expr::Index(..) => { buf.writeln(&format!("let _iter = ({}).into_iter();", expr_code)) } // If accessing `self` then it most likely needs to be @@ -1049,9 +1049,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { Expr::StrLit(s) => self.visit_str_lit(buf, s), Expr::CharLit(s) => self.visit_char_lit(buf, s), Expr::Var(s) => self.visit_var(buf, s), - Expr::VarCall(var, ref args) => self.visit_var_call(buf, var, args)?, Expr::Path(ref path) => self.visit_path(buf, path), - Expr::PathCall(ref path, ref args) => self.visit_path_call(buf, path, args)?, Expr::Array(ref elements) => self.visit_array(buf, elements)?, Expr::Attr(ref obj, name) => self.visit_attr(buf, obj, name)?, Expr::Index(ref obj, ref key) => self.visit_index(buf, obj, key)?, @@ -1060,9 +1058,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { Expr::BinOp(op, ref left, ref right) => self.visit_binop(buf, op, left, right)?, Expr::Range(op, ref left, ref right) => self.visit_range(buf, op, left, right)?, Expr::Group(ref inner) => self.visit_group(buf, inner)?, - Expr::MethodCall(ref obj, method, ref args) => { - self.visit_method_call(buf, obj, method, args)? - } + Expr::Call(ref obj, ref args) => self.visit_call(buf, obj, args)?, Expr::RustMacro(name, args) => self.visit_rust_macro(buf, name, args), }) } @@ -1234,20 +1230,15 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { buf.write("&("); } - let scoped = matches!( - arg, - Expr::Filter(_, _) - | Expr::MethodCall(_, _, _) - | Expr::VarCall(_, _) - | Expr::PathCall(_, _) - ); - - if scoped { - buf.writeln("{")?; - self.visit_expr(buf, arg)?; - buf.writeln("}")?; - } else { - self.visit_expr(buf, arg)?; + match arg { + Expr::Call(left, _) if !matches!(left.as_ref(), Expr::Path(_)) => { + buf.writeln("{")?; + self.visit_expr(buf, arg)?; + buf.writeln("}")?; + } + _ => { + self.visit_expr(buf, arg)?; + } } if borrow { @@ -1283,7 +1274,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { } } self.visit_expr(buf, obj)?; - buf.write(&format!(".{}", attr)); + buf.write(&format!(".{}", normalize_identifier(attr))); Ok(DisplayWrap::Unwrapped) } @@ -1301,15 +1292,14 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { Ok(DisplayWrap::Unwrapped) } - fn visit_method_call( + fn visit_call( &mut self, buf: &mut Buffer, - obj: &Expr<'_>, - method: &str, + left: &Expr<'_>, args: &[Expr<'_>], ) -> Result { - if matches!(obj, Expr::Var("loop")) { - match method { + match left { + Expr::Attr(left, method) if **left == Expr::Var("loop") => match *method { "cycle" => match args { [arg] => { if matches!(arg, Expr::Array(arr) if arr.is_empty()) { @@ -1329,16 +1319,22 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { _ => return Err("loop.cycle(…) expects exactly one argument".into()), }, s => return Err(format!("unknown loop method: {:?}", s).into()), + }, + left => { + match left { + Expr::Var(name) => match self.locals.resolve(name) { + Some(resolved) => buf.write(&resolved), + None => buf.write(&format!("(&self.{})", normalize_identifier(name))), + }, + left => { + self.visit_expr(buf, left)?; + } + } + + buf.write("("); + self._visit_args(buf, args)?; + buf.write(")"); } - } else { - if let Expr::Var("self") = obj { - buf.write("self"); - } else { - self.visit_expr(buf, obj)?; - } - buf.write(&format!(".{}(", normalize_identifier(method))); - self._visit_args(buf, args)?; - buf.write(")"); } Ok(DisplayWrap::Unwrapped) } @@ -1421,24 +1417,6 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { DisplayWrap::Unwrapped } - fn visit_path_call( - &mut self, - buf: &mut Buffer, - path: &[&str], - args: &[Expr<'_>], - ) -> Result { - for (i, part) in path.iter().enumerate() { - if i > 0 { - buf.write("::"); - } - buf.write(part); - } - buf.write("("); - self._visit_args(buf, args)?; - buf.write(")"); - Ok(DisplayWrap::Unwrapped) - } - fn visit_var(&mut self, buf: &mut Buffer, s: &str) -> DisplayWrap { if s == "self" { buf.write(s); @@ -1449,24 +1427,6 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> { DisplayWrap::Unwrapped } - fn visit_var_call( - &mut self, - buf: &mut Buffer, - s: &str, - args: &[Expr<'_>], - ) -> Result { - buf.write("("); - let s = normalize_identifier(s); - if !self.locals.contains(&s) && s != "self" { - buf.write("self."); - } - buf.write(s); - buf.write(")("); - self._visit_args(buf, args)?; - buf.write(")"); - Ok(DisplayWrap::Unwrapped) - } - fn visit_bool_lit(&mut self, buf: &mut Buffer, s: &str) -> DisplayWrap { buf.write(s); DisplayWrap::Unwrapped @@ -1693,14 +1653,6 @@ where } } - fn contains(&self, key: &K) -> bool { - self.scopes.iter().rev().any(|set| set.contains_key(key)) - || match self.parent { - Some(set) => set.contains(key), - None => false, - } - } - /// Iterates the scopes in reverse and returns `Some(LocalMeta)` /// from the first scope where `key` exists. fn get(&self, key: &K) -> Option<&V> { diff --git a/askama_shared/src/parser.rs b/askama_shared/src/parser.rs index f5685b9..f429628 100644 --- a/askama_shared/src/parser.rs +++ b/askama_shared/src/parser.rs @@ -52,9 +52,7 @@ pub enum Expr<'a> { StrLit(&'a str), CharLit(&'a str), Var(&'a str), - VarCall(&'a str, Vec>), Path(Vec<&'a str>), - PathCall(Vec<&'a str>, Vec>), Array(Vec>), Attr(Box>, &'a str), Index(Box>, Box>), @@ -63,7 +61,7 @@ pub enum Expr<'a> { BinOp(&'a str, Box>, Box>), Range(&'a str, Option>>, Option>>), Group(Box>), - MethodCall(Box>, &'a str, Vec>), + Call(Box>, Vec>), RustMacro(&'a str, &'a str), } @@ -86,7 +84,7 @@ impl Expr<'_> { // The result of a call likely doesn't need to be borrowed, // as in that case the call is more likely to return a // reference in the first place then. - VarCall(..) | Path(..) | PathCall(..) | MethodCall(..) => true, + Call(..) | Path(..) => true, // If the `expr` is within a `Unary` or `BinOp` then // an assumption can be made that the operand is copy. // If not, then the value is moved and adding `.clone()` @@ -296,11 +294,6 @@ fn expr_var(i: &str) -> IResult<&str, Expr<'_>> { map(identifier, Expr::Var)(i) } -fn expr_var_call(i: &str) -> IResult<&str, Expr<'_>> { - let (i, (s, args)) = tuple((ws(identifier), arguments))(i)?; - Ok((i, Expr::VarCall(s, args))) -} - fn path(i: &str) -> IResult<&str, Vec<&str>> { let root = opt(value("", ws(tag("::")))); let tail = separated_list1(ws(tag("::")), identifier); @@ -335,11 +328,6 @@ fn expr_path(i: &str) -> IResult<&str, Expr<'_>> { Ok((i, Expr::Path(path))) } -fn expr_path_call(i: &str) -> IResult<&str, Expr<'_>> { - let (i, (path, args)) = tuple((ws(path), arguments))(i)?; - Ok((i, Expr::PathCall(path, args))) -} - fn named_target(i: &str) -> IResult<&str, (&str, Target<'_>)> { let (i, (src, target)) = pair(identifier, opt(preceded(ws(char(':')), target)))(i)?; Ok((i, (src, target.unwrap_or(Target::Name(src))))) @@ -510,52 +498,39 @@ fn expr_single(i: &str) -> IResult<&str, Expr<'_>> { expr_num_lit, expr_str_lit, expr_char_lit, - expr_path_call, expr_path, expr_rust_macro, expr_array_lit, - expr_var_call, expr_var, expr_group, ))(i) } -fn attr(i: &str) -> IResult<&str, (&str, Option>>)> { - let (i, (_, attr, args)) = tuple(( - ws(char('.')), - alt((num_lit, identifier)), - ws(opt(arguments)), - ))(i)?; - Ok((i, (attr, args))) +enum Suffix<'a> { + Attr(&'a str), + Index(Expr<'a>), + Call(Vec>), } -fn expr_attr(i: &str) -> IResult<&str, Expr<'_>> { - let (i, (obj, attrs)) = tuple((expr_single, many0(attr)))(i)?; - - let mut res = obj; - for (aname, args) in attrs { - res = if let Some(args) = args { - Expr::MethodCall(Box::new(res), aname, args) - } else { - Expr::Attr(Box::new(res), aname) - }; - } - - Ok((i, res)) +fn expr_attr(i: &str) -> IResult<&str, Suffix<'_>> { + map( + preceded( + ws(pair(char('.'), not(char('.')))), + cut(alt((num_lit, identifier))), + ), + Suffix::Attr, + )(i) } -fn expr_index(i: &str) -> IResult<&str, Expr<'_>> { - let key = opt(tuple((ws(char('[')), expr_any, ws(char(']'))))); - let (i, (obj, key)) = tuple((expr_attr, key))(i)?; - let key = key.map(|(_, key, _)| key); +fn expr_index(i: &str) -> IResult<&str, Suffix<'_>> { + map( + preceded(ws(char('[')), cut(terminated(expr_any, ws(char(']'))))), + Suffix::Index, + )(i) +} - Ok(( - i, - match key { - Some(key) => Expr::Index(Box::new(obj), Box::new(key)), - None => obj, - }, - )) +fn expr_call(i: &str) -> IResult<&str, Suffix<'_>> { + map(arguments, Suffix::Call)(i) } fn filter(i: &str) -> IResult<&str, (&str, Option>>)> { @@ -564,7 +539,7 @@ fn filter(i: &str) -> IResult<&str, (&str, Option>>)> { } fn expr_filtered(i: &str) -> IResult<&str, Expr<'_>> { - let (i, (obj, filters)) = tuple((expr_unary, many0(filter)))(i)?; + let (i, (obj, filters)) = tuple((expr_prefix, many0(filter)))(i)?; let mut res = obj; for (fname, args) in filters { @@ -581,15 +556,27 @@ fn expr_filtered(i: &str) -> IResult<&str, Expr<'_>> { Ok((i, res)) } -fn expr_unary(i: &str) -> IResult<&str, Expr<'_>> { - let (i, (op, expr)) = tuple((opt(alt((ws(tag("!")), ws(tag("-"))))), expr_index))(i)?; - Ok(( - i, - match op { - Some(op) => Expr::Unary(op, Box::new(expr)), - None => expr, - }, - )) +fn expr_prefix(i: &str) -> IResult<&str, Expr<'_>> { + let (i, (ops, mut expr)) = pair(many0(ws(alt((tag("!"), tag("-"))))), expr_suffix)(i)?; + for op in ops.iter().rev() { + expr = Expr::Unary(op, Box::new(expr)); + } + Ok((i, expr)) +} + +fn expr_suffix(i: &str) -> IResult<&str, Expr<'_>> { + let (mut i, mut expr) = expr_single(i)?; + loop { + let (j, suffix) = opt(alt((expr_attr, expr_index, expr_call)))(i)?; + i = j; + match suffix { + Some(Suffix::Attr(attr)) => expr = Expr::Attr(expr.into(), attr), + Some(Suffix::Index(index)) => expr = Expr::Index(expr.into(), index.into()), + Some(Suffix::Call(args)) => expr = Expr::Call(expr.into(), args), + None => break, + } + } + Ok((i, expr)) } fn expr_rust_macro(i: &str) -> IResult<&str, Expr<'_>> { @@ -1348,7 +1335,10 @@ mod tests { super::parse("{{ Some(123) }}", &s).unwrap(), vec![Node::Expr( Ws(false, false), - Expr::PathCall(vec!["Some"], vec![Expr::NumLit("123")],), + Expr::Call( + Box::new(Expr::Path(vec!["Some"])), + vec![Expr::NumLit("123")] + ), )], ); @@ -1356,14 +1346,14 @@ mod tests { super::parse("{{ Ok(123) }}", &s).unwrap(), vec![Node::Expr( Ws(false, false), - Expr::PathCall(vec!["Ok"], vec![Expr::NumLit("123")],), + Expr::Call(Box::new(Expr::Path(vec!["Ok"])), vec![Expr::NumLit("123")]), )], ); assert_eq!( super::parse("{{ Err(123) }}", &s).unwrap(), vec![Node::Expr( Ws(false, false), - Expr::PathCall(vec!["Err"], vec![Expr::NumLit("123")],), + Expr::Call(Box::new(Expr::Path(vec!["Err"])), vec![Expr::NumLit("123")]), )], ); } @@ -1374,7 +1364,10 @@ mod tests { super::parse("{{ function(\"123\", 3) }}", &Syntax::default()).unwrap(), vec![Node::Expr( Ws(false, false), - Expr::VarCall("function", vec![Expr::StrLit("123"), Expr::NumLit("3")]), + Expr::Call( + Box::new(Expr::Var("function")), + vec![Expr::StrLit("123"), Expr::NumLit("3")] + ), )], ); } @@ -1394,7 +1387,10 @@ mod tests { super::parse("{{ Option::Some(123) }}", &s).unwrap(), vec![Node::Expr( Ws(false, false), - Expr::PathCall(vec!["Option", "Some"], vec![Expr::NumLit("123")],), + Expr::Call( + Box::new(Expr::Path(vec!["Option", "Some"])), + vec![Expr::NumLit("123")], + ), )], ); @@ -1402,8 +1398,8 @@ mod tests { super::parse("{{ self::function(\"123\", 3) }}", &s).unwrap(), vec![Node::Expr( Ws(false, false), - Expr::PathCall( - vec!["self", "function"], + Expr::Call( + Box::new(Expr::Path(vec!["self", "function"])), vec![Expr::StrLit("123"), Expr::NumLit("3")], ), )], @@ -1417,14 +1413,20 @@ mod tests { super::parse("{{ std::string::String::new() }}", &syntax).unwrap(), vec![Node::Expr( Ws(false, false), - Expr::PathCall(vec!["std", "string", "String", "new"], vec![]), + Expr::Call( + Box::new(Expr::Path(vec!["std", "string", "String", "new"])), + vec![] + ), )], ); assert_eq!( super::parse("{{ ::std::string::String::new() }}", &syntax).unwrap(), vec![Node::Expr( Ws(false, false), - Expr::PathCall(vec!["", "std", "string", "String", "new"], vec![]), + Expr::Call( + Box::new(Expr::Path(vec!["", "std", "string", "String", "new"])), + vec![] + ), )], ); } @@ -1586,6 +1588,64 @@ mod tests { ); } + #[test] + fn test_odd_calls() { + use Expr::*; + let syntax = Syntax::default(); + assert_eq!( + super::parse("{{ a[b](c) }}", &syntax).unwrap(), + vec![Node::Expr( + Ws(false, false), + Call( + Box::new(Index(Box::new(Var("a")), Box::new(Var("b")))), + vec![Var("c")], + ), + )], + ); + assert_eq!( + super::parse("{{ (a + b)(c) }}", &syntax).unwrap(), + vec![Node::Expr( + Ws(false, false), + Call( + Box::new(Group(Box::new(BinOp( + "+", + Box::new(Var("a")), + Box::new(Var("b")) + )))), + vec![Var("c")], + ), + )], + ); + assert_eq!( + super::parse("{{ a + b(c) }}", &syntax).unwrap(), + vec![Node::Expr( + Ws(false, false), + BinOp( + "+", + Box::new(Var("a")), + Box::new(Call(Box::new(Var("b")), vec![Var("c")])), + ), + )], + ); + assert_eq!( + super::parse("{{ (-a)(b) }}", &syntax).unwrap(), + vec![Node::Expr( + Ws(false, false), + Call( + Box::new(Group(Box::new(Unary("-", Box::new(Var("a")))))), + vec![Var("b")], + ), + )], + ); + assert_eq!( + super::parse("{{ -a(b) }}", &syntax).unwrap(), + vec![Node::Expr( + Ws(false, false), + Unary("-", Box::new(Call(Box::new(Var("a")), vec![Var("b")])),), + )], + ); + } + #[test] fn test_parse_comments() { let s = &Syntax::default(); -- cgit