Skip to content

Commit

Permalink
Improve plain css parsing.
Browse files Browse the repository at this point in the history
Especially improve some parse error messages.
  • Loading branch information
kaj committed Oct 25, 2024
1 parent d856867 commit f9abd23
Show file tree
Hide file tree
Showing 19 changed files with 122 additions and 52 deletions.
3 changes: 2 additions & 1 deletion rsass/src/css/valueformat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion rsass/src/output/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions rsass/src/parser/css/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<VerboseError<Span<'a>>> {
nom::Err::Error(VerboseError {
errors: vec![(pos, VerboseErrorKind::Context(msg))],
})
}
19 changes: 15 additions & 4 deletions rsass/src/parser/css/strings.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -130,7 +130,18 @@ fn selector_plain_part(input: Span) -> PResult<String> {
}

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<char> {
terminated(char('{'), opt(terminated(is_not("{}"), char('}'))))(input)
}

fn escaped_char(input: Span) -> PResult<char> {
Expand Down
79 changes: 66 additions & 13 deletions rsass/src/parser/css/values.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
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;
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};

Expand All @@ -21,7 +24,16 @@ pub fn slash_list(input: Span) -> PResult<Value> {
Ok((input, list_or_single(list, ListSeparator::Slash)))
}
pub fn slash_list_no_space(input: Span) -> PResult<Value> {
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<Value> {
Expand Down Expand Up @@ -62,6 +74,17 @@ pub fn single(input: Span) -> PResult<Value> {
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,
Expand All @@ -73,13 +96,26 @@ fn string_or_call(input: Span) -> PResult<Value> {
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()))
Expand Down Expand Up @@ -131,6 +167,11 @@ fn single_term(input: Span) -> PResult<Value> {
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),
}
Expand All @@ -146,30 +187,42 @@ fn call_args(input: Span) -> PResult<CallArgs> {
.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<Value> {
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<String> {
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())
Expand Down
16 changes: 7 additions & 9 deletions rsass/src/parser/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,10 @@ impl ParseError {
where
Msg: Into<String>,
{
let msg = if pos.is_at_end() {
String::from("expected more input.")
} else {
msg.into()
};
Self { msg, pos }
Self {
msg: msg.into(),
pos,
}
}
}

Expand All @@ -62,18 +60,18 @@ impl From<nom::error::VerboseError<Span<'_>>> 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())
}
}

Expand Down
9 changes: 9 additions & 0 deletions rsass/src/parser/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> for Span<'a>
Expand Down
12 changes: 6 additions & 6 deletions rsass/src/parser/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,15 +236,15 @@ pub fn single_value(input: Span) -> PResult<Value> {

fn bang(input: Span) -> PResult<Value> {
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)
Expand Down
2 changes: 1 addition & 1 deletion rsass/src/sass/functions/call_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 4 additions & 0 deletions rsass/src/value/list_separator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion rsass/tests/misc/css_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 | ^\
Expand Down
1 change: 0 additions & 1 deletion rsass/tests/spec/css/important.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ mod error {
use super::runner;

#[test]
#[ignore] // wrong error
fn eof_after_bang() {
assert_eq!(
runner().err(
Expand Down
4 changes: 0 additions & 4 deletions rsass/tests/spec/css/plain/error/expression/calculation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ fn runner() -> crate::TestRunner {
}

#[test]
#[ignore] // wrong error
fn interpolation() {
let runner = runner().with_cwd("interpolation");
assert_eq!(
Expand Down Expand Up @@ -42,7 +41,6 @@ fn line_noise() {
);
}
#[test]
#[ignore] // wrong error
fn namespaced_function() {
let runner = runner().with_cwd("namespaced_function");
assert_eq!(
Expand All @@ -57,7 +55,6 @@ fn namespaced_function() {
);
}
#[test]
#[ignore] // wrong error
fn variable() {
let runner = runner().with_cwd("variable");
assert_eq!(
Expand All @@ -72,7 +69,6 @@ fn variable() {
);
}
#[test]
#[ignore] // wrong error
fn wrong_args() {
let runner = runner().with_cwd("wrong_args");
assert_eq!(
Expand Down
2 changes: 0 additions & 2 deletions rsass/tests/spec/css/plain/error/expression/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ fn built_in() {
);
}
#[test]
#[ignore] // wrong error
fn keyword_arguments() {
let runner = runner().with_cwd("keyword_arguments");
assert_eq!(
Expand All @@ -46,7 +45,6 @@ fn keyword_arguments() {
);
}
#[test]
#[ignore] // missing error
fn variable_arguments() {
let runner = runner().with_cwd("variable_arguments");
assert_eq!(
Expand Down
Loading

0 comments on commit f9abd23

Please sign in to comment.