From e8e3cfd9fbe69c6e82ea0f85906265c374626515 Mon Sep 17 00:00:00 2001 From: Rain Date: Sun, 26 Nov 2023 13:21:41 -0800 Subject: [PATCH] [nextest-filtering] better error reporting for invalid escapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously we'd report something like error: expected close parenthesis ╭──── 1 │ package(nexte\$) · ▲ · ╰── missing `)` ╰──── error: expected end of expression ╭──── 1 │ package(nexte\$) · ─┬─ · ╰── unparsed input ╰──── Now, we report: error: invalid escape character ╭──── 1 │ package(nexte\$) · ─┬ · ╰── invalid escape character ╰──── --- nextest-filtering/src/errors.rs | 4 + nextest-filtering/src/parsing.rs | 62 ++++++++++++-- .../src/parsing/unicode_string.rs | 82 +++++++++++++------ 3 files changed, 113 insertions(+), 35 deletions(-) diff --git a/nextest-filtering/src/errors.rs b/nextest-filtering/src/errors.rs index 3f74f2db0af..32b273994f9 100644 --- a/nextest-filtering/src/errors.rs +++ b/nextest-filtering/src/errors.rs @@ -69,6 +69,10 @@ pub enum ParseSingleError { #[error("expected close parenthesis")] ExpectedCloseParenthesis(#[label("missing `)`")] SourceSpan), + /// An invalid escape character was found. + #[error("invalid escape character")] + InvalidEscapeCharacter(#[label("invalid escape character")] SourceSpan), + /// An expression was expected in this position but not found. #[error("expected expression")] ExpectedExpr(#[label("missing expression")] SourceSpan), diff --git a/nextest-filtering/src/parsing.rs b/nextest-filtering/src/parsing.rs index 9f1e8a1a99d..6e8c70e2e1a 100644 --- a/nextest-filtering/src/parsing.rs +++ b/nextest-filtering/src/parsing.rs @@ -212,6 +212,7 @@ impl ExprResult { enum SpanLength { Unknown, Exact(usize), + Offset(isize, usize), } fn expect_inner<'a, F, T>( @@ -226,11 +227,21 @@ where Ok((remaining, out)) => Ok((remaining, Some(out))), Err(nom::Err::Error(err)) | Err(nom::Err::Failure(err)) => { let nom::error::Error { input, .. } = err; - let start = input.location_offset(); - let len = input.fragment().len(); + let fragment_start = input.location_offset(); + let fragment_length = input.fragment().len(); let span = match limit { - SpanLength::Unknown => (start, len).into(), - SpanLength::Exact(x) => (start, x.min(len)).into(), + SpanLength::Unknown => (fragment_start, fragment_length).into(), + SpanLength::Exact(x) => (fragment_start, x.min(fragment_length)).into(), + SpanLength::Offset(offset, x) => { + // e.g. fragment_start = 5, fragment_length = 2, offset = -1, x = 3. + // Here, start = 4. + let effective_start = fragment_start.saturating_add_signed(offset); + // end = 6. + let effective_end = effective_start + fragment_length; + // len = min(3, 6 - 4) = 2. + let len = (effective_end - effective_start).min(x); + (effective_start, len).into() + } }; let err = make_err(span); input.extra.report_error(err); @@ -250,6 +261,17 @@ where expect_inner(parser, make_err, SpanLength::Unknown) } +fn expect_n<'a, F, T>( + parser: F, + make_err: fn(SourceSpan) -> ParseSingleError, + limit: SpanLength, +) -> impl FnMut(Span<'a>) -> IResult<'a, Option> +where + F: FnMut(Span<'a>) -> IResult, +{ + expect_inner(parser, make_err, limit) +} + fn expect_char<'a>( c: char, make_err: fn(SourceSpan) -> ParseSingleError, @@ -305,7 +327,7 @@ fn parse_matcher_text(input: Span) -> IResult> { ParseSingleError::InvalidString, )(input.clone()) { - Ok((i, res)) => (i, res), + Ok((i, res)) => (i, res.flatten()), Err(nom::Err::Incomplete(_)) => { let i = input.slice(input.fragment().len()..); // No need for error reporting, missing closing ')' will be detected after @@ -319,6 +341,7 @@ fn parse_matcher_text(input: Span) -> IResult> { i.extra .report_error(ParseSingleError::InvalidString((start..0).into())); } + Ok((i, res)) } @@ -1227,9 +1250,15 @@ mod tests { } macro_rules! assert_error { - ($error:ident, $name:ident, $start:literal, $end:literal) => { - assert!(matches!($error, ParseSingleError::$name(span) if span == ($start, $end).into())); - }; + ($error:ident, $name:ident, $start:literal, $end:literal) => {{ + let matches = matches!($error, ParseSingleError::$name(span) if span == ($start, $end).into()); + assert!( + matches, + "expected: {:?}, actual: error: {:?}", + ParseSingleError::$name(($start, $end).into()), + $error, + ); + }}; } #[test] @@ -1261,6 +1290,23 @@ mod tests { assert_error!(error, ExpectedCloseParenthesis, 3, 0); } + #[test] + fn test_invalid_escapes() { + let src = r"package(foobar\$\#\@baz)"; + let mut errors = parse_err(src); + assert_eq!(3, errors.len()); + + // Ensure all three errors are reported. + let error = errors.remove(0); + assert_error!(error, InvalidEscapeCharacter, 14, 2); + + let error = errors.remove(0); + assert_error!(error, InvalidEscapeCharacter, 16, 2); + + let error = errors.remove(0); + assert_error!(error, InvalidEscapeCharacter, 18, 2); + } + #[test] fn test_invalid_regex() { let src = "package(/)/)"; diff --git a/nextest-filtering/src/parsing/unicode_string.rs b/nextest-filtering/src/parsing/unicode_string.rs index 76a21d2fbae..45df5b6d4f2 100644 --- a/nextest-filtering/src/parsing/unicode_string.rs +++ b/nextest-filtering/src/parsing/unicode_string.rs @@ -3,7 +3,9 @@ // Adapted from https://github.com/Geal/nom/blob/294ffb3d9e0ade2c3b7ddfff52484b6d643dcce1/examples/string.rs -use super::{IResult, Span}; +use crate::errors::ParseSingleError; + +use super::{expect_n, IResult, Span, SpanLength}; use nom::{ branch::alt, bytes::streaming::{is_not, take_while_m_n}, @@ -56,21 +58,28 @@ fn parse_unicode(input: Span) -> IResult { } #[tracable_parser] -fn parse_escaped_char(input: Span) -> IResult { +fn parse_escaped_char(input: Span) -> IResult> { + let valid = alt(( + parse_unicode, + value('\n', char('n')), + value('\r', char('r')), + value('\t', char('t')), + value('\u{08}', char('b')), + value('\u{0C}', char('f')), + value('\\', char('\\')), + value('/', char('/')), + value(')', char(')')), + value(',', char(',')), + )); preceded( char('\\'), - alt(( - parse_unicode, - value('\n', char('n')), - value('\r', char('r')), - value('\t', char('t')), - value('\u{08}', char('b')), - value('\u{0C}', char('f')), - value('\\', char('\\')), - value('/', char('/')), - value(')', char(')')), - value(',', char(',')), - )), + // If none of the valid characters are found, this will report an error. + expect_n( + valid, + ParseSingleError::InvalidEscapeCharacter, + // -1 to account for the preceding backslash. + SpanLength::Offset(-1, 2), + ), )(input) } @@ -105,25 +114,44 @@ enum StringFragment<'a> { } #[tracable_parser] -fn parse_fragment(input: Span) -> IResult> { +fn parse_fragment(input: Span) -> IResult>> { alt(( map(parse_literal, |span| { - StringFragment::Literal(span.fragment()) + Some(StringFragment::Literal(span.fragment())) + }), + map(parse_escaped_char, |res| { + res.map(StringFragment::EscapedChar) }), - map(parse_escaped_char, StringFragment::EscapedChar), ))(input) } -/// Construct a string by consuming the input until the next unescaped `'` +/// Construct a string by consuming the input until the next unescaped ) or ,. +/// +/// Returns None if the string isn't valid. /// -/// Return Err(Incomplete(1)) if not ending `'` is found +/// Returns Err(Incomplete(1)) if an ending delimiter ) or , is not found. #[tracable_parser] -pub(super) fn parse_string(input: Span) -> IResult { - fold_many0(parse_fragment, String::new, |mut string, fragment| { - match fragment { - StringFragment::Literal(s) => string.push_str(s), - StringFragment::EscapedChar(c) => string.push(c), - } - string - })(input) +pub(super) fn parse_string(input: Span) -> IResult> { + fold_many0( + parse_fragment, + || Some(String::new()), + |string, fragment| { + match (string, fragment) { + (Some(mut string), Some(StringFragment::Literal(s))) => { + string.push_str(s); + Some(string) + } + (Some(mut string), Some(StringFragment::EscapedChar(c))) => { + string.push(c); + Some(string) + } + (Some(_), None) => { + // We encountered a parsing error, and at this point we'll stop returning + // values. + None + } + (None, _) => None, + } + }, + )(input) }