unifyComound() and unifyComplex() no longer move pseudo-classes across pseudo-element boundaries (#2350)

* unifyComound() and unifyComplex() no longer move pseudo-classes across pseudo-element boundaries
* Fix crash in TypeSelector.unify() when unifying an empty list

Author: Goodwine

CHANGELOG.md

@@ -1,3 +1,15 @@
+## 1.79.5
+
+* Changes to how `selector.unify()` and `@extend` combine selectors:
+
+    * The relative order of pseudo-classes (like `:hover`) and pseudo-elements
+      (like `::before`) within each original selector is now preserved when
+      they're combined.
+
+    * Pseudo selectors are now consistently placed at the end of the combined
+      selector, regardless of which selector they came from. Previously, this
+      reordering only applied to pseudo-selectors in the second selector.
+
 ## 1.79.4
 
 ### JS API

lib/src/ast/selector/type.dart

@@ -32,7 +32,7 @@ final class TypeSelector extends SimpleSelector {
   /// @nodoc
   @internal
   List<SimpleSelector>? unify(List<SimpleSelector> compound) {
-    if (compound.first case UniversalSelector() || TypeSelector()) {
+    if (compound.firstOrNull case UniversalSelector() || TypeSelector()) {
       var unified = unifyUniversalAndElement(this, compound.first);
       if (unified == null) return null;
       return [unified, ...compound.skip(1)];

lib/src/extend/functions.dart

@@ -36,7 +36,7 @@ List<ComplexSelector>? unifyComplex(
     List<ComplexSelector> complexes, FileSpan span) {
   if (complexes.length == 1) return complexes;
 
-  List<SimpleSelector>? unifiedBase;
+  CompoundSelector? unifiedBase;
   CssValue<Combinator>? leadingCombinator;
   CssValue<Combinator>? trailingCombinator;
   for (var complex in complexes) {
@@ -67,12 +67,10 @@ List<ComplexSelector>? unifyComplex(
     }
 
     if (unifiedBase == null) {
-      unifiedBase = base.selector.components;
+      unifiedBase = base.selector;
     } else {
-      for (var simple in base.selector.components) {
-        unifiedBase = simple.unify(unifiedBase!); // dart-lang/sdk#45348
-        if (unifiedBase == null) return null;
-      }
+      unifiedBase = unifyCompound(unifiedBase, base.selector);
+      if (unifiedBase == null) return null;
     }
   }
 
@@ -87,7 +85,7 @@ List<ComplexSelector>? unifyComplex(
   var base = ComplexSelector(
       leadingCombinator == null ? const [] : [leadingCombinator],
       [
-        ComplexSelectorComponent(CompoundSelector(unifiedBase!, span),
+        ComplexSelectorComponent(unifiedBase!,
             trailingCombinator == null ? const [] : [trailingCombinator], span)
       ],
       span,
@@ -106,19 +104,44 @@ List<ComplexSelector>? unifyComplex(
 /// Returns a [CompoundSelector] that matches only elements that are matched by
 /// both [compound1] and [compound2].
 ///
-/// The [span] will be used for the new unified selector.
+/// The [compound1]'s `span` will be used for the new unified selector.
+///
+/// This function ensures that the relative order of pseudo-classes (`:`) and
+/// pseudo-elements (`::`) within each input selector is preserved in the
+/// resulting combined selector.
+///
+/// This function enforces a general rule that pseudo-classes (`:`) should come
+/// before pseudo-elements (`::`), but it won't change their order if they were
+/// originally interleaved within a single input selector. This prevents
+/// unintended changes to the selector's meaning. For example, unifying
+/// `::foo:bar` and `:baz` results in `:baz::foo:bar`. `:baz` is a pseudo-class,
+/// so it is moved before the pseudo-class `::foo`. Meanwhile, `:bar` is not
+/// moved before `::foo` because it appeared after `::foo` in the original
+/// selector.
 ///
 /// If no such selector can be produced, returns `null`.
 CompoundSelector? unifyCompound(
     CompoundSelector compound1, CompoundSelector compound2) {
-  var result = compound2.components;
-  for (var simple in compound1.components) {
-    var unified = simple.unify(result);
-    if (unified == null) return null;
-    result = unified;
+  var result = compound1.components;
+  var pseudoResult = <SimpleSelector>[];
+  var pseudoElementFound = false;
+
+  for (var simple in compound2.components) {
+    // All pseudo-classes are unified separately after a pseudo-element to
+    // preserve their relative order with the pseudo-element.
+    if (pseudoElementFound && simple is PseudoSelector) {
+      var unified = simple.unify(pseudoResult);
+      if (unified == null) return null;
+      pseudoResult = unified;
+    } else {
+      pseudoElementFound |= simple is PseudoSelector && simple.isElement;
+      var unified = simple.unify(result);
+      if (unified == null) return null;
+      result = unified;
+    }
   }
 
-  return CompoundSelector(result, compound1.span);
+  return CompoundSelector([...result, ...pseudoResult], compound1.span);
 }
 
 /// Returns a [SimpleSelector] that matches only elements that are matched by

pkg/sass-parser/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.2.4
+
+* No user-visible changes.
+
 ## 0.2.3
 
 * No user-visible changes.

pkg/sass-parser/package.json

@@ -1,6 +1,6 @@
 {
   "name": "sass-parser",
-  "version": "0.2.3",
+  "version": "0.2.4",
   "description": "A PostCSS-compatible wrapper of the official Sass parser",
   "repository": "sass/sass",
   "author": "Google Inc.",

pkg/sass_api/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 12.0.5
+
+* No user-visible changes.
+
 ## 12.0.4
 
 * No user-visible changes.

pkg/sass_api/pubspec.yaml

@@ -2,15 +2,15 @@ name: sass_api
 # Note: Every time we add a new Sass AST node, we need to bump the *major*
 # version because it's a breaking change for anyone who's implementing the
 # visitor interface(s).
-version: 12.0.4
+version: 12.0.5
 description: Additional APIs for Dart Sass.
 homepage: https://github.com/sass/dart-sass
 
 environment:
   sdk: ">=3.0.0 <4.0.0"
 
 dependencies:
-  sass: 1.79.4
+  sass: 1.79.5
 
 dev_dependencies:
   dartdoc: ^8.0.14

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass
-version: 1.79.4
+version: 1.79.5
 description: A Sass implementation in Dart.
 homepage: https://github.com/sass/dart-sass