Improved parse error handling

Many parse errors now match the dart sass error message.  Also allow
"loud" comments in more places.

Use nom "verbose" error handling to build proper parse errors.

Fixes #141.

Author: kaj

Diff for: CHANGELOG.md

@@ -24,6 +24,9 @@ project adheres to
   - Made all color channels f64 instead of Rational (PR #199).
 * Fixed a bug where `clamp(..)` was sometimes evaluated to a value
   even though units wasn't comparable.
+* Improved parse error handling (PR #201, Issue #141).
+  Many parse errors now match the dart sass error message.
+  Also allow "loud" comments in more places.
 * Updated sass-spec test suite to 2024-09-20.
 
 

Diff for: rsass/src/parser/error.rs

@@ -1,6 +1,6 @@
 use super::{PResult, Span};
 use crate::input::SourcePos;
-use nom::Finish;
+use nom::{character::complete::one_of, error::VerboseErrorKind, Finish};
 use std::fmt;
 
 /// An error encountered when parsing sass.
@@ -44,12 +44,36 @@ impl ParseError {
     }
 }
 
-impl From<nom::error::Error<Span<'_>>> for ParseError {
-    fn from(err: nom::error::Error<Span>) -> Self {
-        Self::new(
-            format!("Parse error: {:?}", err.code),
-            err.input.up_to(&err.input).to_owned(),
-        )
+impl From<nom::error::VerboseError<Span<'_>>> for ParseError {
+    fn from(value: nom::error::VerboseError<Span<'_>>) -> Self {
+        let (msg, pos) = value
+            .errors
+            .iter()
+            .filter_map(|(pos, kind)| {
+                match kind {
+                    VerboseErrorKind::Context(ctx) => {
+                        Some((ctx.to_string(), pos))
+                    }
+                    VerboseErrorKind::Char(ch) => {
+                        Some((format!("expected {:?}.", ch.to_string()), pos))
+                    }
+                    VerboseErrorKind::Nom(_) => None, // Try the next one!
+                }
+            })
+            .next()
+            .or_else(|| {
+                value.errors.first().map(|(pos, kind)| {
+                    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)
+                    }
+                })
+            })
+            .unwrap();
+        Self::new(msg, pos.up_to(pos).to_owned())
     }
 }
 

Diff for: rsass/src/parser/formalargs.rs

@@ -4,20 +4,22 @@ use super::value::space_list;
 use super::{PResult, Span};
 use crate::sass::{CallArgs, FormalArgs, Name};
 use nom::bytes::complete::tag;
