more robust support for @forward prefixes, more code coverage

CHANGELOG.md

@@ -12,6 +12,7 @@
 - support `$whiteness` and `$blackness` as arguments to `scale-color(..)`
 - more accurate list separator from `join(..)`
 - resolve unicode edge cases in `str-index(..)`
+- more robust support for `@forward` prefixes
 
 # 0.12.0
 

README.md

@@ -70,9 +70,9 @@ The spec runner does not work on Windows.
 Using a modified version of the spec runner that ignores warnings and error spans (but does include error messages), `grass` achieves the following results:
 
 ```
-2022-12-26
-PASSING: 6077
-FAILING: 828
+2022-01-03
+PASSING: 6118
+FAILING: 787
 TOTAL: 6905
 ```
 

src/builtin/functions/meta.rs

@@ -316,7 +316,7 @@ pub(crate) fn call(mut args: ArgumentResult, visitor: &mut Visitor) -> SassResul
         }
     };
 
-    args.remove_positional(0).unwrap();
+    args.remove_positional(0);
 
     visitor.run_function_callable_with_maybe_evaled(*func, MaybeEvaledArguments::Evaled(args), span)
 }

src/builtin/modules/meta.rs

@@ -32,6 +32,8 @@ fn load_css(mut args: ArgumentResult, visitor: &mut Visitor) -> SassResult<()> {
 
     let with = match args.default_arg(1, "with", Value::Null) {
         Value::Map(map) => Some(map),
+        Value::List(v, ..) if v.is_empty() => Some(SassMap::new()),
+        Value::ArgList(v) if v.is_empty() => Some(SassMap::new()),
         Value::Null => None,
         v => return Err((format!("$with: {} is not a map.", v.inspect(span)?), span).into()),
     };

src/builtin/modules/mod.rs

@@ -14,7 +14,7 @@ use crate::{
     error::SassResult,
     evaluate::{Environment, Visitor},
     selector::ExtensionStore,
-    utils::{BaseMapView, MapView, MergedMapView, PublicMemberMapView},
+    utils::{BaseMapView, MapView, MergedMapView, PrefixedMapView, PublicMemberMapView},
     value::{SassFunction, SassMap, Value},
 };
 
@@ -36,6 +36,61 @@ pub(crate) struct ForwardedModule {
 }
 
 impl ForwardedModule {
+    pub fn new(module: Arc<RefCell<Module>>, rule: AstForwardRule) -> Self {
+        let scope = (*module).borrow().scope();
+
+        let variables = Self::forwarded_map(
+            scope.variables,
+            rule.prefix.as_deref(),
+            rule.shown_variables.as_ref(),
+            rule.hidden_variables.as_ref(),
+        );
+
+        let functions = Self::forwarded_map(
+            scope.functions,
+            rule.prefix.as_deref(),
+            rule.shown_mixins_and_functions.as_ref(),
+            rule.hidden_mixins_and_functions.as_ref(),
+        );
+
+        let mixins = Self::forwarded_map(
+            scope.mixins,
+            rule.prefix.as_deref(),
+            rule.shown_mixins_and_functions.as_ref(),
+            rule.hidden_mixins_and_functions.as_ref(),
+        );
+
+        (*module).borrow_mut().set_scope(ModuleScope {
+            variables,
+            mixins,
+            functions,
+        });
+
+        ForwardedModule {
+            inner: module,
+            forward_rule: rule,
+        }
+    }
+
+    fn forwarded_map<T: Clone + fmt::Debug + 'static>(
+        mut map: Arc<dyn MapView<Value = T>>,
+        prefix: Option<&str>,
+        safelist: Option<&HashSet<Identifier>>,
+        blocklist: Option<&HashSet<Identifier>>,
+    ) -> Arc<dyn MapView<Value = T>> {
+        debug_assert!(safelist.is_none() || blocklist.is_none());
+
+        if prefix.is_none() && safelist.is_none() && blocklist.is_none() {
+            return map;
+        }
+
+        if let Some(prefix) = prefix {
+            map = Arc::new(PrefixedMapView(map, prefix.to_owned()));
+        }
+
+        map
+    }
+
     pub fn if_necessary(
         module: Arc<RefCell<Module>>,
         rule: AstForwardRule,
@@ -54,10 +109,9 @@ impl ForwardedModule {
         {
             module
         } else {
-            Arc::new(RefCell::new(Module::Forwarded(ForwardedModule {
-                inner: module,
-                forward_rule: rule,
-            })))
+            Arc::new(RefCell::new(Module::Forwarded(ForwardedModule::new(
+                module, rule,
+            ))))
         }
     }
 }
@@ -232,6 +286,13 @@ impl Module {
         }
     }
 
