properly evaluated builtin fn if when one arg contains an error

src/args.rs

@@ -36,7 +36,7 @@ impl FuncArgs {
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
-pub(crate) struct CallArgs(pub HashMap<CallArg, Spanned<Value>>, pub Span);
+pub(crate) struct CallArgs(pub HashMap<CallArg, SassResult<Spanned<Value>>>, pub Span);
 
 #[derive(Debug, Clone, Hash, Eq, PartialEq)]
 pub(crate) enum CallArg {
@@ -101,18 +101,22 @@ impl CallArgs {
     /// Get argument by name
     ///
     /// Removes the argument
-    pub fn get_named<T: Into<Identifier>>(&mut self, val: T) -> Option<Spanned<Value>> {
+    pub fn get_named<T: Into<Identifier>>(&mut self, val: T) -> Option<SassResult<Spanned<Value>>> {
         self.0.remove(&CallArg::Named(val.into()))
     }
 
     /// Get a positional argument by 0-indexed position
     ///
     /// Removes the argument
-    pub fn get_positional(&mut self, val: usize) -> Option<Spanned<Value>> {
+    pub fn get_positional(&mut self, val: usize) -> Option<SassResult<Spanned<Value>>> {
         self.0.remove(&CallArg::Positional(val))
     }
 
-    pub fn get<T: Into<Identifier>>(&mut self, position: usize, name: T) -> Option<Spanned<Value>> {
+    pub fn get<T: Into<Identifier>>(
+        &mut self,
+        position: usize,
+        name: T,
+    ) -> Option<SassResult<Spanned<Value>>> {
         match self.get_named(name) {
             Some(v) => Some(v),
             None => self.get_positional(position),
@@ -121,9 +125,9 @@ impl CallArgs {
 
     pub fn get_err(&mut self, position: usize, name: &'static str) -> SassResult<Spanned<Value>> {
         match self.get_named(name) {
-            Some(v) => Ok(v),
+            Some(v) => v,
             None => match self.get_positional(position) {
-                Some(v) => Ok(v),
+                Some(v) => v,
                 None => Err((format!("Missing argument ${}.", name), self.span()).into()),
             },
         }

src/builtin/color/hsl.rs

@@ -184,7 +184,7 @@ fn inner_hsl(name: &'static str, mut args: CallArgs, parser: &mut Parser<'_>) ->
             3,
             "alpha",
             Value::Dimension(Number::one(), Unit::None),
-        ) {
+        )? {
             Value::Dimension(n, Unit::None) => n,
             Value::Dimension(n, Unit::Percent) => n / Number::from(100),
             v @ Value::Dimension(..) => {
@@ -469,7 +469,7 @@ fn invert(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> {
         1,
         "weight",
         Value::Dimension(Number::from(100), Unit::Percent),
-    ) {
+    )? {
         Value::Dimension(n, u) => bound!(args, "weight", n, u, 0, 100) / Number::from(100),
         v => {
             return Err((

src/builtin/color/other.rs

@@ -14,7 +14,7 @@ use crate::{
 
 macro_rules! opt_rgba {
     ($args:ident, $name:ident, $arg:literal, $low:literal, $high:literal, $parser:ident) => {
-        let $name = match $parser.default_named_arg(&mut $args, $arg, Value::Null) {
+        let $name = match $parser.default_named_arg(&mut $args, $arg, Value::Null)? {
             Value::Dimension(n, u) => Some(bound!($args, $arg, n, u, $low, $high)),
             Value::Null => None,
             v => {
@@ -34,7 +34,7 @@ macro_rules! opt_rgba {
 
 macro_rules! opt_hsl {
     ($args:ident, $name:ident, $arg:literal, $low:literal, $high:literal, $parser:ident) => {
-        let $name = match $parser.default_named_arg(&mut $args, $arg, Value::Null) {
+        let $name = match $parser.default_named_arg(&mut $args, $arg, Value::Null)? {
             Value::Dimension(n, u) => {
                 Some(bound!($args, $arg, n, u, $low, $high) / Number::from(100))
             }
@@ -88,7 +88,7 @@ fn change_color(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value
         ))));
     }
 
-    let hue = match parser.default_named_arg(&mut args, "hue", Value::Null) {
+    let hue = match parser.default_named_arg(&mut args, "hue", Value::Null)? {
         Value::Dimension(n, _) => Some(n),
         Value::Null => None,
         v => {
@@ -147,7 +147,7 @@ fn adjust_color(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value
         ))));
     }
 
-    let hue = match parser.default_named_arg(&mut args, "hue", Value::Null) {
+    let hue = match parser.default_named_arg(&mut args, "hue", Value::Null)? {
         Value::Dimension(n, _) => Some(n),
         Value::Null => None,
         v => {
@@ -205,7 +205,7 @@ fn scale_color(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value>
 
     macro_rules! opt_scale_arg {
         ($args:ident, $name:ident, $arg:literal, $low:literal, $high:literal, $parser:ident) => {
-            let $name = match $parser.default_named_arg(&mut $args, $arg, Value::Null) {
+            let $name = match $parser.default_named_arg(&mut $args, $arg, Value::Null)? {
                 Value::Dimension(n, Unit::Percent) => {
                     Some(bound!($args, $arg, n, Unit::Percent, $low, $high) / Number::from(100))
                 }

src/builtin/color/rgb.rs

@@ -305,7 +305,7 @@ fn inner_rgb(name: &'static str, mut args: CallArgs, parser: &mut Parser<'_>) ->
             3,
             "alpha",
             Value::Dimension(Number::one(), Unit::None),
-        ) {
+        )? {
             Value::Dimension(n, Unit::None) => n,
             Value::Dimension(n, Unit::Percent) => n / Number::from(100),
             v @ Value::Dimension(..) => {
@@ -416,7 +416,7 @@ fn mix(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> {
         2,
         "weight",
         Value::Dimension(Number::from(50), Unit::None),
-    ) {
+    )? {
         Value::Dimension(n, u) => bound!(args, "weight", n, u, 0, 100) / Number::from(100),
         v => {
             return Err((

src/builtin/list.rs

@@ -131,7 +131,7 @@ fn append(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> {
         2,
         "separator",
         Value::String("auto".to_owned(), QuoteKind::None),
-    ) {
+    )? {
         Value::String(s, ..) => match s.as_str() {
             "auto" => sep,
             "comma" => ListSeparator::Comma,
@@ -178,7 +178,7 @@ fn join(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> {
         2,
         "separator",
         Value::String("auto".to_owned(), QuoteKind::None),
-    ) {
+    )? {
         Value::String(s, ..) => match s.as_str() {
             "auto" => {
                 if list1.is_empty() || (list1.len() == 1 && sep1 == ListSeparator::Space) {
@@ -214,7 +214,7 @@ fn join(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> {
         3,
         "bracketed",
         Value::String("auto".to_owned(), QuoteKind::None),
-    ) {
+    )? {
         Value::String(s, ..) => match s.as_str() {
             "auto" => brackets,
             _ => Brackets::Bracketed,

src/builtin/math.rs

@@ -137,7 +137,7 @@ fn comparable(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value>
 #[cfg(feature = "random")]
 fn random(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> {
     args.max_args(1)?;
-    let limit = match parser.default_arg(&mut args, 0, "limit", Value::Null) {
+    let limit = match parser.default_arg(&mut args, 0, "limit", Value::Null)? {
         Value::Dimension(n, _) => n,
         Value::Null => {
             let mut rng = rand::thread_rng();

src/builtin/meta.rs

@@ -170,9 +170,9 @@ fn get_function(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value
         }
     };
     let css = parser
-        .default_arg(&mut args, 1, "css", Value::False)
+        .default_arg(&mut args, 1, "css", Value::False)?
         .is_true();
-    let module = match parser.default_arg(&mut args, 2, "module", Value::Null) {
+    let module = match parser.default_arg(&mut args, 2, "module", Value::Null)? {
         Value::String(s, ..) => Some(s),
         Value::Null => None,
         v => {

src/builtin/string.rs

@@ -148,7 +148,7 @@ fn str_slice(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> {
                 .into())
         }
     };
-    let mut end = match parser.default_arg(&mut args, 2, "end-at", Value::Null) {
+    let mut end = match parser.default_arg(&mut args, 2, "end-at", Value::Null)? {
         Value::Dimension(n, Unit::None) if n.is_decimal() => {
             return Err((format!("{} is not an int.", n), args.span()).into())
         }

src/error.rs

@@ -14,6 +14,47 @@ pub struct SassError {
     kind: SassErrorKind,
 }
 
+impl Clone for SassError {
+    fn clone(&self) -> Self {
+        match &self.kind {
+            SassErrorKind::Raw(a, b) => SassError {
+                kind: SassErrorKind::Raw(a.clone(), *b),
+            },
+            SassErrorKind::ParseError { message, loc } => SassError {
+                kind: SassErrorKind::ParseError {
+                    message: message.clone(),
+                    loc: loc.clone(),
+                },
+            },
+            _ => unimplemented!(),
+        }
+    }
+}
+
+impl PartialEq for SassError {
+    fn eq(&self, other: &Self) -> bool {
+        match &self.kind {
+            SassErrorKind::Raw(a, b) => match &other.kind {
+                SassErrorKind::Raw(c, d) => a == c && b == d,
+                _ => false,
+            },
+            SassErrorKind::ParseError {
+                message: message1,
+                loc: loc1,
+            } => match &other.kind {
+                SassErrorKind::ParseError {
+                    message: message2,
+                    loc: loc2,
+                } => message1 == message2 && loc1 == loc2,
+                _ => false,
+            },
+            _ => unimplemented!(),
+        }
+    }
+}
+
+impl Eq for SassError {}
+
 impl SassError {
     pub(crate) fn raw(self) -> (String, Span) {
         match self.kind {

src/parse/args.rs

@@ -178,7 +178,7 @@ impl<'a> Parser<'a> {
                             } else {
                                 CallArg::Named(name.into())
                             },
-                            self.parse_value_from_vec(val)?,
+                            self.parse_value_from_vec(val),
                         );
                         span = span.merge(tok.pos());
                         return Ok(CallArgs(args, span));
@@ -222,22 +222,22 @@ impl<'a> Parser<'a> {
                 match val.node {
                     Value::ArgList(v) => {
                         for arg in v {
-                            args.insert(CallArg::Positional(args.len()), arg);
+                            args.insert(CallArg::Positional(args.len()), Ok(arg));
                         }
                     }
                     Value::List(v, ..) => {
                         for arg in v {
-                            args.insert(CallArg::Positional(args.len()), arg.span(val.span));
+                            args.insert(CallArg::Positional(args.len()), Ok(arg.span(val.span)));
                         }
                     }
                     Value::Map(v) => {
                         for (name, arg) in v.entries() {
                             let name = name.to_css_string(val.span)?.to_string();
-                            args.insert(CallArg::Named(name.into()), arg.span(val.span));
+                            args.insert(CallArg::Named(name.into()), Ok(arg.span(val.span)));
                         }
                     }
                     _ => {
-                        args.insert(CallArg::Positional(args.len()), val);
+                        args.insert(CallArg::Positional(args.len()), Ok(val));
                     }
                 }
             } else {
@@ -247,7 +247,7 @@ impl<'a> Parser<'a> {
                     } else {
                         CallArg::Named(name.as_str().into())
                     },
-                    self.parse_value_from_vec(mem::take(&mut val))?,
+                    self.parse_value_from_vec(mem::take(&mut val)),
                 );
             }
 
@@ -278,20 +278,28 @@ impl<'a> Parser<'a> {
         position: usize,
         name: &'static str,
         default: Value,
-    ) -> Value {
+    ) -> SassResult<Value> {
-        match args.get(position, name) {
+        Ok(match args.get(position, name) {
-            Some(val) => val.node,
+            Some(val) => val?.node,
             None => default,
-        }
+        })
     }
 
     #[allow(clippy::unused_self)]
-    pub fn positional_arg(&self, args: &mut CallArgs, position: usize) -> Option<Spanned<Value>> {
+    pub fn positional_arg(
+        &self,
+        args: &mut CallArgs,
+        position: usize,
+    ) -> Option<SassResult<Spanned<Value>>> {
         args.get_positional(position)
     }
 
     #[allow(dead_code, clippy::unused_self)]
-    fn named_arg(&self, args: &mut CallArgs, name: &'static str) -> Option<Spanned<Value>> {
+    fn named_arg(
+        &self,
+        args: &mut CallArgs,
+        name: &'static str,
+    ) -> Option<SassResult<Spanned<Value>>> {
         args.get_named(name)
     }
 
@@ -301,11 +309,11 @@ impl<'a> Parser<'a> {
         args: &mut CallArgs,
         name: &'static str,
         default: Value,
-    ) -> Value {
+    ) -> SassResult<Value> {
-        match args.get_named(name) {
+        Ok(match args.get_named(name) {
-            Some(val) => val.node,
+            Some(val) => val?.node,
             None => default,
-        }
+        })
     }
 
     #[allow(clippy::unused_self)]
@@ -315,14 +323,14 @@ impl<'a> Parser<'a> {
             .0
             .into_iter()
             .map(|(a, v)| Ok((a.position()?, v)))
-            .collect::<Result<Vec<(usize, Spanned<Value>)>, String>>()
+            .collect::<Result<Vec<(usize, SassResult<Spanned<Value>>)>, String>>()
         {
             Ok(v) => v,
             Err(e) => return Err((format!("No argument named ${}.", e), args.1).into()),
         };
         args.sort_by(|(a1, _), (a2, _)| a1.cmp(a2));
         for arg in args {
-            vals.push(arg.1);
+            vals.push(arg.1?);
         }
         Ok(vals)
     }
@@ -351,14 +359,14 @@ impl<'a> Parser<'a> {
             let val = match args.get(idx, arg.name.clone()) {
                 Some(v) => v,
                 None => match arg.default.as_mut() {
-                    Some(v) => self.parse_value_from_vec(mem::take(v))?,
+                    Some(v) => self.parse_value_from_vec(mem::take(v)),
                     None => {
                         return Err(
                             (format!("Missing argument ${}.", &arg.name), args.span()).into()
                         )
                     }
                 },
-            };
+            }?;
             self.scopes
                 .last_mut()
                 .insert_var(arg.name.clone(), val.clone());

tests/args.rs

@@ -45,6 +45,11 @@ test!(
     "@function foo($a, $b: $a) {\n    @return $b;\n}\n\na {\n    color: foo(2);\n}\n",
     "a {\n  color: 2;\n}\n"
 );
+test!(
+    arg_errors_are_lazy_for_if,
+    "a {\n  color: if(false, unit(foo), red);\n}\n",
+    "a {\n  color: red;\n}\n"
+);
 error!(
     #[ignore = "expects incorrect char, '{'"]
     nothing_after_open,
@@ -54,3 +59,8 @@ error!(
     nothing_after_open_paren_in_fn_args,
     "@function foo(", "Error: expected \")\"."
 );
+error!(
+    args_are_evaluated_eagerly,
+    "@function foo($a) {\n    @return foo;\n}\n\na {\n    color: foo(unit(bar));\n}\n",
+    "Error: $number: bar is not a number."
+);