From f9abd2354694c6aa0ab17894e8615091c908f640 Mon Sep 17 00:00:00 2001 From: Rasmus Kaj Date: Fri, 25 Oct 2024 22:17:57 +0200 Subject: [PATCH] Improve plain css parsing. Especially improve some parse error messages. --- rsass/src/css/valueformat.rs | 3 +- rsass/src/output/transform.rs | 2 +- rsass/src/parser/css/mod.rs | 10 +++ rsass/src/parser/css/strings.rs | 19 ++++- rsass/src/parser/css/values.rs | 79 ++++++++++++++++--- rsass/src/parser/error.rs | 16 ++-- rsass/src/parser/span.rs | 9 +++ rsass/src/parser/value.rs | 12 +-- rsass/src/sass/functions/call_error.rs | 2 +- rsass/src/value/list_separator.rs | 4 + rsass/tests/misc/css_parser.rs | 2 +- rsass/tests/spec/css/important.rs | 1 - .../css/plain/error/expression/calculation.rs | 4 - .../css/plain/error/expression/function.rs | 2 - .../plain/error/expression/interpolation.rs | 3 - .../css/plain/error/expression/parentheses.rs | 1 - .../css/plain/error/expression/variable.rs | 1 - rsass/tests/spec/css/plain/functions.rs | 2 - rsass/tests/spec/css/plain/slash.rs | 2 - 19 files changed, 122 insertions(+), 52 deletions(-) diff --git a/rsass/src/css/valueformat.rs b/rsass/src/css/valueformat.rs index cf2051f6c..ef5fcbe27 100644 --- a/rsass/src/css/valueformat.rs +++ b/rsass/src/css/valueformat.rs @@ -36,7 +36,8 @@ impl<'a> Display for Formatted<'a, Value> { } let t = v .iter() - .filter(|v| !v.is_null() || introspect) + // TODO: Get rid of this filter entirely? + .filter(|v| !v.is_null() || sep.is_slash() || introspect) .map(|v| { let needs_paren = match *v { Value::List(ref v, inner, false) => { diff --git a/rsass/src/output/transform.rs b/rsass/src/output/transform.rs index 71b8163c4..2cfdf6846 100644 --- a/rsass/src/output/transform.rs +++ b/rsass/src/output/transform.rs @@ -280,7 +280,7 @@ fn handle_item( } e => { let pos = pos.in_call(name); - Error::BadCall(format!("{e:?}"), pos, None) + Error::BadCall(e.to_string(), pos, None) } })?; } else { diff --git a/rsass/src/parser/css/mod.rs b/rsass/src/parser/css/mod.rs index 06f10f54c..abe194167 100644 --- a/rsass/src/parser/css/mod.rs +++ b/rsass/src/parser/css/mod.rs @@ -11,6 +11,7 @@ use nom::character::complete::{multispace0, multispace1}; use nom::combinator::{ all_consuming, into, map, map_res, not, opt, peek, recognize, }; +use nom::error::{VerboseError, VerboseErrorKind}; use nom::multi::{fold_many0, many0, many_till}; use nom::sequence::{delimited, preceded, terminated}; use std::str::from_utf8; @@ -154,3 +155,12 @@ pub fn spacelike(input: Span) -> PResult<()> { pub fn opt_spacelike(input: Span) -> PResult<()> { map(multispace0, |_| ())(input) } + +fn nom_err<'a>( + msg: &'static str, + pos: Span<'a>, +) -> nom::Err>> { + nom::Err::Error(VerboseError { + errors: vec![(pos, VerboseErrorKind::Context(msg))], + }) +} diff --git a/rsass/src/parser/css/strings.rs b/rsass/src/parser/css/strings.rs index dd401118d..e3dbe70e0 100644 --- a/rsass/src/parser/css/strings.rs +++ b/rsass/src/parser/css/strings.rs @@ -1,12 +1,12 @@ use super::super::{input_to_str, input_to_string, PResult, Span}; -use super::opt_spacelike; +use super::{nom_err, opt_spacelike}; use crate::css::CssString; use crate::value::Quotes; use nom::branch::alt; use nom::bytes::complete::{is_not, tag, take}; -use nom::character::complete::one_of; +use nom::character::complete::{char, one_of}; use nom::combinator::{ - into, map, map_opt, map_res, not, opt, peek, recognize, value, verify, + into, map, map_opt, map_res, opt, recognize, value, verify, }; use nom::multi::{fold_many0, fold_many1, many0, many_m_n}; use nom::sequence::{delimited, preceded, terminated}; @@ -130,7 +130,18 @@ fn selector_plain_part(input: Span) -> PResult { } fn hash_no_interpolation(input: Span) -> PResult<&str> { - map_res(terminated(tag("#"), peek(not(tag("{")))), input_to_str)(input) + let (next, hash) = tag("#")(input)?; + if let Ok((end, _)) = interpolation_block(next) { + return Err(nom_err( + "Interpolation isn't allowed in plain CSS.", + input.up_to(&end), + )); + } else { + Ok((next, input_to_str(hash).unwrap())) + } +} +fn interpolation_block(input: Span) -> PResult { + terminated(char('{'), opt(terminated(is_not("{}"), char('}'))))(input) } fn escaped_char(input: Span) -> PResult { diff --git a/rsass/src/parser/css/values.rs b/rsass/src/parser/css/values.rs index 1672b52ef..4f75fef53 100644 --- a/rsass/src/parser/css/values.rs +++ b/rsass/src/parser/css/values.rs @@ -1,4 +1,4 @@ -use super::strings; +use super::{nom_err, strings}; use super::{opt_spacelike, PResult, Span}; use crate::css::{BinOp, CallArgs, Value}; use crate::parser::input_to_str; @@ -6,8 +6,11 @@ use crate::parser::value::{numeric, unicode_range_inner}; use crate::value::{ListSeparator, Operator}; use nom::branch::alt; use nom::bytes::complete::{is_not, tag}; -use nom::character::complete::one_of; -use nom::combinator::{into, map, map_opt, opt, peek, value}; +use nom::character::complete::{char, none_of, one_of}; +use nom::combinator::{ + cond, into, map, map_opt, not, opt, peek, recognize, value, +}; +use nom::error::context; use nom::multi::{fold_many0, many0, separated_list0, separated_list1}; use nom::sequence::{delimited, pair, preceded, terminated, tuple}; @@ -21,7 +24,16 @@ pub fn slash_list(input: Span) -> PResult { Ok((input, list_or_single(list, ListSeparator::Slash))) } pub fn slash_list_no_space(input: Span) -> PResult { - let (input, list) = separated_list1(tag("/"), space_list)(input)?; + let (mut input, first) = space_list(input)?; + let mut list = vec![first]; + while let PResult::Ok((rest, _)) = + terminated(tag("/"), peek(not(tag("*"))))(input) + { + let (rest, value) = + alt((map(space_list, Some), value(None, opt_spacelike)))(rest)?; + list.push(value.unwrap_or(Value::Literal("".into()))); + input = rest; + } Ok((input, list_or_single(list, ListSeparator::SlashNoSpace))) } pub fn space_list(input: Span) -> PResult { @@ -62,6 +74,17 @@ pub fn single(input: Span) -> PResult { Some(c) if *c == b'-' || *c == b'.' => { alt((into(numeric), string_or_call))(input) } + Some(b'(') => { + let (end, _) = + delimited(tag("("), none_of(")"), opt(tag(")")))(input)?; + let pos = input.up_to(&end); + Err(nom_err("Parentheses aren't allowed in plain CSS.", pos)) + } + Some(b'$') => { + let (end, _) = preceded(tag("$"), strings::css_string)(input)?; + let pos = input.up_to(&end); + Err(nom_err("Sass variables aren't allowed in plain CSS.", pos)) + } _ => alt(( map(unicode_range_inner, Value::UnicodeRange), string_or_call, @@ -73,13 +96,26 @@ fn string_or_call(input: Span) -> PResult { let (rest, string) = strings::css_string_any(input)?; if string.quotes().is_none() { if let Ok((rest, _)) = terminated(tag("("), opt_spacelike)(rest) { - let endp = preceded(opt_spacelike, tag(")")); + let endp = preceded(opt_spacelike, char(')')); let (rest, args) = if string.value() == "calc" { + if let Ok((end, _)) = preceded(opt_spacelike, tag(")"))(rest) + { + return Err(nom_err( + "Missing argument.", + input.up_to(&end), + )); + } map(terminated(calc_expr, endp), CallArgs::from_single)(rest)? } else { terminated(call_args, endp)(rest)? }; return Ok((rest, Value::Call(string.take_value(), args))); + } else if let Ok((end, _)) = preceded(char('.'), string_or_call)(rest) + { + return Err(nom_err( + "Module namespaces aren't allowed in plain CSS.", + input.up_to(&end), + )); } } Ok((rest, string.into())) @@ -131,6 +167,11 @@ fn single_term(input: Span) -> PResult { calc_expr, preceded(opt_spacelike, tag(")")), )(input), + Some(b'$') => { + let (end, _) = preceded(tag("$"), strings::css_string)(input)?; + let pos = input.up_to(&end); + Err(nom_err("Sass variables aren't allowed in plain CSS.", pos)) + } Some(c) if b'0' <= *c && *c <= b'9' => into(numeric)(input), _ => string_or_call(input), } @@ -146,30 +187,42 @@ fn call_args(input: Span) -> PResult { .map(|(name, val)| (name.into(), val)) .collect(); let (rest, positional) = separated_list0(spaced(","), single_arg)(rest)?; - let (rest, trail) = opt(tag(","))(rest)?; + let (rest, trailing_comma) = + map(opt(spaced(",")), |c| c.is_some())(rest)?; + + let (rest, _) = cond( + trailing_comma, + alt(( + peek(tag(")")), + context("Expected expression.", recognize(single_arg)), + )), + )(rest)?; Ok(( rest, CallArgs { positional, named, - trailing_comma: trail.is_some(), + trailing_comma, }, )) } fn single_arg(input: Span) -> PResult { fn end(input: Span) -> PResult<()> { - peek(preceded(opt_spacelike, map(one_of(",)"), |_| ())))(input) + peek(preceded(opt_spacelike, map(one_of(",)."), |_| ())))(input) + } + match terminated(space_list, end)(input) { + Ok(ok) => Ok(ok), + Err(err) => match terminated(into(ext_arg_as_string), end)(input) { + Ok(ok) => Ok(ok), + Err(_) => Err(err), + }, } - alt(( - terminated(space_list, end), - terminated(into(ext_arg_as_string), end), - ))(input) } fn ext_arg_as_string(input: Span) -> PResult { map_opt(is_not("\"\\;{}()[] ,"), |s: Span| { - if s.is_empty() { + if s.first().map_or(true, |ch| ch.is_ascii_digit()) { None } else { Some(input_to_str(s).ok()?.to_owned()) diff --git a/rsass/src/parser/error.rs b/rsass/src/parser/error.rs index cf8618f50..4056e9e25 100644 --- a/rsass/src/parser/error.rs +++ b/rsass/src/parser/error.rs @@ -35,12 +35,10 @@ impl ParseError { where Msg: Into, { - let msg = if pos.is_at_end() { - String::from("expected more input.") - } else { - msg.into() - }; - Self { msg, pos } + Self { + msg: msg.into(), + pos, + } } } @@ -62,18 +60,18 @@ impl From>> for ParseError { }) .next() .or_else(|| { - value.errors.first().map(|(pos, kind)| { + value.errors.first().map(|(pos, _)| { if pos.is_at_end() { ("expected more input.".to_string(), pos) } else if let PResult::Ok((_, b)) = one_of(")}]")(*pos) { (format!("unmatched \"{b}\"."), pos) } else { - (format!("Parse error: {kind:?}"), pos) + ("Parse error.".to_string(), pos) } }) }) .unwrap(); - Self::new(msg, pos.up_to(pos).to_owned()) + Self::new(msg, pos.sanitize_end().to_owned()) } } diff --git a/rsass/src/parser/span.rs b/rsass/src/parser/span.rs index 957f711a6..5be1a3839 100644 --- a/rsass/src/parser/span.rs +++ b/rsass/src/parser/span.rs @@ -69,6 +69,15 @@ impl<'a> Span<'a> { pub(crate) fn up_to(self, other: &Self) -> Self { self.take(self.offset(other)) } + /// If `self` goes on the end of input, return just the starting point. + /// Otherwise preserve `self` as is. + pub(crate) fn sanitize_end(self) -> Self { + if self.end < self.source.data().len() { + self + } else { + self.up_to(&self) + } + } } impl<'a, T> Compare for Span<'a> diff --git a/rsass/src/parser/value.rs b/rsass/src/parser/value.rs index 55e9a1ebe..094f882c0 100644 --- a/rsass/src/parser/value.rs +++ b/rsass/src/parser/value.rs @@ -236,15 +236,15 @@ pub fn single_value(input: Span) -> PResult { fn bang(input: Span) -> PResult { map( - context( - "Expected \"important\".", - map_res( - preceded( - pair(tag("!"), opt_spacelike), + map_res( + preceded( + pair(tag("!"), opt_spacelike), + context( + "Expected \"important\".", tag("important"), // TODO Pretty much anythig goes, here? ), - input_to_string, ), + input_to_string, ), Value::Bang, )(input) diff --git a/rsass/src/sass/functions/call_error.rs b/rsass/src/sass/functions/call_error.rs index 163b45263..71d0debd3 100644 --- a/rsass/src/sass/functions/call_error.rs +++ b/rsass/src/sass/functions/call_error.rs @@ -43,7 +43,7 @@ impl CallError { Error::BadCall(msg, call_pos.clone().opt_in_calc(), None) } Self::Invalid(err) => { - Error::BadCall(format!("{err:?}"), call_pos.clone(), None) + Error::BadCall(err.to_string(), call_pos.clone(), None) } Self::BadArgument(name, problem) => Error::BadCall( if name.as_ref().is_empty() { diff --git a/rsass/src/value/list_separator.rs b/rsass/src/value/list_separator.rs index 975dd3808..970062bb0 100644 --- a/rsass/src/value/list_separator.rs +++ b/rsass/src/value/list_separator.rs @@ -24,6 +24,10 @@ impl ListSeparator { Self::Space => " ", } } + /// Return trus for slash (with or without space). + pub fn is_slash(&self) -> bool { + matches!(self, Self::Slash | Self::SlashNoSpace) + } } impl Default for ListSeparator { diff --git a/rsass/tests/misc/css_parser.rs b/rsass/tests/misc/css_parser.rs index 368ef0651..de8ea4268 100644 --- a/rsass/tests/misc/css_parser.rs +++ b/rsass/tests/misc/css_parser.rs @@ -15,7 +15,7 @@ fn error_in_right_place() { // Note: The error message should be better, but this is a good place for it. // Specifically, the marker should _not_ indicate the opening brace. Err(String::from( - "Error: Parse error: Nom(Tag)\ + "Error: Parse error.\ \n ,\ \n5 | error here\ \n | ^\ diff --git a/rsass/tests/spec/css/important.rs b/rsass/tests/spec/css/important.rs index 0f3adb38a..8b6b6ab39 100644 --- a/rsass/tests/spec/css/important.rs +++ b/rsass/tests/spec/css/important.rs @@ -14,7 +14,6 @@ mod error { use super::runner; #[test] - #[ignore] // wrong error fn eof_after_bang() { assert_eq!( runner().err( diff --git a/rsass/tests/spec/css/plain/error/expression/calculation.rs b/rsass/tests/spec/css/plain/error/expression/calculation.rs index a254baea3..347b034eb 100644 --- a/rsass/tests/spec/css/plain/error/expression/calculation.rs +++ b/rsass/tests/spec/css/plain/error/expression/calculation.rs @@ -12,7 +12,6 @@ fn runner() -> crate::TestRunner { } #[test] -#[ignore] // wrong error fn interpolation() { let runner = runner().with_cwd("interpolation"); assert_eq!( @@ -42,7 +41,6 @@ fn line_noise() { ); } #[test] -#[ignore] // wrong error fn namespaced_function() { let runner = runner().with_cwd("namespaced_function"); assert_eq!( @@ -57,7 +55,6 @@ fn namespaced_function() { ); } #[test] -#[ignore] // wrong error fn variable() { let runner = runner().with_cwd("variable"); assert_eq!( @@ -72,7 +69,6 @@ fn variable() { ); } #[test] -#[ignore] // wrong error fn wrong_args() { let runner = runner().with_cwd("wrong_args"); assert_eq!( diff --git a/rsass/tests/spec/css/plain/error/expression/function.rs b/rsass/tests/spec/css/plain/error/expression/function.rs index 4569202d4..c58d66780 100644 --- a/rsass/tests/spec/css/plain/error/expression/function.rs +++ b/rsass/tests/spec/css/plain/error/expression/function.rs @@ -31,7 +31,6 @@ fn built_in() { ); } #[test] -#[ignore] // wrong error fn keyword_arguments() { let runner = runner().with_cwd("keyword_arguments"); assert_eq!( @@ -46,7 +45,6 @@ fn keyword_arguments() { ); } #[test] -#[ignore] // missing error fn variable_arguments() { let runner = runner().with_cwd("variable_arguments"); assert_eq!( diff --git a/rsass/tests/spec/css/plain/error/expression/interpolation.rs b/rsass/tests/spec/css/plain/error/expression/interpolation.rs index 9265796cd..c4b506050 100644 --- a/rsass/tests/spec/css/plain/error/expression/interpolation.rs +++ b/rsass/tests/spec/css/plain/error/expression/interpolation.rs @@ -11,7 +11,6 @@ fn runner() -> crate::TestRunner { } #[test] -#[ignore] // wrong error fn calc() { let runner = runner().with_cwd("calc"); assert_eq!( @@ -26,7 +25,6 @@ fn calc() { ); } #[test] -#[ignore] // wrong error fn identifier() { let runner = runner().with_cwd("identifier"); assert_eq!( @@ -56,7 +54,6 @@ fn quoted_string() { ); } #[test] -#[ignore] // wrong error fn standalone() { let runner = runner().with_cwd("standalone"); assert_eq!( diff --git a/rsass/tests/spec/css/plain/error/expression/parentheses.rs b/rsass/tests/spec/css/plain/error/expression/parentheses.rs index 02fee55f3..0436096d0 100644 --- a/rsass/tests/spec/css/plain/error/expression/parentheses.rs +++ b/rsass/tests/spec/css/plain/error/expression/parentheses.rs @@ -8,7 +8,6 @@ fn runner() -> crate::TestRunner { } #[test] -#[ignore] // wrong error fn test() { assert_eq!( runner().err("@use \'plain\'"), diff --git a/rsass/tests/spec/css/plain/error/expression/variable.rs b/rsass/tests/spec/css/plain/error/expression/variable.rs index d41539d63..4ed9243fa 100644 --- a/rsass/tests/spec/css/plain/error/expression/variable.rs +++ b/rsass/tests/spec/css/plain/error/expression/variable.rs @@ -24,7 +24,6 @@ fn declaration() { ); } #[test] -#[ignore] // wrong error fn test_use() { let runner = runner().with_cwd("use"); assert_eq!( diff --git a/rsass/tests/spec/css/plain/functions.rs b/rsass/tests/spec/css/plain/functions.rs index 78b0cbfbc..3ac011669 100644 --- a/rsass/tests/spec/css/plain/functions.rs +++ b/rsass/tests/spec/css/plain/functions.rs @@ -68,7 +68,6 @@ mod error { } #[test] - #[ignore] // wrong error fn empty_second_before_third() { let runner = runner().with_cwd("empty_second_before_third"); assert_eq!( @@ -83,7 +82,6 @@ mod error { ); } #[test] - #[ignore] // wrong error fn invalid_second_arg_syntax() { let runner = runner().with_cwd("invalid_second_arg_syntax"); assert_eq!( diff --git a/rsass/tests/spec/css/plain/slash.rs b/rsass/tests/spec/css/plain/slash.rs index 737b13a20..68839c8c6 100644 --- a/rsass/tests/spec/css/plain/slash.rs +++ b/rsass/tests/spec/css/plain/slash.rs @@ -32,7 +32,6 @@ mod without_intermediate { } #[test] - #[ignore] // unexepected error fn no_whitespace() { let runner = runner().with_cwd("no_whitespace"); assert_eq!( @@ -43,7 +42,6 @@ mod without_intermediate { ); } #[test] - #[ignore] // unexepected error fn whitespace() { let runner = runner().with_cwd("whitespace"); assert_eq!(