Cleanup.

Remove remains of the old selector / logical selector divide.

CHANGELOG.md

@@ -9,7 +9,13 @@ project adheres to
 
 ## Unreleased
 
-(Nothing yet)
+### Breaking changes:
+
+* Replaced the css `Selector` implementation.
+  The new "logical" selector types that was used in selector
+  function in rsass 0.28 is now the only css selector implementation.
+  Most of the api to those types are private.
+  Some will probably be made public after some stabilization period.
 
 ## Release 0.28.10
 

rsass/src/css/selectors/attribute.rs

@@ -1,6 +1,6 @@
 use crate::{css::CssString, output::CssBuf};
 
-/// A logical attribute selector.
+/// An attribute selector.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) struct Attribute {
     /// The attribute name

rsass/src/css/selectors/compound.rs

@@ -1,5 +1,5 @@
 use super::{Attribute, CssSelectorSet, ElemType, Opt, Pseudo, SelectorSet};
-use crate::output::CssBuf;
+use crate::{output::CssBuf, parser::input_span, ParseError};
 use lazy_static::lazy_static;
 use std::fmt;
 
@@ -31,6 +31,23 @@ impl CompoundSelector {
     pub(super) fn has_id(&self) -> bool {
         self.id.is_some()
     }
+    pub(super) fn cant_append(&self) -> bool {
+        self.is_empty()
+            || self.element.as_ref().map_or(false, ElemType::cant_append)
+    }
+    pub(super) fn append(&self, other: &Self) -> Result<Self, ParseError> {
+        let mut s = CssBuf::new(Default::default());
+        self.write_to(&mut s);
+        other.write_to(&mut s);
+        let s = s.take();
+        if s.is_empty() {
+            Ok(Self::default())
+        } else {
+            let span = input_span(s);
+            Ok(ParseError::check(parser::compound_selector(span.borrow()))?)
+        }
+    }
+
     pub(super) fn is_rootish(&self) -> bool {
         self.pseudo.iter().any(Pseudo::is_rootish)
     }
@@ -152,7 +169,7 @@ impl CompoundSelector {
 
     pub fn write_to(&self, buf: &mut CssBuf) {
         if self.backref.is_some() {
-            buf.add_str("&");
+            buf.add_char('&');
         }
         if let Some(e) = &self.element {
             if !e.is_any()
@@ -165,15 +182,15 @@ impl CompoundSelector {
             }
         }
         for p in &self.placeholders {
-            buf.add_str("%");
+            buf.add_char('%');
             buf.add_str(p);
         }
         if let Some(id) = &self.id {
-            buf.add_str("#");
+            buf.add_char('#');
             buf.add_str(id);
         }
         for c in &self.classes {
-            buf.add_str(".");
+            buf.add_char('.');
             buf.add_str(c);
         }
         for attr in &self.attr {

rsass/src/css/selectors/elemtype.rs

@@ -2,7 +2,7 @@ use crate::output::CssBuf;
 use std::fmt;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub(super) struct ElemType {
+pub(crate) struct ElemType {
     s: String,
 }
 
@@ -16,6 +16,9 @@ impl ElemType {
     pub fn is_any(&self) -> bool {
         self.s == "*"
     }
+    pub(super) fn cant_append(&self) -> bool {
+        self.s.starts_with('*') || self.s.starts_with('|')
+    }
 
     pub fn is_superselector(&self, sub: &Self) -> bool {
         let (e_ns, e_name) = self.split_ns();
@@ -48,11 +51,9 @@ impl ElemType {
     }
 
     fn split_ns(&self) -> (Option<&str>, &str) {
-        let mut e = self.s.splitn(2, '|');
-        match (e.next(), e.next()) {
-            (Some(ns), Some(elem)) => (Some(ns), elem),
-            (Some(elem), None) => (None, elem),
-            _ => unreachable!(),
+        match self.s.split_once('|') {
+            Some((ns, name)) => (Some(ns), name),
+            None => (None, &self.s),
         }
     }
 

rsass/src/css/selectors/error.rs

@@ -21,7 +21,8 @@ impl From<ParseError> for BadSelector0 {
     }
 }
 
-/// The error when a [Value] cannot be converted to a [Selectors] or [Selector].
+/// The error when a [`Value`] cannot be converted to a
+/// [`SelectorSet`][super::SelectorSet] or [`Selector`][super::Selector].
 #[derive(Debug)]
 pub enum BadSelector {
     /// The value was not the expected type of list or string.

rsass/src/css/selectors/mod.rs

@@ -9,9 +9,9 @@ mod context;
 mod cssselectorset;
 mod elemtype;
 mod error;
-mod logical;
 mod opt;
 mod pseudo;
+mod selector;
 mod selectorset;
 
 use self::attribute::Attribute;
@@ -21,7 +21,7 @@ use self::pseudo::Pseudo;
 pub use context::SelectorCtx;
 pub(crate) use cssselectorset::CssSelectorSet;
 pub use error::BadSelector;
-pub use logical::Selector;
+pub use selector::Selector;
 pub use selectorset::SelectorSet;
 
 pub(crate) mod parser {
@@ -30,7 +30,7 @@ pub(crate) mod parser {
     pub(super) use super::elemtype::parser::{
         elem_name, keyframe_stop, name_opt_ns,
     };
-    pub(super) use super::logical::parser::selector;
     pub(super) use super::pseudo::parser::pseudo;
+    pub(super) use super::selector::parser::selector;
     pub(crate) use super::selectorset::parser::selector_set;
 }

rsass/src/css/selectors/logical.rs → rsass/src/css/selectors/selector.rs

@@ -1,13 +1,6 @@
-//! A logical selector is a css selector, but representend in a way
-//! that I hope make implementing the sass selector functions easier.
-//!
-//! In the future, I might use this as the primary (only) css selector
-//! implementation.  But as that is a major breaking change, I keep
-//! these types internal for now.
 use super::compound::CompoundSelector;
 use super::error::BadSelector0;
-use super::parser::compound_selector;
-use super::{BadSelector, CssSelectorSet, Opt, Pseudo, SelectorSet};
+use super::{BadSelector, CssSelectorSet, Opt, SelectorSet};
 use crate::css::Value;
 use crate::output::CssBuf;
 use crate::parser::input_span;
@@ -19,9 +12,9 @@ use std::iter::once;
 
 type RelBox = Box<(RelKind, Selector)>;
 
-/// A selector more aimed at making it easy to implement selector functions.
+/// A css selector.
 ///
-/// A logical selector is a sequence of compound selectors, joined by
+/// A selector is a sequence of compound selectors, joined by
 /// relational operators (where the "ancestor" relation is just
 /// whitespace in the text representation).
 #[derive(Default, Clone, PartialEq, Eq)]
@@ -72,34 +65,18 @@ impl Selector {
 
     pub(super) fn append(&self, other: &Self) -> Result<Self, AppendError> {
         if self.is_local_empty() {
-            return Err(AppendError::Parent);
-        }
-
-        let mut result = other.to_owned();
-        if let Some(rel) = result.rel_of.take() {
-            result.rel_of = Some(Box::new((rel.0, self.append(&rel.1)?)));
-            Ok(result)
+            Err(AppendError::Parent)
+        } else if let Some(rel) = other.rel_of.clone() {
+            Ok(Self {
+                rel_of: Some(Box::new((rel.0, self.append(&rel.1)?))),
+                compound: other.compound.clone(),
+            })
+        } else if other.compound.cant_append() {
+            Err(AppendError::Sub)
         } else {
-            let rel_of = self.rel_of.clone();
-            if result.is_local_empty() {
-                return Err(AppendError::Sub);
-            }
-            let s = self.clone().last_compound_str();
-            let other = result.last_compound_str();
-            if other
-                .bytes()
-                .next()
-                .map_or(true, |c| c == b'*' || c == b'|')
-            {
-                return Err(AppendError::Sub);
-            }
-            let s = s + &other;
-            let span = input_span(s);
             Ok(Self {
-                rel_of,
-                compound: ParseError::check(compound_selector(
-                    span.borrow(),
-                ))?,
+                rel_of: self.rel_of.clone(),
+                compound: self.compound.append(&other.compound)?,
             })
         }
     }
@@ -170,32 +147,20 @@ impl Selector {
         self = self.resolve_ref_in_pseudo(ctx);
         let rel_of = self.rel_of.take();
 
-        let result = if self.compound.has_backref() {
+        let result = if self.compound.backref.is_some() {
             self.compound.backref = None;
             ctx.s
                 .s
                 .iter()
                 .flat_map(|s| {
-                    let mut buf = CssBuf::new(Default::default());
-                    s.compound.write_to(&mut buf);
-                    self.compound.write_to(&mut buf);
-                    let buf = buf.take();
-                    let compound = if buf.is_empty() {
-                        CompoundSelector::default()
-                    } else {
-                        let span = input_span(buf);
-                        ParseError::check(compound_selector(span.borrow()))
-                            .unwrap()
-                    };
-                    let result = Selector {
-                        rel_of: self.rel_of.clone(),
-                        compound,
-                    };
                     Selector {
                         rel_of: s.rel_of.clone(),
                         compound: CompoundSelector::default(),
                     }
-                    .unify(result)
+                    .unify(Selector {
+                        rel_of: self.rel_of.clone(),
+                        compound: s.compound.append(&self.compound).unwrap(),
+                    })
                 })
                 .collect()
         } else {
@@ -446,34 +411,24 @@ impl Selector {
         self.compound.write_to(buf)
     }
 
-    pub(super) fn into_string_vec(mut self) -> Vec<String> {
-        let mut vec =
-            if let Some((kind, sel)) = self.rel_of.take().map(|b| *b) {
-                let mut vec = sel.into_string_vec();
-                if let Some(symbol) = kind.symbol() {
-                    vec.push(symbol.to_string());
-                }
-                vec
-            } else {
-                Vec::new()
-            };
-        let last = self.last_compound_str();
-        if !last.is_empty() {
-            vec.push(last);
+    pub(super) fn into_string_vec(self) -> Vec<String> {
+        let mut vec = if let Some((kind, sel)) = self.rel_of.map(|b| *b) {
+            let mut vec = sel.into_string_vec();
+            if let Some(symbol) = kind.symbol() {
+                vec.push(symbol.to_string());
+            }
+            vec
+        } else {
+            Vec::new()
+        };
+        if !self.compound.is_empty() {
+            let mut buf = CssBuf::new(Default::default());
+            self.compound.write_to(&mut buf);
+            vec.push(String::from_utf8_lossy(&buf.take()).to_string());
         }
         vec
     }
 
-    fn last_compound_str(self) -> String {
-        let mut buf = CssBuf::new(Default::default());
-        self.compound.write_to(&mut buf);
-        String::from_utf8_lossy(&buf.take()).to_string()
-    }
-
-    fn pseudo_element(&self) -> Option<&Pseudo> {
-        self.compound.pseudo_element()
-    }
-
     /// Internal (the api is [`TryFrom`]).
     pub(super) fn _try_from_value(v: &Value) -> Result<Self, BadSelector0> {
         match v {
@@ -580,8 +535,7 @@ fn unify_relbox(a: RelBox, b: RelBox) -> Option<Vec<RelBox>> {
             if b.is_local_superselector(&a) {
                 as_rel_vec(Ancestor, a._unify(b)?)
             } else if a.is_local_superselector(&b)
-                || have_same(&a.compound.id, &b.compound.id)
-                || have_same(&a.pseudo_element(), &b.pseudo_element())
+                || a.compound.must_not_inherit(&b.compound)
             {
                 as_rel_vec(Ancestor, b._unify(a)?)
             } else {
@@ -599,7 +553,7 @@ fn unify_relbox(a: RelBox, b: RelBox) -> Option<Vec<RelBox>> {
                 as_rel_vec(Sibling, once(b))
             } else if b.is_superselector(&a) {
                 as_rel_vec(Sibling, once(a))
-            } else if !have_same(&a.compound.id, &b.compound.id) {
+            } else if !a.compound.must_not_inherit(&b.compound) {
                 as_rel_vec(
                     Sibling,
                     b.clone()
@@ -616,7 +570,7 @@ fn unify_relbox(a: RelBox, b: RelBox) -> Option<Vec<RelBox>> {
         | ((Sibling, b_s), (a_k @ AdjacentSibling, a_s)) => {
             if b_s.is_superselector(&a_s) {
                 as_rel_vec(a_k, once(a_s))
-            } else if a_s.compound.id.is_some() || b_s.compound.id.is_some() {
+            } else if a_s.compound.has_id() || b_s.compound.has_id() {
                 as_rel_vec(a_k, a_s.with_rel_of(Sibling, b_s))
             } else {
                 as_rel_vec(
@@ -642,10 +596,6 @@ fn unify_relbox(a: RelBox, b: RelBox) -> Option<Vec<RelBox>> {
     })
 }
 
-fn have_same<T: Eq>(one: &Option<T>, other: &Option<T>) -> bool {
-    one.is_some() && one == other
-}
-
 impl TryFrom<Value> for Selector {
     type Error = BadSelector;
 

rsass/src/sass/selectors.rs

@@ -6,8 +6,7 @@
 //!
 //! This _may_ change to a something like a tree of operators with
 //! leafs of simple selectors in some future release.
-use crate::css::parser::selector_set;
-use crate::css::{self, SelectorSet};
+use crate::css::{self, parser::selector_set};
 use crate::parser::input_span;
 use crate::sass::SassString;
 use crate::{Error, ParseError, ScopeRef};
@@ -35,12 +34,12 @@ impl Selectors {
     }
 
     /// Evaluate any interpolation in these Selectors.
-    pub fn eval(&self, scope: ScopeRef) -> Result<SelectorSet, Error> {
+    pub fn eval(&self, scope: ScopeRef) -> Result<css::SelectorSet, Error> {
         let mut s = Vec::new();
         for sel in &self.s {
             s.extend(sel.eval(scope.clone())?);
         }
-        Ok(SelectorSet { s })
+        Ok(css::SelectorSet { s })
     }
     fn write_eval(
         &self,