Add proper handling of @keyframes.

CHANGELOG.md

@@ -32,6 +32,9 @@ project adheres to
 * Improved parse error handling (PR #201, Issue #141).
   Many parse errors now match the dart sass error message.
   Also allow "loud" comments in more places.
+* Added proper handling of `@keyframes`.  This is a breaking change for
+  adding new variants to public enums (PR #178).
+* Handle trailing comma in function arguments in plain css correctly.
 * Updated sass-spec test suite to 2024-10-10.
 
 

rsass/src/css/atrule.rs

@@ -1,6 +1,6 @@
 use super::{
-    BodyItem, Comment, CustomProperty, Import, Item, MediaRule, Property,
-    Rule, Value,
+    BodyItem, Comment, CustomProperty, Import, Item, Keyframes, MediaRule,
+    Property, Rule, Value,
 };
 use crate::output::CssBuf;
 use std::io::{self, Write};
@@ -71,6 +71,8 @@ pub enum AtRuleBodyItem {
     MediaRule(MediaRule),
     /// An `@` rule.
     AtRule(AtRule),
+    /// Keyframes
+    Keyframes(Keyframes),
 }
 
 impl AtRuleBodyItem {
@@ -83,6 +85,7 @@ impl AtRuleBodyItem {
             Self::CustomProperty(cp) => cp.write(buf),
             Self::MediaRule(rule) => rule.write(buf)?,
             Self::AtRule(rule) => rule.write(buf)?,
+            Self::Keyframes(rule) => rule.write(buf)?,
         }
         Ok(())
     }
@@ -140,6 +143,12 @@ impl TryFrom<Item> for AtRuleBodyItem {
             Item::MediaRule(x) => Ok(x.into()),
             Item::AtRule(x) => Ok(x.into()),
             Item::Separator => Err("separator not supported here"),
+            Item::Keyframes(x) => Ok(x.into()),
         }
     }
 }
+impl From<Keyframes> for AtRuleBodyItem {
+    fn from(value: Keyframes) -> Self {
+        AtRuleBodyItem::Keyframes(value)
+    }
+}

rsass/src/css/item.rs

@@ -1,4 +1,4 @@
-use super::{AtRule, Comment, CssString, MediaRule, Rule, Value};
+use super::{AtRule, Comment, CssString, Keyframes, MediaRule, Rule, Value};
 use crate::output::CssBuf;
 use std::io::{self, Write};
 
@@ -15,6 +15,8 @@ pub enum Item {
     MediaRule(MediaRule),
     /// An (unknown) `@` rule.
     AtRule(AtRule),
+    /// An `@keyframes` rule.
+    Keyframes(Keyframes),
     /// An extra newline for grouping (unless compressed format).
     Separator,
 }
@@ -27,6 +29,7 @@ impl Item {
             Self::Rule(rule) => rule.write(buf)?,
             Self::MediaRule(rule) => rule.write(buf)?,
             Self::AtRule(atrule) => atrule.write(buf)?,
+            Self::Keyframes(rule) => rule.write(buf)?,
             Self::Separator => buf.opt_nl(),
         }
         Ok(())
@@ -53,6 +56,11 @@ impl From<AtRule> for Item {
         Self::AtRule(value)
     }
 }