+    fn set_scope(&mut self, new_scope: ModuleScope) {
+        match self {
+            Self::Builtin { scope } | Self::Environment { scope, .. } => *scope = new_scope,
+            Self::Forwarded(forwarded) => (*forwarded.inner).borrow_mut().set_scope(new_scope),
+        }
+    }
+
     pub fn get_var(&self, name: Spanned<Identifier>) -> SassResult<Value> {
         let scope = self.scope();
 

src/utils/map_view.rs

@@ -293,7 +293,11 @@ impl<V: fmt::Debug + Clone> MapView for MergedMapView<V> {
     }
 
     fn iter(&self) -> Vec<(Identifier, Self::Value)> {
-        unimplemented!()
+        self.1
+            .iter()
+            .copied()
+            .map(|name| (name, self.get(name).unwrap()))
+            .collect()
     }
 }
 
@@ -340,6 +344,10 @@ impl<V: fmt::Debug + Clone, T: MapView<Value = V> + Clone> MapView for PublicMem
     }
 
     fn iter(&self) -> Vec<(Identifier, Self::Value)> {
-        unimplemented!()
+        self.0
+            .iter()
+            .into_iter()
+            .filter(|(name, _)| Identifier::is_public(name))
+            .collect()
     }
 }

tests/forward.rs

@@ -208,6 +208,115 @@ fn member_import_precedence_top_level() {
         &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
     );
 }
+
+#[test]
+fn member_as_function() {
+    let mut fs = TestFs::new();
+
+    fs.add_file("_midstream.scss", r#"@forward "upstream" as d-*;"#);
+    fs.add_file(
+        "_upstream.scss",
+        r#"
+            @function c() {
+                @return e;
+            }
+        "#,
+    );
+
+    let input = r#"
+        @use "midstream";
+
+        a {
+            b: midstream.d-c();
+        }
+    "#;
+
+    assert_eq!(
+        "a {\n  b: e;\n}\n",
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
+    );
+}
+
+#[test]
+fn member_as_mixin() {
+    let mut fs = TestFs::new();
+
+    fs.add_file("_midstream.scss", r#"@forward "upstream" as b-*;"#);
+    fs.add_file(
+        "_upstream.scss",
+        r#"
+            @mixin a() {
+                c {
+                    d: e
+                }
+            }
+        "#,
+    );
+
+    let input = r#"
+        @use "midstream";
+
+        @include midstream.b-a;
+    "#;
+
+    assert_eq!(
+        "c {\n  d: e;\n}\n",
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
+    );
+}
+
+#[test]
+fn member_as_variable_use() {
+    let mut fs = TestFs::new();
+
+    fs.add_file("_midstream.scss", r#"@forward "upstream" as d-*;"#);
+    fs.add_file(
+        "_upstream.scss",
+        r#"
+            $c: e;
+        "#,
+    );
+
+    let input = r#"
+        @use "midstream";
+
+        a {b: midstream.$d-c}
+    "#;
+
+    assert_eq!(
+        "a {\n  b: e;\n}\n",
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
+    );
+}
+
+#[test]
+fn member_as_variable_assignment_toplevel() {
+    let mut fs = TestFs::new();
+
+    fs.add_file("_midstream.scss", r#"@forward "upstream" as d-*;"#);
+    fs.add_file(
+        "_upstream.scss",
+        r#"
+            $a: old value;
+
+            @function get-a() {@return $a}
+        "#,
+    );
+
+    let input = r#"
+        @use "midstream";
+
+        midstream.$d-a: new value;
+
+        b {c: midstream.d-get-a()};
+    "#;
+
+    assert_eq!(
+        "b {\n  c: new value;\n}\n",
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
+    );
+}
+
 error!(
     after_style_rule,
     r#"

tests/meta.rs

@@ -314,5 +314,19 @@ test!(
     }",
     "a {\n  color: true;\n}\n"
 );
+error!(
+    call_single_arg_is_named,
+    "a {
+      color: call($function: get-function(\"red\"));
+    }",
+    "Error: Missing argument $color."
+);
+test!(
+    call_all_args_named,
+    "a {
+      color: call($function: get-function(\"red\"), $color: #fff);
+    }",
+    "a {\n  color: 255;\n}\n"
+);
 
 // todo: if() with different combinations of named and positional args

tests/use.rs

@@ -587,4 +587,57 @@ fn use_with_through_forward_multiple() {
     );
 }
 
+#[test]
+fn module_functions_empty() {
+    let mut fs = TestFs::new();
+
+    fs.add_file("_other.scss", r#""#);
+
+    let input = r#"
+        @use "sass:meta";
+        @use "other";
+
+        a {
+            b: meta.inspect(meta.module-functions("other"))
+        }
+    "#;
+
+    assert_eq!(
+        "a {\n  b: ();\n}\n",
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
+    );
+}
+
+#[test]
+fn module_functions_through_forward() {
+    let mut fs = TestFs::new();
+
+    fs.add_file(
+        "_a.scss",
+        r#"
+        @forward "b";
+    "#,
+    );
+    fs.add_file(
+        "_b.scss",
+        r#"
+        @function foo() {}
+    "#,
+    );
+
+    let input = r#"
+        @use "sass:meta";
+        @use "a";
+
+        a {
+            b: meta.inspect(meta.module-functions("a"))
+        }
+    "#;
+
+    assert_eq!(
+        "a {\n  b: (\"foo\": get-function(\"foo\"));\n}\n",
+        &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
+    );
+}
+
 // todo: refactor these tests to use testfs where possible