From beb64abac4c2abcdab400540477f215acde6ce9c Mon Sep 17 00:00:00 2001
From: greenwoodcm <greenwd@amazon.com>
Date: Sun, 9 Jul 2023 10:55:45 -0700
Subject: [PATCH] support `$keys...` argument for `map.get` (#83)

per the sass-lang docs [1], the user should be able to invoke
`map.get($my-map, "key1", "key2")` to perform a nested lookup
of the two keys.  the current implementation fails if provided
more than two arguments to `map.get`.  this change implements
the nested get.  fixes #80.

[1] https://sass-lang.com/documentation/modules/map/
---
 crates/compiler/src/ast/args.rs              | 12 ++++---
 crates/compiler/src/builtin/functions/map.rs | 35 ++++++++++++++++++--
 crates/lib/tests/map.rs                      | 35 ++++++++++++++++++++
 3 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/crates/compiler/src/ast/args.rs b/crates/compiler/src/ast/args.rs
index c0e79e2..9e929b7 100644
--- a/crates/compiler/src/ast/args.rs
+++ b/crates/compiler/src/ast/args.rs
@@ -233,10 +233,14 @@ impl ArgumentResult {
     pub(crate) fn min_args(&self, min: usize) -> SassResult<()> {
         let len = self.len();
         if len < min {
-            if min == 1 {
-                return Err(("At least one argument must be passed.", self.span()).into());
-            }
-            todo!("min args greater than one")
+            let phrase = match min {
+                1 => "one argument",
+                2 => "two arguments",
+                3 => "three arguments",
+                _ => todo!("min args greater than three"),
+            };
+
+            return Err((format!("At least {phrase} must be passed."), self.span()).into());
         }
         Ok(())
     }
diff --git a/crates/compiler/src/builtin/functions/map.rs b/crates/compiler/src/builtin/functions/map.rs
index 129a205..c4db25a 100644
--- a/crates/compiler/src/builtin/functions/map.rs
+++ b/crates/compiler/src/builtin/functions/map.rs
@@ -1,12 +1,43 @@
 use crate::builtin::builtin_imports::*;
 
+/// map.get($map, $key, $keys...)
+/// map-get($map, $key, $keys...)
+///
+/// If $keys is empty, returns the value in $map associated with $key.
+/// If $map doesn’t have a value associated with $key, returns null.
+/// If $keys is not empty, follows the set of keys including $key and
+/// excluding the last key in $keys, from left to right, to find the
+/// nested map targeted for searching.
+/// Returns the value in the targeted map associated with the last key
+/// in $keys.
+/// Returns null if the map does not have a value associated with the
+/// key, or if any key in $keys is missing from a map or references a
+/// value that is not a map.
+///
+/// https://sass-lang.com/documentation/modules/map/
 pub(crate) fn map_get(mut args: ArgumentResult, visitor: &mut Visitor) -> SassResult<Value> {
-    args.max_args(2)?;
     let key = args.get_err(1, "key")?;
     let map = args
         .get_err(0, "map")?
         .assert_map_with_name("map", args.span())?;
-    Ok(map.get(&key).unwrap_or(Value::Null))
+
+    // since we already extracted the map and first key,
+    // neither will be returned in the variadic args list
+    let keys = args.get_variadic()?;
+
+    let mut val = map.get(&key).unwrap_or(Value::Null);
+    for key in keys {
+        // if at any point we find a value that's not a map,
+        // we return null
+        let val_map = match val.try_map() {
+            Some(val_map) => val_map,
+            None => return Ok(Value::Null),
+        };
+
+        val = val_map.get(&key).unwrap_or(Value::Null);
+    }
+
+    Ok(val)
 }
 
 pub(crate) fn map_has_key(mut args: ArgumentResult, visitor: &mut Visitor) -> SassResult<Value> {
diff --git a/crates/lib/tests/map.rs b/crates/lib/tests/map.rs
index 967a1c2..d6dbfb3 100644
--- a/crates/lib/tests/map.rs
+++ b/crates/lib/tests/map.rs
@@ -5,6 +5,11 @@ test!(
     "a {\n  color: map-get((a: b), a);\n}\n",
     "a {\n  color: b;\n}\n"
 );
+test!(
+    map_get_key_exists_named,
+    "a {\n  color: map-get($map: (a: b), $key: a);\n}\n",
+    "a {\n  color: b;\n}\n"
+);
 test!(
     map_get_key_does_not_exist,
     "a {\n  color: map-get((a: b), foo);\n}\n",
@@ -19,6 +24,36 @@ error!(
     map_get_non_map,
     "a {\n  color: map-get(foo, foo);\n}\n", "Error: $map: foo is not a map."
 );
+test!(
+    map_get_nested,
+    "a {\n  color: map-get((a: (b: (c: d))), a, b, c);\n}\n",
+    "a {\n  color: d;\n}\n"
+);
+// it's an odd thing to do, but the spec suggests that the user
+// can call the function like:
+//     map.get("key2", "key3", $map: $my-map, $key: "key1")
+// in this case we are to use the named argument $key as the
+// first key and use the positional arguments at the front as
+// $keys.  this test verifies this behavior.
+test!(
+    map_get_nested_named_and_positional,
+    "a {\n  color: map-get(b, c, $map: (a: (b: (c: d))), $key: a);\n}\n",
+    "a {\n  color: d;\n}\n"
+);
+test!(
+    map_get_nested_key_does_not_exist,
+    "a {\n  color: map-get((a: (b: (c: d))), a, d, e, f);\n}\n",
+    ""
+);
+test!(
+    map_get_nested_non_map,
+    "a {\n  color: map-get((a: (b: c)), a, b, c, d);\n}\n",
+    ""
+);
+error!(
+    map_get_no_args,
+    "a {\n  color: map-get();\n}\n", "Error: Missing argument $key."
+);
 error!(
     map_get_one_arg,
     "a {\n  color: map-get(1);\n}\n", "Error: Missing argument $key."