Skip to content

Commit

Permalink
Fix @forward statement altering the scope of the forwarded module (#85
Browse files Browse the repository at this point in the history
)

* avoid mutating forwarded module scope

* fix by adding scope to ForwardedModule instead

- add a test reproducing the issue

* fix verbose and no-color sass cli flags
  • Loading branch information
kketch authored Sep 28, 2023
1 parent 05bad84 commit f7f620e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 19 deletions.
25 changes: 9 additions & 16 deletions crates/compiler/src/builtin/modules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ impl ShadowedModule {

#[derive(Debug, Clone)]
pub(crate) struct ForwardedModule {
scope: ModuleScope,
#[allow(dead_code)]
inner: Arc<RefCell<Module>>,
#[allow(dead_code)]
forward_rule: AstForwardRule,
Expand Down Expand Up @@ -138,15 +140,16 @@ impl ForwardedModule {
rule.hidden_mixins_and_functions.as_ref(),
);

(*module).borrow_mut().set_scope(ModuleScope {
let scope = ModuleScope {
variables,
mixins,
functions,
});
};

ForwardedModule {
inner: module,
forward_rule: rule,
scope,
}
}

Expand Down Expand Up @@ -362,17 +365,8 @@ impl Module {
match self {
Self::Builtin { scope }
| Self::Environment { scope, .. }
| Self::Forwarded(ForwardedModule { scope, .. })
| Self::Shadowed(ShadowedModule { scope, .. }) => scope.clone(),
Self::Forwarded(forwarded) => (*forwarded.inner).borrow().scope(),
}
}

fn set_scope(&mut self, new_scope: ModuleScope) {
match self {
Self::Builtin { scope }
| Self::Environment { scope, .. }
| Self::Shadowed(ShadowedModule { scope, .. }) => *scope = new_scope,
Self::Forwarded(forwarded) => (*forwarded.inner).borrow_mut().set_scope(new_scope),
}
}

Expand Down Expand Up @@ -402,10 +396,9 @@ impl Module {
Self::Builtin { .. } => {
return Err(("Cannot modify built-in variable.", name.span).into())
}
Self::Environment { scope, .. } | Self::Shadowed(ShadowedModule { scope, .. }) => {
scope.clone()
}
Self::Forwarded(forwarded) => (*forwarded.inner).borrow_mut().scope(),
Self::Environment { scope, .. }
| Self::Forwarded(ForwardedModule { scope, .. })
| Self::Shadowed(ShadowedModule { scope, .. }) => scope.clone(),
};

if scope.variables.insert(name.node, value).is_none() {
Expand Down
4 changes: 1 addition & 3 deletions crates/lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ pub use grass_compiler::{

/// Include CSS in your binary at compile time from a Sass source file
///
/// ```
/// static CSS: &str = grass::include!("../static/_index.scss");
/// ```
/// `static CSS: &str = grass::include!("../static/_index.scss");`
///
/// This requires the `"macro"` feature, which is not enabled by default.
///
Expand Down
2 changes: 2 additions & 0 deletions crates/lib/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,14 @@ fn cli() -> Command {
.arg(
Arg::new("NO_COLOR")
.short('c')
.action(ArgAction::SetTrue)
.long("no-color")
.hide(true)
.help("Whether to use terminal colors for messages.")
)
.arg(
Arg::new("VERBOSE")
.action(ArgAction::SetTrue)
.long("verbose")
.hide(true)
.help("Print all deprecation warnings even when they're repetitive.")
Expand Down
34 changes: 34 additions & 0 deletions crates/lib/tests/forward.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,40 @@ fn import_forwarded_first_no_use() {
);
}

#[test]
fn forward_same_module_with_and_without_prefix() {
let mut fs = TestFs::new();

fs.add_file(
"_midstream.scss",
r#"
@forward "upstream";
@forward "upstream" as b-*;
"#,
);
fs.add_file(
"_upstream.scss",
r#"
@mixin a() {
c {
d: e
}
}
"#,
);

let input = r#"
@use "midstream";
@include midstream.a;
"#;

assert_eq!(
"c {\n d: e;\n}\n",
&grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input)
);
}

error!(
after_style_rule,
r#"
Expand Down

0 comments on commit f7f620e

Please sign in to comment.