From e31f4e210e383982ce14f1e61c49e520416dcb83 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 25 Nov 2024 16:56:11 -0600 Subject: [PATCH] refactor(parser): Be explicit about `i`nput state (#275) * refactor(parser): In 'filter', name a checkpoint * refactor(parser): In 'Target::named', name a checkpoint * refactor(parser): In 'Target::named', remove a checkpoint * refactor(parser): In 'Target::rest', name a checkpoint * refactor(parser): In 'Target::parse_one', name a checkpoint * refactor(parser): In 'Target::parse', align the input * refactor(parser): In 'Expr::is_as', name a checkpoint * refactor(parser): In 'Expr::concat', name a checkpoint * refactor(parser): In 'Suffix::parse', name a checkpoint * refactor(parser): In 'Node::break', name a checkpoint * refactor(parser): In 'Node::continue', name a checkpoint * refactor(parser): In 'Macro::parse', name a checkpoint * refactor(parser): In 'When::else', name a checkpoint --- rinja_parser/src/expr.rs | 58 ++++++++++++++++++++++++-------------- rinja_parser/src/lib.rs | 7 +++-- rinja_parser/src/node.rs | 37 +++++++++++++----------- rinja_parser/src/target.rs | 36 ++++++++++++----------- 4 files changed, 82 insertions(+), 56 deletions(-) diff --git a/rinja_parser/src/expr.rs b/rinja_parser/src/expr.rs index e6c595fc..eb9741d8 100644 --- a/rinja_parser/src/expr.rs +++ b/rinja_parser/src/expr.rs @@ -249,17 +249,19 @@ impl<'a> Expr<'a> { fn concat(i: &'a str, level: Level) -> ParseResult<'a, WithSpan<'a, Self>> { fn concat_expr(i: &str, level: Level) -> ParseResult<'_, Option>>> { let ws1 = |i| opt(skip_ws1).parse_next(i); - let (j, data) = opt((ws1, '~', ws1, |i| Expr::muldivmod(i, level))).parse_next(i)?; + + let start = i; + let (i, data) = opt((ws1, '~', ws1, |i| Expr::muldivmod(i, level))).parse_next(i)?; if let Some((t1, _, t2, expr)) = data { if t1.is_none() || t2.is_none() { return Err(winnow::error::ErrMode::Cut(ErrorContext::new( "the concat operator `~` must be surrounded by spaces", - i, + start, ))); } - Ok((j, Some(expr))) + Ok((i, Some(expr))) } else { - Ok((j, None)) + Ok((i, None)) } } @@ -282,12 +284,13 @@ impl<'a> Expr<'a> { fn is_as(i: &'a str, level: Level) -> ParseResult<'a, WithSpan<'a, Self>> { let start = i; - let (before_keyword, lhs) = Self::filtered(i, level)?; - let (j, rhs) = opt(ws(identifier)).parse_next(before_keyword)?; + let (i, lhs) = Self::filtered(i, level)?; + let before_keyword = i; + let (i, rhs) = opt(ws(identifier)).parse_next(i)?; let i = match rhs { - Some("is") => j, + Some("is") => i, Some("as") => { - let (i, target) = opt(identifier).parse_next(j)?; + let (i, target) = opt(identifier).parse_next(i)?; let target = target.unwrap_or_default(); if crate::PRIMITIVE_TYPES.contains(&target) { return Ok((i, WithSpan::new(Self::As(Box::new(lhs), target), start))); @@ -306,7 +309,10 @@ impl<'a> Expr<'a> { ))); } } - _ => return Ok((before_keyword, lhs)), + _ => { + let i = before_keyword; + return Ok((i, lhs)); + } }; let (i, rhs) = @@ -545,6 +551,7 @@ impl<'a> Suffix<'a> { let (_, level) = level.nest(i)?; let (mut i, mut expr) = Expr::single(i, level)?; loop { + let before_suffix = i; let (j, suffix) = opt(alt(( Self::attr, |i| Self::index(i, level), @@ -553,27 +560,36 @@ impl<'a> Suffix<'a> { Self::r#macro, ))) .parse_next(i)?; + i = j; match suffix { - Some(Self::Attr(attr)) => expr = WithSpan::new(Expr::Attr(expr.into(), attr), i), - Some(Self::Index(index)) => { - expr = WithSpan::new(Expr::Index(expr.into(), index.into()), i); + Some(Self::Attr(attr)) => { + expr = WithSpan::new(Expr::Attr(expr.into(), attr), before_suffix) } - Some(Self::Call(args)) => expr = WithSpan::new(Expr::Call(expr.into(), args), i), - Some(Self::Try) => expr = WithSpan::new(Expr::Try(expr.into()), i), + Some(Self::Index(index)) => { + expr = WithSpan::new(Expr::Index(expr.into(), index.into()), before_suffix); + } + Some(Self::Call(args)) => { + expr = WithSpan::new(Expr::Call(expr.into(), args), before_suffix) + } + Some(Self::Try) => expr = WithSpan::new(Expr::Try(expr.into()), before_suffix), Some(Self::MacroCall(args)) => match expr.inner { - Expr::Path(path) => expr = WithSpan::new(Expr::RustMacro(path, args), i), - Expr::Var(name) => expr = WithSpan::new(Expr::RustMacro(vec![name], args), i), + Expr::Path(path) => { + expr = WithSpan::new(Expr::RustMacro(path, args), before_suffix) + } + Expr::Var(name) => { + expr = WithSpan::new(Expr::RustMacro(vec![name], args), before_suffix) + } _ => { - return Err( - winnow::error::ErrMode::from_error_kind(i, ErrorKind::Tag).cut() - ); + return Err(winnow::error::ErrMode::from_error_kind( + before_suffix, + ErrorKind::Tag, + ) + .cut()); } }, None => break, } - - i = j; } Ok((i, expr)) } diff --git a/rinja_parser/src/lib.rs b/rinja_parser/src/lib.rs index 6f27b493..84e6349c 100644 --- a/rinja_parser/src/lib.rs +++ b/rinja_parser/src/lib.rs @@ -961,10 +961,11 @@ fn filter<'a>( i: &'a str, level: &mut Level, ) -> ParseResult<'a, (&'a str, Option>>>)> { - let (j, _) = ws(('|', not('|'))).parse_next(i)?; + let start = i; + let (i, _) = ws(('|', not('|'))).parse_next(i)?; - *level = level.nest(i)?.1; - cut_err((ws(identifier), opt(|i| Expr::arguments(i, *level, false)))).parse_next(j) + *level = level.nest(start)?.1; + cut_err((ws(identifier), opt(|i| Expr::arguments(i, *level, false)))).parse_next(i) } /// Returns the common parts of two paths. diff --git a/rinja_parser/src/node.rs b/rinja_parser/src/node.rs index f7f56e93..2a23686f 100644 --- a/rinja_parser/src/node.rs +++ b/rinja_parser/src/node.rs @@ -131,14 +131,16 @@ impl<'a> Node<'a> { ws(keyword("break")), opt(Whitespace::parse), ); - let (j, (pws, _, nws)) = p.parse_next(i)?; + + let start = i; + let (i, (pws, _, nws)) = p.parse_next(i)?; if !s.is_in_loop() { return Err(winnow::error::ErrMode::Cut(ErrorContext::new( "you can only `break` inside a `for` loop", - i, + start, ))); } - Ok((j, Self::Break(WithSpan::new(Ws(pws, nws), i)))) + Ok((i, Self::Break(WithSpan::new(Ws(pws, nws), start)))) } fn r#continue(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { @@ -147,14 +149,16 @@ impl<'a> Node<'a> { ws(keyword("continue")), opt(Whitespace::parse), ); - let (j, (pws, _, nws)) = p.parse_next(i)?; + + let start = i; + let (i, (pws, _, nws)) = p.parse_next(i)?; if !s.is_in_loop() { return Err(winnow::error::ErrMode::Cut(ErrorContext::new( "you can only `continue` inside a `for` loop", - i, + start, ))); } - Ok((j, Self::Continue(WithSpan::new(Ws(pws, nws), i)))) + Ok((i, Self::Continue(WithSpan::new(Ws(pws, nws), start)))) } fn expr(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { @@ -263,7 +267,6 @@ pub struct When<'a> { impl<'a> When<'a> { fn r#else(i: &'a str, s: &State<'_>) -> ParseResult<'a, WithSpan<'a, Self>> { - let start = i; let mut p = ( |i| s.tag_block_start(i), opt(Whitespace::parse), @@ -277,13 +280,15 @@ impl<'a> When<'a> { ), ), ); - let (new_i, (_, pws, _, (nws, _, nodes))) = p.parse_next(i)?; + + let start = i; + let (i, (_, pws, _, (nws, _, nodes))) = p.parse_next(i)?; Ok(( - new_i, + i, WithSpan::new( Self { ws: Ws(pws, nws), - target: vec![Target::Placeholder(WithSpan::new((), i))], + target: vec![Target::Placeholder(WithSpan::new((), start))], nodes, }, start, @@ -664,11 +669,11 @@ impl<'a> Macro<'a> { }), ), ); - let (j, (pws1, _, (name, params, nws1, _))) = start.parse_next(i)?; + let (i, (pws1, _, (name, params, nws1, _))) = start.parse_next(i)?; if is_rust_keyword(name) { return Err(winnow::error::ErrMode::Cut(ErrorContext::new( format!("'{name}' is not a valid name for a macro"), - i, + start_s, ))); } @@ -677,17 +682,17 @@ impl<'a> Macro<'a> { let mut iter = params.iter(); while let Some((arg_name, default_value)) = iter.next() { - check_duplicated_name(&mut names, arg_name, i)?; + check_duplicated_name(&mut names, arg_name, start_s)?; if default_value.is_some() { for (new_arg_name, default_value) in iter.by_ref() { - check_duplicated_name(&mut names, new_arg_name, i)?; + check_duplicated_name(&mut names, new_arg_name, start_s)?; if default_value.is_none() { return Err(winnow::error::ErrMode::Cut(ErrorContext::new( format!( "all arguments following `{arg_name}` should have a default \ value, `{new_arg_name}` doesn't have a default value" ), - i, + start_s, ))); } } @@ -719,7 +724,7 @@ impl<'a> Macro<'a> { ), ), ); - let (i, (contents, (_, pws2, _, nws2))) = end.parse_next(j)?; + let (i, (contents, (_, pws2, _, nws2))) = end.parse_next(i)?; Ok(( i, diff --git a/rinja_parser/src/target.rs b/rinja_parser/src/target.rs index 08f56f83..e4af8849 100644 --- a/rinja_parser/src/target.rs +++ b/rinja_parser/src/target.rs @@ -1,5 +1,5 @@ use winnow::Parser; -use winnow::combinator::{alt, opt, preceded, separated1}; +use winnow::combinator::{alt, opt, peek, preceded, separated1}; use winnow::token::one_of; use crate::{ @@ -27,14 +27,13 @@ pub enum Target<'a> { impl<'a> Target<'a> { /// Parses multiple targets with `or` separating them pub(super) fn parse(i: &'a str, s: &State<'_>) -> ParseResult<'a, Self> { - let (new_i, target) = separated1(|i| s.nest(i, |i| Self::parse_one(i, s)), ws("or")) + separated1(|i| s.nest(i, |i| Self::parse_one(i, s)), ws("or")) .map(|v: Vec<_>| v) .map(|mut opts| match opts.len() { 1 => opts.pop().unwrap(), _ => Self::OrChain(opts), }) - .parse_next(i)?; - Ok((new_i, target)) + .parse_next(i) } /// Parses a single target without an `or`, unless it is wrapped in parentheses. @@ -102,12 +101,13 @@ impl<'a> Target<'a> { } // neither literal nor struct nor path - let (new_i, name) = identifier.parse_next(i)?; + let i_before_identifier = i; + let (i, name) = identifier.parse_next(i)?; let target = match name { - "_" => Self::Placeholder(WithSpan::new((), i)), - _ => verify_name(i, name)?, + "_" => Self::Placeholder(WithSpan::new((), i_before_identifier)), + _ => verify_name(i_before_identifier, name)?, }; - Ok((new_i, target)) + Ok((i, target)) } fn lit(i: &'a str) -> ParseResult<'a, Self> { @@ -126,10 +126,11 @@ impl<'a> Target<'a> { alt((Self::rest, |i| Self::parse(i, s))).parse_next(i) } - fn named(init_i: &'a str, s: &State<'_>) -> ParseResult<'a, (&'a str, Self)> { - let (i, rest) = opt(Self::rest.with_recognized()).parse_next(init_i)?; + fn named(i: &'a str, s: &State<'_>) -> ParseResult<'a, (&'a str, Self)> { + let start = i; + let (i, rest) = opt(Self::rest.with_recognized()).parse_next(i)?; if let Some(rest) = rest { - let (_, chr) = ws(opt(one_of([',', ':']))).parse_next(i)?; + let (i, chr) = peek(ws(opt(one_of([',', ':'])))).parse_next(i)?; if let Some(chr) = chr { return Err(winnow::error::ErrMode::Cut(ErrorContext::new( format!( @@ -150,25 +151,28 @@ impl<'a> Target<'a> { return Ok((i, (rest.1, rest.0))); } + let i = start; let (i, (src, target)) = - (identifier, opt(preceded(ws(':'), |i| Self::parse(i, s)))).parse_next(init_i)?; + (identifier, opt(preceded(ws(':'), |i| Self::parse(i, s)))).parse_next(i)?; if src == "_" { + let i = start; return Err(winnow::error::ErrMode::Cut(ErrorContext::new( "cannot use placeholder `_` as source in named struct", - init_i, + i, ))); } let target = match target { Some(target) => target, - None => verify_name(init_i, src)?, + None => verify_name(start, src)?, }; Ok((i, (src, target))) } - fn rest(start: &'a str) -> ParseResult<'a, Self> { - let (i, (ident, _)) = (opt((identifier, ws('@'))), "..").parse_next(start)?; + fn rest(i: &'a str) -> ParseResult<'a, Self> { + let start = i; + let (i, (ident, _)) = (opt((identifier, ws('@'))), "..").parse_next(i)?; Ok(( i, Self::Rest(WithSpan::new(ident.map(|(ident, _)| ident), start)),