fix longstanding @extend bug related to selector lists

src/parse/mod.rs

@@ -1193,6 +1193,8 @@ impl<'a> Parser<'a> {
 
         let extend_rule = ExtendRule::new(value.clone(), is_optional, self.span_before);
 
+        let super_selector = self.super_selectors.last();
+
         for complex in value.0.components {
             if complex.components.len() != 1 || !complex.components.first().unwrap().is_compound() {
                 // If the selector was a compound selector but not a simple
@@ -1214,7 +1216,7 @@ impl<'a> Parser<'a> {
             }
 
             self.extender.add_extension(
-                self.super_selectors.last().clone().0,
+                super_selector.clone().0,
                 compound.components.first().unwrap(),
                 &extend_rule,
                 &None,

src/selector/extend/extended_selector.rs

@@ -36,3 +36,22 @@ impl ExtendedSelector {
         self.0.replace(selector);
     }
 }
+
+#[derive(Clone, Debug)]
+pub(crate) struct SelectorHashSet(Vec<ExtendedSelector>);
+
+impl SelectorHashSet {
+    pub const fn new() -> Self {
+        Self(Vec::new())
+    }
+
+    pub fn insert(&mut self, selector: ExtendedSelector) {
+        if !self.0.contains(&selector) {
+            self.0.push(selector);
+        }
+    }
+
+    pub fn into_iter(self) -> std::vec::IntoIter<ExtendedSelector> {
+        self.0.into_iter()
+    }
+}

src/selector/extend/mod.rs

@@ -15,6 +15,7 @@ use super::{
 };
 
 pub(crate) use extended_selector::ExtendedSelector;
+use extended_selector::SelectorHashSet;
 use extension::Extension;
 pub(crate) use functions::unify_complex;
 use functions::{paths, weave};
@@ -64,7 +65,7 @@ pub(crate) struct Extender {
     ///
     /// This is used to find which selectors an `@extend` applies to and adjust
     /// them.
-    selectors: HashMap<SimpleSelector, HashSet<ExtendedSelector>>,
+    selectors: HashMap<SimpleSelector, SelectorHashSet>,
 
     /// A map from all extended simple selectors to the sources of those
     /// extensions.
@@ -867,8 +868,6 @@ impl Extender {
     ///
     /// The `media_query_context` is the media query context in which the selector was
     /// defined, or `None` if it was defined at the top level of the document.
-    // todo: the docs are wrong, and we may want to consider returning an `Rc<RefCell<SelectorList>>`
-    // the reason we don't is that it would interfere with hashing
     pub fn add_selector(
         &mut self,
         mut selector: SelectorList,
@@ -915,7 +914,7 @@ impl Extender {
                     for simple in component.components {
                         self.selectors
                             .entry(simple.clone())
-                            .or_insert_with(HashSet::new)
+                            .or_insert_with(SelectorHashSet::new)
                             .insert(selector.clone());
 
                         if let SimpleSelector::Pseudo(Pseudo {
@@ -953,12 +952,6 @@ impl Extender {
 
         let mut new_extensions: Option<IndexMap<ComplexSelector, Extension>> = None;
 
-        let mut sources = self
-            .extensions
-            .entry(target.clone())
-            .or_insert_with(IndexMap::new)
-            .clone();
-
         for complex in extender.components {
             let state = Extension {
                 specificity: complex.max_specificity(),
@@ -972,6 +965,11 @@ impl Extender {
                 right: None,
             };
 
+            let sources = self
+                .extensions
+                .entry(target.clone())
+                .or_insert_with(IndexMap::new);
+
             if let Some(existing_state) = sources.get(&complex) {
                 // If there's already an extend from `extender` to `target`, we don't need
                 // to re-run the extension. We may need to mark the extension as
@@ -1004,39 +1002,28 @@ impl Extender {
                     .get_or_insert_with(IndexMap::new)
                     .insert(complex.clone(), state.clone());
             }
+        }
 
-            let new_extensions = if let Some(new) = new_extensions.clone() {
+        let new_extensions = if let Some(new) = new_extensions {
-                new
+            new
-            } else {
+        } else {
-                // TODO: HACK: we extend by sources here, but we should be able to mutate sources directly
+            return;
-                self.extensions
+        };
-                    .get_mut(target)
-                    .get_or_insert(&mut IndexMap::new())
-                    .extend(sources);
-                return;
-            };
 
-            let mut new_extensions_by_target = HashMap::new();
+        let mut new_extensions_by_target = HashMap::new();
-            new_extensions_by_target.insert(target.clone(), new_extensions);
+        new_extensions_by_target.insert(target.clone(), new_extensions);
 
-            if let Some(existing_extensions) = existing_extensions.clone() {
+        if let Some(existing_extensions) = existing_extensions {
-                let additional_extensions =
+            let additional_extensions =
-                    self.extend_existing_extensions(existing_extensions, &new_extensions_by_target);
+                self.extend_existing_extensions(existing_extensions, &new_extensions_by_target);
-                if let Some(additional_extensions) = additional_extensions {
+            if let Some(additional_extensions) = additional_extensions {
-                    map_add_all_2(&mut new_extensions_by_target, additional_extensions);
+                map_add_all_2(&mut new_extensions_by_target, additional_extensions);
-                }
-            }
-
-            if let Some(selectors) = selectors.clone() {
-                self.extend_existing_selectors(selectors, &new_extensions_by_target);
             }
         }
 
-        // TODO: HACK: we extend by sources here, but we should be able to mutate sources directly
+        if let Some(selectors) = selectors {
-        self.extensions
+            self.extend_existing_selectors(selectors, &new_extensions_by_target);
-            .get_mut(target)
+        }
-            .get_or_insert(&mut IndexMap::new())
-            .extend(sources);
     }
 
     /// Extend `extensions` using `new_extensions`.
@@ -1145,10 +1132,10 @@ impl Extender {
     /// Extend `extensions` using `new_extensions`.
     fn extend_existing_selectors(
         &mut self,
-        selectors: HashSet<ExtendedSelector>,
+        selectors: SelectorHashSet,
         new_extensions: &HashMap<SimpleSelector, IndexMap<ComplexSelector, Extension>>,
     ) {
-        for mut selector in selectors {
+        for mut selector in selectors.into_iter() {
             let old_value = selector.clone().into_selector().0;
             selector.set_inner(self.extend_list(
                 old_value.clone(),

tests/extend.rs

@@ -1635,7 +1635,6 @@ test!(
     "~ .foo {\n  a: b;\n}\n"
 );
 test!(
-    #[ignore = "Rc<RefCell<Selector>>"]
     nested_selector_with_child_selector_hack_extender_and_extendee_newline,
     "> .foo {a: b}\nflip,\n> foo bar {@extend .foo}\n",
     "> .foo, > flip,\n> foo bar {\n  a: b;\n}\n"
@@ -1843,5 +1842,41 @@ test!(
     ",
     ".foo, .bang, .baz {\n  a: b;\n}\n\n.bar, .bang, .baz {\n  x: y;\n}\n"
 );
+test!(
+    selector_list_after_selector,
+    "a {
+        color: red;
+    }
+
+    b,
+    c {
+        @extend a;
+    }",
+    "a, b,\nc {\n  color: red;\n}\n"
+);
+test!(
+    selector_list_before_selector,
+    "b, c {
+        @extend a;
+    }
+
+    a {
+        color: red;
+    }",
+    "a, b, c {\n  color: red;\n}\n"
+);
+test!(
+    selector_list_of_selector_pseudo_classes_after_selector,
+    "foo {
+        color: black;
+    }
+
+    a:current(foo),
+    :current(foo) {
+        @extend foo;
+    }",
+    "foo, a:current(foo),\n:current(foo) {\n  color: black;\n}\n"
+);
 
 // todo: extend_loop (massive test)
+// todo: extend tests in folders