+impl From<Keyframes> for Item {
+    fn from(value: Keyframes) -> Self {
+        Item::Keyframes(value)
+    }
+}
 impl From<MediaRule> for Item {
     fn from(value: MediaRule) -> Self {
         Self::MediaRule(value)

rsass/src/css/keyframes.rs

@@ -0,0 +1,58 @@
+use super::{Comment, Property};
+use crate::output::CssBuf;
+use std::io::{self, Write};
+
+/// An `@keyframes` rule in css.
+#[derive(Clone, Debug)]
+pub struct Keyframes {
+    name: String,
+    items: Vec<KfItem>,
+}
+
+impl Keyframes {
+    pub(crate) fn new(name: String, items: Vec<KfItem>) -> Self {
+        Keyframes { name, items }
+    }
+    pub(crate) fn write(&self, buf: &mut CssBuf) -> io::Result<()> {
+        write!(buf, "@keyframes {}", self.name)?;
+        if let &[KfItem::Comment(ref single)] = &self.items[..] {
+            buf.add_one("{ ", "{");
+            single.write(buf);
+            buf.pop_nl();
+            buf.add_one(" }", "}");
+        } else {
+            buf.start_block();
+            for item in &self.items {
+                match item {
+                    KfItem::Stop(name, rules) => {
+                        buf.do_indent_no_nl();
+                        if let Some((first, rest)) = name.split_first() {
+                            buf.add_str(first);
+                            for name in rest {
+                                buf.add_one(", ", ",");
+                                buf.add_str(name);
+                            }
+                        }
+                        buf.start_block();
+                        for rule in rules {
+                            rule.write(buf);
+                        }
+                        buf.end_block();
+                    }
+                    KfItem::Comment(comment) => comment.write(buf),
+                }
+            }
+            buf.end_block();
+        }
+        Ok(())
+    }
+}
+
+/// An item in keyframes, either a stop or a comment.
+#[derive(Clone, Debug)]
+pub enum KfItem {
+    /// A stop
+    Stop(Vec<String>, Vec<Property>),
+    /// A comment
+    Comment(Comment),
+}

rsass/src/css/mod.rs

@@ -4,6 +4,7 @@ mod binop;
 mod call_args;
 mod comment;
 mod item;
+mod keyframes;
 mod mediarule;
 mod rule;
 mod selectors;
@@ -17,6 +18,7 @@ pub use self::binop::BinOp;
 pub use self::call_args::CallArgs;
 pub use self::comment::Comment;
 pub use self::item::{Import, Item};
+pub use self::keyframes::{Keyframes, KfItem};
 pub use self::mediarule::{MediaArgs, MediaRule};
 pub use self::rule::{BodyItem, CustomProperty, Property, Rule};
 pub use self::selectors::{BadSelector, Selector, SelectorSet};

rsass/src/output/cssdest.rs

@@ -270,6 +270,7 @@ impl<'a> CssDestination for AtRuleDest<'a> {
             // FIXME: This should bubble or something?
             Item::MediaRule(r) => r.into(),
             Item::AtRule(r) => r.into(),
+            Item::Keyframes(k) => k.into(),
             Item::Separator => return Ok(()), // Not pushed?
         });
         Ok(())
@@ -386,10 +387,11 @@ impl<'a> CssDestination for AtMediaDest<'a> {
             Item::Comment(c) => c.into(),
             Item::Import(i) => i.into(),
             Item::Rule(r) => r.into(),
+            Item::AtRule(r) => r.into(),
+            Item::Keyframes(k) => k.into(),
             // FIXME: Check if the args can be merged!
             // Or is that a separate pass after building a first css tree?
             Item::MediaRule(r) => r.into(),
-            Item::AtRule(r) => r.into(),
             Item::Separator => return Ok(()), // Not pushed?
         });
         Ok(())

rsass/src/output/transform.rs

@@ -4,9 +4,10 @@
 use super::cssdest::CssDestination;
 use super::CssData;
 use crate::css::{self, AtRule, Import, SelectorCtx};
+use crate::css::{Comment, Property};
 use crate::error::ResultPos;
 use crate::input::{Context, Loader, Parsed, SourceKind};
-use crate::sass::{get_global_module, Expose, Item, UseAs};
+use crate::sass::{get_global_module, Expose, Item, KfItem, UseAs};
 use crate::value::ValueRange;
 use crate::{Error, Invalid, ScopeRef};
 
@@ -201,6 +202,42 @@ fn handle_item(
                 dest.push_import(Import::new(name, args));
             }
         }
