A bunch of Error refactoring.

CHANGELOG.md

@@ -9,8 +9,16 @@ project adheres to
 
 ## Unreleased
 
+### Breaking changes
+
+* Changes in `Error` representation.  Many errors are now constructed
+  like `Invalid::Something.at(pos)` (PR #145).
+
+### Improvements
+
 * Allow interpolation in css min and max function arguments.
 * The url for `@use` and `@forward` must be quoted.
+* Some `@` rules are now forbidden in some places as they should (PR #145).
 
 Thanks to @fasterthanlime (again) for reporting the problem with
 interpolation in min and max.

src/css/call_args.rs

@@ -1,4 +1,4 @@
-use super::Value;
+use super::{is_not, Value};
 use crate::ordermap::OrderMap;
 use crate::sass::{ArgsError, Name};
 use crate::value::ListSeparator;
@@ -59,7 +59,7 @@ impl CallArgs {
                 Value::Literal(s) => {
                     self.named.insert(s.value().into(), v);
                 }
-                x => return Err(Error::bad_value("a string", &x)),
+                x => return Err(Error::error(is_not(&x, "a string"))),
             }
         }
         Ok(())

src/css/value.rs

@@ -148,8 +148,8 @@ impl Value {
                 .value
                 .clone()
                 .into_integer()
-                .map_err(|_| Error::bad_value("an integer", self)),
-            v => Err(Error::bad_value("a number", v)),
+                .map_err(|_| Error::error(is_not(self, "an integer"))),
+            v => Err(Error::error(is_not(v, "a number"))),
         }
     }
 
@@ -191,13 +191,9 @@ impl Value {
             }
             Value::List(v, _, _) => v,
             Value::Map(map) => map
