From da3c3eabfc6a7e4740d4e1ecb91795f814410420 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Thu, 30 Jul 2020 23:40:34 -0400 Subject: [PATCH] refactor how `@content` scoping is handled --- src/atrule/mixin.rs | 13 ++++++++ src/parse/mixin.rs | 54 ++++++++++++++++++++++++--------- src/scope.rs | 23 +++++++------- tests/mixins.rs | 74 +++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 132 insertions(+), 32 deletions(-) diff --git a/src/atrule/mixin.rs b/src/atrule/mixin.rs index 82c8b93..a85bcf8 100644 --- a/src/atrule/mixin.rs +++ b/src/atrule/mixin.rs @@ -26,6 +26,19 @@ impl Mixin { #[derive(Debug, Clone)] pub(crate) struct Content { + /// The literal block, serialized as a list of tokens pub content: Option>, + + /// Optional args, e.g. `@content(a, b, c);` pub content_args: Option, + + /// The number of scopes at the use of `@include` + /// + /// This is used to "reset" back to the state of the `@include` + /// without actually cloning the scope or putting it in an `Rc` + pub scope_len: usize, + + /// Whether or not the mixin this `@content` block is inside of was + /// declared in the global scope + pub declared_at_root: bool, } diff --git a/src/parse/mixin.rs b/src/parse/mixin.rs index 84830d1..db4a5a1 100644 --- a/src/parse/mixin.rs +++ b/src/parse/mixin.rs @@ -8,6 +8,7 @@ use crate::{ args::{CallArgs, FuncArgs}, atrule::{Content, Mixin}, error::SassResult, + scope::Scopes, utils::read_until_closing_curly_brace, Token, }; @@ -129,6 +130,8 @@ impl<'a> Parser<'a> { let scope = self.eval_args(fn_args, args)?; + let scope_len = self.scopes.len(); + if declared_at_root { mem::swap(self.scopes, self.content_scopes); } @@ -138,6 +141,8 @@ impl<'a> Parser<'a> { self.content.push(Content { content, content_args, + scope_len, + declared_at_root, }); let body = Parser { @@ -177,26 +182,35 @@ impl<'a> Parser<'a> { .into()); } - if let Some(Token { kind: '(', .. }) = self.toks.peek() { - self.toks.next(); - let args = self.parse_call_args()?; - if let Some(Some(content_args)) = self.content.last().map(|v| v.content_args.clone()) { - args.max_args(content_args.len())?; - - let scope = self.eval_args(content_args, args)?; - self.content_scopes.merge(scope); - } else { - args.max_args(0)?; - } - } - Ok(if let Some(content) = self.content.pop() { + let (mut scope_at_decl, mixin_scope) = if content.declared_at_root { + (mem::take(self.content_scopes), Scopes::new()) + } else { + mem::take(self.scopes).split_off(content.scope_len) + }; + + let mut entered_scope = false; + + if let Some(Token { kind: '(', .. }) = self.toks.peek() { + self.toks.next(); + let args = self.parse_call_args()?; + if let Some(ref content_args) = content.content_args { + args.max_args(content_args.len())?; + + let scope = self.eval_args(content_args.clone(), args)?; + scope_at_decl.enter_scope(scope); + entered_scope = true; + } else { + args.max_args(0)?; + } + } + let stmts = if let Some(body) = content.content.clone() { Parser { toks: &mut body.into_iter().peekmore(), map: self.map, path: self.path, - scopes: self.content_scopes, + scopes: &mut scope_at_decl, global_scope: self.global_scope, super_selectors: self.super_selectors, span_before: self.span_before, @@ -213,6 +227,18 @@ impl<'a> Parser<'a> { Vec::new() }; + if entered_scope { + scope_at_decl.exit_scope(); + } + + scope_at_decl.merge(mixin_scope); + + if content.declared_at_root { + *self.content_scopes = scope_at_decl; + } else { + *self.scopes = scope_at_decl; + } + self.content.push(content); stmts diff --git a/src/scope.rs b/src/scope.rs index 25b84ef..69fb0db 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -73,12 +73,6 @@ impl Scope { } self.functions.contains_key(&name) } - - fn merge(&mut self, other: Scope) { - self.vars.extend(other.vars); - self.mixins.extend(other.mixins); - self.functions.extend(other.functions); - } } #[derive(Debug, Default)] @@ -89,6 +83,15 @@ impl Scopes { Self(Vec::new()) } + pub fn len(&self) -> usize { + self.0.len() + } + + pub fn split_off(mut self, len: usize) -> (Scopes, Scopes) { + let split = self.0.split_off(len); + (self, Scopes(split)) + } + pub fn enter_new_scope(&mut self) { self.0.push(Scope::new()); } @@ -101,12 +104,8 @@ impl Scopes { self.0.pop(); } - pub fn merge(&mut self, other: Scope) { - if let Some(scope) = self.0.last_mut() { - scope.merge(other) - } else { - self.0.push(other) - } + pub fn merge(&mut self, mut other: Self) { + self.0.append(&mut other.0); } } diff --git a/tests/mixins.rs b/tests/mixins.rs index 79cbb5c..3864e53 100644 --- a/tests/mixins.rs +++ b/tests/mixins.rs @@ -381,20 +381,20 @@ test!( "@mixin foo { color: foo; } - + a { @include foo(); - + @mixin foo { color: bar; } - + a { @mixin foo { color: baz; } } - + @include foo(); }", "a {\n color: foo;\n color: bar;\n}\n" @@ -451,7 +451,7 @@ test!( ); test!( can_access_variables_declared_before_content, - "@mixin foo { + "@mixin foo { $a: red; @content; @@ -459,12 +459,74 @@ test!( color: $a; } - a { @include foo; }", "a {\n color: red;\n}\n" ); +test!( + content_contains_variable_declared_in_outer_scope_not_declared_at_root, + "a { + $a: red; + + @mixin foo { + @content; + } + + @include foo { + color: $a; + } + }", + "a {\n color: red;\n}\n" +); +test!( + content_contains_variable_declared_in_outer_scope_declared_at_root, + "@mixin foo { + @content; + } + + a { + $a: red; + + @include foo { + color: $a; + } + }", + "a {\n color: red;\n}\n" +); +test!( + content_contains_variable_declared_in_outer_scope_not_declared_at_root_and_modified, + "a { + $a: red; + + @mixin foo { + $a: green; + @content; + } + + @include foo { + color: $a; + } + }", + "a {\n color: green;\n}\n" +); +test!( + content_contains_variable_declared_in_outer_scope_declared_at_root_and_modified, + "@mixin foo { + $a: green; + @content; + } + + a { + $a: red; + + + @include foo { + color: $a; + } + }", + "a {\n color: red;\n}\n" +); error!( mixin_in_function, "@function foo() {