Refactor how function call args are parsed

src/args.rs

@@ -2,6 +2,7 @@ use std::collections::BTreeMap;
 use std::iter::Peekable;
 
 use crate::common::{Scope, Symbol};
+use crate::error::SassResult;
 use crate::utils::{devour_whitespace, devour_whitespace_or_comment};
 use crate::value::Value;
 use crate::{Token, TokenKind};
@@ -127,141 +128,79 @@ pub(crate) fn eat_func_args<I: Iterator<Item = Token>>(
 pub(crate) fn eat_call_args<I: Iterator<Item = Token>>(
     toks: &mut Peekable<I>,
     scope: &Scope,
-) -> CallArgs {
+) -> SassResult<CallArgs> {
     let mut args: BTreeMap<String, Value> = BTreeMap::new();
     devour_whitespace_or_comment(toks);
-    let mut name: Option<String> = None;
+    let mut name: String;
-    let mut val = Vec::new();
+    let mut val: Vec<Token> = Vec::new();
-    while let Some(Token { kind, pos }) = toks.next() {
+    loop {
-        match kind {
+        match toks.peek().unwrap().kind {
-            TokenKind::Variable(v) => {
+            TokenKind::Variable(_) => {
+                let v = toks.next().unwrap();
                 devour_whitespace_or_comment(toks);
-                match toks.peek() {
+                if toks.next().unwrap().equals_symbol(Symbol::Colon) {
-                    Some(Token {
+                    name = v.kind.to_string();
-                        kind: TokenKind::Symbol(Symbol::Colon),
+                } else {
-                        ..
+                    val.push(v);
-                    }) => name = Some(v),
+                    name = args.len().to_string();
-                    Some(Token {
-                        kind: TokenKind::Symbol(Symbol::Comma),
-                        ..
-                    }) => {
-                        toks.next();
-                        match name {
-                            Some(ref name) => {
-                                args.insert(name.clone(), scope.get_var(&v).unwrap().clone())
-                            }
-                            None => args.insert(
-                                format!("{}", args.len()),
-                                scope.get_var(&v).unwrap().clone(),
-                            ),
-                        };
-                        if let Some(ref mut s) = name {
-                            s.clear();
-                        }
-                        val.clear();
-                    }
-                    Some(Token {
-                        kind: TokenKind::Symbol(Symbol::CloseParen),
-                        ..
-                    }) => {
-                        toks.next();
-                        match name {
-                            Some(name) => args.insert(name, scope.get_var(&v).unwrap().clone()),
-                            None => args.insert(
-                                format!("{}", args.len()),
-                                scope.get_var(&v).unwrap().clone(),
-                            ),
-                        };
-                        break;
-                    }
-                    _ => todo!("unexpected token after variable in call args"),
-                }
-            }
-            TokenKind::Symbol(Symbol::Colon) => {
-                devour_whitespace_or_comment(toks);
-                while let Some(tok) = toks.peek() {
-                    match &tok.kind {
-                        TokenKind::Symbol(Symbol::Comma) => {
-                            toks.next();
-                            args.insert(
-                                name.clone().unwrap(),
-                                Value::from_tokens(&mut val.clone().into_iter().peekable(), scope)
-                                    .unwrap(),
-                            );
-                            if let Some(ref mut s) = name {
-                                s.clear();
-                            }
-                            val.clear();
-                            break;
-                        }
-                        TokenKind::Symbol(Symbol::CloseParen) => {
-                            args.insert(
-                                name.clone().unwrap(),
-                                Value::from_tokens(&mut val.clone().into_iter().peekable(), scope)
-                                    .unwrap(),
-                            );
-                            break;
-                        }
-                        _ => val.push(toks.next().expect("we know this exists!")),
-                    }
-                }
-            }
-            TokenKind::Symbol(Symbol::OpenParen) => {
-                val.push(Token { kind, pos });
-                let mut unclosed_parens = 0;
-                while let Some(tok) = toks.next() {
-                    match &tok.kind {
-                        TokenKind::Symbol(Symbol::OpenParen) => {
-                            unclosed_parens += 1;
-                        }
-                        TokenKind::Symbol(Symbol::CloseParen) => {
-                            if unclosed_parens <= 1 {
-                                val.push(tok);
-                                break;
-                            } else {
-                                unclosed_parens -= 1;
-                            }
-                        }
-                        _ => {}
-                    }
-                    val.push(tok);
                 }
             }
             TokenKind::Symbol(Symbol::CloseParen) => {
-                if val.is_empty() {
+                toks.next();
-                    break;
+                return Ok(CallArgs(args));
-                }
-                match name {
-                    Some(name) => args.insert(
-                        name,
-                        Value::from_tokens(&mut val.into_iter().peekable(), scope).unwrap(),
-                    ),
-                    None => args.insert(
-                        format!("{}", args.len()),
-                        Value::from_tokens(&mut val.into_iter().peekable(), scope).unwrap(),
-                    ),
-                };
-                break;
             }
-            TokenKind::Symbol(Symbol::Comma) => {
+            _ => name = args.len().to_string(),
-                match name {
-                    Some(ref name) => args.insert(
-                        name.clone(),
-                        Value::from_tokens(&mut val.clone().into_iter().peekable(), scope).unwrap(),
-                    ),
-                    None => args.insert(
-                        format!("{}", args.len()),
-                        Value::from_tokens(&mut val.clone().into_iter().peekable(), scope).unwrap(),
-                    ),
-                };
-                if let Some(ref mut s) = name {
-                    s.clear();
-                }
-                val.clear();
-            }
-            _ => val.push(Token { kind, pos }),
         }
         devour_whitespace_or_comment(toks);
+
+        while let Some(tok) = toks.next() {
+            match tok.kind {
+                TokenKind::Symbol(Symbol::CloseParen) => {
+                    args.insert(
+                        name,
+                        Value::from_tokens(&mut val.clone().into_iter().peekable(), scope)?,
+                    );
+                    return Ok(CallArgs(args));
+                }
+                TokenKind::Symbol(Symbol::Comma) => break,
+                TokenKind::Symbol(Symbol::OpenParen) => {
+                    val.push(tok);
+                    val.extend(read_until_close_paren(toks));
+                }
+                _ => val.push(tok),
+            }
+        }
+
+        args.insert(
+            name,
+            Value::from_tokens(&mut val.clone().into_iter().peekable(), scope)?,
+        );
+        val.clear();
+        devour_whitespace_or_comment(toks);
+
+        if toks.peek().is_none() {
+            return Ok(CallArgs(args));
+        }
     }
-    CallArgs(args)
+}
+
+fn read_until_close_paren<I: Iterator<Item = Token>>(toks: &mut Peekable<I>) -> Vec<Token> {
+    let mut v = Vec::new();
+    let mut scope = 0;
+    while let Some(tok) = toks.next() {
+        match tok.kind {
+            TokenKind::Symbol(Symbol::CloseParen) => {
+                if scope <= 1 {
+                    v.push(tok);
+                    return v;
+                } else {
+                    scope -= 1;
+                }
+            }
+            TokenKind::Symbol(Symbol::OpenParen) => scope += 1,
+            _ => {}
+        }
+        v.push(tok)
+    }
+    v
 }

src/lib.rs

@@ -96,6 +96,12 @@ pub(crate) struct Token {
     pub kind: TokenKind,
 }
 
+impl Token {
+    pub fn equals_symbol(&self, s: Symbol) -> bool {
+        self.kind.equals_symbol(s)
+    }
+}
+
 impl IsWhitespace for Token {
     fn is_whitespace(&self) -> bool {
         if let TokenKind::Whitespace(_) = self.kind {
@@ -147,6 +153,12 @@ pub(crate) enum TokenKind {
     Interpolation,
 }
 
+impl TokenKind {
+    pub fn equals_symbol(&self, s: Symbol) -> bool {
+        self == &TokenKind::Symbol(s)
+    }
+}
+
 impl Display for TokenKind {
     #[inline]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -159,7 +171,7 @@ impl Display for TokenKind {
             TokenKind::Attribute(s) => write!(f, "{}", s),
             TokenKind::Keyword(kw) => write!(f, "{}", kw),
             TokenKind::MultilineComment(s) => write!(f, "/*{}*/", s),
-            TokenKind::Variable(s) => write!(f, "${}", s),
+            TokenKind::Variable(s) => write!(f, "{}", s),
             TokenKind::Interpolation => {
                 panic!("we don't want to format TokenKind::Interpolation using Display")
             }

src/mixin.rs

@@ -141,7 +141,7 @@ pub(crate) fn eat_include<I: Iterator<Item = Token>>(
         match tok.kind {
             TokenKind::Symbol(Symbol::SemiColon) => CallArgs::new(),
             TokenKind::Symbol(Symbol::OpenParen) => {
-                let tmp = eat_call_args(toks, scope);
+                let tmp = eat_call_args(toks, scope)?;
                 devour_whitespace(toks);
                 if let Some(tok) = toks.next() {
                     assert_eq!(tok.kind, TokenKind::Symbol(Symbol::SemiColon));

src/value/parse.rs

@@ -223,7 +223,7 @@ impl Value {
                         let func = match scope.get_fn(&s) {
                             Ok(f) => f,
                             Err(_) => match GLOBAL_FUNCTIONS.get(&s) {
-                                Some(f) => return f(&eat_call_args(toks, scope), scope),
+                                Some(f) => return f(&eat_call_args(toks, scope)?, scope),
                                 None => {
                                     s.push('(');
                                     let mut unclosed_parens = 0;
@@ -260,7 +260,7 @@ impl Value {
                                 }
                             },
                         };
-                        Ok(func.clone().args(&eat_call_args(toks, scope)).call())
+                        Ok(func.clone().args(&eat_call_args(toks, scope)?).call())
                     }
                     _ => {
                         if let Ok(c) = crate::color::ColorName::try_from(s.as_ref()) {

tests/color.rs

@@ -81,11 +81,11 @@ test!(
     opacity_function_number_unit,
     "a {\n  color: opacity(1px);\n}\n"
 );
-// blocked on better function-call argument parsing
+// blocked on better value parsing
-// specifically, passing lists as values
+// specifically, this is parsed as a number and a list
 // test!(
 //     rgba_one_arg,
-//     "a {\n  color: rgba(1 2 3);;\n}\n",
+//     "a {\n  color: rgba(1 2 3);\n}\n",
 //     "a {\n  color: #010203;\n}\n"
 // );
 test!(
@@ -293,17 +293,16 @@ test!(
     "a {\n  color: adjust-hue(#811, 45deg);\n}\n",
     "a {\n  color: #886a11;\n}\n"
 );
-// blocked on better parsing of call args
+test!(
-// test!(
+    adjust_hue_named_args,
-//     adjust_hue_named_args,
+    "a {\n  color: adjust-hue($color: hsl(120, 30%, 90%), $degrees: 60deg);\n}\n",
-//     "a {\n  color: adjust-hue($color: hsl(120, 30%, 90%), $degrees: 60deg);\n}\n",
+    "a {\n  color: #deeded;\n}\n"
-//     "a {\n  color: #deeded;\n}\n"
+);
-// );
+test!(
-// test!(
+    lighten_named_args,
-//     lighten_named_args,
+    "a {\n  color: lighten($color: hsl(0, 0%, 0%), $amount: 30%);\n}\n",
-//     "a {\n  color: lighten($color: hsl(0, 0%, 0%), $amount: 30%);\n}\n",
+    "a {\n  color: #4d4d4d;\n}\n"
-//     "a {\n  color: #deeded;\n}\n"
+);
-// );
 test!(
     lighten_basic,
     "a {\n  color: lighten(hsl(0, 0%, 0%), 30%);\n}\n",
@@ -316,12 +315,11 @@ test!(
     // blocked on recognizing when to use 3-hex over 6-hex
     "a {\n  color: #ee0000;\n}\n"
 );
-// blocked on better parsing of call args
+test!(
-// test!(
+    darken_named_args,
-//     darken_named_args,
+    "a {\n  color: darken($color: hsl(25, 100%, 80%), $amount: 30%);\n}\n",
-//     "a {\n  color: darken($color: hsl(25, 100%, 80%), $amount: 30%);\n}\n",
+    "a {\n  color: #ff6a00;\n}\n"
-//     "a {\n  color: #ff6a00;\n}\n"
+);
-// );
 test!(
     darken_basic,
     "a {\n  color: darken(hsl(25, 100%, 80%), 30%);\n}\n",
@@ -334,12 +332,11 @@ test!(
     // blocked on recognizing when to use 3-hex over 6-hex
     "a {\n  color: #220000;\n}\n"
 );
-// blocked on better parsing of call args
+test!(
-// test!(
+    saturate_named_args,
-//     saturate_named_args,
+    "a {\n  color: saturate($color: hsl(25, 100%, 80%), $amount: 30%);\n}\n",
-//     "a {\n  color: saturate($color: hsl(25, 100%, 80%), $amount: 30%);\n}\n",
+    "a {\n  color: #ffc499;\n}\n"
-//     "a {\n  color: #ff6a00;\n}\n"
+);
-// );
 test!(
     saturate_basic,
     "a {\n  color: saturate(hsl(120, 30%, 90%), 20%);\n}\n",
@@ -350,12 +347,11 @@ test!(
     "a {\n  color: saturate(#855, 20%);\n}\n",
     "a {\n  color: #9e3f3f;\n}\n"
 );
-// blocked on better parsing of call args
+test!(
-// test!(
+    desaturate_named_args,
-//     desaturate_named_args,
+    "a {\n  color: desaturate($color: hsl(25, 100%, 80%), $amount: 30%);\n}\n",
-//     "a {\n  color: desaturate($color: hsl(25, 100%, 80%), $amount: 30%);\n}\n",
+    "a {\n  color: #f0c6a8;\n}\n"
-//     "a {\n  color: #ff6a00;\n}\n"
+);
-// );
 test!(
     desaturate_basic,
     "a {\n  color: desaturate(hsl(120, 30%, 90%), 20%);\n}\n",
@@ -402,6 +398,7 @@ test!(
     "a {\n  color: gray;\n}\n"
 );
 test!(grayscale_number, "a {\n  color: grayscale(15%);\n}\n");
+// handle special functions better!
 // test!(
 //     grayscale_number_casing,
 //     "a {\n  color: grAyscaLe(15%);\n}\n"

tests/strings.rs

@@ -98,11 +98,8 @@ test!(
     "a {\n  color: unquote('');\n}\n",
     ""
 );
-// blocked on refactoring how function-call args are parsed
+test!(
-// right now, whitespace is eaten between idents with no
+    str_len_space,
-// regard for quotes
+    "a {\n  color: str-length(\"foo bar\");\n}\n",
-// test!(
+    "a {\n  color: 7;\n}\n"
-//     str_len_space,
+);
-//     "a {\n  color: str-length(\"foo bar\");\n}\n",
-//     "a {\n  color: 7;\n}\n"
-// );