+        Item::KeyFrames(name, stops) => {
+            let name = name.evaluate(scope.clone())?.unquote();
+            let rules = stops
+                .iter()
+                .map(|item| match item {
+                    KfItem::Stop(name, rules) => Ok(Some(css::KfItem::Stop(
+                        name.iter()
+                            .map(|name| {
+                                Ok(name.evaluate(scope.clone())?.take_value())
+                            })
+                            .collect::<Result<_, Error>>()?,
+                        rules
+                            .iter()
+                            .map(|(name, val)| {
+                                let name = name.evaluate(scope.clone())?;
+                                let val = val.evaluate(scope.clone())?;
+                                Ok(Property::new(name.take_value(), val))
+                            })
+                            .collect::<Result<_, Error>>()?,
+                    ))),
+                    KfItem::VariableDeclaration(var) => {
+                        var.evaluate(&scope)?;
+                        Ok(None)
+                    }
+                    KfItem::Comment(comment) => {
+                        let comment = Comment::from(
+                            comment.evaluate(scope.clone())?.value(),
+                        );
+                        Ok(Some(css::KfItem::Comment(comment)))
+                    }
+                })
+                .filter_map(|r| r.transpose())
+                .collect::<Result<_, Error>>()?;
+            dest.push_item(css::Keyframes::new(name, rules).into())
+                .no_pos()?;
+        }
         Item::AtRoot(ref selectors, ref body) => {
             let selectors = selectors.eval(scope.clone())?;
             let ctx = scope.get_selectors().at_root(selectors);
@@ -231,7 +268,9 @@ fn handle_item(
             let args = args.evaluate(scope.clone())?;
             if let Some(ref body) = *body {
                 let mut atrule = dest.start_atrule(name.clone(), args);
-                let local = if name == "keyframes" {
+                let local = if name == "keyframes"
+                    || (name.starts_with("-") && name.ends_with("-keyframes"))
+                {
                     ScopeRef::sub_selectors(scope, SelectorCtx::root())
                 } else {
                     ScopeRef::sub(scope)
@@ -470,15 +509,14 @@ fn check_body(body: &[Item], context: BodyContext) -> Result<(), Error> {
     Ok(())
 }
 
-const CSS_AT_RULES: [&str; 16] = [
+const CSS_AT_RULES: [&str; 15] = [
     "charset",
     "color-profile",
     "counter-style",
     "document",
     "font-face",
     "font-feature-values",
     "import",
-    "keyframes",
     "layer",
     "media",
     "namespace",

rsass/src/parser/keyframes.rs

@@ -0,0 +1,76 @@
+use super::strings::{sass_string, sass_string_ext2};
+use super::util::{comment, ignore_comments, opt_spacelike};
+use super::value::{number, value_expression};
+use super::PResult;
+use super::{variable_declaration, Span};
+use crate::sass::{Item, KfItem, SassString, Value};
+use nom::branch::alt;
+use nom::bytes::complete::tag;
+use nom::combinator::{map, map_res, opt, recognize};
+use nom::multi::{many_till, separated_list1};
+use nom::sequence::{delimited, pair, preceded, terminated};
+use std::str::from_utf8;
+
+pub fn keyframes2(input: Span) -> PResult<Item> {
+    let (input, setname) =
+        dbg!(terminated(sass_string_ext2, opt_spacelike)(input))?;
+    let (input, (body, _end)) = preceded(
+        terminated(tag("{"), opt_spacelike),
+        many_till(
+            alt((
+                map(
+                    pair(
+                        separated_list1(
+                            delimited(opt_spacelike, tag(","), opt_spacelike),
+                            stop_name,
+                        ),
+                        preceded(
+                            delimited(opt_spacelike, tag("{"), opt_spacelike),
+                            map(
+                                many_till(
+                                    body_rule,
+                                    terminated(
+                                        terminated(tag("}"), opt_spacelike),
+                                        opt(tag(";")),
+                                    ),
+                                ),
+                                |(a, _)| a,
+                            ),
+                        ),
+                    ),
+                    |(names, body)| KfItem::Stop(names, body),
+                ),
+                map(
+                    terminated(variable_declaration, opt_spacelike),
+                    KfItem::VariableDeclaration,
+                ),
+                map(comment, KfItem::Comment),
+            )),
+            terminated(terminated(tag("}"), opt_spacelike), opt(tag(";"))),
+        ),
+    )(input)?;
+    Ok((input, Item::KeyFrames(setname, body)))
+}
+
+fn body_rule(input: Span) -> PResult<(SassString, Value)> {
+    pair(
+        terminated(
+            sass_string,
+            delimited(ignore_comments, tag(":"), ignore_comments),
+        ),
+        terminated(
+            value_expression,
+            delimited(opt_spacelike, opt(tag(";")), opt_spacelike),
+        ),
+    )(input)
+}
+
+fn stop_name(input: Span) -> PResult<SassString> {
+    alt((
+        map_res(recognize(terminated(number, tag("%"))), |s| {
+            from_utf8(s.fragment())
+                .map(|s| SassString::from(s.to_lowercase()))
+        }),
+        sass_string,
+    ))(input)
+}

rsass/src/parser/mod.rs

@@ -3,6 +3,7 @@ mod css_function;
 mod error;
 pub mod formalargs;
 mod imports;
+mod keyframes;
 mod media;
 pub mod selectors;
 mod span;
@@ -30,7 +31,9 @@ use self::value::{
     value_expression,
 };
 use crate::input::{SourceFile, SourceName, SourcePos};
-use crate::sass::parser::{variable_declaration2, variable_declaration_mod};
+use crate::sass::parser::{
+    variable_declaration, variable_declaration2, variable_declaration_mod,
+};
 use crate::sass::{Callable, FormalArgs, Item, Name, Selectors, Value};
 use crate::value::ListSeparator;
 #[cfg(test)]
@@ -224,6 +227,7 @@ fn at_rule2(input0: Span) -> PResult<Item> {
         "if" => if_statement2(input),
         "import" => import2(input),
         "include" => mixin_call(input0, input),
+        "keyframes" => keyframes::keyframes2(input),
         "media" => media::rule(input0, input),
         "mixin" => mixin_declaration2(input),
         "return" => return_stmt2(input0, input),

rsass/src/parser/strings.rs

@@ -120,6 +120,13 @@ pub fn sass_string_ext(input: Span) -> PResult<SassString> {
     Ok((input, SassString::new(parts, Quotes::None)))
 }
 
+/// An unquoted string that may contain the `$` sign.
+pub fn sass_string_ext2(input: Span) -> PResult<SassString> {
+    let (input, parts) =
+        many0(alt((string_part_interpolation, extended_part2)))(input)?;
+    Ok((input, SassString::new(parts, Quotes::None)))
+}
+
 fn unquoted_first_part(input: Span) -> PResult<String> {
     let (input, first) = alt((
         map(str_plain_part, String::from),
@@ -416,6 +423,17 @@ pub fn extended_part(input: Span) -> PResult<StringPart> {
     Ok((input, StringPart::Raw(part)))
 }
 
+fn extended_part2(input: Span) -> PResult<StringPart> {
+    let (input, part) = map_res(
+        recognize(pair(
+            verify(take_char, |c| is_ext_str_start_char(c) || *c == '$'),
+            many0(verify(take_char, is_ext_str_char)),
+        )),
+        input_to_string,
+    )(input)?;
+    Ok((input, StringPart::Raw(part)))
+}
+
 fn is_ext_str_start_char(c: &char) -> bool {
     is_name_char(c)
         || *c == '*'