Refactor with common Closure.

CHANGELOG.md

@@ -15,15 +15,17 @@ project adheres to
   `Invalid::Something.at(pos)` (PR #145).
 * `@extend` is still unsupported, but now some uses of it (e.g. in control
   structures) will result in an error instead of wrong output.
-* Some `sass::Item` alternatives now contain a `Callable`, combining a
-  `FormalArgs` with a body (a `Vec<Item>`).  And `sass::Item::Call` now has
-  a `CallArgs` (PR #146).
+* Some `sass::Item` alternatives now contain a `Callable`, combining
+  `FormalArgs` with a body (a `Vec<Item>`).  And `sass::Item::Content` now
+  has a `CallArgs`.  Also, `MixinDeclImpl` is replaced with
+  `sass::Closure`. (PR #146).
 * Remove deprecated methods `css::Value::integer_value()` and
   `Number::is_integer()`.
 
 ### Improvements
 
-* The `@content` can have arguments when declaring and calling a mixin.
+* The `@content` can have arguments when declaring and calling a mixin
+  (PR #146).
 * Allow interpolation in css min and max function arguments.
 * The url for `@use` and `@forward` must be quoted.
 * Some `@` rules are now forbidden in some places as they should (PR #145).

src/output/transform.rs

@@ -10,9 +10,7 @@ use crate::css::{BodyItem, Comment, Import, Property, Rule, Selectors};
 use crate::error::{Error, Invalid};
 use crate::file_context::FileContext;
 use crate::parser::Parsed;
-use crate::sass::{
-    get_global_module, Expose, Function, Item, MixinDecl, Name, UseAs,
-};
+use crate::sass::{get_global_module, Expose, Item, Name, UseAs};
 use crate::value::ValueRange;
 use crate::ScopeRef;
 use std::io::Write;
@@ -322,18 +320,15 @@ fn handle_item(
                 return Err(Invalid::FunctionName.at(p));
             }
             check_body(&body.body, BodyContext::Function)?;
-            scope.define_function(
-                name.into(),
-                Function::closure(body.clone(), scope.clone()),
-            );
+            scope.define_function(name.into(), body.closure(&scope).into());
         }
         Item::Return(_, ref pos) => {
             return Err(Invalid::AtRule.at(pos.clone()));
         }
 
         Item::MixinDeclaration(ref name, ref body) => {
             check_body(&body.body, BodyContext::Mixin)?;
-            scope.define_mixin(name.into(), MixinDecl::new(body, &scope))
+            scope.define_mixin(name.into(), body.closure(&scope).into())
         }
         Item::MixinCall(ref name, ref args, ref body, ref pos) => {
             if let Some(mixin) = scope.get_mixin(&name.into()) {
@@ -344,7 +339,7 @@ fn handle_item(
                     pos,
                     file_context,
                 )?;
-                mixin.define_content(&scope, body, pos);
+                mixin.define_content(&scope, body);
                 handle_parsed(
                     mixin.body,
                     head,
@@ -372,23 +367,21 @@ fn handle_item(
             if let Some(content) =
                 scope.get_mixin(&Name::from_static("%%BODY%%"))
             {
-                if !content.is_no_body() {
-                    let mixin = content.get(
-                        "@content",
-                        scope,
-                        args,
-                        pos,
-                        file_context,
-                    )?;
-                    handle_parsed(
-                        mixin.body,
-                        head,
-                        rule,
-                        buf,
-                        mixin.scope,
-                        file_context,
-                    )?;
-                }
+                let mixin = content.get(
+                    "@content",
+                    scope,
+                    args,
+                    pos,
+                    file_context,
+                )?;
+                handle_parsed(
+                    mixin.body,
+                    head,
+                    rule,
+                    buf,
+                    mixin.scope,
+                    file_context,
+                )?;
             }
         }
 

src/sass/callable.rs

@@ -1,5 +1,5 @@
 use super::{FormalArgs, Item};
-use crate::SourcePos;
+use crate::{ScopeRef, SourcePos};
 
 /// The callable part of a sass mixin or function.
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd)]
@@ -22,4 +22,18 @@ impl Callable {
             decl,
         }
     }
+    /// Combine this callable with a scope to get a closure.
+    pub fn closure(&self, scope: &ScopeRef) -> Closure {
+        Closure {
+            body: self.clone(),
+            scope: scope.clone(),
+        }
+    }
+}
+
+/// A callable combined with the runtime scope where it was declared.
+#[derive(Clone)]
+pub struct Closure {
+    pub(crate) body: Callable,
+    pub(crate) scope: ScopeRef,
 }

src/sass/functions/mod.rs

@@ -1,4 +1,4 @@
-use super::{Callable, FormalArgs, Name};
+use super::{Closure, FormalArgs, Name};
 use crate::css::{self, is_not, CallArgs, CssString, Value};
 use crate::error::Error;
 use crate::output::{Format, Formatted};
@@ -134,18 +134,6 @@ impl Function {
         }
     }
 
-    /// Create a new `Function` from a scss implementation.
-    ///
-    /// The scope is where the function is defined, used to bind any
-    /// non-parameter names in the body.
-    pub fn closure(body: Callable, scope: ScopeRef) -> Self {
-        Function {
-            args: body.args,
-            pos: body.decl,
-            body: FuncImpl::UserDefined(scope, body.body),
-        }
-    }
-
     /// Call the function from a given scope and with a given set of
     /// arguments.
     pub fn call(
@@ -183,6 +171,16 @@ impl Function {
     }
 }
 
+impl From<Closure> for Function {
+    fn from(c: Closure) -> Self {
+        Function {
+            args: c.body.args,
+            pos: c.body.decl,
+            body: FuncImpl::UserDefined(c.scope, c.body.body),
+        }
+    }
+}
+
 lazy_static! {
     static ref MODULES: BTreeMap<&'static str, Scope> = {
         let mut modules = BTreeMap::new();

src/sass/mixin.rs

@@ -1,5 +1,5 @@
 use super::functions::get_string;
-use super::{CallArgs, Callable, FormalArgs, Name, Value};
+use super::{CallArgs, Callable, Closure, FormalArgs, Name, Value};
 use crate::css::{CssString, ValueToMapError};
 use crate::file_context::FileContext;
 use crate::ordermap::OrderMap;
@@ -11,36 +11,20 @@ use std::convert::TryInto;
 #[derive(Clone)]
 pub enum MixinDecl {
     /// an actual mixin
-    Sass(MixinDeclImpl),
+    Sass(Closure),
     /// The body of a mixin call that does not have a body
     NoBody,
     /// The special `load-css` mixin.
     LoadCss,
 }
 
-#[derive(Clone)]
-pub struct MixinDeclImpl {
-    body: Callable,
-    scope: ScopeRef,
-    pos: SourcePos,
-}
-
-impl From<MixinDeclImpl> for MixinDecl {
-    fn from(decl: MixinDeclImpl) -> Self {
+impl From<Closure> for MixinDecl {
+    fn from(decl: Closure) -> Self {
         MixinDecl::Sass(decl)
     }
 }
 
 impl MixinDecl {
-    /// Create a new Mixin.
-    pub fn new(body: &Callable, scope: &ScopeRef) -> Self {
-        MixinDeclImpl {
-            body: body.clone(),
-            scope: scope.clone(),
-            pos: body.decl.clone(),
-        }
-        .into()
-    }
     pub(crate) fn get(
         self,
         name: &str,
@@ -63,13 +47,13 @@ impl MixinDecl {
                         .map_err(|e| {
                             e.decl_called(
                                 call_pos.in_call(name),
-                                decl.pos.clone(),
+                                decl.body.decl.clone(),
                             )
                         })?,
                     body: Parsed::Scss(decl.body.body),
                 })
             }
-            MixinDecl::NoBody => unreachable!(),
+            MixinDecl::NoBody => Ok(Mixin::empty(scope)),
             MixinDecl::LoadCss => {
                 let fargs = FormalArgs::new(vec![
                     (name!(url), None),
@@ -97,10 +81,7 @@ impl MixinDecl {
                     .any(|name| *name == url.value())
                 {
                     if with.unwrap_or_default().is_empty() {
-                        return Ok(Mixin {
-                            scope,
-                            body: Parsed::Css(vec![]),
-                        });
+                        return Ok(Mixin::empty(scope));
                     } else {
                         return Err(Error::BadCall(
                             format!(
@@ -151,21 +132,21 @@ pub struct Mixin {
 }
 
 impl Mixin {
+    fn empty(scope: ScopeRef) -> Self {
+        Mixin {
+            scope,
+            body: Parsed::Css(vec![]),
+        }
+    }
     pub(crate) fn define_content(
         &self,
         scope: &ScopeRef,
         body: &Option<Callable>,
-        pos: &SourcePos,
     ) {
         self.scope.define_mixin(
             Name::from_static("%%BODY%%"),
             match body {
-                Some(body) => MixinDeclImpl {
-                    body: body.clone(),
-                    scope: scope.clone(),
-                    pos: pos.clone(),
-                }
-                .into(),
+                Some(body) => body.closure(scope).into(),
                 None => MixinDecl::NoBody,
             },
         )

src/sass/mod.rs

@@ -22,7 +22,7 @@ mod string;
 mod value;
 
 pub use self::call_args::CallArgs;
-pub use self::callable::Callable;
+pub use self::callable::{Callable, Closure};
 pub use self::formal_args::{ArgsError, FormalArgs};
 pub use self::functions::{get_global_module, Function};
 pub use self::item::{Expose, Item, UseAs};