refactor attribute parsing

src/selector/attribute.rs

@@ -1,151 +1,221 @@
-use std::fmt::{self, Display};
+use std::fmt::{self, Display, Write};
 
 use peekmore::PeekMoreIterator;
 
 use codemap::Span;
 
 use super::{Selector, SelectorKind};
+use crate::common::QuoteKind;
 use crate::error::SassResult;
 use crate::scope::Scope;
-use crate::utils::{
+use crate::utils::{devour_whitespace, eat_ident, is_ident, parse_quoted_string};
-    devour_whitespace, eat_ident, is_ident_char, parse_interpolation, parse_quoted_string,
+use crate::value::Value;
-};
 use crate::Token;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub(crate) struct Attribute {
-    attr: String,
+    attr: QualifiedName,
     value: String,
     modifier: Option<char>,
-    kind: AttributeKind,
+    op: AttributeOp,
+    span: Span,
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+struct QualifiedName {
+    pub ident: String,
+    pub namespace: Option<String>,
+}
+
+impl Display for QualifiedName {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        if let Some(namespace) = &self.namespace {
+            write!(f, "{}|", namespace)?;
+        }
+        f.write_str(&self.ident)
+    }
+}
+
+fn attribute_name<I: Iterator<Item = Token>>(
+    toks: &mut PeekMoreIterator<I>,
+    scope: &Scope,
+    super_selector: &Selector,
+    start: Span,
+) -> SassResult<QualifiedName> {
+    let next = toks.peek().ok_or(("Expected identifier.", start))?;
+    if next.kind == '*' {
+        let pos = next.pos;
+        toks.next();
+        if toks.peek().ok_or(("expected \"|\".", pos))?.kind != '|' {
+            return Err(("expected \"|\".", pos).into());
+        } else {
+            toks.next();
+        }
+        let ident = eat_ident(toks, scope, super_selector)?.node;
+        return Ok(QualifiedName {
+            ident,
+            namespace: Some('*'.to_string()),
+        });
+    }
+    let name_or_namespace = eat_ident(toks, scope, super_selector)?;
+    match toks.peek() {
+        Some(v) if v.kind != '|' => {
+            return Ok(QualifiedName {
+                ident: name_or_namespace.node,
+                namespace: None,
+            });
+        }
+        Some(..) => {}
+        None => return Err(("expected more input.", name_or_namespace.span).into()),
+    }
+    match toks.peek_forward(1) {
+        Some(v) if v.kind == '=' => {
+            toks.peek_backward(1).unwrap();
+            return Ok(QualifiedName {
+                ident: name_or_namespace.node,
+                namespace: None,
+            });
+        }
+        Some(..) => {
+            toks.peek_backward(1).unwrap();
+        }
+        None => return Err(("expected more input.", name_or_namespace.span).into()),
+    }
+    toks.next();
+    let ident = eat_ident(toks, scope, super_selector)?.node;
+    Ok(QualifiedName {
+        ident,
+        namespace: Some(name_or_namespace.node),
+    })
+}
+
+fn attribute_operator<I: Iterator<Item = Token>>(
+    toks: &mut PeekMoreIterator<I>,
+    start: Span,
+) -> SassResult<AttributeOp> {
+    let op = match toks.next().ok_or(("Expected \"]\".", start))?.kind {
+        '=' => return Ok(AttributeOp::Equals),
+        '~' => AttributeOp::Include,
+        '|' => AttributeOp::Dash,
+        '^' => AttributeOp::Prefix,
+        '$' => AttributeOp::Suffix,
+        '*' => AttributeOp::Contains,
+        _ => return Err(("Expected \"]\".", start).into()),
+    };
+    if toks.next().ok_or(("expected \"=\".", start))?.kind != '=' {
+        return Err(("expected \"=\".", start).into());
+    }
+    Ok(op)
+}
 impl Attribute {
     pub fn from_tokens<I: Iterator<Item = Token>>(
         toks: &mut PeekMoreIterator<I>,
         scope: &Scope,
         super_selector: &Selector,
-        mut start: Span,
+        start: Span,
     ) -> SassResult<SelectorKind> {
         devour_whitespace(toks);
-        let next_tok = toks.peek().ok_or(("Expected identifier.", start))?;
+        let attr = attribute_name(toks, scope, super_selector, start)?;
-        let attr = match next_tok.kind {
-            c if is_ident_char(c) => {
-                let i = eat_ident(toks, scope, super_selector)?;
-                start = i.span;
-                i.node
-            }
-            '#' => {
-                start.merge(toks.next().unwrap().pos());
-                if toks.next().ok_or(("Expected expression.", start))?.kind == '{' {
-                    let interpolation = parse_interpolation(toks, scope, super_selector)?;
-                    interpolation.node.to_css_string(interpolation.span)?
-                } else {
-                    return Err(("Expected expression.", start).into());
-                }
-            }
-            _ => return Err(("Expected identifier.", start).into()),
-        };
-
         devour_whitespace(toks);
-
+        if toks.peek().ok_or(("expected more input.", start))?.kind == ']' {
-        let next = toks.next().ok_or(("expected \"]\".", start))?;
+            toks.next();
-
-        let kind = match next.kind {
-            c if is_ident_char(c) => return Err(("Expected \"]\".", next.pos()).into()),
-            ']' => {
             return Ok(SelectorKind::Attribute(Attribute {
-                    kind: AttributeKind::Any,
                 attr,
                 value: String::new(),
                 modifier: None,
+                op: AttributeOp::Any,
+                span: start,
             }));
         }
-            '=' => AttributeKind::Equals,
-            '~' => AttributeKind::Include,
-            '|' => AttributeKind::Dash,
-            '^' => AttributeKind::Prefix,
-            '$' => AttributeKind::Suffix,
-            '*' => AttributeKind::Contains,
-            _ => return Err(("expected \"]\".", next.pos()).into()),
-        };
-
-        if kind != AttributeKind::Equals {
-            let next = toks.next().ok_or(("expected \"=\".", next.pos()))?;
-            match next.kind {
-                '=' => {}
-                _ => return Err(("expected \"=\".", next.pos()).into()),
-            }
-        }
 
+        let op = attribute_operator(toks, start)?;
         devour_whitespace(toks);
 
-        let next = toks.next().ok_or(("Expected identifier.", next.pos()))?;
+        let peek = toks.peek().ok_or(("expected more input.", start))?;
 
-        let value = match next.kind {
+        let value = match peek.kind {
-            v @ 'a'..='z' | v @ 'A'..='Z' | v @ '-' | v @ '_' => {
+            q @ '\'' | q @ '"' => {
-                format!("{}{}", v, eat_ident(toks, scope, super_selector)?.node)
+                toks.next();
+                match parse_quoted_string(toks, scope, q, super_selector)?.node {
+                    Value::Ident(s, ..) => s,
+                    _ => unreachable!(),
                 }
-            q @ '"' | q @ '\'' => {
-                parse_quoted_string(toks, scope, q, super_selector)?.to_css_string(next.pos())?
             }
-            _ => return Err(("Expected identifier.", next.pos()).into()),
+            _ => eat_ident(toks, scope, super_selector)?.node,
         };
-
         devour_whitespace(toks);
 
-        let next = toks.next().ok_or(("expected \"]\".", next.pos()))?;
+        let peek = toks.peek().ok_or(("expected more input.", start))?;
 
-        let modifier = match next.kind {
+        let modifier = match peek.kind {
-            ']' => {
+            c if c.is_alphabetic() => Some(c),
-                return Ok(SelectorKind::Attribute(Attribute {
+            _ => None,
-                    kind,
-                    attr,
-                    value,
-                    modifier: None,
-                }))
-            }
-            v @ 'a'..='z' | v @ 'A'..='Z' => {
-                let next = toks.next().ok_or(("expected \"]\".", next.pos()))?;
-                match next.kind {
-                    ']' => {}
-                    _ => return Err(("expected \"]\".", next.pos()).into()),
-                }
-                Some(v)
-            }
-            _ => return Err(("expected \"]\".", next.pos()).into()),
         };
 
+        let pos = peek.pos();
+
+        if modifier.is_some() {
+            toks.next();
+            devour_whitespace(toks);
+        }
+
+        if toks.peek().ok_or(("expected \"]\".", pos))?.kind != ']' {
+            return Err(("expected \"]\".", pos).into());
+        } else {
+            toks.next();
+        }
+
         Ok(SelectorKind::Attribute(Attribute {
-            kind,
+            op,
             attr,
             value,
             modifier,
+            span: start,
         }))
     }
 }
 
 impl Display for Attribute {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let modifier = if let Some(c) = self.modifier {
+        f.write_char('[')?;
-            format!(" {}", c)
+        write!(f, "{}", self.attr)?;
-        } else {
+
-            String::new()
+        if self.op != AttributeOp::Any {
-        };
+            f.write_str(self.op.into())?;
-        match self.kind {
+            if is_ident(&self.value) && !self.value.starts_with("--") {
-            AttributeKind::Any => write!(f, "[{}{}]", self.attr, modifier),
+                f.write_str(&self.value)?;
-            AttributeKind::Equals => write!(f, "[{}={}{}]", self.attr, self.value, modifier),
+
-            AttributeKind::Include => write!(f, "[{}~={}{}]", self.attr, self.value, modifier),
+                if self.modifier.is_some() {
-            AttributeKind::Dash => write!(f, "[{}|={}{}]", self.attr, self.value, modifier),
+                    f.write_char(' ')?;
-            AttributeKind::Prefix => write!(f, "[{}^={}{}]", self.attr, self.value, modifier),
-            AttributeKind::Suffix => write!(f, "[{}$={}{}]", self.attr, self.value, modifier),
-            AttributeKind::Contains => write!(f, "[{}*={}{}]", self.attr, self.value, modifier),
                 }
+            } else {
+                // todo: remove unwrap by not doing this in display
+                // or having special emitter for quoted strings?
+                // (also avoids the clone because we can consume/modify self)
+                f.write_str(
+                    &Value::Ident(self.value.clone(), QuoteKind::Quoted)
+                        .to_css_string(self.span)
+                        .unwrap(),
+                )?;
+                // todo: this space is not emitted when `compressed` output
+                if self.modifier.is_some() {
+                    f.write_char(' ')?;
+                }
+            }
+
+            if let Some(c) = self.modifier {
+                f.write_char(c)?;
+            }
+        }
+
+        f.write_char(']')?;
+
+        Ok(())
     }
 }
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
-enum AttributeKind {
+enum AttributeOp {
     /// \[attr\]
     ///
     /// Represents elements with an attribute name of `attr`
@@ -184,3 +254,17 @@ enum AttributeKind {
     /// `value` within the string
     Contains,
 }
+
+impl Into<&'static str> for AttributeOp {
+    fn into(self) -> &'static str {
+        match self {
+            Self::Any => "",
+            Self::Equals => "=",
+            Self::Include => "~=",
+            Self::Dash => "|=",
+            Self::Prefix => "^=",
+            Self::Suffix => "$=",
+            Self::Contains => "*=",
+        }
+    }
+}

src/utils/chars.rs

@@ -26,10 +26,6 @@ pub(crate) fn read_until_char<I: Iterator<Item = Token>>(
     v
 }
 
-pub(crate) fn is_ident_char(c: char) -> bool {
-    c.is_ascii_alphabetic() || c == '_' || c == '\\' || (!c.is_ascii() && !c.is_control())
-}
-
 pub(crate) fn hex_char_for(number: u32) -> char {
     assert!(number < 0x10);
     std::char::from_u32(if number < 0xA {

src/utils/strings.rs

@@ -12,6 +12,38 @@ use crate::{Scope, Token};
 
 use super::{as_hex, hex_char_for, is_name, is_name_start, parse_interpolation};
 
+pub(crate) fn is_ident(s: &str) -> bool {
+    let mut chars = s.chars().peekable();
+    match chars.next() {
+        Some(c) if is_name_start(c) && !c.is_numeric() => {}
+        Some(..) | None => return false,
+    }
+    while let Some(c) = chars.next() {
+        if c == '\\' {
+            for _ in 0..6 {
+                let next = match chars.next() {
+                    Some(t) => t,
+                    None => return true,
+                };
+                if !next.is_ascii_hexdigit() {
+                    break;
+                }
+            }
+            match chars.peek() {
+                Some(c) if c.is_whitespace() => {
+                    chars.next();
+                }
+                _ => {}
+            };
+            continue;
+        }
+        if !is_name(c) {
+            return false;
+        }
+    }
+    true
+}
+
 fn ident_body_no_interpolation<I: Iterator<Item = Token>>(
     toks: &mut PeekMoreIterator<I>,
     unit: bool,

tests/selectors.rs

@@ -65,17 +65,40 @@ test!(
     "[attr=val] {\n  color: red;\n}\n"
 );
 test!(
-    #[ignore]
     selector_attribute_removes_single_quotes,
     "[attr='val'] {\n  color: red;\n}\n",
     "[attr=val] {\n  color: red;\n}\n"
 );
 test!(
-    #[ignore]
     selector_attribute_removes_double_quotes,
     "[attr=\"val\"] {\n  color: red;\n}\n",
     "[attr=val] {\n  color: red;\n}\n"
 );
+test!(
+    selector_attribute_quotes_non_ident,
+    "[attr=\"1\"] {\n  color: red;\n}\n"
+);
+test!(
+    selector_attribute_quotes_custom_property,
+    "[attr=\"--foo\"] {\n  color: red;\n}\n"
+);
+test!(
+    selector_attribute_unquoted_escape,
+    "[attr=v\\al] {\n  color: red;\n}\n",
+    "[attr=v\\a l] {\n  color: red;\n}\n"
+);
+test!(
+    selector_attribute_quoted_escape,
+    "[attr=\"v\\al\"] {\n  color: red;\n}\n"
+);
+test!(
+    selector_attribute_namespace,
+    "[*|foo] {\n  color: red;\n}\n"
+);
+error!(
+    selector_attribute_missing_equal,
+    "[a~b] {\n  color: red;\n}\n", "Error: expected \"=\"."
+);
 test!(
     selector_attribute_maintains_quotes_around_invalid_identifier,
     "[attr=\"val.\"] {\n  color: red;\n}\n"