Stop emitting mixed-decls in a bunch of unnecessary cases (#2342)

lib/src/ast/css/declaration.dart

@@ -2,10 +2,13 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'package:meta/meta.dart';
 import 'package:source_span/source_span.dart';
+import 'package:stack_trace/stack_trace.dart';
 
 import '../../value.dart';
 import 'node.dart';
+import 'style_rule.dart';
 import 'value.dart';
 
 /// A plain CSS declaration (that is, a `name: value` pair).
@@ -16,6 +19,22 @@ abstract interface class CssDeclaration implements CssNode {
   /// The value of this declaration.
   CssValue<Value> get value;
 
+  /// A list of style rules that appeared before this declaration in the Sass
+  /// input but after it in the CSS output.
+  ///
+  /// These are used to emit mixed declaration deprecation warnings during
+  /// serialization, so we can check based on specificity whether the warnings
+  /// are really necessary without worrying about `@extend` potentially changing
+  /// things up.
+  @internal
+  List<CssStyleRule> get interleavedRules;
+
+  /// The stack trace indicating where this node was created.
+  ///
+  /// This is used to emit interleaved declaration warnings, and is only set if
+  /// [interleavedRules] isn't empty.
+  Trace? get trace;
+
   /// The span for [value] that should be emitted to the source map.
   ///
   /// When the declaration's expression is just a variable, this is the span

lib/src/ast/css/modifiable/declaration.dart

@@ -3,11 +3,13 @@
 // https://opensource.org/licenses/MIT.
 
 import 'package:source_span/source_span.dart';
+import 'package:stack_trace/stack_trace.dart';
 
 import '../../../value.dart';
 import '../../../visitor/interface/modifiable_css.dart';
 import '../declaration.dart';
 import '../value.dart';
+import '../style_rule.dart';
 import 'node.dart';
 
 /// A modifiable version of [CssDeclaration] for use in the evaluation step.
@@ -16,15 +18,23 @@ final class ModifiableCssDeclaration extends ModifiableCssNode
   final CssValue<String> name;
   final CssValue<Value> value;
   final bool parsedAsCustomProperty;
+  final List<CssStyleRule> interleavedRules;
+  final Trace? trace;
   final FileSpan valueSpanForMap;
   final FileSpan span;
 
   bool get isCustomProperty => name.value.startsWith('--');
 
   /// Returns a new CSS declaration with the given properties.
   ModifiableCssDeclaration(this.name, this.value, this.span,
-      {required this.parsedAsCustomProperty, FileSpan? valueSpanForMap})
-      : valueSpanForMap = valueSpanForMap ?? value.span {
+      {required this.parsedAsCustomProperty,
+      Iterable<CssStyleRule>? interleavedRules,
+      this.trace,
+      FileSpan? valueSpanForMap})
+      : interleavedRules = interleavedRules == null
+            ? const []
+            : List.unmodifiable(interleavedRules),
+        valueSpanForMap = valueSpanForMap ?? value.span {
     if (parsedAsCustomProperty) {
       if (!isCustomProperty) {
         throw ArgumentError(

lib/src/ast/css/modifiable/node.dart

@@ -13,7 +13,6 @@ import '../node.dart';
 /// modification should only be done within the evaluation step, so the
 /// unmodifiable types are used elsewhere to enforce that constraint.
 abstract base class ModifiableCssNode extends CssNode {
-  /// The node that contains this, or `null` for the root [CssStylesheet] node.
   ModifiableCssParentNode? get parent => _parent;
   ModifiableCssParentNode? _parent;
 

lib/src/ast/css/node.dart

@@ -15,6 +15,10 @@ import 'style_rule.dart';
 /// A statement in a plain CSS syntax tree.
 @sealed
 abstract class CssNode implements AstNode {
+  /// The node that contains this, or `null` for the root [CssStylesheet] node.
+  @internal
+  CssParentNode? get parent;
+
   /// Whether this was generated from the last node in a nested Sass tree that
   /// got flattened during evaluation.
   bool get isGroupEnd;

lib/src/ast/css/stylesheet.dart

@@ -13,6 +13,7 @@ import 'node.dart';
 ///
 /// This is the root plain CSS node. It contains top-level statements.
 class CssStylesheet extends CssParentNode {
+  CssParentNode? get parent => null;
   final List<CssNode> children;
   final FileSpan span;
   bool get isGroupEnd => false;

lib/src/async_compile.dart

@@ -173,6 +173,7 @@ Future<CompileResult> _compileStylesheet(
       useSpaces: useSpaces,
       indentWidth: indentWidth,
       lineFeed: lineFeed,
+      logger: logger,
       sourceMap: sourceMap,
       charset: charset);
 

lib/src/compile.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_compile.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: 69b31749dc94c7f717e9d395327e4209c4d3feb0
+// Checksum: 4d72aeb3c39a2e607d1889755e07b7e489eddfa6
 //
 // ignore_for_file: unused_import
 
@@ -182,6 +182,7 @@ CompileResult _compileStylesheet(
       useSpaces: useSpaces,
       indentWidth: indentWidth,
       lineFeed: lineFeed,
+      logger: logger,
       sourceMap: sourceMap,
       charset: charset);
 

lib/src/visitor/async_evaluate.dart

@@ -1198,20 +1198,43 @@ final class _EvaluateVisitor
           node.span);
     }
 
-    if (_parent.parent!.children.last case var sibling
-        when _parent != sibling) {
-      _warn(
-          "Sass's behavior for declarations that appear after nested\n"
-          "rules will be changing to match the behavior specified by CSS in an "
-          "upcoming\n"
-          "version. To keep the existing behavior, move the declaration above "
-          "the nested\n"
-          "rule. To opt into the new behavior, wrap the declaration in `& "
-          "{}`.\n"
-          "\n"
-          "More info: https://sass-lang.com/d/mixed-decls",
-          MultiSpan(node.span, 'declaration', {sibling.span: 'nested rule'}),
-          Deprecation.mixedDecls);
+    var siblings = _parent.parent!.children;
+    var interleavedRules = <CssStyleRule>[];
+    if (siblings.last != _parent &&
+        // Reproduce this condition from [_warn] so that we don't add anything to
+        // [interleavedRules] for declarations in dependencies.
+        !(_quietDeps &&
+            (_inDependency || (_currentCallable?.inDependency ?? false)))) {
+      loop:
+      for (var sibling in siblings.skip(siblings.indexOf(_parent) + 1)) {
+        switch (sibling) {
+          case CssComment():
+            continue loop;
+
+          case CssStyleRule rule:
+            interleavedRules.add(rule);
+
+          case _:
+            // Always warn for siblings that aren't style rules, because they
+            // add no specificity and they're nested in the same parent as this
+            // declaration.
+            _warn(
+                "Sass's behavior for declarations that appear after nested\n"
+                "rules will be changing to match the behavior specified by CSS "
+                "in an upcoming\n"
+                "version. To keep the existing behavior, move the declaration "
+                "above the nested\n"
+                "rule. To opt into the new behavior, wrap the declaration in "
+                "`& {}`.\n"
+                "\n"
+                "More info: https://sass-lang.com/d/mixed-decls",
+                MultiSpan(
+                    node.span, 'declaration', {sibling.span: 'nested rule'}),
+                Deprecation.mixedDecls);
+            interleavedRules.clear();
+            break;
+        }
+      }
     }
 
     var name = await _interpolationToValue(node.name, warnForColor: true);
@@ -1227,6 +1250,8 @@ final class _EvaluateVisitor
         _parent.addChild(ModifiableCssDeclaration(
             name, CssValue(value, expression.span), node.span,
             parsedAsCustomProperty: node.isCustomProperty,
+            interleavedRules: interleavedRules,
+            trace: interleavedRules.isEmpty ? null : _stackTrace(node.span),
             valueSpanForMap:
                 _sourceMap ? node.value.andThen(_expressionNode)?.span : null));
       } else if (name.value.startsWith('--')) {

lib/src/visitor/evaluate.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_evaluate.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: de25b9055a73f1c7ebe7a707139e6c789a2866dd
+// Checksum: ca67afd2df1c970eb887d4a24c7fe838c2aaec60
 //
 // ignore_for_file: unused_import
 
@@ -1196,20 +1196,43 @@ final class _EvaluateVisitor
           node.span);
     }
 
-    if (_parent.parent!.children.last case var sibling
-        when _parent != sibling) {
-      _warn(
-          "Sass's behavior for declarations that appear after nested\n"
-          "rules will be changing to match the behavior specified by CSS in an "
-          "upcoming\n"
-          "version. To keep the existing behavior, move the declaration above "
-          "the nested\n"
-          "rule. To opt into the new behavior, wrap the declaration in `& "
-          "{}`.\n"
-          "\n"
-          "More info: https://sass-lang.com/d/mixed-decls",
-          MultiSpan(node.span, 'declaration', {sibling.span: 'nested rule'}),
-          Deprecation.mixedDecls);
+    var siblings = _parent.parent!.children;
+    var interleavedRules = <CssStyleRule>[];
+    if (siblings.last != _parent &&
+        // Reproduce this condition from [_warn] so that we don't add anything to
+        // [interleavedRules] for declarations in dependencies.
+        !(_quietDeps &&
+            (_inDependency || (_currentCallable?.inDependency ?? false)))) {
+      loop:
+      for (var sibling in siblings.skip(siblings.indexOf(_parent) + 1)) {
+        switch (sibling) {
+          case CssComment():
+            continue loop;
+
+          case CssStyleRule rule:
+            interleavedRules.add(rule);
+
+          case _:
+            // Always warn for siblings that aren't style rules, because they
+            // add no specificity and they're nested in the same parent as this
+            // declaration.
+            _warn(
+                "Sass's behavior for declarations that appear after nested\n"
+                "rules will be changing to match the behavior specified by CSS "
+                "in an upcoming\n"
+                "version. To keep the existing behavior, move the declaration "
+                "above the nested\n"
+                "rule. To opt into the new behavior, wrap the declaration in "
+                "`& {}`.\n"
+                "\n"
+                "More info: https://sass-lang.com/d/mixed-decls",
+                MultiSpan(
+                    node.span, 'declaration', {sibling.span: 'nested rule'}),
+                Deprecation.mixedDecls);
+            interleavedRules.clear();
+            break;
+        }
+      }
     }
 
     var name = _interpolationToValue(node.name, warnForColor: true);
@@ -1225,6 +1248,8 @@ final class _EvaluateVisitor
         _parent.addChild(ModifiableCssDeclaration(
             name, CssValue(value, expression.span), node.span,
             parsedAsCustomProperty: node.isCustomProperty,
+            interleavedRules: interleavedRules,
+            trace: interleavedRules.isEmpty ? null : _stackTrace(node.span),
             valueSpanForMap:
                 _sourceMap ? node.value.andThen(_expressionNode)?.span : null));
       } else if (name.value.startsWith('--')) {

lib/src/visitor/serialize.dart

@@ -6,17 +6,21 @@ import 'dart:math' as math;
 import 'dart:typed_data';
 
 import 'package:charcode/charcode.dart';
+import 'package:collection/collection.dart';
 import 'package:source_maps/source_maps.dart';
 import 'package:string_scanner/string_scanner.dart';
 
 import '../ast/css.dart';
 import '../ast/node.dart';
 import '../ast/selector.dart';
 import '../color_names.dart';
+import '../deprecation.dart';
 import '../exception.dart';
+import '../logger.dart';
 import '../parse/parser.dart';
 import '../utils.dart';
 import '../util/character.dart';
+import '../util/multi_span.dart';
 import '../util/no_source_map_buffer.dart';
 import '../util/nullable.dart';
 import '../util/number.dart';
@@ -48,6 +52,7 @@ SerializeResult serialize(CssNode node,
     bool useSpaces = true,
     int? indentWidth,
     LineFeed? lineFeed,
+    Logger? logger,
     bool sourceMap = false,
     bool charset = true}) {
   indentWidth ??= 2;
@@ -57,6 +62,7 @@ SerializeResult serialize(CssNode node,
       useSpaces: useSpaces,
       indentWidth: indentWidth,
       lineFeed: lineFeed,
+      logger: logger,
       sourceMap: sourceMap);
   node.accept(visitor);
   var css = visitor._buffer.toString();
@@ -128,6 +134,12 @@ final class _SerializeVisitor
   /// The characters to use for a line feed.
   final LineFeed _lineFeed;
 
+  /// The logger to use to print warnings.
+  ///
+  /// This should only be used for statement-level serialization. It's not
+  /// guaranteed to be the main user-provided logger for expressions.
+  final Logger _logger;
+
   /// Whether we're emitting compressed output.
   bool get _isCompressed => _style == OutputStyle.compressed;
 
@@ -138,14 +150,16 @@ final class _SerializeVisitor
       bool useSpaces = true,
       int? indentWidth,
       LineFeed? lineFeed,
+      Logger? logger,
       bool sourceMap = true})
       : _buffer = sourceMap ? SourceMapBuffer() : NoSourceMapBuffer(),
         _style = style ?? OutputStyle.expanded,
         _inspect = inspect,
         _quote = quote,
         _indentCharacter = useSpaces ? $space : $tab,
         _indentWidth = indentWidth ?? 2,
-        _lineFeed = lineFeed ?? LineFeed.lf {
+        _lineFeed = lineFeed ?? LineFeed.lf,
+        _logger = logger ?? const Logger.stderr() {
     RangeError.checkValueInInterval(_indentWidth, 0, 10, "indentWidth");
   }
 
@@ -329,6 +343,33 @@ final class _SerializeVisitor
   }
 
   void visitCssDeclaration(CssDeclaration node) {
+    if (node.interleavedRules.isNotEmpty) {
+      var declSpecificities = _specificities(node.parent!);
+      for (var rule in node.interleavedRules) {
+        var ruleSpecificities = _specificities(rule);
+
+        // If the declaration can never match with the same specificity as one
+        // of its sibling rules, then ordering will never matter and there's no
+        // need to warn about the declaration being re-ordered.
+        if (!declSpecificities.any(ruleSpecificities.contains)) continue;
+
+        _logger.warnForDeprecation(
+            Deprecation.mixedDecls,
+            "Sass's behavior for declarations that appear after nested\n"
+            "rules will be changing to match the behavior specified by CSS in an "
+            "upcoming\n"
+            "version. To keep the existing behavior, move the declaration above "
+            "the nested\n"
+            "rule. To opt into the new behavior, wrap the declaration in `& "
+            "{}`.\n"
+            "\n"
+            "More info: https://sass-lang.com/d/mixed-decls",
+            span:
+                MultiSpan(node.span, 'declaration', {rule.span: 'nested rule'}),
+            trace: node.trace);
+      }
+    }
+
     _writeIndentation();
 
     _write(node.name);
@@ -363,6 +404,22 @@ final class _SerializeVisitor
     }
   }
 
+  /// Returns the set of possible specificities which which [node] might match.
+  Set<int> _specificities(CssParentNode node) {
+    if (node case CssStyleRule rule) {
+      // Plain CSS style rule nesting implicitly wraps parent selectors in
+      // `:is()`, so they all match with the highest specificity among any of
+      // them.
+      var parent = node.parent.andThen(_specificities)?.max ?? 0;
+      return {
+        for (var selector in rule.selector.components)
+          parent + selector.specificity
+      };
+    } else {
+      return node.parent.andThen(_specificities) ?? const {0};
+    }
+  }
+
   /// Emits the value of [node], with all newlines followed by whitespace
   void _writeFoldedValue(CssDeclaration node) {
     var scanner = StringScanner((node.value.value as SassString).text);