-use nom::combinator::{map, map_res, opt};
+use nom::character::complete::char;
+use nom::combinator::{cut, map, map_res, opt};
+use nom::error::context;
 use nom::multi::separated_list0;
 use nom::sequence::{delimited, pair, preceded, terminated};
 
 pub fn formal_args(input: Span) -> PResult<FormalArgs> {
-    let (input, _) = terminated(tag("("), opt_spacelike)(input)?;
+    let (input, _) = terminated(char('('), opt_spacelike)(input)?;
     let (input, v) = separated_list0(
         preceded(tag(","), opt_spacelike),
         map(
             pair(
                 delimited(tag("$"), name, opt_spacelike),
                 opt(delimited(
                     terminated(tag(":"), opt_spacelike),
-                    space_list,
+                    cut(context("Expected expression.", space_list)),
                     opt_spacelike,
                 )),
             ),
@@ -26,7 +28,7 @@ pub fn formal_args(input: Span) -> PResult<FormalArgs> {
     )(input)?;
     let (input, _) = terminated(opt(tag(",")), opt_spacelike)(input)?;
     let (input, va) = terminated(opt(tag("...")), opt_spacelike)(input)?;
-    let (input, _) = tag(")")(input)?;
+    let (input, _) = char(')')(input)?;
     Ok((
         input,
         if va.is_none() {
@@ -39,28 +41,31 @@ pub fn formal_args(input: Span) -> PResult<FormalArgs> {
 
 pub fn call_args(input: Span) -> PResult<CallArgs> {
     delimited(
-        terminated(tag("("), opt_spacelike),
+        terminated(char('('), opt_spacelike),
         map_res(
             pair(
                 separated_list0(
                     terminated(tag(","), opt_spacelike),
                     pair(
-                        opt(delimited(
-                            tag("$"),
-                            map(name, Name::from),
+                        opt(map(
                             delimited(
-                                ignore_comments,
-                                tag(":"),
-                                opt_spacelike,
+                                tag("$"),
+                                name,
+                                delimited(
+                                    ignore_comments,
+                                    char(':'),
+                                    opt_spacelike,
+                                ),
                             ),
+                            Name::from,
                         )),
                         terminated(space_list, opt_spacelike),
                     ),
                 ),
-                opt(terminated(tag(","), opt_spacelike)),
+                opt(terminated(char(','), opt_spacelike)),
             ),
             |(args, trail)| CallArgs::new(args, trail.is_some()),
         ),
-        tag(")"),
+        cut(char(')')),
     )(input)
 }

Diff for: rsass/src/parser/imports.rs

@@ -3,12 +3,14 @@ use super::strings::{
     name, sass_string, sass_string_dq, sass_string_sq, special_url,
 };
 use super::util::{ignore_comments, opt_spacelike, semi_or_end};
-use super::value::space_list;
+use super::value::{identifier, space_list};
 use super::{media, position, PResult, Span};
 use crate::sass::{Expose, Item, Name, SassString, UseAs, Value};
 use nom::branch::alt;
 use nom::bytes::complete::tag;
-use nom::combinator::{map, opt, value};
+use nom::character::complete::char;
+use nom::combinator::{cut, map, opt, value};
+use nom::error::context;
 use nom::multi::{separated_list0, separated_list1};
 use nom::sequence::{delimited, pair, preceded, terminated, tuple};
 use std::collections::BTreeSet;
@@ -43,12 +45,15 @@ pub fn use2<'a>(start: Span, input: Span<'a>) -> PResult<'a, Item> {
     map(
         terminated(
             tuple((
-                terminated(quoted_sass_string, opt_spacelike),
+                context(
+                    "Expected string.",
+                    terminated(quoted_sass_string, ignore_comments),
+                ),
                 opt(preceded(
-                    terminated(tag("with"), opt_spacelike),
+                    terminated(tag("with"), ignore_comments),
                     with_arg,
                 )),
-                opt(preceded(terminated(tag("as"), opt_spacelike), as_arg)),
+                opt(preceded(terminated(tag("as"), ignore_comments), as_arg)),
                 position,
             )),
             semi_or_end,
@@ -65,24 +70,28 @@ pub fn use2<'a>(start: Span, input: Span<'a>) -> PResult<'a, Item> {
 }
 
 pub fn forward2<'a>(start: Span, input: Span<'a>) -> PResult<'a, Item> {
-    let (mut end, path) =
-        terminated(quoted_sass_string, opt_spacelike)(input)?;
+    let (mut end, path) = context(
+        "Expected string.",
+        terminated(quoted_sass_string, opt_spacelike),
+    )(input)?;
     let mut found_as = None;
     let mut expose = Expose::All;
     let mut found_with = None;
-    while let Ok((rest, arg)) = terminated(name, opt_spacelike)(end) {
+    while let Ok((rest, arg)) =
+        delimited(ignore_comments, name, ignore_comments)(end)
+    {
         end = match arg.as_ref() {
-            "as" if found_as.is_none() => {
-                let (i, a) = as_arg(rest)?;
+            "as" if found_as.is_none() && found_with.is_none() => {
+                let (i, a) = fwd_as_arg(rest)?;
                 found_as = Some(a);
                 i
             }
-            "hide" if expose == Expose::All => {
+            "hide" if expose == Expose::All && found_with.is_none() => {
                 let (i, (funs, vars)) = exposed_names(rest)?;
                 expose = Expose::Hide(funs, vars);
                 i
             }
-            "show" if expose == Expose::All => {
+            "show" if expose == Expose::All && found_with.is_none() => {
                 let (i, (funs, vars)) = exposed_names(rest)?;
                 expose = Expose::Show(funs, vars);
                 i
@@ -92,12 +101,7 @@ pub fn forward2<'a>(start: Span, input: Span<'a>) -> PResult<'a, Item> {
                 found_with = Some(w);
                 i
             }
-            _ => {
-                return Err(nom::Err::Error(nom::error::Error::new(
-                    end,
-                    nom::error::ErrorKind::MapRes,
-                )));
-            }
+            _ => break,
         };
     }
     let (rest, ()) = semi_or_end(end)?;
@@ -119,7 +123,10 @@ fn exposed_names(input: Span) -> PResult<(BTreeSet<Name>, BTreeSet<Name>)> {
             terminated(tag(","), opt_spacelike),
             pair(
                 map(opt(tag("$")), |v| v.is_some()),
-                map(terminated(name, opt_spacelike), Name::from),
+                cut(context(
+                    "Expected variable, mixin, or function name",
+                    map(terminated(name, opt_spacelike), Name::from),
+                )),
             ),
         ),
         |items| {
@@ -146,24 +153,28 @@ fn as_arg(input: Span) -> PResult<UseAs> {
     )(input)
 }
 
+fn fwd_as_arg(input: Span) -> PResult<UseAs> {
+    map(terminated(identifier, char('*')), UseAs::Prefix)(input)
+}
+
 fn with_arg(input: Span) -> PResult<Vec<(Name, Value, bool)>> {
     delimited(
-        terminated(tag("("), opt_spacelike),
-        separated_list0(
+        terminated(char('('), ignore_comments),
+        separated_list1(
             comma,
             tuple((
                 delimited(
-                    tag("$"),
-                    map(name, Name::from),
-                    delimited(opt_spacelike, tag(":"), opt_spacelike),
+                    char('$'),
+                    map(identifier, Name::from),
+                    delimited(ignore_comments, char(':'), ignore_comments),
                 ),
-                terminated(space_list, opt_spacelike),
+                terminated(space_list, ignore_comments),
                 map(opt(terminated(tag("!default"), opt_spacelike)), |o| {
                     o.is_some()
                 }),
             )),
         ),
-        delimited(opt(comma), tag(")"), opt_spacelike),
+        delimited(opt(comma), char(')'), opt_spacelike),
     )(input)
 }
 
@@ -172,5 +183,5 @@ fn quoted_sass_string(input: Span) -> PResult<SassString> {
 }
 
 fn comma(input: Span) -> PResult<()> {
-    map(terminated(tag(","), ignore_comments), |_| ())(input)
+    delimited(ignore_comments, map(tag(","), |_| ()), ignore_comments)(input)
 }

Diff for: rsass/src/parser/media.rs

@@ -4,7 +4,7 @@ use super::strings::{sass_string_dq, sass_string_sq};
 use super::util::{ignore_comments, opt_spacelike, semi_or_end};
 use super::value::{
     self, any_additive_expr, any_product, bracket_list, dictionary,
-    function_call_or_string, variable,
+    function_call_or_string_rulearg, variable,
 };
 use super::{body_block, list_or_single, PResult};
 use crate::sass::{BinOp, Item, Value};
@@ -53,7 +53,7 @@ pub fn args(input: Span) -> PResult<Value> {
                     bracket_list,
                     into(value::numeric),
                     variable,
-                    map(function_call_or_string, |s| match s {
+                    map(function_call_or_string_rulearg, |s| match s {
                         Value::Literal(s) => Value::Literal({
                             let lower = s
                                 .single_raw()