allow redeclaration of module variables

src/builtin/modules/mod.rs

@@ -22,7 +22,13 @@ mod selector;
 mod string;
 
 #[derive(Debug, Default)]
-pub(crate) struct Module(pub Scope);
+pub(crate) struct Module {
+    pub scope: Scope,
+
+    /// Whether or not this module is builtin
+    /// e.g. `"sass:math"`
+    is_builtin: bool,
+}
 
 #[derive(Debug, Default)]
 pub(crate) struct Modules(BTreeMap<Identifier, Module>);
@@ -83,9 +89,27 @@ impl Modules {
                 .into()),
         }
     }
+
+    pub fn get_mut(&mut self, name: Identifier, span: Span) -> SassResult<&mut Module> {
+        match self.0.get_mut(&name) {
+            Some(v) => Ok(v),
+            None => Err((
+                format!(
+                    "There is no module with the namespace \"{}\".",
+                    name.as_str()
+                ),
+                span,
+            )
+                .into()),
+        }
+    }
 }
 
 impl Module {
+    pub fn new_builtin() -> Self {
+        Module { scope: Scope::default(), is_builtin: true }
+    }
+
     pub fn get_var(&self, name: Spanned<Identifier>) -> SassResult<&Value> {
         if name.node.as_str().starts_with('-') {
             return Err((
@@ -95,12 +119,36 @@ impl Module {
                 .into());
         }
 
-        match self.0.vars.get(&name.node) {
+        match self.scope.vars.get(&name.node) {
             Some(v) => Ok(v),
             None => Err(("Undefined variable.", name.span).into()),
         }
     }
 
+    pub fn update_var(&mut self, name: Spanned<Identifier>, value: Value) -> SassResult<()> {
+        if self.is_builtin {
+            return Err((
+                "Cannot modify built-in variable.",
+                name.span,
+            )
+                .into());
+        }
+
+        if name.node.as_str().starts_with('-') {
+            return Err((
+                "Private members can't be accessed from outside their modules.",
+                name.span,
+            )
+                .into());
+        }
+
+        if self.scope.insert_var(name.node, value).is_some() {
+            Ok(())
+        } else {
+            Err(("Undefined variable.", name.span).into())
+        }
+    }
+
     pub fn get_mixin(&self, name: Spanned<Identifier>) -> SassResult<Mixin> {
         if name.node.as_str().starts_with('-') {
             return Err((
@@ -110,18 +158,18 @@ impl Module {
                 .into());
         }
 
-        match self.0.mixins.get(&name.node) {
+        match self.scope.mixins.get(&name.node) {
             Some(v) => Ok(v.clone()),
             None => Err(("Undefined mixin.", name.span).into()),
         }
     }
 
     pub fn insert_builtin_mixin(&mut self, name: &'static str, mixin: BuiltinMixin) {
-        self.0.mixins.insert(name.into(), Mixin::Builtin(mixin));
+        self.scope.mixins.insert(name.into(), Mixin::Builtin(mixin));
     }
 
     pub fn insert_builtin_var(&mut self, name: &'static str, value: Value) {
-        self.0.vars.insert(name.into(), value);
+        self.scope.vars.insert(name.into(), value);
     }
 
     pub fn get_fn(&self, name: Spanned<Identifier>) -> SassResult<Option<SassFunction>> {
@@ -133,15 +181,15 @@ impl Module {
                 .into());
         }
 
-        Ok(self.0.functions.get(&name.node).cloned())
+        Ok(self.scope.functions.get(&name.node).cloned())
     }
 
     pub fn var_exists(&self, name: Identifier) -> bool {
-        !name.as_str().starts_with('-') && self.0.var_exists(name)
+        !name.as_str().starts_with('-') && self.scope.var_exists(name)
     }
 
     pub fn mixin_exists(&self, name: Identifier) -> bool {
-        !name.as_str().starts_with('-') && self.0.mixin_exists(name)
+        !name.as_str().starts_with('-') && self.scope.mixin_exists(name)
     }
 
     pub fn insert_builtin(
@@ -150,14 +198,14 @@ impl Module {
         function: fn(CallArgs, &mut Parser<'_>) -> SassResult<Value>,
     ) {
         let ident = name.into();
-        self.0
+        self.scope
             .functions
             .insert(ident, SassFunction::Builtin(Builtin::new(function), ident));
     }
 
     pub fn functions(&self) -> SassMap {
         SassMap::new_with(
-            self.0
+            self.scope
                 .functions
                 .iter()
                 .filter(|(key, _)| !key.as_str().starts_with('-'))
@@ -173,7 +221,7 @@ impl Module {
 
     pub fn variables(&self) -> SassMap {
         SassMap::new_with(
-            self.0
+            self.scope
                 .vars
                 .iter()
                 .filter(|(key, _)| !key.as_str().starts_with('-'))
@@ -187,49 +235,49 @@ impl Module {
         )
     }
 
-    pub const fn new_from_scope(scope: Scope) -> Self {
+    pub const fn new_from_scope(scope: Scope, is_builtin: bool) -> Self {
-        Module(scope)
+        Module { scope, is_builtin }
     }
 }
 
 pub(crate) fn declare_module_color() -> Module {
-    let mut module = Module::default();
+    let mut module = Module::new_builtin();
     color::declare(&mut module);
     module
 }
 
 pub(crate) fn declare_module_list() -> Module {
-    let mut module = Module::default();
+    let mut module = Module::new_builtin();
     list::declare(&mut module);
     module
 }
 
 pub(crate) fn declare_module_map() -> Module {
-    let mut module = Module::default();
+    let mut module = Module::new_builtin();
     map::declare(&mut module);
     module
 }
 
 pub(crate) fn declare_module_math() -> Module {
-    let mut module = Module::default();
+    let mut module = Module::new_builtin();
     math::declare(&mut module);
     module
 }
 
 pub(crate) fn declare_module_meta() -> Module {
-    let mut module = Module::default();
+    let mut module = Module::new_builtin();
     meta::declare(&mut module);
     module
 }
 
 pub(crate) fn declare_module_selector() -> Module {
-    let mut module = Module::default();
+    let mut module = Module::new_builtin();
     selector::declare(&mut module);
     module
 }
 
 pub(crate) fn declare_module_string() -> Module {
-    let mut module = Module::default();
+    let mut module = Module::new_builtin();
     string::declare(&mut module);
     module
 }

src/parse/common.rs

@@ -2,7 +2,7 @@ use std::ops::{BitAnd, BitOr};
 
 use codemap::Spanned;
 
-use crate::{interner::InternedString, value::Value};
+use crate::{common::Identifier, interner::InternedString, value::Value};
 
 #[derive(Debug, Clone)]
 pub(crate) struct NeverEmptyVec<T> {
@@ -42,6 +42,7 @@ impl<T> NeverEmptyVec<T> {
 pub(super) enum SelectorOrStyle {
     Selector(String),
     Style(InternedString, Option<Box<Spanned<Value>>>),
+    ModuleVariableRedeclaration(Identifier),
 }
 
 #[derive(Debug, Copy, Clone)]

src/parse/mod.rs

@@ -11,6 +11,7 @@ use crate::{
         AtRuleKind, SupportsRule, UnknownAtRule,
     },
     builtin::modules::{ModuleConfig, Modules},
+    common::Identifier,
     error::SassResult,
     scope::{Scope, Scopes},
     selector::{
@@ -27,6 +28,7 @@ use crate::{
 
 use common::{Comment, ContextFlags, NeverEmptyVec, SelectorOrStyle};
 pub(crate) use value::{HigherIntermediateValue, ValueVisitor};
+use variable::VariableValue;
 
 mod args;
 pub mod common;
@@ -131,6 +133,41 @@ impl<'a> Parser<'a> {
         }
     }
 
+    fn parse_module_variable_redeclaration(&mut self, module: Identifier) -> SassResult<()> {
+        let variable = self
+            .parse_identifier_no_interpolation(false)?
+            .map_node(|n| n.into());
+
+        self.whitespace_or_comment();
+        self.expect_char(':')?;
+
+        let VariableValue {
+            val_toks,
+            global,
+            default,
+        } = self.parse_variable_value()?;
+
+        if global {
+            return Err((
+                "!global isn't allowed for variables in other modules.",
+                variable.span,
+            )
+                .into());
+        }
+
+        if default {
+            return Ok(());
+        }
+
+        let value = self.parse_value_from_vec(val_toks, true)?;
+
+        self.modules
+            .get_mut(module, variable.span)?
+            .update_var(variable, value.node)?;
+
+        Ok(())
+    }
+
     fn parse_stmt(&mut self) -> SassResult<Vec<Stmt>> {
         let mut stmts = Vec::new();
         while let Some(Token { kind, pos }) = self.toks.peek() {
@@ -294,6 +331,9 @@ impl<'a> Parser<'a> {
                     }
                     if self.flags.in_keyframes() {
                         match self.is_selector_or_style()? {
+                            SelectorOrStyle::ModuleVariableRedeclaration(module) => {
+                                self.parse_module_variable_redeclaration(module)?
+                            }
                             SelectorOrStyle::Style(property, value) => {
                                 if let Some(value) = value {
                                     stmts.push(Stmt::Style(Style { property, value }));
@@ -321,6 +361,9 @@ impl<'a> Parser<'a> {
                     }
 
                     match self.is_selector_or_style()? {
+                        SelectorOrStyle::ModuleVariableRedeclaration(module) => {
+                            self.parse_module_variable_redeclaration(module)?
+                        }
                         SelectorOrStyle::Style(property, value) => {
                             if let Some(value) = value {
                                 stmts.push(Stmt::Style(Style { property, value }));

src/parse/module.rs

@@ -143,7 +143,7 @@ impl<'a> Parser<'a> {
                             .into());
                     }
 
-                    (Module::new_from_scope(global_scope), stmts)
+                    (Module::new_from_scope(global_scope, false), stmts)
                 } else {
                     return Err(("Can't find stylesheet to import.", self.span_before).into());
                 }

src/parse/style.rs

@@ -111,43 +111,62 @@ impl<'a> Parser<'a> {
         let mut property = self.parse_identifier()?.node;
         let whitespace_after_property = self.whitespace();
 
-        if let Some(Token { kind: ':', .. }) = self.toks.peek() {
+        match self.toks.peek() {
-            self.toks.next();
+            Some(Token { kind: ':', .. }) => {
-            if let Some(Token { kind, .. }) = self.toks.peek() {
+                self.toks.next();
-                return Ok(match kind {
+                if let Some(Token { kind, .. }) = self.toks.peek() {
-                    ':' => {
+                    return Ok(match kind {
-                        if whitespace_after_property {
+                        ':' => {
-                            property.push(' ');
+                            if whitespace_after_property {
-                        }
+                                property.push(' ');
-                        property.push(':');
-                        SelectorOrStyle::Selector(property)
-                    }
-                    c if is_name(*c) => {
-                        if let Some(toks) = self.parse_style_value_when_no_space_after_semicolon() {
-                            let len = toks.len();
-                            if let Ok(val) = self.parse_value_from_vec(toks, false) {
-                                self.toks.take(len).for_each(drop);
-                                return Ok(SelectorOrStyle::Style(
-                                    InternedString::get_or_intern(property),
-                                    Some(Box::new(val)),
-                                ));
                             }
+                            property.push(':');
+                            SelectorOrStyle::Selector(property)
                         }
+                        c if is_name(*c) => {
+                            if let Some(toks) =
+                                self.parse_style_value_when_no_space_after_semicolon()
+                            {
+                                let len = toks.len();
+                                if let Ok(val) = self.parse_value_from_vec(toks, false) {
+                                    self.toks.take(len).for_each(drop);
+                                    return Ok(SelectorOrStyle::Style(
+                                        InternedString::get_or_intern(property),
+                                        Some(Box::new(val)),
+                                    ));
+                                }
+                            }
 
-                        if whitespace_after_property {
+                            if whitespace_after_property {
-                            property.push(' ');
+                                property.push(' ');
+                            }
+                            property.push(':');
+                            return Ok(SelectorOrStyle::Selector(property));
                         }
-                        property.push(':');
+                        _ => SelectorOrStyle::Style(InternedString::get_or_intern(property), None),
-                        return Ok(SelectorOrStyle::Selector(property));
+                    });
+                }
+            }
+            Some(Token { kind: '.', .. }) => {
+                if matches!(self.toks.peek_next(), Some(Token { kind: '$', .. })) {
+                    self.toks.next();
+                    self.toks.next();
+                    return Ok(SelectorOrStyle::ModuleVariableRedeclaration(
+                        property.into(),
+                    ));
+                } else {
+                    if whitespace_after_property {
+                        property.push(' ');
                     }
-                    _ => SelectorOrStyle::Style(InternedString::get_or_intern(property), None),
+                    return Ok(SelectorOrStyle::Selector(property));
-                });
+                }
             }
-        } else {
+            _ => {
-            if whitespace_after_property {
+                if whitespace_after_property {
-                property.push(' ');
+                    property.push(' ');
+                }
+                return Ok(SelectorOrStyle::Selector(property));
             }
-            return Ok(SelectorOrStyle::Selector(property));
         }
         Err(("expected \"{\".", self.span_before).into())
     }

src/parse/variable.rs

@@ -8,10 +8,10 @@ use crate::{
 use super::Parser;
 
 #[derive(Debug)]
-struct VariableValue {
+pub(crate) struct VariableValue {
-    val_toks: Vec<Token>,
+    pub val_toks: Vec<Token>,
-    global: bool,
+    pub global: bool,
-    default: bool,
+    pub default: bool,
 }
 
 impl VariableValue {
@@ -88,7 +88,7 @@ impl<'a> Parser<'a> {
         Ok(())
     }
 
-    fn parse_variable_value(&mut self) -> SassResult<VariableValue> {
+    pub(super) fn parse_variable_value(&mut self) -> SassResult<VariableValue> {
         let mut default = false;
         let mut global = false;
 

src/scope.rs

@@ -81,7 +81,7 @@ impl Scope {
     }
 
     pub fn merge_module(&mut self, other: Module) {
-        self.merge(other.0);
+        self.merge(other.scope);
     }
 }
 

tests/use.rs

@@ -253,3 +253,66 @@ fn use_with_same_variable_multiple_times() {
         input
     );
 }
+
+#[test]
+fn use_variable_redeclaration_var_dne() {
+    let input = "@use \"use_variable_redeclaration_var_dne\" as mod;\nmod.$a: red;";
+    tempfile!("use_variable_redeclaration_var_dne.scss", "");
+
+    assert_err!("Error: Undefined variable.", input);
+}
+
+#[test]
+fn use_variable_redeclaration_global() {
+    let input = "@use \"use_variable_redeclaration_global\" as mod;\nmod.$a: red !global;";
+    tempfile!("use_variable_redeclaration_global.scss", "$a: green;");
+
+    assert_err!(
+        "Error: !global isn't allowed for variables in other modules.",
+        input
+    );
+}
+
+#[test]
+fn use_variable_redeclaration_simple() {
+    let input =
+        "@use \"use_variable_redeclaration_simple\" as mod;\nmod.$a: red; a { color: mod.$a; }";
+    tempfile!("use_variable_redeclaration_simple.scss", "$a: green;");
+
+    assert_eq!(
+        "a {\n  color: red;\n}\n",
+        &grass::from_string(input.to_string(), &grass::Options::default()).expect(input)
+    );
+}
+
+#[test]
+fn use_variable_redeclaration_default() {
+    let input = "@use \"use_variable_redeclaration_default\" as mod;\nmod.$a: 1 % red !default; a { color: mod.$a; }";
+    tempfile!("use_variable_redeclaration_default.scss", "$a: green;");
+
+    assert_eq!(
+        "a {\n  color: green;\n}\n",
+        &grass::from_string(input.to_string(), &grass::Options::default()).expect(input)
+    );
+}
+
+#[test]
+fn use_variable_redeclaration_private() {
+    let input = "@use \"use_variable_redeclaration_private\" as mod;\nmod.$-a: red;";
+    tempfile!("use_variable_redeclaration_private.scss", "$a: green;");
+
+    assert_err!(
+        "Error: Private members can't be accessed from outside their modules.",
+        input
+    );
+}
+
+#[test]
+fn use_variable_redeclaration_builtin() {
+    let input = "@use \"sass:math\";\nmath.$e: red;";
+
+    assert_err!(
+        "Error: Cannot modify built-in variable.",
+        input
+    );
+}