From e94525f483196757f744d5e42e5a8870d7e388c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Fri, 28 Jun 2024 03:59:38 +0200 Subject: [PATCH 1/7] Add support for `..` in let pattern matching for structs --- rinja_derive/src/generator.rs | 9 +++- rinja_parser/src/node.rs | 69 ++++++++++++++++++++++++++----- testing/tests/rest_pattern.rs | 78 +++++++++++++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 testing/tests/rest_pattern.rs diff --git a/rinja_derive/src/generator.rs b/rinja_derive/src/generator.rs index 9e7ed9fd..f2fffbb3 100644 --- a/rinja_derive/src/generator.rs +++ b/rinja_derive/src/generator.rs @@ -1788,8 +1788,8 @@ impl<'a> Generator<'a> { target: &Target<'a>, ) { match target { - Target::Name("_") => { - buf.write("_"); + Target::Placeholder(s) | Target::Rest(s) => { + buf.write(s); } Target::Name(name) => { let name = normalize_identifier(name); @@ -1824,6 +1824,11 @@ impl<'a> Generator<'a> { buf.write(SeparatedPath(path)); buf.write(" { "); for (name, target) in targets { + if let Target::Rest(s) = target { + buf.write(s); + continue; + } + buf.write(normalize_identifier(name)); buf.write(": "); self.visit_target(buf, initialized, false, target); diff --git a/rinja_parser/src/node.rs b/rinja_parser/src/node.rs index c8ddbf4c..2c1dccb9 100644 --- a/rinja_parser/src/node.rs +++ b/rinja_parser/src/node.rs @@ -2,7 +2,7 @@ use std::str; use nom::branch::alt; use nom::bytes::complete::{tag, take_till}; -use nom::character::complete::char; +use nom::character::complete::{char, one_of}; use nom::combinator::{ complete, consumed, cut, eof, map, map_res, not, opt, peek, recognize, value, }; @@ -11,7 +11,7 @@ use nom::error_position; use nom::multi::{fold_many0, many0, many1, separated_list0, separated_list1}; use nom::sequence::{delimited, pair, preceded, terminated, tuple}; -use crate::{not_ws, ErrorContext, ParseResult, WithSpan}; +use crate::{not_ws, ErrorContext, ParseErr, ParseResult, WithSpan}; use super::{ bool_lit, char_lit, filter, identifier, is_ws, keyword, num_lit, path_or_identifier, skip_till, @@ -188,6 +188,8 @@ pub enum Target<'a> { BoolLit(&'a str), Path(Vec<&'a str>), OrChain(Vec>), + Placeholder(&'a str), + Rest(&'a str), } impl<'a> Target<'a> { @@ -221,7 +223,7 @@ impl<'a> Target<'a> { return Ok((i, Self::Tuple(Vec::new(), Vec::new()))); } - let (i, first_target) = Self::parse(i, s)?; + let (i, first_target) = Self::unnamed(i, s)?; let (i, is_unused_paren) = opt_closing_paren(i)?; if is_unused_paren { return Ok((i, first_target)); @@ -230,7 +232,7 @@ impl<'a> Target<'a> { let mut targets = vec![first_target]; let (i, _) = cut(tuple(( fold_many0( - preceded(ws(char(',')), |i| Self::parse(i, s)), + preceded(ws(char(',')), |i| Self::unnamed(i, s)), || (), |_, target| { targets.push(target); @@ -239,7 +241,7 @@ impl<'a> Target<'a> { opt(ws(char(','))), ws(cut(char(')'))), )))(i)?; - return Ok((i, Self::Tuple(Vec::new(), targets))); + return Ok((i, Self::Tuple(Vec::new(), only_one_rest_pattern(targets)?))); } let path = |i| { @@ -260,11 +262,11 @@ impl<'a> Target<'a> { let (i, targets) = alt(( map(char(')'), |_| Vec::new()), terminated( - cut(separated_list1(ws(char(',')), |i| Self::parse(i, s))), + cut(separated_list1(ws(char(',')), |i| Self::unnamed(i, s))), pair(opt(ws(char(','))), ws(cut(char(')')))), ), ))(i)?; - return Ok((i, Self::Tuple(path, targets))); + return Ok((i, Self::Tuple(path, only_one_rest_pattern(targets)?))); } let (i, is_named_struct) = opt_opening_brace(i)?; @@ -284,7 +286,11 @@ impl<'a> Target<'a> { // neither literal nor struct nor path let (new_i, name) = identifier(i)?; - Ok((new_i, Self::verify_name(i, name)?)) + let target = match name { + "_" => Self::Placeholder(name), + _ => Self::verify_name(i, name)?, + }; + Ok((new_i, target)) } fn lit(i: &'a str) -> ParseResult<'a, Self> { @@ -296,20 +302,46 @@ impl<'a> Target<'a> { ))(i) } + fn unnamed(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { + alt((Self::rest, |i| Self::parse(i, s)))(i) + } + fn named(init_i: &'a str, s: &State<'_>) -> ParseResult<'a, (&'a str, Self)> { + let (i, rest) = opt(consumed(Self::rest))(init_i)?; + if let Some(rest) = rest { + let (_, chr) = ws(opt(one_of(",:")))(i)?; + if let Some(chr) = chr { + return Err(nom::Err::Failure(ErrorContext::new( + format!("unexpected `{chr}` character after `..`"), + i, + ))); + } + return Ok((i, rest)); + } + let (i, (src, target)) = pair( identifier, opt(preceded(ws(char(':')), |i| Self::parse(i, s))), )(init_i)?; + if src == "_" { + return Err(nom::Err::Failure(ErrorContext::new( + "cannot use placeholder `_` as source in named struct", + init_i, + ))); + } + let target = match target { Some(target) => target, None => Self::verify_name(init_i, src)?, }; - Ok((i, (src, target))) } + fn rest(i: &'a str) -> ParseResult<'a, Self> { + map(tag(".."), Self::Rest)(i) + } + fn verify_name(input: &'a str, name: &'a str) -> Result>> { match name { "self" | "writer" => Err(nom::Err::Failure(ErrorContext::new( @@ -321,6 +353,23 @@ impl<'a> Target<'a> { } } +fn only_one_rest_pattern(targets: Vec>) -> Result>, ParseErr<'_>> { + let snd_wildcard = targets + .iter() + .filter_map(|t| match t { + Target::Rest(s) => Some(s), + _ => None, + }) + .nth(1); + if let Some(snd_wildcard) = snd_wildcard { + return Err(nom::Err::Failure(ErrorContext::new( + "`..` can only be used once per tuple pattern", + snd_wildcard, + ))); + } + Ok(targets) +} + #[derive(Debug, PartialEq)] pub struct When<'a> { pub ws: Ws, @@ -347,7 +396,7 @@ impl<'a> When<'a> { WithSpan::new( Self { ws: Ws(pws, nws), - target: Target::Name("_"), + target: Target::Placeholder("_"), nodes, }, start, diff --git a/testing/tests/rest_pattern.rs b/testing/tests/rest_pattern.rs new file mode 100644 index 00000000..28e4d933 --- /dev/null +++ b/testing/tests/rest_pattern.rs @@ -0,0 +1,78 @@ +use rinja::Template; + +#[test] +fn a() { + #[derive(Template)] + #[template(source = "{% if let (a, ..) = abc %}-{{a}}-{% endif %}", ext = "txt")] + struct Tmpl { + abc: (u32, u32, u32), + } + + assert_eq!(Tmpl { abc: (1, 2, 3) }.to_string(), "-1-"); +} + +#[test] +fn ab() { + #[derive(Template)] + #[template( + source = "{% if let (a, b, ..) = abc %}-{{a}}{{b}}-{% endif %}", + ext = "txt" + )] + struct Tmpl { + abc: (u32, u32, u32), + } + + assert_eq!(Tmpl { abc: (1, 2, 3) }.to_string(), "-12-"); +} + +#[test] +fn abc() { + #[derive(Template)] + #[template( + source = "{% if let (a, b, c, ..) = abc %}-{{a}}{{b}}{{c}}-{% endif %}", + ext = "txt" + )] + struct Tmpl1 { + abc: (u32, u32, u32), + } + + assert_eq!(Tmpl1 { abc: (1, 2, 3) }.to_string(), "-123-"); + + assert_eq!(Tmpl2 { abc: (1, 2, 3) }.to_string(), "-123-"); + + #[derive(Template)] + #[template( + source = "{% if let (a, b, c, ..) = abc %}-{{a}}{{b}}{{c}}-{% endif %}", + ext = "txt" + )] + struct Tmpl2 { + abc: (u32, u32, u32), + } + + assert_eq!(Tmpl2 { abc: (1, 2, 3) }.to_string(), "-123-"); +} + +#[test] +fn bc() { + #[derive(Template)] + #[template( + source = "{% if let (.., b, c) = abc %}-{{b}}{{c}}-{% endif %}", + ext = "txt" + )] + struct Tmpl { + abc: (u32, u32, u32), + } + + assert_eq!(Tmpl { abc: (1, 2, 3) }.to_string(), "-23-"); +} + +#[test] +fn c() { + #[derive(Template)] + #[template(source = "{% if let (.., c) = abc %}-{{c}}-{% endif %}", ext = "txt")] + struct Tmpl { + abc: (u32, u32, u32), + } + + assert_eq!(Tmpl { abc: (1, 2, 3) }.to_string(), "-3-"); +} From 6ee37650982033ad16e39f5c108cd33f6a0933d7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 27 Jun 2024 22:04:44 +0200 Subject: [PATCH 2/7] Add regression test for `..` pattern in structs --- testing/tests/let_destructoring.rs | 26 ++++++++++++ .../tests/ui/let_destructuring_has_rest.rs | 40 +++++++++++++++++++ .../ui/let_destructuring_has_rest.stderr | 26 ++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 testing/tests/ui/let_destructuring_has_rest.rs create mode 100644 testing/tests/ui/let_destructuring_has_rest.stderr diff --git a/testing/tests/let_destructoring.rs b/testing/tests/let_destructoring.rs index 5d433028..aaa84839 100644 --- a/testing/tests/let_destructoring.rs +++ b/testing/tests/let_destructoring.rs @@ -119,3 +119,29 @@ fn test_let_destruct_with_path_and_with_keyword() { }; assert_eq!(t.render().unwrap(), "hello"); } + +#[derive(Template)] +#[template( + source = " +{%- if let RestPattern2 { a, b } = x -%}hello {{ a }}{%- endif -%} +{%- if let RestPattern2 { a, b, } = x -%}hello {{ b }}{%- endif -%} +{%- if let RestPattern2 { a, .. } = x -%}hello {{ a }}{%- endif -%} +", + ext = "html" +)] +struct RestPattern { + x: RestPattern2, +} + +struct RestPattern2 { + a: u32, + b: u32, +} + +#[test] +fn test_has_rest_pattern() { + let t = RestPattern { + x: RestPattern2 { a: 0, b: 1 }, + }; + assert_eq!(t.render().unwrap(), "hello 0hello 1hello 0"); +} diff --git a/testing/tests/ui/let_destructuring_has_rest.rs b/testing/tests/ui/let_destructuring_has_rest.rs new file mode 100644 index 00000000..4301bd24 --- /dev/null +++ b/testing/tests/ui/let_destructuring_has_rest.rs @@ -0,0 +1,40 @@ +use rinja::Template; + +struct X { + a: u32, + b: u32, +} + +#[derive(Template)] +#[template(source = " +{%- if let X { a, .. } = x -%}hello {{ a }}{%- endif -%} +", ext = "html")] +struct T1 { + x: X, +} + +#[derive(Template)] +#[template(source = " +{%- if let X { a, .., } = x -%}hello {{ a }}{%- endif -%} +", ext = "html")] +struct T2 { + x: X, +} + +#[derive(Template)] +#[template(source = " +{%- if let X { a .. } = x -%}hello {{ a }}{%- endif -%} +", ext = "html")] +struct T3 { + x: X, +} + +#[derive(Template)] +#[template(source = " +{%- if let X { .. } = x -%}hello {{ a }}{%- endif -%} +", ext = "html")] +struct T4 { + x: X, +} + +fn main() {} diff --git a/testing/tests/ui/let_destructuring_has_rest.stderr b/testing/tests/ui/let_destructuring_has_rest.stderr new file mode 100644 index 00000000..fd22deb5 --- /dev/null +++ b/testing/tests/ui/let_destructuring_has_rest.stderr @@ -0,0 +1,26 @@ +error: unexpected `,` character after `..` + failed to parse template source at row 2, column 20 near: + ", } = x -%}hello {{ a }}{%- endif -%}\n" + --> tests/ui/let_destructuring_has_rest.rs:16:10 + | +16 | #[derive(Template)] + | ^^^^^^^^ + | + = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: failed to parse template source at row 2, column 17 near: + ".. } = x -%}hello {{ a }}{%- endif -%}\n" + --> tests/ui/let_destructuring_has_rest.rs:24:10 + | +24 | #[derive(Template)] + | ^^^^^^^^ + | + = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0609]: no field `a` on type `&T4` + --> tests/ui/let_destructuring_has_rest.rs:32:10 + | +32 | #[derive(Template)] + | ^^^^^^^^ unknown field + | + = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) From 6b8b97672abebc7daa3f46ec6bc2d8cc25de3233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Fri, 28 Jun 2024 22:48:45 +0200 Subject: [PATCH 3/7] Better error message on missing comma --- rinja_parser/src/node.rs | 81 ++++++++++--------- .../ui/let_destructuring_has_rest.stderr | 3 +- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/rinja_parser/src/node.rs b/rinja_parser/src/node.rs index 2c1dccb9..fc8f5f35 100644 --- a/rinja_parser/src/node.rs +++ b/rinja_parser/src/node.rs @@ -8,8 +8,8 @@ use nom::combinator::{ }; use nom::error::ErrorKind; use nom::error_position; -use nom::multi::{fold_many0, many0, many1, separated_list0, separated_list1}; -use nom::sequence::{delimited, pair, preceded, terminated, tuple}; +use nom::multi::{many0, many1, separated_list0, separated_list1}; +use nom::sequence::{delimited, pair, preceded, tuple}; use crate::{not_ws, ErrorContext, ParseErr, ParseResult, WithSpan}; @@ -207,7 +207,6 @@ impl<'a> Target<'a> { /// Parses a single target without an `or`, unless it is wrapped in parentheses. fn parse_one(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { let mut opt_opening_paren = map(opt(ws(char('('))), |o| o.is_some()); - let mut opt_closing_paren = map(opt(ws(char(')'))), |o| o.is_some()); let mut opt_opening_brace = map(opt(ws(char('{'))), |o| o.is_some()); let (i, lit) = opt(Self::lit)(i)?; @@ -218,29 +217,10 @@ impl<'a> Target<'a> { // match tuples and unused parentheses let (i, target_is_tuple) = opt_opening_paren(i)?; if target_is_tuple { - let (i, is_empty_tuple) = opt_closing_paren(i)?; - if is_empty_tuple { - return Ok((i, Self::Tuple(Vec::new(), Vec::new()))); + let (i, (singleton, mut targets)) = collect_targets(i, s, ')', Self::unnamed)?; + if singleton { + return Ok((i, targets.pop().unwrap())); } - - let (i, first_target) = Self::unnamed(i, s)?; - let (i, is_unused_paren) = opt_closing_paren(i)?; - if is_unused_paren { - return Ok((i, first_target)); - } - - let mut targets = vec![first_target]; - let (i, _) = cut(tuple(( - fold_many0( - preceded(ws(char(',')), |i| Self::unnamed(i, s)), - || (), - |_, target| { - targets.push(target); - }, - ), - opt(ws(char(','))), - ws(cut(char(')'))), - )))(i)?; return Ok((i, Self::Tuple(Vec::new(), only_one_rest_pattern(targets)?))); } @@ -259,25 +239,13 @@ impl<'a> Target<'a> { let (i, is_unnamed_struct) = opt_opening_paren(i)?; if is_unnamed_struct { - let (i, targets) = alt(( - map(char(')'), |_| Vec::new()), - terminated( - cut(separated_list1(ws(char(',')), |i| Self::unnamed(i, s))), - pair(opt(ws(char(','))), ws(cut(char(')')))), - ), - ))(i)?; + let (i, (_, targets)) = collect_targets(i, s, ')', Self::unnamed)?; return Ok((i, Self::Tuple(path, only_one_rest_pattern(targets)?))); } let (i, is_named_struct) = opt_opening_brace(i)?; if is_named_struct { - let (i, targets) = alt(( - map(char('}'), |_| Vec::new()), - terminated( - cut(separated_list1(ws(char(',')), |i| Self::named(i, s))), - pair(opt(ws(char(','))), ws(cut(char('}')))), - ), - ))(i)?; + let (i, (_, targets)) = collect_targets(i, s, '}', Self::named)?; return Ok((i, Self::Struct(path, targets))); } @@ -353,6 +321,41 @@ impl<'a> Target<'a> { } } +fn collect_targets<'a, T>( + i: &'a str, + s: &State<'_>, + delim: char, + mut one: impl FnMut(&'a str, &State<'_>) -> ParseResult<'a, T>, +) -> ParseResult<'a, (bool, Vec)> { + let opt_comma = |i| map(ws(opt(char(','))), |o| o.is_some())(i); + let opt_end = |i| map(ws(opt(char(delim))), |o| o.is_some())(i); + + let (i, has_end) = opt_end(i)?; + if has_end { + return Ok((i, (false, Vec::new()))); + } + + let (i, targets) = opt(separated_list1(ws(char(',')), |i| one(i, s)))(i)?; + let Some(targets) = targets else { + return Err(nom::Err::Failure(ErrorContext::new( + "expected comma separated list of members", + i, + ))); + }; + + let (i, (has_comma, has_end)) = pair(opt_comma, opt_end)(i)?; + if !has_end { + let msg = match has_comma { + true => format!("expected member, or `{delim}` as terminator"), + false => format!("expected `,` for more members, or `{delim}` as terminator"), + }; + return Err(nom::Err::Failure(ErrorContext::new(msg, i))); + } + + let singleton = !has_comma && targets.len() == 1; + Ok((i, (singleton, targets))) +} + fn only_one_rest_pattern(targets: Vec>) -> Result>, ParseErr<'_>> { let snd_wildcard = targets .iter() diff --git a/testing/tests/ui/let_destructuring_has_rest.stderr b/testing/tests/ui/let_destructuring_has_rest.stderr index fd22deb5..fe1b84fa 100644 --- a/testing/tests/ui/let_destructuring_has_rest.stderr +++ b/testing/tests/ui/let_destructuring_has_rest.stderr @@ -8,7 +8,8 @@ error: unexpected `,` character after `..` | = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) -error: failed to parse template source at row 2, column 17 near: +error: expected `,` for more members, or `}` as terminator + failed to parse template source at row 2, column 17 near: ".. } = x -%}hello {{ a }}{%- endif -%}\n" --> tests/ui/let_destructuring_has_rest.rs:24:10 | From e84e1a6af271433e0e403ef6d5be4514bc54b802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Fri, 28 Jun 2024 22:56:34 +0200 Subject: [PATCH 4/7] Move Target into its own file --- rinja_derive/src/generator.rs | 5 +- rinja_parser/src/lib.rs | 3 + rinja_parser/src/node.rs | 212 +--------------------------------- rinja_parser/src/target.rs | 210 +++++++++++++++++++++++++++++++++ 4 files changed, 221 insertions(+), 209 deletions(-) create mode 100644 rinja_parser/src/target.rs diff --git a/rinja_derive/src/generator.rs b/rinja_derive/src/generator.rs index f2fffbb3..e63cb5c9 100644 --- a/rinja_derive/src/generator.rs +++ b/rinja_derive/src/generator.rs @@ -12,10 +12,9 @@ use crate::input::{Source, TemplateInput}; use crate::{CompileError, CRATE}; use parser::node::{ - Call, Comment, CondTest, FilterBlock, If, Include, Let, Lit, Loop, Match, Target, Whitespace, - Ws, + Call, Comment, CondTest, FilterBlock, If, Include, Let, Lit, Loop, Match, Whitespace, Ws, }; -use parser::{Expr, Filter, Node, WithSpan}; +use parser::{Expr, Filter, Node, Target, WithSpan}; use quote::quote; pub(crate) struct Generator<'a> { diff --git a/rinja_parser/src/lib.rs b/rinja_parser/src/lib.rs index ae88a544..a7ffb195 100644 --- a/rinja_parser/src/lib.rs +++ b/rinja_parser/src/lib.rs @@ -22,6 +22,9 @@ pub mod expr; pub use expr::{Expr, Filter}; pub mod node; pub use node::Node; + +mod target; +pub use target::Target; #[cfg(test)] mod tests; diff --git a/rinja_parser/src/node.rs b/rinja_parser/src/node.rs index fc8f5f35..de209921 100644 --- a/rinja_parser/src/node.rs +++ b/rinja_parser/src/node.rs @@ -2,20 +2,16 @@ use std::str; use nom::branch::alt; use nom::bytes::complete::{tag, take_till}; -use nom::character::complete::{char, one_of}; -use nom::combinator::{ - complete, consumed, cut, eof, map, map_res, not, opt, peek, recognize, value, -}; +use nom::character::complete::char; +use nom::combinator::{complete, consumed, cut, eof, map, not, opt, peek, recognize, value}; use nom::error::ErrorKind; use nom::error_position; -use nom::multi::{many0, many1, separated_list0, separated_list1}; +use nom::multi::{many0, many1, separated_list0}; use nom::sequence::{delimited, pair, preceded, tuple}; -use crate::{not_ws, ErrorContext, ParseErr, ParseResult, WithSpan}; - -use super::{ - bool_lit, char_lit, filter, identifier, is_ws, keyword, num_lit, path_or_identifier, skip_till, - str_lit, ws, Expr, Filter, PathOrIdentifier, State, +use crate::{ + filter, identifier, is_ws, keyword, not_ws, skip_till, str_lit, ws, ErrorContext, Expr, Filter, + ParseResult, State, Target, WithSpan, }; #[derive(Debug, PartialEq)] @@ -177,202 +173,6 @@ impl<'a> Node<'a> { } } -#[derive(Clone, Debug, PartialEq)] -pub enum Target<'a> { - Name(&'a str), - Tuple(Vec<&'a str>, Vec>), - Struct(Vec<&'a str>, Vec<(&'a str, Target<'a>)>), - NumLit(&'a str), - StrLit(&'a str), - CharLit(&'a str), - BoolLit(&'a str), - Path(Vec<&'a str>), - OrChain(Vec>), - Placeholder(&'a str), - Rest(&'a str), -} - -impl<'a> Target<'a> { - /// Parses multiple targets with `or` separating them - pub(super) fn parse(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { - map( - separated_list1(ws(tag("or")), |i| s.nest(i, |i| Self::parse_one(i, s))), - |mut opts| match opts.len() { - 1 => opts.pop().unwrap(), - _ => Self::OrChain(opts), - }, - )(i) - } - - /// Parses a single target without an `or`, unless it is wrapped in parentheses. - fn parse_one(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { - let mut opt_opening_paren = map(opt(ws(char('('))), |o| o.is_some()); - let mut opt_opening_brace = map(opt(ws(char('{'))), |o| o.is_some()); - - let (i, lit) = opt(Self::lit)(i)?; - if let Some(lit) = lit { - return Ok((i, lit)); - } - - // match tuples and unused parentheses - let (i, target_is_tuple) = opt_opening_paren(i)?; - if target_is_tuple { - let (i, (singleton, mut targets)) = collect_targets(i, s, ')', Self::unnamed)?; - if singleton { - return Ok((i, targets.pop().unwrap())); - } - return Ok((i, Self::Tuple(Vec::new(), only_one_rest_pattern(targets)?))); - } - - let path = |i| { - map_res(path_or_identifier, |v| match v { - PathOrIdentifier::Path(v) => Ok(v), - PathOrIdentifier::Identifier(v) => Err(v), - })(i) - }; - - // match structs - let (i, path) = opt(path)(i)?; - if let Some(path) = path { - let i_before_matching_with = i; - let (i, _) = opt(ws(keyword("with")))(i)?; - - let (i, is_unnamed_struct) = opt_opening_paren(i)?; - if is_unnamed_struct { - let (i, (_, targets)) = collect_targets(i, s, ')', Self::unnamed)?; - return Ok((i, Self::Tuple(path, only_one_rest_pattern(targets)?))); - } - - let (i, is_named_struct) = opt_opening_brace(i)?; - if is_named_struct { - let (i, (_, targets)) = collect_targets(i, s, '}', Self::named)?; - return Ok((i, Self::Struct(path, targets))); - } - - return Ok((i_before_matching_with, Self::Path(path))); - } - - // neither literal nor struct nor path - let (new_i, name) = identifier(i)?; - let target = match name { - "_" => Self::Placeholder(name), - _ => Self::verify_name(i, name)?, - }; - Ok((new_i, target)) - } - - fn lit(i: &'a str) -> ParseResult<'a, Self> { - alt(( - map(str_lit, Self::StrLit), - map(char_lit, Self::CharLit), - map(num_lit, Self::NumLit), - map(bool_lit, Self::BoolLit), - ))(i) - } - - fn unnamed(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { - alt((Self::rest, |i| Self::parse(i, s)))(i) - } - - fn named(init_i: &'a str, s: &State<'_>) -> ParseResult<'a, (&'a str, Self)> { - let (i, rest) = opt(consumed(Self::rest))(init_i)?; - if let Some(rest) = rest { - let (_, chr) = ws(opt(one_of(",:")))(i)?; - if let Some(chr) = chr { - return Err(nom::Err::Failure(ErrorContext::new( - format!("unexpected `{chr}` character after `..`"), - i, - ))); - } - return Ok((i, rest)); - } - - let (i, (src, target)) = pair( - identifier, - opt(preceded(ws(char(':')), |i| Self::parse(i, s))), - )(init_i)?; - - if src == "_" { - return Err(nom::Err::Failure(ErrorContext::new( - "cannot use placeholder `_` as source in named struct", - init_i, - ))); - } - - let target = match target { - Some(target) => target, - None => Self::verify_name(init_i, src)?, - }; - Ok((i, (src, target))) - } - - fn rest(i: &'a str) -> ParseResult<'a, Self> { - map(tag(".."), Self::Rest)(i) - } - - fn verify_name(input: &'a str, name: &'a str) -> Result>> { - match name { - "self" | "writer" => Err(nom::Err::Failure(ErrorContext::new( - format!("cannot use `{name}` as a name"), - input, - ))), - _ => Ok(Self::Name(name)), - } - } -} - -fn collect_targets<'a, T>( - i: &'a str, - s: &State<'_>, - delim: char, - mut one: impl FnMut(&'a str, &State<'_>) -> ParseResult<'a, T>, -) -> ParseResult<'a, (bool, Vec)> { - let opt_comma = |i| map(ws(opt(char(','))), |o| o.is_some())(i); - let opt_end = |i| map(ws(opt(char(delim))), |o| o.is_some())(i); - - let (i, has_end) = opt_end(i)?; - if has_end { - return Ok((i, (false, Vec::new()))); - } - - let (i, targets) = opt(separated_list1(ws(char(',')), |i| one(i, s)))(i)?; - let Some(targets) = targets else { - return Err(nom::Err::Failure(ErrorContext::new( - "expected comma separated list of members", - i, - ))); - }; - - let (i, (has_comma, has_end)) = pair(opt_comma, opt_end)(i)?; - if !has_end { - let msg = match has_comma { - true => format!("expected member, or `{delim}` as terminator"), - false => format!("expected `,` for more members, or `{delim}` as terminator"), - }; - return Err(nom::Err::Failure(ErrorContext::new(msg, i))); - } - - let singleton = !has_comma && targets.len() == 1; - Ok((i, (singleton, targets))) -} - -fn only_one_rest_pattern(targets: Vec>) -> Result>, ParseErr<'_>> { - let snd_wildcard = targets - .iter() - .filter_map(|t| match t { - Target::Rest(s) => Some(s), - _ => None, - }) - .nth(1); - if let Some(snd_wildcard) = snd_wildcard { - return Err(nom::Err::Failure(ErrorContext::new( - "`..` can only be used once per tuple pattern", - snd_wildcard, - ))); - } - Ok(targets) -} - #[derive(Debug, PartialEq)] pub struct When<'a> { pub ws: Ws, diff --git a/rinja_parser/src/target.rs b/rinja_parser/src/target.rs new file mode 100644 index 00000000..85eb1fea --- /dev/null +++ b/rinja_parser/src/target.rs @@ -0,0 +1,210 @@ +use nom::branch::alt; +use nom::bytes::complete::tag; +use nom::character::complete::{char, one_of}; +use nom::combinator::{consumed, map, map_res, opt}; +use nom::multi::separated_list1; +use nom::sequence::{pair, preceded}; + +use crate::{ + bool_lit, char_lit, identifier, keyword, num_lit, path_or_identifier, str_lit, ws, + ErrorContext, ParseErr, ParseResult, PathOrIdentifier, State, +}; + +#[derive(Clone, Debug, PartialEq)] +pub enum Target<'a> { + Name(&'a str), + Tuple(Vec<&'a str>, Vec>), + Struct(Vec<&'a str>, Vec<(&'a str, Target<'a>)>), + NumLit(&'a str), + StrLit(&'a str), + CharLit(&'a str), + BoolLit(&'a str), + Path(Vec<&'a str>), + OrChain(Vec>), + Placeholder(&'a str), + Rest(&'a str), +} + +impl<'a> Target<'a> { + /// Parses multiple targets with `or` separating them + pub(super) fn parse(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { + map( + separated_list1(ws(tag("or")), |i| s.nest(i, |i| Self::parse_one(i, s))), + |mut opts| match opts.len() { + 1 => opts.pop().unwrap(), + _ => Self::OrChain(opts), + }, + )(i) + } + + /// Parses a single target without an `or`, unless it is wrapped in parentheses. + fn parse_one(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { + let mut opt_opening_paren = map(opt(ws(char('('))), |o| o.is_some()); + let mut opt_opening_brace = map(opt(ws(char('{'))), |o| o.is_some()); + + let (i, lit) = opt(Self::lit)(i)?; + if let Some(lit) = lit { + return Ok((i, lit)); + } + + // match tuples and unused parentheses + let (i, target_is_tuple) = opt_opening_paren(i)?; + if target_is_tuple { + let (i, (singleton, mut targets)) = collect_targets(i, s, ')', Self::unnamed)?; + if singleton { + return Ok((i, targets.pop().unwrap())); + } + return Ok((i, Self::Tuple(Vec::new(), only_one_rest_pattern(targets)?))); + } + + let path = |i| { + map_res(path_or_identifier, |v| match v { + PathOrIdentifier::Path(v) => Ok(v), + PathOrIdentifier::Identifier(v) => Err(v), + })(i) + }; + + // match structs + let (i, path) = opt(path)(i)?; + if let Some(path) = path { + let i_before_matching_with = i; + let (i, _) = opt(ws(keyword("with")))(i)?; + + let (i, is_unnamed_struct) = opt_opening_paren(i)?; + if is_unnamed_struct { + let (i, (_, targets)) = collect_targets(i, s, ')', Self::unnamed)?; + return Ok((i, Self::Tuple(path, only_one_rest_pattern(targets)?))); + } + + let (i, is_named_struct) = opt_opening_brace(i)?; + if is_named_struct { + let (i, (_, targets)) = collect_targets(i, s, '}', Self::named)?; + return Ok((i, Self::Struct(path, targets))); + } + + return Ok((i_before_matching_with, Self::Path(path))); + } + + // neither literal nor struct nor path + let (new_i, name) = identifier(i)?; + let target = match name { + "_" => Self::Placeholder(name), + _ => verify_name(i, name)?, + }; + Ok((new_i, target)) + } + + fn lit(i: &'a str) -> ParseResult<'a, Self> { + alt(( + map(str_lit, Self::StrLit), + map(char_lit, Self::CharLit), + map(num_lit, Self::NumLit), + map(bool_lit, Self::BoolLit), + ))(i) + } + + fn unnamed(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { + alt((Self::rest, |i| Self::parse(i, s)))(i) + } + + fn named(init_i: &'a str, s: &State<'_>) -> ParseResult<'a, (&'a str, Self)> { + let (i, rest) = opt(consumed(Self::rest))(init_i)?; + if let Some(rest) = rest { + let (_, chr) = ws(opt(one_of(",:")))(i)?; + if let Some(chr) = chr { + return Err(nom::Err::Failure(ErrorContext::new( + format!("unexpected `{chr}` character after `..`"), + i, + ))); + } + return Ok((i, rest)); + } + + let (i, (src, target)) = pair( + identifier, + opt(preceded(ws(char(':')), |i| Self::parse(i, s))), + )(init_i)?; + + if src == "_" { + return Err(nom::Err::Failure(ErrorContext::new( + "cannot use placeholder `_` as source in named struct", + init_i, + ))); + } + + let target = match target { + Some(target) => target, + None => verify_name(init_i, src)?, + }; + Ok((i, (src, target))) + } + + fn rest(i: &'a str) -> ParseResult<'a, Self> { + map(tag(".."), Self::Rest)(i) + } +} + +fn verify_name<'a>( + input: &'a str, + name: &'a str, +) -> Result, nom::Err>> { + match name { + "self" | "writer" => Err(nom::Err::Failure(ErrorContext::new( + format!("cannot use `{name}` as a name"), + input, + ))), + _ => Ok(Target::Name(name)), + } +} + +fn collect_targets<'a, T>( + i: &'a str, + s: &State<'_>, + delim: char, + mut one: impl FnMut(&'a str, &State<'_>) -> ParseResult<'a, T>, +) -> ParseResult<'a, (bool, Vec)> { + let opt_comma = |i| map(ws(opt(char(','))), |o| o.is_some())(i); + let opt_end = |i| map(ws(opt(char(delim))), |o| o.is_some())(i); + + let (i, has_end) = opt_end(i)?; + if has_end { + return Ok((i, (false, Vec::new()))); + } + + let (i, targets) = opt(separated_list1(ws(char(',')), |i| one(i, s)))(i)?; + let Some(targets) = targets else { + return Err(nom::Err::Failure(ErrorContext::new( + "expected comma separated list of members", + i, + ))); + }; + + let (i, (has_comma, has_end)) = pair(opt_comma, opt_end)(i)?; + if !has_end { + let msg = match has_comma { + true => format!("expected member, or `{delim}` as terminator"), + false => format!("expected `,` for more members, or `{delim}` as terminator"), + }; + return Err(nom::Err::Failure(ErrorContext::new(msg, i))); + } + + let singleton = !has_comma && targets.len() == 1; + Ok((i, (singleton, targets))) +} + +fn only_one_rest_pattern(targets: Vec>) -> Result>, ParseErr<'_>> { + let snd_wildcard = targets + .iter() + .filter_map(|t| match t { + Target::Rest(s) => Some(s), + _ => None, + }) + .nth(1); + if let Some(snd_wildcard) = snd_wildcard { + return Err(nom::Err::Failure(ErrorContext::new( + "`..` can only be used once per tuple pattern", + snd_wildcard, + ))); + } + Ok(targets) +} From cf4f514d7e9dc70076f3669d3a5b3512d2ea73a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Fri, 28 Jun 2024 23:02:07 +0200 Subject: [PATCH 5/7] Move passing tests --- testing/tests/let_destructoring.rs | 37 +++++++++++++++++++ .../tests/ui/let_destructuring_has_rest.rs | 14 ++----- .../ui/let_destructuring_has_rest.stderr | 24 ++++++------ 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/testing/tests/let_destructoring.rs b/testing/tests/let_destructoring.rs index aaa84839..9d687a5f 100644 --- a/testing/tests/let_destructoring.rs +++ b/testing/tests/let_destructoring.rs @@ -145,3 +145,40 @@ fn test_has_rest_pattern() { }; assert_eq!(t.render().unwrap(), "hello 0hello 1hello 0"); } + +struct X { + a: u32, + b: u32, +} + +#[derive(Template)] +#[template(source = " +{%- if let X { a, .. } = x -%}hello {{ a }}{%- endif -%} +", ext = "html")] +struct T1 { + x: X, +} + +#[test] +fn test_t1() { + let t = T1 { + x: X { a: 1, b: 2 }, + }; + assert_eq!(t.render().unwrap(), "hello 1"); +} + +#[derive(Template)] +#[template(source = " +{%- if let X { .. } = x -%}hello{%- endif -%} +", ext = "html")] +struct T2 { + x: X, +} + +#[test] +fn test_t2() { + let t = T2 { + x: X { a: 1, b: 2 }, + }; + assert_eq!(t.render().unwrap(), "hello"); +} diff --git a/testing/tests/ui/let_destructuring_has_rest.rs b/testing/tests/ui/let_destructuring_has_rest.rs index 4301bd24..c65fbf51 100644 --- a/testing/tests/ui/let_destructuring_has_rest.rs +++ b/testing/tests/ui/let_destructuring_has_rest.rs @@ -7,7 +7,7 @@ struct X { #[derive(Template)] #[template(source = " -{%- if let X { a, .. } = x -%}hello {{ a }}{%- endif -%} +{%- if let X { a, .., } = x -%}hello {{ a }}{%- endif -%} ", ext = "html")] struct T1 { x: X, @@ -15,7 +15,7 @@ struct T1 { #[derive(Template)] #[template(source = " -{%- if let X { a, .., } = x -%}hello {{ a }}{%- endif -%} +{%- if let X { a .. } = x -%}hello {{ a }}{%- endif -%} ", ext = "html")] struct T2 { x: X, @@ -23,18 +23,10 @@ struct T2 { #[derive(Template)] #[template(source = " -{%- if let X { a .. } = x -%}hello {{ a }}{%- endif -%} +{%- if let X { a, 1 } = x -%}hello {{ a }}{%- endif -%} ", ext = "html")] struct T3 { x: X, } -#[derive(Template)] -#[template(source = " -{%- if let X { .. } = x -%}hello {{ a }}{%- endif -%} -", ext = "html")] -struct T4 { - x: X, -} - fn main() {} diff --git a/testing/tests/ui/let_destructuring_has_rest.stderr b/testing/tests/ui/let_destructuring_has_rest.stderr index fe1b84fa..28b747e5 100644 --- a/testing/tests/ui/let_destructuring_has_rest.stderr +++ b/testing/tests/ui/let_destructuring_has_rest.stderr @@ -1,6 +1,16 @@ error: unexpected `,` character after `..` failed to parse template source at row 2, column 20 near: ", } = x -%}hello {{ a }}{%- endif -%}\n" + --> tests/ui/let_destructuring_has_rest.rs:8:10 + | +8 | #[derive(Template)] + | ^^^^^^^^ + | + = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: expected `,` for more members, or `}` as terminator + failed to parse template source at row 2, column 17 near: + ".. } = x -%}hello {{ a }}{%- endif -%}\n" --> tests/ui/let_destructuring_has_rest.rs:16:10 | 16 | #[derive(Template)] @@ -8,20 +18,12 @@ error: unexpected `,` character after `..` | = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) -error: expected `,` for more members, or `}` as terminator - failed to parse template source at row 2, column 17 near: - ".. } = x -%}hello {{ a }}{%- endif -%}\n" +error: expected member, or `}` as terminator + failed to parse template source at row 2, column 18 near: + "1 } = x -%}hello {{ a }}{%- endif -%}\n" --> tests/ui/let_destructuring_has_rest.rs:24:10 | 24 | #[derive(Template)] | ^^^^^^^^ | = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) - -error[E0609]: no field `a` on type `&T4` - --> tests/ui/let_destructuring_has_rest.rs:32:10 - | -32 | #[derive(Template)] - | ^^^^^^^^ unknown field - | - = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) From 0d8373e3b21a8c90e72038a95542c6a37c6a9426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Fri, 28 Jun 2024 23:06:21 +0200 Subject: [PATCH 6/7] Linted --- testing/tests/let_destructoring.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/testing/tests/let_destructoring.rs b/testing/tests/let_destructoring.rs index 9d687a5f..383d56d5 100644 --- a/testing/tests/let_destructoring.rs +++ b/testing/tests/let_destructoring.rs @@ -146,15 +146,19 @@ fn test_has_rest_pattern() { assert_eq!(t.render().unwrap(), "hello 0hello 1hello 0"); } +#[allow(dead_code)] struct X { a: u32, b: u32, } #[derive(Template)] -#[template(source = " +#[template( + source = " {%- if let X { a, .. } = x -%}hello {{ a }}{%- endif -%} -", ext = "html")] +", + ext = "html" +)] struct T1 { x: X, } @@ -168,9 +172,12 @@ fn test_t1() { } #[derive(Template)] -#[template(source = " +#[template( + source = " {%- if let X { .. } = x -%}hello{%- endif -%} -", ext = "html")] +", + ext = "html" +)] struct T2 { x: X, } From f4ccdb6585f98273d4f8f8660fc5156f880c9dac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Kijewski?= Date: Fri, 28 Jun 2024 23:32:13 +0200 Subject: [PATCH 7/7] Better information that `..` must come last in a named struct --- rinja_parser/src/target.rs | 5 +++- .../tests/ui/let_destructuring_has_rest.rs | 16 +++++++++++++ .../ui/let_destructuring_has_rest.stderr | 23 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/rinja_parser/src/target.rs b/rinja_parser/src/target.rs index 85eb1fea..5f6a7c5e 100644 --- a/rinja_parser/src/target.rs +++ b/rinja_parser/src/target.rs @@ -113,7 +113,10 @@ impl<'a> Target<'a> { let (_, chr) = ws(opt(one_of(",:")))(i)?; if let Some(chr) = chr { return Err(nom::Err::Failure(ErrorContext::new( - format!("unexpected `{chr}` character after `..`"), + format!( + "unexpected `{chr}` character after `..`\n\ + note that in a named struct, `..` must come last to ignore other members" + ), i, ))); } diff --git a/testing/tests/ui/let_destructuring_has_rest.rs b/testing/tests/ui/let_destructuring_has_rest.rs index c65fbf51..fe002138 100644 --- a/testing/tests/ui/let_destructuring_has_rest.rs +++ b/testing/tests/ui/let_destructuring_has_rest.rs @@ -29,4 +29,20 @@ struct T3 { x: X, } +#[derive(Template)] +#[template(source = " +{%- if let X { a, .., b } = x -%}hello {{ a }}{%- endif -%} +", ext = "html")] +struct T4 { + x: X, +} + +#[derive(Template)] +#[template(source = " +{%- if let X { .., b } = x -%}hello {{ a }}{%- endif -%} +", ext = "html")] +struct T5 { + x: X, +} + fn main() {} diff --git a/testing/tests/ui/let_destructuring_has_rest.stderr b/testing/tests/ui/let_destructuring_has_rest.stderr index 28b747e5..7be69205 100644 --- a/testing/tests/ui/let_destructuring_has_rest.stderr +++ b/testing/tests/ui/let_destructuring_has_rest.stderr @@ -1,4 +1,5 @@ error: unexpected `,` character after `..` + note that in a named struct, `..` must come last to ignore other members failed to parse template source at row 2, column 20 near: ", } = x -%}hello {{ a }}{%- endif -%}\n" --> tests/ui/let_destructuring_has_rest.rs:8:10 @@ -27,3 +28,25 @@ error: expected member, or `}` as terminator | ^^^^^^^^ | = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: unexpected `,` character after `..` + note that in a named struct, `..` must come last to ignore other members + failed to parse template source at row 2, column 20 near: + ", b } = x -%}hello {{ a }}{%- endif -%}\n" + --> tests/ui/let_destructuring_has_rest.rs:32:10 + | +32 | #[derive(Template)] + | ^^^^^^^^ + | + = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: unexpected `,` character after `..` + note that in a named struct, `..` must come last to ignore other members + failed to parse template source at row 2, column 17 near: + ", b } = x -%}hello {{ a }}{%- endif -%}\n" + --> tests/ui/let_destructuring_has_rest.rs:40:10 + | +40 | #[derive(Template)] + | ^^^^^^^^ + | + = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)