From 8d91a3a0dcb8f930616aba8b3dd3993738d27bcf Mon Sep 17 00:00:00 2001 From: Vincent Prouillet Date: Mon, 26 Aug 2024 12:26:14 +0200 Subject: [PATCH] Allow subscripts on literal arrays/map (#42) --- README.md | 1 + tera/bytes.py | 4 +- tera/src/functions.rs | 22 +++--- tera/src/lib.rs | 2 +- tera/src/parsing/ast.rs | 30 ++++++-- tera/src/parsing/parser.rs | 54 ++++++++----- .../success/expr/expressions.txt | 2 + .../rendering_inputs/success/literals.txt | 4 +- ...r_expressions_success@expressions.txt.snap | 2 + ..._rendering__rendering_ok@literals.txt.snap | 2 + tera/src/tera.rs | 8 +- tera/src/tests.rs | 1 - tera/src/value/mod.rs | 75 +++++++------------ tera/src/vm/interpreter.rs | 15 ++-- 14 files changed, 128 insertions(+), 94 deletions(-) diff --git a/README.md b/README.md index 431ead2..83ed871 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ TODO: - [x] Uncomment all tests using filters/functions/tests - [x] Parsing errors should report with the source context like Rust errors with the right spans - [ ] Add more helpful errors when loading templates (eg a forloop with for key, key in value/missing includes etc) +- [ ] Some constant folding: maths, subscript on literals - [x] Allow escape chars (eg \n) in strings concat, there was an issue about that in Zola - [ ] Feature to load templates from a glob with optional dep - [x] MAYBE: add a way to add global context to the Tera struct that are passed on every render automatically diff --git a/tera/bytes.py b/tera/bytes.py index 3957cdf..046e25e 100644 --- a/tera/bytes.py +++ b/tera/bytes.py @@ -1,9 +1,11 @@ import dis +import ast text = """ -"majeur" if age >= 18 else "mineur" +[1,2,3][0] """ +print(ast.dump(ast.parse(text))) print(dis.dis(text)) """ diff --git a/tera/src/functions.rs b/tera/src/functions.rs index 8027912..7925e9a 100644 --- a/tera/src/functions.rs +++ b/tera/src/functions.rs @@ -1,9 +1,9 @@ -use std::sync::Arc; use crate::args::Kwargs; use crate::errors::{Error, TeraResult}; -use crate::Value; use crate::value::FunctionResult; use crate::vm::state::State; +use crate::Value; +use std::sync::Arc; /// The function function type definition pub trait Function: Sync + Send + 'static { @@ -18,25 +18,25 @@ pub trait Function: Sync + Send + 'static { } impl Function for Func - where - Func: Fn(Kwargs, &State) -> Res + Sync + Send + 'static, - Res: FunctionResult, +where + Func: Fn(Kwargs, &State) -> Res + Sync + Send + 'static, + Res: FunctionResult, { fn call(&self, kwargs: Kwargs, state: &State) -> Res { (self)(kwargs, state) } } -type FunctionFunc = dyn Fn( Kwargs, &State) -> TeraResult + Sync + Send + 'static; +type FunctionFunc = dyn Fn(Kwargs, &State) -> TeraResult + Sync + Send + 'static; #[derive(Clone)] pub(crate) struct StoredFunction(Arc); impl StoredFunction { pub fn new(f: Func) -> Self - where - Func: Function, - Res: FunctionResult, + where + Func: Function, + Res: FunctionResult, { let closure = move |kwargs, state: &State| -> TeraResult { f.call(kwargs, state).into_result() @@ -55,7 +55,9 @@ pub(crate) fn range(kwargs: Kwargs, _: &State) -> TeraResult> { let end = kwargs.must_get::("end")?; let step_by = kwargs.get::("step_by")?.unwrap_or(1); if start > end { - return Err(Error::message("Function `range` was called with a `start` argument greater than the `end` one")); + return Err(Error::message( + "Function `range` was called with a `start` argument greater than the `end` one", + )); } Ok((start..end).step_by(step_by).collect()) diff --git a/tera/src/lib.rs b/tera/src/lib.rs index edcedcd..7cd1a97 100644 --- a/tera/src/lib.rs +++ b/tera/src/lib.rs @@ -2,12 +2,12 @@ mod args; mod context; mod errors; mod filters; +mod functions; mod parsing; mod reporting; mod template; mod tera; mod tests; -mod functions; mod utils; pub mod value; pub(crate) mod vm; diff --git a/tera/src/parsing/ast.rs b/tera/src/parsing/ast.rs index 91ba2d5..911f0bd 100644 --- a/tera/src/parsing/ast.rs +++ b/tera/src/parsing/ast.rs @@ -4,7 +4,7 @@ use std::fmt; use std::sync::Arc; use crate::utils::{Span, Spanned}; -use crate::value::{Key, Value}; +use crate::value::{format_map, Key, Value}; use crate::HashMap; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -90,12 +90,11 @@ impl fmt::Display for BinaryOperator { pub enum Expression { /// A constant: string, number, boolean, array or null Const(Spanned), - /// An array that contains things not + /// An array that contains things that we need to look up in the context Array(Spanned), - /// A hashmap defined in the template + /// A hashmap defined in the template where we need to look up in the context Map(Spanned), /// A variable to look up in the context. - /// Note that Var(Spanned), /// The `.` getter, as in item.field GetAttr(Spanned), @@ -118,6 +117,14 @@ impl Expression { matches!(self, Expression::Const(..)) } + pub fn is_array_or_map_literal(&self) -> bool { + match self { + Expression::Const(c) => c.as_map().is_some() || c.as_vec().is_some(), + Expression::Map(_) | Expression::Array(_) => true, + _ => false, + } + } + pub(crate) fn as_value(&self) -> Option { match self { Expression::Const(c) => Some(c.node().clone()), @@ -202,6 +209,9 @@ impl fmt::Display for Expression { Value::String(s, _) => write!(f, "'{}'", *s), Value::I64(s) => write!(f, "{}", *s), Value::F64(s) => write!(f, "{}", *s), + Value::U64(s) => write!(f, "{}", *s), + Value::U128(s) => write!(f, "{}", *s), + Value::I128(s) => write!(f, "{}", *s), Value::Bool(s) => write!(f, "{}", *s), Value::Array(s) => { write!(f, "[")?; @@ -217,7 +227,17 @@ impl fmt::Display for Expression { write!(f, "]") } Value::Null => write!(f, "null"), - _ => unreachable!(), + Value::Undefined => write!(f, "undefined"), + Value::Bytes(_) => write!(f, ""), + Value::Map(s) => { + let mut buf: Vec = Vec::new(); + format_map(s, &mut buf).expect("failed to write map to vec"); + write!( + f, + "{}", + std::str::from_utf8(&buf).expect("valid utf-8 in display") + ) + } }, Map(i) => write!(f, "{}", **i), Array(i) => write!(f, "{}", **i), diff --git a/tera/src/parsing/parser.rs b/tera/src/parsing/parser.rs index f22b719..7511a9b 100644 --- a/tera/src/parsing/parser.rs +++ b/tera/src/parsing/parser.rs @@ -179,6 +179,29 @@ impl<'a> Parser<'a> { .any(|b| *b == BodyContext::ForLoop) } + // Parse something in brackets [..] after an ident or a literal array/map + fn parse_subscript(&mut self, expr: Expression) -> TeraResult { + expect_token!(self, Token::LeftBracket, "[")?; + self.num_left_brackets += 1; + if self.num_left_brackets > MAX_NUM_LEFT_BRACKETS { + return Err(Error::syntax_error( + format!("Identifiers can only have up to {MAX_NUM_LEFT_BRACKETS} nested brackets."), + &self.current_span, + )); + } + + let sub_expr = self.parse_expression(0)?; + let mut span = expr.span().clone(); + span.expand(&self.current_span); + + let expr = Expression::GetItem(Spanned::new(GetItem { expr, sub_expr }, span)); + + expect_token!(self, Token::RightBracket, "]")?; + self.num_left_brackets -= 1; + + Ok(expr) + } + /// Can be just an ident or a macro call/fn fn parse_ident(&mut self, ident: &str) -> TeraResult { let mut start_span = self.current_span.clone(); @@ -226,26 +249,9 @@ impl<'a> Parser<'a> { )); } } + // Subscript Some(Ok((Token::LeftBracket, _))) => { - expect_token!(self, Token::LeftBracket, "[")?; - self.num_left_brackets += 1; - if self.num_left_brackets > MAX_NUM_LEFT_BRACKETS { - return Err(Error::syntax_error( - format!("Identifiers can only have up to {MAX_NUM_LEFT_BRACKETS} nested brackets."), - &self.current_span, - )); - } - - let sub_expr = self.parse_expression(0)?; - start_span.expand(&self.current_span); - - expr = Expression::GetItem(Spanned::new( - GetItem { expr, sub_expr }, - start_span.clone(), - )); - - expect_token!(self, Token::RightBracket, "]")?; - self.num_left_brackets -= 1; + expr = self.parse_subscript(expr)?; } // Function Some(Ok((Token::LeftParen, _))) => { @@ -542,6 +548,16 @@ impl<'a> Parser<'a> { Token::Ident("and") => BinaryOperator::And, Token::Ident("or") => BinaryOperator::Or, Token::Ident("is") => BinaryOperator::Is, + // A subscript. Should only be here after a literal array or map + Token::LeftBracket => { + if !lhs.is_array_or_map_literal() { + return Err(Error::syntax_error( + format!("Subscript is only allowed after a map or array literal"), + &self.current_span, + )); + } + return self.parse_subscript(lhs); + } // A ternary Token::Ident("if") => { self.next_or_error()?; diff --git a/tera/src/snapshot_tests/parser_inputs/success/expr/expressions.txt b/tera/src/snapshot_tests/parser_inputs/success/expr/expressions.txt index 925a99d..043f4e6 100644 --- a/tera/src/snapshot_tests/parser_inputs/success/expr/expressions.txt +++ b/tera/src/snapshot_tests/parser_inputs/success/expr/expressions.txt @@ -105,6 +105,8 @@ {{ (person in groupA) and person in groupB }} {{ (query.tags ~ ' ' ~ tags) | trim }} {{ a and a is containing(pat='a') or b and b is containing(pat='b') }} +{{ ['up', 'down', 'left', 'right'][1] }} +{{ {"hello": "world"}["hello"] }} // Macro calls {{ macros::input(label='Name', type='text') }} diff --git a/tera/src/snapshot_tests/rendering_inputs/success/literals.txt b/tera/src/snapshot_tests/rendering_inputs/success/literals.txt index 0cadb74..4ee227c 100644 --- a/tera/src/snapshot_tests/rendering_inputs/success/literals.txt +++ b/tera/src/snapshot_tests/rendering_inputs/success/literals.txt @@ -19,4 +19,6 @@ Hello {{ 2 / 0.5 }} {{ 2.1 / 0.5 }} {{ true and 10 }} -{{ true and not 10 }} \ No newline at end of file +{{ true and not 10 }} +{{ ['up', 'down', 'left', 'right'][1] }} +{{ {"hello": "world"}["hello"] }} \ No newline at end of file diff --git a/tera/src/snapshot_tests/snapshots/tera__snapshot_tests__parser__parser_expressions_success@expressions.txt.snap b/tera/src/snapshot_tests/snapshots/tera__snapshot_tests__parser__parser_expressions_success@expressions.txt.snap index b6c4522..09e20d5 100644 --- a/tera/src/snapshot_tests/snapshots/tera__snapshot_tests__parser__parser_expressions_success@expressions.txt.snap +++ b/tera/src/snapshot_tests/snapshots/tera__snapshot_tests__parser__parser_expressions_success@expressions.txt.snap @@ -86,6 +86,8 @@ three' indent{}) (and (in person groupA) (in person groupB)) (| (~ (~ query.tags ' ') tags) trim{}) (and (or (and a (is a containing{pat='a'})) b) (is b containing{pat='b'})) +["up", "down", "left", "right"][1] +{hello: 'world'}['hello'] macros::input{label='Name', type='text'} (| macros::input{} safe{}) macros::input{n=(- a 1)} diff --git a/tera/src/snapshot_tests/snapshots/tera__snapshot_tests__rendering__rendering_ok@literals.txt.snap b/tera/src/snapshot_tests/snapshots/tera__snapshot_tests__rendering__rendering_ok@literals.txt.snap index a592a2d..09158af 100644 --- a/tera/src/snapshot_tests/snapshots/tera__snapshot_tests__rendering__rendering_ok@literals.txt.snap +++ b/tera/src/snapshot_tests/snapshots/tera__snapshot_tests__rendering__rendering_ok@literals.txt.snap @@ -25,3 +25,5 @@ true 4.2 10 false +down +world diff --git a/tera/src/tera.rs b/tera/src/tera.rs index cfde2b8..1648159 100644 --- a/tera/src/tera.rs +++ b/tera/src/tera.rs @@ -1,12 +1,12 @@ use crate::args::ArgFromValue; use crate::errors::{Error, TeraResult}; use crate::filters::{Filter, StoredFilter}; +use crate::functions::{Function, StoredFunction}; use crate::template::{find_parents, Template}; use crate::tests::{StoredTest, Test, TestResult}; use crate::value::FunctionResult; use crate::vm::interpreter::VirtualMachine; use crate::{escape_html, Context, HashMap}; -use crate::functions::{Function, StoredFunction}; /// Default template name used for `Tera::render_str` and `Tera::one_off`. const ONE_OFF_TEMPLATE_NAME: &str = "__tera_one_off"; @@ -137,9 +137,9 @@ impl Tera { /// /// If a function with that name already exists, it will be overwritten pub fn register_function(&mut self, name: &'static str, func: Func) - where - Func: Function, - Res: FunctionResult, + where + Func: Function, + Res: FunctionResult, { self.functions.insert(name, StoredFunction::new(func)); } diff --git a/tera/src/tests.rs b/tera/src/tests.rs index 8e86330..4ce8c3f 100644 --- a/tera/src/tests.rs +++ b/tera/src/tests.rs @@ -8,7 +8,6 @@ use crate::value::{Key, Map}; use crate::vm::state::State; use crate::{HashMap, Value}; - pub trait TestResult { fn into_result(self) -> TeraResult; } diff --git a/tera/src/value/mod.rs b/tera/src/value/mod.rs index c7c6847..614ea51 100644 --- a/tera/src/value/mod.rs +++ b/tera/src/value/mod.rs @@ -27,6 +27,30 @@ pub type Map = HashMap, Value>; #[cfg(feature = "preserve_order")] pub type Map = IndexMap, Value>; +#[inline] +pub(crate) fn format_map(map: &Map, f: &mut impl std::io::Write) -> std::io::Result<()> { + let mut key_val: Box<_> = map.iter().collect(); + // Keys are sorted to have deterministic output + // TODO: Consider using sort_unstable_by_key + key_val.sort_by_key(|elem| elem.0); + f.write_all(b"{")?; + for (idx, (key, value)) in key_val.iter().enumerate() { + if idx > 0 { + f.write_all(b", ")?; + } + key.format(f)?; + f.write_all(b": ")?; + if value.as_str().is_some() { + f.write_all(b"'")?; + } + value.format(f)?; + if value.as_str().is_some() { + f.write_all(b"'")?; + } + } + f.write_all(b"}") +} + #[derive(Debug, Copy, Clone, Eq, PartialEq)] pub enum StringKind { Normal, @@ -173,19 +197,11 @@ impl Value { Value::Array(v) => { let mut it = v.iter(); f.write_all(b"[")?; - // First value - if let Some(elem) = it.next() { - if elem.as_str().is_some() { - f.write_all(b"'")?; + + for (idx, elem) in v.iter().enumerate() { + if idx > 0 { + f.write_all(b", ")?; } - elem.format(f)?; - if elem.as_str().is_some() { - f.write_all(b"'")?; - } - } - // Every other value - for elem in it { - f.write_all(b", ")?; if elem.as_str().is_some() { f.write_all(b"'")?; } @@ -196,40 +212,7 @@ impl Value { } f.write_all(b"]") } - Value::Map(v) => { - let mut key_val: Box<_> = v.iter().collect(); - // Keys are sorted to have deterministic output - // TODO: Consider using sort_unstable_by_key - key_val.sort_by_key(|elem| elem.0); - let mut it = key_val.iter(); - f.write_all(b"{")?; - // First value - if let Some((key, value)) = it.next() { - key.format(f)?; - f.write_all(b": ")?; - if value.as_str().is_some() { - f.write_all(b"'")?; - } - value.format(f)?; - if value.as_str().is_some() { - f.write_all(b"'")?; - } - } - // Every other value - for (key, value) in it { - f.write_all(b", ")?; - key.format(f)?; - f.write_all(b": ")?; - if value.as_str().is_some() { - f.write_all(b"'")?; - } - value.format(f)?; - if value.as_str().is_some() { - f.write_all(b"'")?; - } - } - f.write_all(b"}") - } + Value::Map(v) => format_map(v, f), Value::F64(v) => { #[cfg(feature = "no-fmt")] { diff --git a/tera/src/vm/interpreter.rs b/tera/src/vm/interpreter.rs index 1018757..4aa7b89 100644 --- a/tera/src/vm/interpreter.rs +++ b/tera/src/vm/interpreter.rs @@ -232,18 +232,21 @@ impl<'tera> VirtualMachine<'tera> { state.stack.push(Value::Null, None); } else { if let Some(f) = self.tera.functions.get(name.as_str()) { - let val = - match f.call(Kwargs::new(kwargs.into_map().unwrap()), &state) { - Ok(v) => v, - Err(err) => rendering_error!(format!("{err}"), span), - }; + let val = match f.call(Kwargs::new(kwargs.into_map().unwrap()), &state) + { + Ok(v) => v, + Err(err) => rendering_error!(format!("{err}"), span), + }; state .stack .push(val.into(), span.as_ref().map(|c| Cow::Owned(c.clone()))); } else { // TODO: we _should_ be able to track that at compile time - rendering_error!(format!("This function is not registered in Tera"), span) + rendering_error!( + format!("This function is not registered in Tera"), + span + ) } } }