Proper errors from the parser. (#201)

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

This should fix #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()