-                .iter()
-                .map(|&(ref k, ref v)| {
-                    Value::List(
-                        vec![k.clone(), v.clone()],
-                        Some(ListSeparator::Space),
-                        false,
-                    )
+                .into_iter()
+                .map(|(k, v)| {
+                    Value::List(vec![k, v], Some(ListSeparator::Space), false)
                 })
                 .collect(),
             Value::Paren(v) => v.iter_items(),

src/error.rs

@@ -1,7 +1,5 @@
-use crate::css::Value;
-use crate::output::Format;
 use crate::parser::{ParseError, SourcePos};
-use crate::sass::Name;
+use crate::sass::{ArgsError, Name};
 use crate::value::RangeError;
 use std::convert::From;
 use std::{fmt, io};
@@ -18,13 +16,11 @@ pub enum Error {
     /// A bad call to a builtin function, with call- and optionally
     /// declaration position.
     BadCall(String, SourcePos, Option<SourcePos>),
-    /// Some kind of illegal value.
-    BadValue(String),
     /// An illegal value for a specific parameter.
     BadArgument(Name, String),
     /// The pos here is the function declaration.
     /// This error will be wrapped in a BadCall, giving the pos of the call.
-    BadArguments(String, SourcePos),
+    BadArguments(ArgsError, SourcePos),
     /// Tried to import file at pos while already importing it at pos.
     ImportLoop(SourcePos, SourcePos),
     /// A range error
@@ -42,29 +38,6 @@ pub enum Error {
 impl std::error::Error for Error {}
 
 impl Error {
-    /// A bad value with an "(actual) is not (expected)" message.
-    pub fn bad_value(expected: &str, actual: &Value) -> Self {
-        Error::BadValue(format!(
-            "Error: {} is not {}.",
-            actual.format(Format::introspect()),
-            expected,
-        ))
-    }
-
-    /// Wrong kind of argument to a sass function.
-    /// `expected` is a string describing what the parameter should
-    /// have been, `actual` is the argument.
-    pub fn bad_arg(
-        name: Name,
-        actual: &Value,
-        problem: &'static str,
-    ) -> Error {
-        Error::BadArgument(
-            name,
-            format!("{} {}", actual.format(Format::introspect()), problem),
-        )
-    }
-
     /// A generic error message.
     pub fn error<T: AsRef<str>>(msg: T) -> Self {
         Error::S(format!("Error: {}", msg.as_ref()))
@@ -116,7 +89,6 @@ impl fmt::Display for Error {
                 pos.show(out)
             }
             Error::BadRange(ref err) => err.fmt(out),
-            Error::BadValue(ref err) => err.fmt(out),
             // fallback
             ref x => write!(out, "{:?}", x),
         }
@@ -174,7 +146,6 @@ impl From<RangeError> for Error {
 ///
 /// Should be combined with a position to get an [Error].
 #[derive(Debug)]
-#[allow(unused)] // fixme
 pub enum Invalid {
     /// An undefined variable was used at source pos.
     UndefinedVariable,

src/output/transform.rs

@@ -363,10 +363,6 @@ fn handle_item(
                     file_context,
                 )
                 .map_err(|e: Error| match e {
-                    Error::BadArguments(msg, decl) => {
-                        let pos = pos.in_call(name);
-                        Error::BadCall(msg, pos, Some(decl))
-                    }
                     Error::Invalid(err, _) => err.at(pos.clone()),
                     e => {
                         let pos = pos.in_call(name);

src/sass/formal_args.rs

@@ -1,7 +1,7 @@
-use crate::css;
 use crate::error::Error;
 use crate::sass::{Name, Value};
 use crate::ScopeRef;
+use crate::{css, SourcePos};
 use std::fmt;
 
 /// The declared arguments of a mixin or function declaration.
@@ -67,9 +67,7 @@ impl FormalArgs {
                 } else if let Some(default) = default {
                     argscope.define(
                         name.clone(),
-                        &default
-                            .do_evaluate(argscope.clone(), true)
-                            .map_err(ArgsError::Eval)?,
+                        &default.do_evaluate(argscope.clone(), true)?,
                     );
                 } else {
                     return Err(ArgsError::Missing(name.clone()));
@@ -124,7 +122,25 @@ pub enum ArgsError {
     /// Got unexpected named argumet
     Unexpected(Name),
     /// An error evaluating one of the arguments.
-    Eval(Error),
+    Eval(Box<Error>),
+}
+
+impl ArgsError {
+    /// This argument error happend for args declared at the given pos.
+    pub fn declared_at(self, pos: &SourcePos) -> Error {
+        match self {
+            ArgsError::Eval(e) => *e,
+            ae => Error::BadArguments(ae, pos.clone()),
+        }
+    }
+    /// This argument error happened while calling for `call` a function
+    /// declared at `decl` position.
+    pub fn decl_called(self, call: SourcePos, decl: SourcePos) -> Error {
+        match self {
+            ArgsError::Eval(e) => *e,
+            ae => Error::BadCall(ae.to_string(), call, Some(decl)),
+        }
+    }
 }
 
 impl fmt::Display for ArgsError {
@@ -157,6 +173,12 @@ impl fmt::Display for ArgsError {
     }
 }
 
+impl From<Error> for ArgsError {
+    fn from(e: Error) -> ArgsError {
+        ArgsError::Eval(Box::new(e))
+    }
+}
+
 // Note: this is only for some special cases, normally the "context"
 // of a function declaration pos is required.
 impl From<ArgsError> for Error {

src/sass/functions/color/mod.rs

@@ -174,11 +174,10 @@ fn make_call(name: &str, args: Vec<Value>) -> Value {
 
 fn bad_arg(err: ArgsError, name: &Name, args: &FormalArgs) -> Error {
     match err {
-        ArgsError::Eval(e) => e,
-        ae => Error::BadArguments(
-            ae.to_string(),
-            SourcePos::mock_function(name, args, ""),
-        ),
+        ArgsError::Eval(e) => *e,
+        ae => {
+            Error::BadArguments(ae, SourcePos::mock_function(name, args, ""))
+        }
     }
 }
 

src/sass/functions/list.rs

@@ -92,9 +92,7 @@ pub fn create_module() -> Scope {
     def_va!(f, slash(elements), |s| {
         let list = get_va_list(s, name!(elements))?;
         if list.len() < 2 {
-            return Err(Error::BadValue(
-                "Error: At least two elements are required.".into(),
-            ));
+            return Err(Error::error("At least two elements are required."));
         }
         Ok(Value::List(list, Some(ListSeparator::Slash), false))
     });

src/sass/functions/math.rs

@@ -26,7 +26,8 @@ pub fn create_module() -> Scope {
             (Value::Color(a, _), Value::Numeric(b, _)) if b.is_no_unit() => {
                 let bn = b
                     .as_ratio()
-                    .map_err(|e| Error::BadValue(e.to_string()))?;
+                    .map_err(|e| e.to_string())
+                    .named(name!(number2))?;
                 Ok((a.to_rgba().as_ref() / bn).into())
             }
             (Value::Numeric(ref a, _), Value::Numeric(ref b, _)) => {

src/sass/functions/mod.rs

@@ -182,10 +182,9 @@ impl Function {
         def: ScopeRef,
         args: CallArgs,
     ) -> Result<ScopeRef, Error> {
-        self.args.eval(def, args).map_err(|e| match e {
-            sass::ArgsError::Eval(e) => e,
-            ae => Error::BadArguments(ae.to_string(), self.pos.clone()),
-        })
+        self.args
+            .eval(def, args)
+            .map_err(|e| e.declared_at(&self.pos))
     }
 }
 

src/sass/mixin.rs

@@ -64,17 +64,13 @@ impl MixinDecl {
                         .args
                         .eval(
                             ScopeRef::sub_selectors(decl.scope.clone(), sel),
-                            call_args
-                                .evaluate(scope)
-                                .map_err(crate::sass::ArgsError::Eval)?,
+                            call_args.evaluate(scope)?,
                         )
-                        .map_err(|e| match e {
-                            crate::sass::ArgsError::Eval(e) => e,
-                            ae => Error::BadCall(
-                                ae.to_string(),
+                        .map_err(|e| {
+                            e.decl_called(
                                 call_pos.in_call(name),
-                                Some(decl.pos.clone()),
-                            ),
+                                decl.pos.clone(),
+                            )
                         })?,
                     body: Parsed::Scss(decl.body),
                 })
@@ -92,20 +88,8 @@ impl MixinDecl {
                 );
                 let call_pos2 = call_pos.clone();
                 let argscope = fargs
-                    .eval(
-                        scope.clone(),
-                        call_args
-                            .evaluate(scope.clone())
-                            .map_err(crate::sass::ArgsError::Eval)?,
-                    )
-                    .map_err(|e| match e {
-                        crate::sass::ArgsError::Eval(e) => e,
-                        ae => Error::BadCall(
-                            ae.to_string(),
-                            call_pos2,
-                            Some(pos),
-                        ),
-                    })?;
+                    .eval(scope.clone(), call_args.evaluate(scope.clone())?)
+                    .map_err(|e| e.decl_called(call_pos2, pos))?;
                 let call_pos2 = call_pos.clone();
                 let url = get_string(&argscope, name!(url)).map_err(|e| {
                     Error::BadCall(e.to_string(), call_pos2, None)