From e59796780bad7f0d9dd0ae28105a02a2332f606b Mon Sep 17 00:00:00 2001 From: Juan Azambuja Date: Mon, 18 Sep 2023 10:30:13 -0300 Subject: [PATCH] feat: Support arbitrary keyword args on filtes --- crates/core/src/parser/filter.rs | 2 + crates/core/src/runtime/expression.rs | 32 +++++ crates/derive/src/filter_parameters.rs | 117 ++++++++++++++++-- .../src/parse_filter/filter_reflection.rs | 11 +- tests/derive_macros.rs | 53 ++++++++ .../keyword_group.rs | 82 ++++++++++++ tests/derive_macros_test_filters/mod.rs | 2 + 7 files changed, 287 insertions(+), 12 deletions(-) create mode 100644 tests/derive_macros_test_filters/keyword_group.rs diff --git a/crates/core/src/parser/filter.rs b/crates/core/src/parser/filter.rs index b35a08b3e..9176f4c0b 100644 --- a/crates/core/src/parser/filter.rs +++ b/crates/core/src/parser/filter.rs @@ -24,6 +24,7 @@ pub struct ParameterReflection { pub trait FilterParametersReflection { fn positional_parameters() -> &'static [ParameterReflection]; fn keyword_parameters() -> &'static [ParameterReflection]; + fn keyword_group_parameters() -> &'static [ParameterReflection]; } /// A trait that holds the information of a filter about itself, such as @@ -42,6 +43,7 @@ pub trait FilterReflection { fn positional_parameters(&self) -> &'static [ParameterReflection]; fn keyword_parameters(&self) -> &'static [ParameterReflection]; + fn keyword_group_parameters(&self) -> &'static [ParameterReflection]; } /// A trait that declares and holds the parameters of a filter. diff --git a/crates/core/src/runtime/expression.rs b/crates/core/src/runtime/expression.rs index af0ab76ae..bddd095a8 100644 --- a/crates/core/src/runtime/expression.rs +++ b/crates/core/src/runtime/expression.rs @@ -1,6 +1,7 @@ use std::fmt; use crate::error::Result; +use crate::model::Object; use crate::model::Scalar; use crate::model::Value; use crate::model::ValueCow; @@ -9,6 +10,8 @@ use crate::model::ValueView; use super::variable::Variable; use super::Runtime; +use std::collections::HashMap; + /// An un-evaluated `Value`. #[derive(Debug, Clone, PartialEq)] pub enum Expression { @@ -16,19 +19,30 @@ pub enum Expression { Variable(Variable), /// Evaluated. Literal(Value), + /// Used for evaluating object literals, + ObjectLiteral(ObjectLiteral), } +type ObjectLiteral = HashMap; + impl Expression { /// Create an expression from a scalar literal. pub fn with_literal>(literal: S) -> Self { Expression::Literal(Value::scalar(literal)) } + /// Creates an expression from an object literal (used when parsing filter + /// arguments) + pub fn with_object_literal(object_literal_expr: ObjectLiteral) -> Self { + Expression::ObjectLiteral(object_literal_expr) + } + /// Convert into a literal if possible. pub fn into_literal(self) -> Option { match self { Expression::Literal(x) => Some(x), Expression::Variable(_) => None, + Expression::ObjectLiteral(_) => None, } } @@ -37,6 +51,7 @@ impl Expression { match self { Expression::Literal(_) => None, Expression::Variable(x) => Some(x), + Expression::ObjectLiteral(_) => None, } } @@ -48,6 +63,16 @@ impl Expression { let path = x.try_evaluate(runtime)?; runtime.try_get(&path) } + Expression::ObjectLiteral(ref obj_lit) => { + let obj = obj_lit + .iter() + .map(|(key, expr)| match expr.try_evaluate(runtime) { + Some(result) => (key.into(), result.to_value()), + None => (key.into(), Value::Nil), + }) + .collect::(); + Some(ValueCow::Owned(obj.to_value())) + } } } @@ -57,8 +82,14 @@ impl Expression { Expression::Literal(ref x) => ValueCow::Borrowed(x), Expression::Variable(ref x) => { let path = x.evaluate(runtime)?; + runtime.get(&path)? } + Expression::ObjectLiteral(obj_lit) => obj_lit + .iter() + .map(|(key, expr)| (key.into(), expr.evaluate(runtime).unwrap().to_value())) + .collect::() + .into(), }; Ok(val) } @@ -69,6 +100,7 @@ impl fmt::Display for Expression { match self { Expression::Literal(ref x) => write!(f, "{}", x.source()), Expression::Variable(ref x) => write!(f, "{}", x), + Expression::ObjectLiteral(ref x) => write!(f, "{:?}", x), } } } diff --git a/crates/derive/src/filter_parameters.rs b/crates/derive/src/filter_parameters.rs index 0d88c4a9a..aa31c5315 100644 --- a/crates/derive/src/filter_parameters.rs +++ b/crates/derive/src/filter_parameters.rs @@ -83,6 +83,19 @@ impl<'a> FilterParameters<'a> { )); } + if fields.more_than_one_keyword_group_parameter() { + let grouped_keyword_fields = fields + .parameters + .iter() + .filter(|parameter| parameter.is_keyword_group()) + .collect::>(); + + return Err(Error::new_spanned( + grouped_keyword_fields.first(), + "Found more than one keyword_group parameter, this is not allowed.", + )); + } + let name = ident; let evaluated_name = Self::parse_attrs(attrs)? .unwrap_or_else(|| Ident::new(&format!("Evaluated{}", name), Span::call_site())); @@ -115,6 +128,16 @@ impl<'a> FilterParametersFields<'a> { .find(|parameter| !parameter.is_optional()) } + /// Predicate that indicates the presence of more than one keyword group + /// argument + fn more_than_one_keyword_group_parameter(&self) -> bool { + self.parameters + .iter() + .filter(|parameter| parameter.is_keyword_group()) + .count() + > 1 + } + /// Tries to create a new `FilterParametersFields` from the given `Fields` fn from_fields(fields: &'a Fields) -> Result { match fields { @@ -256,6 +279,11 @@ impl<'a> FilterParameter<'a> { self.meta.mode == FilterParameterMode::Keyword } + /// Returns whether this is a keyword list field. + fn is_keyword_group(&self) -> bool { + self.meta.mode == FilterParameterMode::KeywordGroup + } + /// Returns the name of this parameter in liquid. /// /// That is, by default, the name of the field as a string. However, @@ -279,6 +307,7 @@ impl<'a> ToTokens for FilterParameter<'a> { enum FilterParameterMode { Keyword, Positional, + KeywordGroup, } impl FromStr for FilterParameterMode { @@ -286,6 +315,7 @@ impl FromStr for FilterParameterMode { fn from_str(s: &str) -> std::result::Result { match s { "keyword" => Ok(FilterParameterMode::Keyword), + "keyword_group" => Ok(FilterParameterMode::KeywordGroup), "positional" => Ok(FilterParameterMode::Positional), s => Err(format!( "Expected either \"keyword\" or \"positional\". Found \"{}\".", @@ -424,6 +454,15 @@ fn generate_construct_positional_field( } } +/// Generates the statement that assigns the keyword list argument. +fn generate_construct_keyword_group_field(field: &FilterParameter<'_>) -> TokenStream { + let name = &field.name; + + quote! { + let #name = Expression::with_object_literal(keyword_as_map); + } +} + /// Generates the statement that evaluates the `Expression` fn generate_evaluate_field(field: &FilterParameter<'_>) -> TokenStream { let name = &field.name; @@ -582,6 +621,13 @@ fn generate_impl_filter_parameters(filter_parameters: &FilterParameters<'_>) -> .iter() .filter(|parameter| parameter.is_keyword()); + let keyword_group_fields = fields + .parameters + .iter() + .filter(|parameter| parameter.is_keyword_group()); + + let group_keyword_param_exists = keyword_group_fields.peekable().peek().is_some(); + let match_keyword_parameters_arms = fields .parameters .iter() @@ -597,6 +643,55 @@ fn generate_impl_filter_parameters(filter_parameters: &FilterParameters<'_>) -> quote!{ let #field = #field.ok_or_else(|| ::liquid_core::error::Error::with_msg(concat!("Expected named argument `", #liquid_name, "`")))?; } }); + let keyword_group_fields_handling_blocks = fields + .parameters + .iter() + .filter(|parameter| parameter.is_keyword_group()) + .map(generate_construct_keyword_group_field) + .collect::>(); + + let keyword_not_found_in_params_block = if group_keyword_param_exists { + // If there is a parameter that indicates all keywords should be grouped + // in an object, we generate an empty matching arm to prevent an error from + // being returned when a parsed keyword argument is not defines as a param. + quote! { + {} + } + } else { + // If there is no parameter that indicates all keywords should be grouped, + // an error is returned when a keyword argument is found but has not being + // declared. + quote! { + { + return ::std::result::Result::Err(::liquid_core::error::Error::with_msg(format!("Unexpected named argument `{}`", keyword))) + } + } + }; + + let assign_grouped_keyword_block = if group_keyword_param_exists { + keyword_group_fields_handling_blocks + .first() + .unwrap() + .clone() + } else { + quote! {} + }; + + let keywords_handling_block = quote! { + let mut keyword_as_map: std::collections::HashMap = std::collections::HashMap::new(); + #(let mut #keyword_fields = ::std::option::Option::None;)* + #[allow(clippy::never_loop)] // This is not obfuscating the code because it's generated by a macro + while let ::std::option::Option::Some(arg) = args.keyword.next() { + keyword_as_map.insert(arg.0.into(), arg.1.clone()); + match arg.0 { + #(#match_keyword_parameters_arms)* + keyword => #keyword_not_found_in_params_block + } + } + #assign_grouped_keyword_block + #(#unwrap_required_keyword_fields)* + }; + quote! { impl<'a> ::liquid_core::parser::FilterParameters<'a> for #name { type EvaluatedFilterParameters = #evaluated_name<'a>; @@ -606,18 +701,10 @@ fn generate_impl_filter_parameters(filter_parameters: &FilterParameters<'_>) -> if let ::std::option::Option::Some(arg) = args.positional.next() { return ::std::result::Result::Err(#too_many_args); } - - #(let mut #keyword_fields = ::std::option::Option::None;)* - #[allow(clippy::never_loop)] // This is not obfuscating the code because it's generated by a macro - while let ::std::option::Option::Some(arg) = args.keyword.next() { - match arg.0 { - #(#match_keyword_parameters_arms)* - keyword => return ::std::result::Result::Err(::liquid_core::error::Error::with_msg(format!("Unexpected named argument `{}`", keyword))), - } - } - #(#unwrap_required_keyword_fields)* + #keywords_handling_block Ok( #name { #comma_separated_field_names } ) + } fn evaluate(&'a self, runtime: &'a dyn ::liquid_core::runtime::Runtime) -> ::liquid_core::error::Result { @@ -692,6 +779,12 @@ fn generate_impl_reflection(filter_parameters: &FilterParameters<'_>) -> TokenSt .filter(|parameter| parameter.is_keyword()) .map(generate_parameter_reflection); + let kwg_params_reflection = fields + .parameters + .iter() + .filter(|parameter| parameter.is_keyword_group()) + .map(generate_parameter_reflection); + let pos_params_reflection = fields .parameters .iter() @@ -707,6 +800,10 @@ fn generate_impl_reflection(filter_parameters: &FilterParameters<'_>) -> TokenSt fn keyword_parameters() -> &'static [::liquid_core::parser::ParameterReflection] { &[ #(#kw_params_reflection)* ] } + + fn keyword_group_parameters() -> &'static [::liquid_core::parser::ParameterReflection] { + &[ #(#kwg_params_reflection)* ] + } } } } diff --git a/crates/derive/src/parse_filter/filter_reflection.rs b/crates/derive/src/parse_filter/filter_reflection.rs index cf35d09e8..1c068d65b 100644 --- a/crates/derive/src/parse_filter/filter_reflection.rs +++ b/crates/derive/src/parse_filter/filter_reflection.rs @@ -15,15 +15,18 @@ fn generate_reflection(filter_parser: &ParseFilter<'_>) -> Result { let impl_filter_reflection = filter_parser.generate_impl(quote! { ::liquid_core::parser::FilterReflection }); - let (positional_parameters, keyword_parameters) = if let Some(parameters_struct_name) = + let (positional_parameters, keyword_parameters, keyword_group_parameters) = if let Some( + parameters_struct_name, + ) = parameters_struct_name { ( quote_spanned! {parameters_struct_name.span()=> <#parameters_struct_name as ::liquid_core::parser::FilterParametersReflection>::positional_parameters() }, quote_spanned! {parameters_struct_name.span()=> <#parameters_struct_name as ::liquid_core::parser::FilterParametersReflection>::keyword_parameters() }, + quote_spanned! {parameters_struct_name.span()=> <#parameters_struct_name as ::liquid_core::parser::FilterParametersReflection>::keyword_group_parameters() }, ) } else { - (quote! { &[] }, quote! { &[] }) + (quote! { &[] }, quote! { &[] }, quote! { &[] }) }; Ok(quote! { @@ -42,6 +45,10 @@ fn generate_reflection(filter_parser: &ParseFilter<'_>) -> Result { fn keyword_parameters(&self) -> &'static [::liquid_core::parser::ParameterReflection] { #keyword_parameters } + + fn keyword_group_parameters(&self) -> &'static [::liquid_core::parser::ParameterReflection] { + #keyword_group_parameters + } } }) } diff --git a/tests/derive_macros.rs b/tests/derive_macros.rs index 1ebd87060..85b3e338a 100644 --- a/tests/derive_macros.rs +++ b/tests/derive_macros.rs @@ -9,6 +9,7 @@ fn build_parser() -> Parser { ParserBuilder::new() .filter(derive_macros_test_filters::TestPositionalFilterParser) .filter(derive_macros_test_filters::TestKeywordFilterParser) + .filter(derive_macros_test_filters::TestKeywordGroupFilterParser) .filter(derive_macros_test_filters::TestMixedFilterParser) .filter(derive_macros_test_filters::TestParameterlessFilterParser) .build() @@ -101,6 +102,27 @@ pub fn test_derive_keyword_filter_ok() { assert_eq!(rendered, expected); } +#[test] +pub fn test_derive_keyword_group_filter_ok() { + let parser = build_parser(); + + let template = parser + .parse(concat!( + "{{ 0 | kwg: optional:\"str\", required: true }}\n", + "{{ 0 | kwg: required: false }}" + )) + .unwrap(); + let expected = concat!( + "\n", + "" + ); + + let globals = liquid::Object::new(); + let rendered = template.render(&globals).unwrap(); + + assert_eq!(rendered, expected); +} + #[test] pub fn test_derive_keyword_filter_err() { let parser = build_parser(); @@ -141,6 +163,37 @@ pub fn test_derive_keyword_filter_reflection() { assert_eq!(kw_args[1].is_optional, false); } +#[test] +pub fn test_derive_keyword_group_filter_reflection() { + let filter = derive_macros_test_filters::TestKeywordGroupFilterParser; + + assert_eq!(filter.name(), "kwg"); + assert_eq!( + filter.description(), + "Filter to test keyword group arguments." + ); + assert!(filter.positional_parameters().is_empty()); + let kw_args = filter.keyword_parameters(); + let kwg_args = filter.keyword_group_parameters(); + + assert_eq!(kw_args[0].name, "optional"); + assert_eq!(kw_args[0].description, "Optional keyword argument."); + assert_eq!(kw_args[0].is_optional, true); + + assert_eq!(kw_args[1].name, "required"); + assert_eq!( + kw_args[1].description, + "Required keyword argument. Must be a boolean." + ); + assert_eq!(kw_args[1].is_optional, false); + + assert_eq!(kwg_args[0].name, "all_keywords"); + assert_eq!( + kwg_args[0].description, + "Keyword group that contains all keyword parameters." + ); +} + #[test] pub fn test_derive_mixed_filter_ok() { let parser = build_parser(); diff --git a/tests/derive_macros_test_filters/keyword_group.rs b/tests/derive_macros_test_filters/keyword_group.rs new file mode 100644 index 000000000..b8856f3de --- /dev/null +++ b/tests/derive_macros_test_filters/keyword_group.rs @@ -0,0 +1,82 @@ +use liquid_core::Expression; +use liquid_core::Result; +use liquid_core::Runtime; +use liquid_core::{ + Display_filter, Filter, FilterParameters, FilterReflection, FromFilterParameters, ParseFilter, +}; +use liquid_core::{Value, ValueView}; + +#[derive(Debug, FilterParameters)] +struct TestKeywordGroupFilterParameters { + #[parameter( + description = "Optional keyword argument.", + arg_type = "str", + mode = "keyword" + )] + optional: Option, + + #[parameter( + description = "Required keyword argument. Must be a boolean.", + arg_type = "bool", + mode = "keyword" + )] + required: Expression, + + #[parameter( + description = "Keyword group that contains all keyword parameters.", + mode = "keyword_group" + )] + all_keywords: Expression, +} + +#[derive(Clone, ParseFilter, FilterReflection)] +#[filter( + name = "kwg", + description = "Filter to test keyword group arguments.", + parameters(TestKeywordGroupFilterParameters), + parsed(TestKeywordGroupFilter) +)] +pub struct TestKeywordGroupFilterParser; + +#[derive(Debug, FromFilterParameters, Display_filter)] +#[name = "kwg"] +pub struct TestKeywordGroupFilter { + #[parameters] + args: TestKeywordGroupFilterParameters, +} + +impl Filter for TestKeywordGroupFilter { + fn evaluate(&self, _input: &dyn ValueView, runtime: &dyn Runtime) -> Result { + let args = self.args.evaluate(runtime)?; + + let required = args.required; + + let binding = args.all_keywords.to_value(); + let keyword_group = binding.as_object().unwrap(); + let required_from_keyword_group = keyword_group + .get("required") + .unwrap() + .as_scalar() + .unwrap() + .to_bool() + .unwrap(); + + let result = if let Some(optional) = args.optional { + let optional_from_keyword_group = keyword_group.get("optional").unwrap().to_kstr(); + format!( + "", + optional, + required, + required_from_keyword_group, + optional_from_keyword_group + ) + } else { + format!( + "", + required, required_from_keyword_group + ) + }; + + Ok(Value::scalar(result)) + } +} diff --git a/tests/derive_macros_test_filters/mod.rs b/tests/derive_macros_test_filters/mod.rs index 7bf4a89ce..ed946ec0a 100644 --- a/tests/derive_macros_test_filters/mod.rs +++ b/tests/derive_macros_test_filters/mod.rs @@ -1,10 +1,12 @@ mod keyword; +mod keyword_group; mod mixed; mod parameterless; mod positional; mod stateful; pub use self::keyword::TestKeywordFilterParser; +pub use self::keyword_group::TestKeywordGroupFilterParser; pub use self::mixed::TestMixedFilterParser; pub use self::parameterless::TestParameterlessFilterParser; pub use self::positional::TestPositionalFilterParser;