diff --git a/CHANGELOG.md b/CHANGELOG.md index 58517e1..a6befcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ --> +# 0.13.0 + # 0.12.4 - implement builtin map-module functions `map.deep-merge(..)` and `map.deep-remove(..)` diff --git a/README.md b/README.md index f8a0d6f..8921d0b 100644 --- a/README.md +++ b/README.md @@ -29,13 +29,6 @@ All known missing features and bugs are tracked in [#19](https://github.com/conn `grass` is benchmarked against `dart-sass` and `sassc` (`libsass`) [here](https://github.com/connorskees/sass-perf). In general, `grass` appears to be ~2x faster than `dart-sass` and ~1.7x faster than `sassc`. -## Web Assembly - -`grass` experimentally releases a -[WASM version of the library to npm](https://www.npmjs.com/package/@connorskees/grass), -compiled using wasm-bindgen. To use `grass` in your JavaScript projects, run -`npm install @connorskees/grass` to add it to your package.json. This version of grass is not currently well documented, but one can find example usage in the [`grassmeister` repository](https://github.com/connorskees/grassmeister). - ## Cargo Features ### commandline @@ -84,9 +77,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-05-11 +2022-05-20 PASSING: 6277 -FAILING: 596 +FAILING: 548 TOTAL: 6905 ``` diff --git a/crates/compiler/src/ast/stmt.rs b/crates/compiler/src/ast/stmt.rs index 537b323..c9306f1 100644 --- a/crates/compiler/src/ast/stmt.rs +++ b/crates/compiler/src/ast/stmt.rs @@ -415,6 +415,13 @@ impl ConfiguredValue { configuration_span: Some(configuration_span), } } + + pub fn implicit(value: Value) -> Self { + Self { + value, + configuration_span: None, + } + } } #[derive(Debug, Clone)] diff --git a/crates/compiler/src/builtin/modules/mod.rs b/crates/compiler/src/builtin/modules/mod.rs index 4b6ec66..0159e96 100644 --- a/crates/compiler/src/builtin/modules/mod.rs +++ b/crates/compiler/src/builtin/modules/mod.rs @@ -14,7 +14,9 @@ use crate::{ error::SassResult, evaluate::{Environment, Visitor}, selector::ExtensionStore, - utils::{BaseMapView, MapView, MergedMapView, PrefixedMapView, PublicMemberMapView}, + utils::{ + BaseMapView, LimitedMapView, MapView, MergedMapView, PrefixedMapView, PublicMemberMapView, + }, value::{SassFunction, SassMap, Value}, }; @@ -28,6 +30,82 @@ mod meta; mod selector; mod string; +/// A [Module] that only exposes members that aren't shadowed by a given +/// blocklist of member names. +#[derive(Debug, Clone)] +pub(crate) struct ShadowedModule { + #[allow(dead_code)] + inner: Arc>, + scope: ModuleScope, +} + +impl ShadowedModule { + pub fn new( + module: Arc>, + variables: Option<&HashSet>, + functions: Option<&HashSet>, + mixins: Option<&HashSet>, + ) -> Self { + let module_scope = module.borrow().scope(); + + let variables = Self::shadowed_map(Arc::clone(&module_scope.variables), variables); + let functions = Self::shadowed_map(Arc::clone(&module_scope.functions), functions); + let mixins = Self::shadowed_map(Arc::clone(&module_scope.mixins), mixins); + + let new_scope = ModuleScope { + variables, + functions, + mixins, + }; + + Self { + inner: module, + scope: new_scope, + } + } + + fn needs_blocklist( + map: Arc>, + blocklist: Option<&HashSet>, + ) -> bool { + blocklist.is_some() + && !map.is_empty() + && blocklist.unwrap().iter().any(|key| map.contains_key(*key)) + } + + fn shadowed_map( + map: Arc>, + blocklist: Option<&HashSet>, + ) -> Arc> { + match blocklist { + Some(..) if !Self::needs_blocklist(Arc::clone(&map), blocklist) => map, + Some(blocklist) => Arc::new(LimitedMapView::blocklist(map, blocklist)), + None => map, + } + } + + pub fn if_necessary( + module: Arc>, + variables: Option<&HashSet>, + functions: Option<&HashSet>, + mixins: Option<&HashSet>, + ) -> Option>> { + let module_scope = module.borrow().scope(); + + let needs_blocklist = Self::needs_blocklist(Arc::clone(&module_scope.variables), variables) + || Self::needs_blocklist(Arc::clone(&module_scope.functions), functions) + || Self::needs_blocklist(Arc::clone(&module_scope.mixins), mixins); + + if needs_blocklist { + Some(Arc::new(RefCell::new(Module::Shadowed(Self::new( + module, variables, functions, mixins, + ))))) + } else { + None + } + } +} + #[derive(Debug, Clone)] pub(crate) struct ForwardedModule { inner: Arc>, @@ -149,10 +227,11 @@ pub(crate) enum Module { scope: ModuleScope, }, Forwarded(ForwardedModule), + Shadowed(ShadowedModule), } #[derive(Debug, Clone)] -pub(crate) struct Modules(BTreeMap>>); +pub(crate) struct Modules(pub BTreeMap>>); impl Modules { pub fn new() -> Self { @@ -279,16 +358,20 @@ impl Module { } } - fn scope(&self) -> ModuleScope { + pub(crate) fn scope(&self) -> ModuleScope { match self { - Self::Builtin { scope } | Self::Environment { scope, .. } => scope.clone(), + Self::Builtin { scope } + | Self::Environment { 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, .. } => *scope = new_scope, + Self::Builtin { scope } + | Self::Environment { scope, .. } + | Self::Shadowed(ShadowedModule { scope, .. }) => *scope = new_scope, Self::Forwarded(forwarded) => (*forwarded.inner).borrow_mut().set_scope(new_scope), } } @@ -319,7 +402,9 @@ impl Module { Self::Builtin { .. } => { return Err(("Cannot modify built-in variable.", name.span).into()) } - Self::Environment { scope, .. } => scope.clone(), + Self::Environment { scope, .. } | Self::Shadowed(ShadowedModule { scope, .. }) => { + scope.clone() + } Self::Forwarded(forwarded) => (*forwarded.inner).borrow_mut().scope(), }; diff --git a/crates/compiler/src/evaluate/env.rs b/crates/compiler/src/evaluate/env.rs index 2e612c8..8b59abb 100644 --- a/crates/compiler/src/evaluate/env.rs +++ b/crates/compiler/src/evaluate/env.rs @@ -1,14 +1,20 @@ use codemap::{Span, Spanned}; use crate::{ - ast::{AstForwardRule, Configuration, Mixin}, - builtin::modules::{ForwardedModule, Module, ModuleScope, Modules}, + ast::{AstForwardRule, Configuration, ConfiguredValue, Mixin}, + builtin::modules::{ForwardedModule, Module, ModuleScope, Modules, ShadowedModule}, common::Identifier, error::SassResult, selector::ExtensionStore, value::{SassFunction, Value}, }; -use std::{cell::RefCell, collections::BTreeMap, sync::Arc}; +use std::{ + cell::RefCell, + collections::{BTreeMap, HashSet}, + sync::Arc, +}; + +type Mutable = Arc>; use super::{scope::Scopes, visitor::CallableContentBlock}; @@ -19,6 +25,9 @@ pub(crate) struct Environment { pub global_modules: Vec>>, pub content: Option>, pub forwarded_modules: Arc>>>>, + pub imported_modules: Arc>>>>, + #[allow(clippy::type_complexity)] + pub nested_forwarded_modules: Option>>>>>, } impl Environment { @@ -29,6 +38,8 @@ impl Environment { global_modules: Vec::new(), content: None, forwarded_modules: Arc::new(RefCell::new(Vec::new())), + imported_modules: Arc::new(RefCell::new(Vec::new())), + nested_forwarded_modules: None, } } @@ -39,6 +50,8 @@ impl Environment { global_modules: self.global_modules.iter().map(Arc::clone).collect(), content: self.content.as_ref().map(Arc::clone), forwarded_modules: Arc::clone(&self.forwarded_modules), + imported_modules: Arc::clone(&self.imported_modules), + nested_forwarded_modules: self.nested_forwarded_modules.as_ref().map(Arc::clone), } } @@ -49,6 +62,8 @@ impl Environment { global_modules: Vec::new(), content: self.content.as_ref().map(Arc::clone), forwarded_modules: Arc::clone(&self.forwarded_modules), + imported_modules: Arc::clone(&self.imported_modules), + nested_forwarded_modules: self.nested_forwarded_modules.as_ref().map(Arc::clone), } } @@ -61,101 +76,167 @@ impl Environment { } } + /// Makes the members forwarded by [module] available in the current + /// environment. + /// + /// This is called when [module] is `@import`ed. pub fn import_forwards(&mut self, _env: Module) { - // if (module is _EnvironmentModule) { - // var forwarded = module._environment._forwardedModules; - // if (forwarded == null) return; + if let Module::Environment { env, .. } = _env { + let mut forwarded = env.forwarded_modules; - // // Omit modules from [forwarded] that are already globally available and - // // forwarded in this module. - // var forwardedModules = _forwardedModules; - // if (forwardedModules != null) { - // forwarded = { - // for (var entry in forwarded.entries) - // if (!forwardedModules.containsKey(entry.key) || - // !_globalModules.containsKey(entry.key)) - // entry.key: entry.value, - // }; - // } else { - // forwardedModules = _forwardedModules ??= {}; - // } + if (*forwarded).borrow().is_empty() { + return; + } - // var forwardedVariableNames = - // forwarded.keys.expand((module) => module.variables.keys).toSet(); - // var forwardedFunctionNames = - // forwarded.keys.expand((module) => module.functions.keys).toSet(); - // var forwardedMixinNames = - // forwarded.keys.expand((module) => module.mixins.keys).toSet(); + // Omit modules from [forwarded] that are already globally available and + // forwarded in this module. + let forwarded_modules = Arc::clone(&self.forwarded_modules); + if !(*forwarded_modules).borrow().is_empty() { + // todo: intermediate name + let mut x = Vec::new(); + for entry in (*forwarded).borrow().iter() { + if !forwarded_modules + .borrow() + .iter() + .any(|module| Arc::ptr_eq(module, entry)) + || !self + .global_modules + .iter() + .any(|module| Arc::ptr_eq(module, entry)) + { + x.push(Arc::clone(entry)); + } + } - // if (atRoot) { - // // Hide members from modules that have already been imported or - // // forwarded that would otherwise conflict with the @imported members. - // for (var entry in _importedModules.entries.toList()) { - // var module = entry.key; - // var shadowed = ShadowedModuleView.ifNecessary(module, - // variables: forwardedVariableNames, - // mixins: forwardedMixinNames, - // functions: forwardedFunctionNames); - // if (shadowed != null) { - // _importedModules.remove(module); - // if (!shadowed.isEmpty) _importedModules[shadowed] = entry.value; - // } - // } + forwarded = Arc::new(RefCell::new(x)); + } - // for (var entry in forwardedModules.entries.toList()) { - // var module = entry.key; - // var shadowed = ShadowedModuleView.ifNecessary(module, - // variables: forwardedVariableNames, - // mixins: forwardedMixinNames, - // functions: forwardedFunctionNames); - // if (shadowed != null) { - // forwardedModules.remove(module); - // if (!shadowed.isEmpty) forwardedModules[shadowed] = entry.value; - // } - // } + let forwarded_var_names = forwarded + .borrow() + .iter() + .flat_map(|module| (*module).borrow().scope().variables.keys()) + .collect::>(); + let forwarded_fn_names = forwarded + .borrow() + .iter() + .flat_map(|module| (*module).borrow().scope().functions.keys()) + .collect::>(); + let forwarded_mixin_names = forwarded + .borrow() + .iter() + .flat_map(|module| (*module).borrow().scope().mixins.keys()) + .collect::>(); - // _importedModules.addAll(forwarded); - // forwardedModules.addAll(forwarded); - // } else { - // (_nestedForwardedModules ??= - // List.generate(_variables.length - 1, (_) => [])) - // .last - // .addAll(forwarded.keys); - // } + if self.at_root() { + let mut to_remove = Vec::new(); - // // Remove existing member definitions that are now shadowed by the - // // forwarded modules. - // for (var variable in forwardedVariableNames) { - // _variableIndices.remove(variable); - // _variables.last.remove(variable); - // _variableNodes.last.remove(variable); - // } - // for (var function in forwardedFunctionNames) { - // _functionIndices.remove(function); - // _functions.last.remove(function); - // } - // for (var mixin in forwardedMixinNames) { - // _mixinIndices.remove(mixin); - // _mixins.last.remove(mixin); - // } - // } - // todo!() + // Hide members from modules that have already been imported or + // forwarded that would otherwise conflict with the @imported members. + for (idx, module) in (*self.imported_modules).borrow().iter().enumerate() { + let shadowed = ShadowedModule::if_necessary( + Arc::clone(module), + Some(&forwarded_var_names), + Some(&forwarded_fn_names), + Some(&forwarded_mixin_names), + ); + + if shadowed.is_some() { + to_remove.push(idx); + } + } + + let mut imported_modules = (*self.imported_modules).borrow_mut(); + + for &idx in to_remove.iter().rev() { + imported_modules.remove(idx); + } + + to_remove.clear(); + + for (idx, module) in (*self.forwarded_modules).borrow().iter().enumerate() { + let shadowed = ShadowedModule::if_necessary( + Arc::clone(module), + Some(&forwarded_var_names), + Some(&forwarded_fn_names), + Some(&forwarded_mixin_names), + ); + + if shadowed.is_some() { + to_remove.push(idx); + } + } + + let mut forwarded_modules = (*self.forwarded_modules).borrow_mut(); + + for &idx in to_remove.iter().rev() { + forwarded_modules.remove(idx); + } + + imported_modules.extend(forwarded.borrow().iter().map(Arc::clone)); + forwarded_modules.extend(forwarded.borrow().iter().map(Arc::clone)); + } else { + self.scopes.last_variable_index = None; + self.nested_forwarded_modules + .get_or_insert_with(|| { + Arc::new(RefCell::new( + (0..self.scopes.len()) + .map(|_| Arc::new(RefCell::new(Vec::new()))) + .collect(), + )) + }) + .borrow_mut() + .last_mut() + .unwrap() + .borrow_mut() + .extend(forwarded.borrow().iter().map(Arc::clone)); + } + + // Remove existing member definitions that are now shadowed by the + // forwarded modules. + for variable in forwarded_var_names { + (*self.scopes.variables) + .borrow_mut() + .last_mut() + .unwrap() + .borrow_mut() + .remove(&variable); + } + self.scopes.last_variable_index = None; + + for func in forwarded_fn_names { + (*self.scopes.functions) + .borrow_mut() + .last_mut() + .unwrap() + .borrow_mut() + .remove(&func); + } + for mixin in forwarded_mixin_names { + (*self.scopes.mixins) + .borrow_mut() + .last_mut() + .unwrap() + .borrow_mut() + .remove(&mixin); + } + } } pub fn to_implicit_configuration(&self) -> Configuration { - // var configuration = {}; - // for (var i = 0; i < _variables.length; i++) { - // var values = _variables[i]; - // var nodes = _variableNodes[i]; - // for (var entry in values.entries) { - // // Implicit configurations are never invalid, making [configurationSpan] - // // unnecessary, so we pass null here to avoid having to compute it. - // configuration[entry.key] = - // ConfiguredValue.implicit(entry.value, nodes[entry.key]!); - // } - // } - // return Configuration.implicit(configuration); - todo!() + let mut configuration = BTreeMap::new(); + + let variables = (*self.scopes.variables).borrow(); + + for variables in variables.iter() { + let entries = (**variables).borrow(); + for (key, value) in entries.iter() { + // Implicit configurations are never invalid, making [configurationSpan] + // unnecessary, so we pass null here to avoid having to compute it. + configuration.insert(*key, ConfiguredValue::implicit(value.clone())); + } + } + + Configuration::implicit(configuration) } pub fn forward_module(&mut self, module: Arc>, rule: AstForwardRule) { @@ -274,24 +355,22 @@ impl Environment { } if is_global || self.at_root() { - // // Don't set the index if there's already a variable with the given name, - // // since local accesses should still return the local variable. - // _variableIndices.putIfAbsent(name, () { - // _lastVariableName = name; - // _lastVariableIndex = 0; - // return 0; - // }); + // If this module doesn't already contain a variable named [name], try + // setting it in a global module. + if !self.scopes.global_var_exists(name.node) { + let module_with_name = self.from_one_module(name.node, "variable", |module| { + if module.borrow().var_exists(*name) { + Some(Arc::clone(module)) + } else { + None + } + }); - // // If this module doesn't already contain a variable named [name], try - // // setting it in a global module. - // if (!_variables.first.containsKey(name)) { - // var moduleWithName = _fromOneModule(name, "variable", - // (module) => module.variables.containsKey(name) ? module : null); - // if (moduleWithName != null) { - // moduleWithName.setVariable(name, value, nodeWithSpan); - // return; - // } - // } + if let Some(module_with_name) = module_with_name { + module_with_name.borrow_mut().update_var(name, value)?; + return Ok(()); + } + } self.scopes.insert_var(0, name.node, value); return Ok(()); @@ -334,33 +413,19 @@ impl Environment { } fn get_variable_from_global_modules(&self, name: Identifier) -> Option { - for module in &self.global_modules { - if (**module).borrow().var_exists(name) { - return (**module).borrow().get_var_no_err(name); - } - } - - None + self.from_one_module(name, "variable", |module| { + (**module).borrow().get_var_no_err(name) + }) } fn get_function_from_global_modules(&self, name: Identifier) -> Option { - for module in &self.global_modules { - if (**module).borrow().fn_exists(name) { - return (**module).borrow().get_fn(name); - } - } - - None + self.from_one_module(name, "function", |module| (**module).borrow().get_fn(name)) } fn get_mixin_from_global_modules(&self, name: Identifier) -> Option { - for module in &self.global_modules { - if (**module).borrow().mixin_exists(name) { - return (**module).borrow().get_mixin_no_err(name); - } - } - - None + self.from_one_module(name, "mixin", |module| { + (**module).borrow().get_mixin_no_err(name) + }) } pub fn add_module( @@ -396,4 +461,61 @@ impl Environment { Arc::new(RefCell::new(Module::new_env(self, extension_store))) } + + fn from_one_module( + &self, + _name: Identifier, + _ty: &str, + callback: impl Fn(&Arc>) -> Option, + ) -> Option { + if let Some(nested_forwarded_modules) = &self.nested_forwarded_modules { + for modules in nested_forwarded_modules.borrow().iter().rev() { + for module in modules.borrow().iter().rev() { + if let Some(value) = callback(module) { + return Some(value); + } + } + } + } + + for module in self.imported_modules.borrow().iter() { + if let Some(value) = callback(module) { + return Some(value); + } + } + + let mut value: Option = None; + // Object? identity; + + for module in self.global_modules.iter() { + let value_in_module = match callback(module) { + Some(v) => v, + None => continue, + }; + + value = Some(value_in_module); + + // Object? identityFromModule = valueInModule is AsyncCallable + // ? valueInModule + // : module.variableIdentity(name); + // if (identityFromModule == identity) continue; + + // if (value != null) { + // var spans = _globalModules.entries.map( + // (entry) => callback(entry.key).andThen((_) => entry.value.span)); + + // throw MultiSpanSassScriptException( + // 'This $type is available from multiple global modules.', + // '$type use', { + // for (var span in spans) + // if (span != null) span: 'includes $type' + // }); + // } + + // value = valueInModule; + // identity = identityFromModule; + } + + value + } } diff --git a/crates/compiler/src/evaluate/scope.rs b/crates/compiler/src/evaluate/scope.rs index 579b1c3..4e31471 100644 --- a/crates/compiler/src/evaluate/scope.rs +++ b/crates/compiler/src/evaluate/scope.rs @@ -17,9 +17,9 @@ use crate::{ #[allow(clippy::type_complexity)] #[derive(Debug, Default, Clone)] pub(crate) struct Scopes { - variables: Arc>>>>>, - mixins: Arc>>>>>, - functions: Arc>>>>>, + pub(crate) variables: Arc>>>>>, + pub(crate) mixins: Arc>>>>>, + pub(crate) functions: Arc>>>>>, len: Arc>, pub last_variable_index: Option<(Identifier, usize)>, } @@ -167,6 +167,10 @@ impl Scopes { false } + + pub fn global_var_exists(&self, name: Identifier) -> bool { + self.global_variables().borrow().contains_key(&name) + } } /// Mixins diff --git a/crates/compiler/src/evaluate/visitor.rs b/crates/compiler/src/evaluate/visitor.rs index 2cfa22d..b163357 100644 --- a/crates/compiler/src/evaluate/visitor.rs +++ b/crates/compiler/src/evaluate/visitor.rs @@ -113,6 +113,8 @@ pub struct Visitor<'a> { pub(crate) extender: ExtensionStore, pub(crate) current_import_path: PathBuf, pub(crate) is_plain_css: bool, + pub(crate) modules: BTreeMap>>, + pub(crate) active_modules: BTreeSet, css_tree: CssTree, parent: Option, configuration: Arc>, @@ -157,6 +159,8 @@ impl<'a> Visitor<'a> { configuration: Arc::new(RefCell::new(Configuration::empty())), is_plain_css: false, import_nodes: Vec::new(), + modules: BTreeMap::new(), + active_modules: BTreeSet::new(), options, span_before, map, @@ -166,6 +170,7 @@ impl<'a> Visitor<'a> { } pub(crate) fn visit_stylesheet(&mut self, mut style_sheet: StyleSheet) -> SassResult<()> { + self.active_modules.insert(style_sheet.url.clone()); let was_in_plain_css = self.is_plain_css; self.is_plain_css = style_sheet.is_plain_css; mem::swap(&mut self.current_import_path, &mut style_sheet.url); @@ -178,6 +183,8 @@ impl<'a> Visitor<'a> { mem::swap(&mut self.current_import_path, &mut style_sheet.url); self.is_plain_css = was_in_plain_css; + self.active_modules.remove(&style_sheet.url); + Ok(()) } @@ -366,6 +373,8 @@ impl<'a> Visitor<'a> { ))) } + /// Remove configured values from [upstream] that have been removed from + /// [downstream], unless they match a name in [except]. fn remove_used_configuration( upstream: &Arc>, downstream: &Arc>, @@ -534,6 +543,40 @@ impl<'a> Visitor<'a> { // todo: different errors based on this _names_in_errors: bool, ) -> SassResult>> { + let url = stylesheet.url.clone(); + + // todo: use canonical url for modules + if let Some(already_loaded) = self.modules.get(&stylesheet.url) { + let current_configuration = + configuration.unwrap_or_else(|| Arc::clone(&self.configuration)); + + if !current_configuration.borrow().is_implicit() { + // if (!_moduleConfigurations[url]!.sameOriginal(currentConfiguration) && + // currentConfiguration is ExplicitConfiguration) { + // var message = namesInErrors + // ? "${p.prettyUri(url)} was already loaded, so it can't be " + // "configured using \"with\"." + // : "This module was already loaded, so it can't be configured using " + // "\"with\"."; + + // var existingSpan = _moduleNodes[url]?.span; + // var configurationSpan = configuration == null + // ? currentConfiguration.nodeWithSpan.span + // : null; + // var secondarySpans = { + // if (existingSpan != null) existingSpan: "original load", + // if (configurationSpan != null) configurationSpan: "configuration" + // }; + + // throw secondarySpans.isEmpty + // ? _exception(message) + // : _multiSpanException(message, "new load", secondarySpans); + // } + } + + return Ok(Arc::clone(already_loaded)); + } + let env = Environment::new(); let mut extension_store = ExtensionStore::new(self.span_before); @@ -589,6 +632,8 @@ impl<'a> Visitor<'a> { let module = env.to_module(extension_store); + self.modules.insert(url, Arc::clone(&module)); + Ok(module) } @@ -635,7 +680,7 @@ impl<'a> Visitor<'a> { callback( self, Arc::new(RefCell::new(builtin)), - StyleSheet::new(false, PathBuf::from("")), + StyleSheet::new(false, url.to_path_buf()), )?; return Ok(()); } @@ -643,8 +688,22 @@ impl<'a> Visitor<'a> { // todo: decide on naming convention for style_sheet vs stylesheet let stylesheet = self.load_style_sheet(url.to_string_lossy().as_ref(), false, span)?; + let canonical_url = self + .options + .fs + .canonicalize(&stylesheet.url) + .unwrap_or_else(|_| stylesheet.url.clone()); + + if self.active_modules.contains(&canonical_url) { + return Err(("Module loop: this module is already being loaded.", span).into()); + } + + self.active_modules.insert(canonical_url.clone()); + let module = self.execute(stylesheet.clone(), configuration, names_in_errors)?; + self.active_modules.remove(&canonical_url); + callback(self, module, stylesheet)?; Ok(()) @@ -832,6 +891,7 @@ impl<'a> Visitor<'a> { span: Span, ) -> SassResult { if let Some(name) = self.find_import(url.as_ref()) { + let name = self.options.fs.canonicalize(&name).unwrap_or(name); if let Some(style_sheet) = self.import_cache.get(&name) { return Ok(style_sheet.clone()); } @@ -876,6 +936,14 @@ impl<'a> Visitor<'a> { fn visit_dynamic_import_rule(&mut self, dynamic_import: &AstSassImport) -> SassResult<()> { let stylesheet = self.load_style_sheet(&dynamic_import.url, true, dynamic_import.span)?; + let url = stylesheet.url.clone(); + + if self.active_modules.contains(&url) { + return Err(("This file is already being loaded.", dynamic_import.span).into()); + } + + self.active_modules.insert(url.clone()); + // If the imported stylesheet doesn't use any modules, we can inject its // CSS directly into the current stylesheet. If it does use modules, we // need to put its CSS into an intermediate [ModifiableCssStylesheet] so @@ -924,6 +992,7 @@ impl<'a> Visitor<'a> { self.env.import_forwards(module); if loads_user_defined_modules { + // todo: // if (module.transitivelyContainsCss) { // // If any transitively used module contains extensions, we need to // // clone all modules' CSS. Otherwise, it's possible that they'll be @@ -940,6 +1009,8 @@ impl<'a> Visitor<'a> { // } } + self.active_modules.remove(&url); + Ok(()) } diff --git a/crates/compiler/src/fs.rs b/crates/compiler/src/fs.rs index 9dced3c..db58b51 100644 --- a/crates/compiler/src/fs.rs +++ b/crates/compiler/src/fs.rs @@ -1,6 +1,6 @@ use std::{ io::{self, Error, ErrorKind}, - path::Path, + path::{Path, PathBuf}, }; /// A trait to allow replacing the file system lookup mechanisms. @@ -18,6 +18,11 @@ pub trait Fs: std::fmt::Debug { fn is_file(&self, path: &Path) -> bool; /// Read the entire contents of a file into a bytes vector. fn read(&self, path: &Path) -> io::Result>; + + /// Canonicalize a file path + fn canonicalize(&self, path: &Path) -> io::Result { + Ok(path.to_path_buf()) + } } /// Use [`std::fs`] to read any files from disk. @@ -41,6 +46,11 @@ impl Fs for StdFs { fn read(&self, path: &Path) -> io::Result> { std::fs::read(path) } + + #[inline] + fn canonicalize(&self, path: &Path) -> io::Result { + std::fs::canonicalize(path) + } } /// A file system implementation that acts like it’s completely empty. diff --git a/crates/compiler/src/parse/css.rs b/crates/compiler/src/parse/css.rs index dc5fae5..81e4256 100644 --- a/crates/compiler/src/parse/css.rs +++ b/crates/compiler/src/parse/css.rs @@ -38,15 +38,15 @@ impl<'a> BaseParser<'a> for CssParser<'a> { } impl<'a> StylesheetParser<'a> for CssParser<'a> { - fn is_plain_css(&mut self) -> bool { + fn is_plain_css(&self) -> bool { true } - fn is_indented(&mut self) -> bool { + fn is_indented(&self) -> bool { false } - fn path(&mut self) -> &'a Path { + fn path(&self) -> &'a Path { self.path } @@ -58,7 +58,7 @@ impl<'a> StylesheetParser<'a> for CssParser<'a> { self.options } - fn flags(&mut self) -> &ContextFlags { + fn flags(&self) -> &ContextFlags { &self.flags } diff --git a/crates/compiler/src/parse/sass.rs b/crates/compiler/src/parse/sass.rs index 2b1f1d3..1d1b997 100644 --- a/crates/compiler/src/parse/sass.rs +++ b/crates/compiler/src/parse/sass.rs @@ -71,15 +71,15 @@ impl<'a> BaseParser<'a> for SassParser<'a> { } impl<'a> StylesheetParser<'a> for SassParser<'a> { - fn is_plain_css(&mut self) -> bool { + fn is_plain_css(&self) -> bool { false } - fn is_indented(&mut self) -> bool { + fn is_indented(&self) -> bool { true } - fn path(&mut self) -> &'a Path { + fn path(&self) -> &'a Path { self.path } @@ -91,7 +91,7 @@ impl<'a> StylesheetParser<'a> for SassParser<'a> { self.options } - fn flags(&mut self) -> &ContextFlags { + fn flags(&self) -> &ContextFlags { &self.flags } diff --git a/crates/compiler/src/parse/scss.rs b/crates/compiler/src/parse/scss.rs index 6be08cb..77c8d5a 100644 --- a/crates/compiler/src/parse/scss.rs +++ b/crates/compiler/src/parse/scss.rs @@ -50,15 +50,15 @@ impl<'a> BaseParser<'a> for ScssParser<'a> { } impl<'a> StylesheetParser<'a> for ScssParser<'a> { - fn is_plain_css(&mut self) -> bool { + fn is_plain_css(&self) -> bool { false } - fn is_indented(&mut self) -> bool { + fn is_indented(&self) -> bool { false } - fn path(&mut self) -> &'a Path { + fn path(&self) -> &'a Path { self.path } @@ -74,7 +74,7 @@ impl<'a> StylesheetParser<'a> for ScssParser<'a> { 0 } - fn flags(&mut self) -> &ContextFlags { + fn flags(&self) -> &ContextFlags { &self.flags } diff --git a/crates/compiler/src/parse/stylesheet.rs b/crates/compiler/src/parse/stylesheet.rs index 6ac1562..0a137c6 100644 --- a/crates/compiler/src/parse/stylesheet.rs +++ b/crates/compiler/src/parse/stylesheet.rs @@ -28,15 +28,15 @@ use super::{ /// SCSS share the behavior pub(crate) trait StylesheetParser<'a>: BaseParser<'a> + Sized { // todo: make constant? - fn is_plain_css(&mut self) -> bool; + fn is_plain_css(&self) -> bool; // todo: make constant? - fn is_indented(&mut self) -> bool; + fn is_indented(&self) -> bool; fn options(&self) -> &Options; - fn path(&mut self) -> &Path; + fn path(&self) -> &Path; fn map(&mut self) -> &mut CodeMap; fn span_before(&self) -> Span; fn current_indentation(&self) -> usize; - fn flags(&mut self) -> &ContextFlags; + fn flags(&self) -> &ContextFlags; fn flags_mut(&mut self) -> &mut ContextFlags; #[allow(clippy::type_complexity)] @@ -185,7 +185,13 @@ pub(crate) trait StylesheetParser<'a>: BaseParser<'a> + Sized { } fn __parse(&mut self) -> SassResult { - let mut style_sheet = StyleSheet::new(self.is_plain_css(), self.path().to_path_buf()); + let mut style_sheet = StyleSheet::new( + self.is_plain_css(), + self.options() + .fs + .canonicalize(self.path()) + .unwrap_or_else(|_| self.path().to_path_buf()), + ); // Allow a byte-order mark at the beginning of the document. self.scan_char('\u{feff}'); diff --git a/crates/compiler/src/utils/map_view.rs b/crates/compiler/src/utils/map_view.rs index b75671f..b8b794d 100644 --- a/crates/compiler/src/utils/map_view.rs +++ b/crates/compiler/src/utils/map_view.rs @@ -180,6 +180,17 @@ impl + Clone> MapView for PrefixedM } } +/// A mostly-unmodifiable view of a map that only allows certain keys to be +/// accessed. +/// +/// Whether or not the underlying map contains keys that aren't allowed, this +/// view will behave as though it doesn't contain them. +/// +/// The underlying map's values may change independently of this view, but its +/// set of keys may not. +/// +/// This is unmodifiable *except for the [remove] method*, which is used for +/// `@used with` to mark configured variables as used. #[derive(Debug, Clone)] pub(crate) struct LimitedMapView + Clone>( pub T, @@ -197,11 +208,11 @@ impl + Clone> LimitedMapView Self(map, keys) } - pub fn blocklist(map: T, keys: &HashSet) -> Self { - let keys = keys - .iter() - .copied() - .filter(|key| !map.contains_key(*key)) + pub fn blocklist(map: T, blocklist: &HashSet) -> Self { + let keys = map + .keys() + .into_iter() + .filter(|key| !blocklist.contains(key)) .collect(); Self(map, keys) diff --git a/crates/lib/tests/forward.rs b/crates/lib/tests/forward.rs index 86ac00c..b7c5702 100644 --- a/crates/lib/tests/forward.rs +++ b/crates/lib/tests/forward.rs @@ -334,6 +334,173 @@ fn forward_module_with_error() { ); } +#[test] +fn use_with_multi_load_forward() { + let mut fs = TestFs::new(); + + fs.add_file( + "_midstream.scss", + r#" + @forward "upstream"; + "#, + ); + fs.add_file( + "_upstream.scss", + r#" + $a: original !default; + "#, + ); + + let input = r#" + @use "upstream" with ($a: configured); + @use "midstream"; + b {c: midstream.$a} + "#; + + assert_eq!( + "b {\n c: configured;\n}\n", + &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input) + ); +} + +#[test] +fn forward_member_import_precedence_nested() { + let mut fs = TestFs::new(); + + fs.add_file( + "_midstream.scss", + r#" + @forward "upstream"; + "#, + ); + fs.add_file( + "_upstream.scss", + r#" + $a: in-upstream; + "#, + ); + + let input = r#" + b { + $a: in-input; + + @import "midstream"; + + c: $a; + } + "#; + + assert_eq!( + "b {\n c: in-upstream;\n}\n", + &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input) + ); +} + +#[test] +fn forward_with_through_forward_hide() { + let mut fs = TestFs::new(); + + fs.add_file( + "_downstream.scss", + r#" + @forward "midstream" with ($a: configured); + "#, + ); + fs.add_file( + "_midstream.scss", + r#" + @forward "upstream" hide $b; + "#, + ); + fs.add_file( + "_upstream.scss", + r#" + $a: original !default; + b {c: $a} + "#, + ); + + let input = r#" + @use "downstream"; + "#; + + assert_eq!( + "b {\n c: configured;\n}\n", + &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input) + ); +} + +#[test] +fn forward_with_through_forward_show() { + let mut fs = TestFs::new(); + + fs.add_file( + "_downstream.scss", + r#" + @forward "midstream" with ($a: configured); + "#, + ); + fs.add_file( + "_midstream.scss", + r#" + @forward "upstream" show $a; + "#, + ); + fs.add_file( + "_upstream.scss", + r#" + $a: original !default; + b {c: $a} + "#, + ); + + let input = r#" + @use "downstream"; + "#; + + assert_eq!( + "b {\n c: configured;\n}\n", + &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input) + ); +} + +#[test] +#[ignore = "incorrectly thinks there's a module loop"] +fn import_forwarded_first_no_use() { + let mut fs = TestFs::new(); + + fs.add_file( + "first.scss", + r#" + $variable: value; + "#, + ); + fs.add_file( + "first.import.scss", + r#" + @forward "first"; + "#, + ); + fs.add_file( + "second.scss", + r#" + a { + b: $variable; + } + "#, + ); + + let input = r#" + @import "first"; + @import "second"; + "#; + + assert_eq!( + "a {\n b: value;\n}\n", + &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input) + ); +} + error!( after_style_rule, r#" diff --git a/crates/lib/tests/imports.rs b/crates/lib/tests/imports.rs index 9a71dd4..97916c3 100644 --- a/crates/lib/tests/imports.rs +++ b/crates/lib/tests/imports.rs @@ -349,6 +349,22 @@ fn imports_same_file_thrice() { &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input) ); } +#[test] +fn imports_self() { + let mut fs = TestFs::new(); + + fs.add_file("input.scss", r#"@import "input";"#); + + let input = r#" + @import "input"; + "#; + + assert_err!( + input, + "Error: This file is already being loaded.", + &grass::Options::default().fs(&fs) + ); +} #[test] fn imports_explicit_file_extension() { diff --git a/crates/lib/tests/use.rs b/crates/lib/tests/use.rs index 8d5c6b9..27d1374 100644 --- a/crates/lib/tests/use.rs +++ b/crates/lib/tests/use.rs @@ -845,6 +845,37 @@ fn import_module_using_same_builtin_module_has_styles() { ); } +#[test] +fn use_member_global_variable_assignment_toplevel() { + let mut fs = TestFs::new(); + + fs.add_file( + "other.scss", + r#" + $member: value; + + @function get-member() { + @return $member + } + "#, + ); + + let input = r#" + @use "other" as *; + + $member: new value; + + a { + b: get-member() + } + "#; + + assert_eq!( + "a {\n b: new value;\n}\n", + &grass::from_string(input.to_string(), &grass::Options::default().fs(&fs)).expect(input) + ); +} + #[test] #[ignore = "we don't hermetically evaluate @extend"] fn use_module_with_extend() {