Skip to content

Commit f826ed2

Browse files
nex3nex3
authored
Stop emitting mixed-decls in a bunch of unnecessary cases (#2342)
1 parent 2f0d0da commit f826ed2

File tree

10 files changed

+176
-34
lines changed

10 files changed

+176
-34
lines changed

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22
// MIT-style license that can be found in the LICENSE file or at
33
// https://opensource.org/licenses/MIT.
44

5+
import 'package:meta/meta.dart';
56
import 'package:source_span/source_span.dart';
7+
import 'package:stack_trace/stack_trace.dart';
68

79
import '../../value.dart';
810
import 'node.dart';
11+
import 'style_rule.dart';
912
import 'value.dart';
1013

1114
/// A plain CSS declaration (that is, a `name: value` pair).
@@ -16,6 +19,22 @@ abstract interface class CssDeclaration implements CssNode {
1619
/// The value of this declaration.
1720
CssValue<Value> get value;
1821

22+
/// A list of style rules that appeared before this declaration in the Sass
23+
/// input but after it in the CSS output.
24+
///
25+
/// These are used to emit mixed declaration deprecation warnings during
26+
/// serialization, so we can check based on specificity whether the warnings
27+
/// are really necessary without worrying about `@extend` potentially changing
28+
/// things up.
29+
@internal
30+
List<CssStyleRule> get interleavedRules;
31+
32+
/// The stack trace indicating where this node was created.
33+
///
34+
/// This is used to emit interleaved declaration warnings, and is only set if
35+
/// [interleavedRules] isn't empty.
36+
Trace? get trace;
37+
1938
/// The span for [value] that should be emitted to the source map.
2039
///
2140
/// When the declaration's expression is just a variable, this is the span

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
// https://opensource.org/licenses/MIT.
44

55
import 'package:source_span/source_span.dart';
6+
import 'package:stack_trace/stack_trace.dart';
67

78
import '../../../value.dart';
89
import '../../../visitor/interface/modifiable_css.dart';
910
import '../declaration.dart';
1011
import '../value.dart';
12+
import '../style_rule.dart';
1113
import 'node.dart';
1214

1315
/// A modifiable version of [CssDeclaration] for use in the evaluation step.
@@ -16,15 +18,23 @@ final class ModifiableCssDeclaration extends ModifiableCssNode
1618
final CssValue<String> name;
1719
final CssValue<Value> value;
1820
final bool parsedAsCustomProperty;
21+
final List<CssStyleRule> interleavedRules;
22+
final Trace? trace;
1923
final FileSpan valueSpanForMap;
2024
final FileSpan span;
2125

2226
bool get isCustomProperty => name.value.startsWith('--');
2327

2428
/// Returns a new CSS declaration with the given properties.
2529
ModifiableCssDeclaration(this.name, this.value, this.span,
26-
{required this.parsedAsCustomProperty, FileSpan? valueSpanForMap})
27-
: valueSpanForMap = valueSpanForMap ?? value.span {
30+
{required this.parsedAsCustomProperty,
31+
Iterable<CssStyleRule>? interleavedRules,
32+
this.trace,
33+
FileSpan? valueSpanForMap})
34+
: interleavedRules = interleavedRules == null
35+
? const []
36+
: List.unmodifiable(interleavedRules),
37+
valueSpanForMap = valueSpanForMap ?? value.span {
2838
if (parsedAsCustomProperty) {
2939
if (!isCustomProperty) {
3040
throw ArgumentError(

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import '../node.dart';
1313
/// modification should only be done within the evaluation step, so the
1414
/// unmodifiable types are used elsewhere to enforce that constraint.
1515
abstract base class ModifiableCssNode extends CssNode {
16-
/// The node that contains this, or `null` for the root [CssStylesheet] node.
1716
ModifiableCssParentNode? get parent => _parent;
1817
ModifiableCssParentNode? _parent;
1918

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import 'style_rule.dart';
1515
/// A statement in a plain CSS syntax tree.
1616
@sealed
1717
abstract class CssNode implements AstNode {
18+
/// The node that contains this, or `null` for the root [CssStylesheet] node.
19+
@internal
20+
CssParentNode? get parent;
21+
1822
/// Whether this was generated from the last node in a nested Sass tree that
1923
/// got flattened during evaluation.
2024
bool get isGroupEnd;

‎lib/src/ast/css/stylesheet.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'node.dart';
1313
///
1414
/// This is the root plain CSS node. It contains top-level statements.
1515
class CssStylesheet extends CssParentNode {
16+
CssParentNode? get parent => null;
1617
final List<CssNode> children;
1718
final FileSpan span;
1819
bool get isGroupEnd => false;

‎lib/src/async_compile.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ Future<CompileResult> _compileStylesheet(
173173
useSpaces: useSpaces,
174174
indentWidth: indentWidth,
175175
lineFeed: lineFeed,
176+
logger: logger,
176177
sourceMap: sourceMap,
177178
charset: charset);
178179

‎lib/src/compile.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// DO NOT EDIT. This file was generated from async_compile.dart.
66
// See tool/grind/synchronize.dart for details.
77
//
8-
// Checksum: 69b31749dc94c7f717e9d395327e4209c4d3feb0
8+
// Checksum: 4d72aeb3c39a2e607d1889755e07b7e489eddfa6
99
//
1010
// ignore_for_file: unused_import
1111

@@ -182,6 +182,7 @@ CompileResult _compileStylesheet(
182182
useSpaces: useSpaces,
183183
indentWidth: indentWidth,
184184
lineFeed: lineFeed,
185+
logger: logger,
185186
sourceMap: sourceMap,
186187
charset: charset);
187188

‎lib/src/visitor/async_evaluate.dart

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,20 +1198,43 @@ final class _EvaluateVisitor
11981198
node.span);
11991199
}
12001200

1201-
if (_parent.parent!.children.last case var sibling
1202-
when _parent != sibling) {
1203-
_warn(
1204-
"Sass's behavior for declarations that appear after nested\n"
1205-
"rules will be changing to match the behavior specified by CSS in an "
1206-
"upcoming\n"
1207-
"version. To keep the existing behavior, move the declaration above "
1208-
"the nested\n"
1209-
"rule. To opt into the new behavior, wrap the declaration in `& "
1210-
"{}`.\n"
1211-
"\n"
1212-
"More info: https://sass-lang.com/d/mixed-decls",
1213-
MultiSpan(node.span, 'declaration', {sibling.span: 'nested rule'}),
1214-
Deprecation.mixedDecls);
1201+
var siblings = _parent.parent!.children;
1202+
var interleavedRules = <CssStyleRule>[];
1203+
if (siblings.last != _parent &&
1204+
// Reproduce this condition from [_warn] so that we don't add anything to
1205+
// [interleavedRules] for declarations in dependencies.
1206+
!(_quietDeps &&
1207+
(_inDependency || (_currentCallable?.inDependency ?? false)))) {
1208+
loop:
1209+
for (var sibling in siblings.skip(siblings.indexOf(_parent) + 1)) {
1210+
switch (sibling) {
1211+
case CssComment():
1212+
continue loop;
1213+
1214+
case CssStyleRule rule:
1215+
interleavedRules.add(rule);
1216+
1217+
case _:
1218+
// Always warn for siblings that aren't style rules, because they
1219+
// add no specificity and they're nested in the same parent as this
1220+
// declaration.
1221+
_warn(
1222+
"Sass's behavior for declarations that appear after nested\n"
1223+
"rules will be changing to match the behavior specified by CSS "
1224+
"in an upcoming\n"
1225+
"version. To keep the existing behavior, move the declaration "
1226+
"above the nested\n"
1227+
"rule. To opt into the new behavior, wrap the declaration in "
1228+
"`& {}`.\n"
1229+
"\n"
1230+
"More info: https://sass-lang.com/d/mixed-decls",
1231+
MultiSpan(
1232+
node.span, 'declaration', {sibling.span: 'nested rule'}),
1233+
Deprecation.mixedDecls);
1234+
interleavedRules.clear();
1235+
break;
1236+
}
1237+
}
12151238
}
12161239

12171240
var name = await _interpolationToValue(node.name, warnForColor: true);
@@ -1227,6 +1250,8 @@ final class _EvaluateVisitor
12271250
_parent.addChild(ModifiableCssDeclaration(
12281251
name, CssValue(value, expression.span), node.span,
12291252
parsedAsCustomProperty: node.isCustomProperty,
1253+
interleavedRules: interleavedRules,
1254+
trace: interleavedRules.isEmpty ? null : _stackTrace(node.span),
12301255
valueSpanForMap:
12311256
_sourceMap ? node.value.andThen(_expressionNode)?.span : null));
12321257
} else if (name.value.startsWith('--')) {

‎lib/src/visitor/evaluate.dart

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// DO NOT EDIT. This file was generated from async_evaluate.dart.
66
// See tool/grind/synchronize.dart for details.
77
//
8-
// Checksum: de25b9055a73f1c7ebe7a707139e6c789a2866dd
8+
// Checksum: ca67afd2df1c970eb887d4a24c7fe838c2aaec60
99
//
1010
// ignore_for_file: unused_import
1111

@@ -1196,20 +1196,43 @@ final class _EvaluateVisitor
11961196
node.span);
11971197
}
11981198

1199-
if (_parent.parent!.children.last case var sibling
1200-
when _parent != sibling) {
1201-
_warn(
1202-
"Sass's behavior for declarations that appear after nested\n"
1203-
"rules will be changing to match the behavior specified by CSS in an "
1204-
"upcoming\n"
1205-
"version. To keep the existing behavior, move the declaration above "
1206-
"the nested\n"
1207-
"rule. To opt into the new behavior, wrap the declaration in `& "
1208-
"{}`.\n"
1209-
"\n"
1210-
"More info: https://sass-lang.com/d/mixed-decls",
1211-
MultiSpan(node.span, 'declaration', {sibling.span: 'nested rule'}),
1212-
Deprecation.mixedDecls);
1199+
var siblings = _parent.parent!.children;
1200+
var interleavedRules = <CssStyleRule>[];
1201+
if (siblings.last != _parent &&
1202+
// Reproduce this condition from [_warn] so that we don't add anything to
1203+
// [interleavedRules] for declarations in dependencies.
1204+
!(_quietDeps &&
1205+
(_inDependency || (_currentCallable?.inDependency ?? false)))) {
1206+
loop:
1207+
for (var sibling in siblings.skip(siblings.indexOf(_parent) + 1)) {
1208+
switch (sibling) {
1209+
case CssComment():
1210+
continue loop;
1211+
1212+
case CssStyleRule rule:
1213+
interleavedRules.add(rule);
1214+
1215+
case _:
1216+
// Always warn for siblings that aren't style rules, because they
1217+
// add no specificity and they're nested in the same parent as this
1218+
// declaration.
1219+
_warn(
1220+
"Sass's behavior for declarations that appear after nested\n"
1221+
"rules will be changing to match the behavior specified by CSS "
1222+
"in an upcoming\n"
1223+
"version. To keep the existing behavior, move the declaration "
1224+
"above the nested\n"
1225+
"rule. To opt into the new behavior, wrap the declaration in "
1226+
"`& {}`.\n"
1227+
"\n"
1228+
"More info: https://sass-lang.com/d/mixed-decls",
1229+
MultiSpan(
1230+
node.span, 'declaration', {sibling.span: 'nested rule'}),
1231+
Deprecation.mixedDecls);
1232+
interleavedRules.clear();
1233+
break;
1234+
}
1235+
}
12131236
}
12141237

12151238
var name = _interpolationToValue(node.name, warnForColor: true);
@@ -1225,6 +1248,8 @@ final class _EvaluateVisitor
12251248
_parent.addChild(ModifiableCssDeclaration(
12261249
name, CssValue(value, expression.span), node.span,
12271250
parsedAsCustomProperty: node.isCustomProperty,
1251+
interleavedRules: interleavedRules,
1252+
trace: interleavedRules.isEmpty ? null : _stackTrace(node.span),
12281253
valueSpanForMap:
12291254
_sourceMap ? node.value.andThen(_expressionNode)?.span : null));
12301255
} else if (name.value.startsWith('--')) {

‎lib/src/visitor/serialize.dart

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,21 @@ import 'dart:math' as math;
66
import 'dart:typed_data';
77

88
import 'package:charcode/charcode.dart';
9+
import 'package:collection/collection.dart';
910
import 'package:source_maps/source_maps.dart';
1011
import 'package:string_scanner/string_scanner.dart';
1112

1213
import '../ast/css.dart';
1314
import '../ast/node.dart';
1415
import '../ast/selector.dart';
1516
import '../color_names.dart';
17+
import '../deprecation.dart';
1618
import '../exception.dart';
19+
import '../logger.dart';
1720
import '../parse/parser.dart';
1821
import '../utils.dart';
1922
import '../util/character.dart';
23+
import '../util/multi_span.dart';
2024
import '../util/no_source_map_buffer.dart';
2125
import '../util/nullable.dart';
2226
import '../util/number.dart';
@@ -48,6 +52,7 @@ SerializeResult serialize(CssNode node,
4852
bool useSpaces = true,
4953
int? indentWidth,
5054
LineFeed? lineFeed,
55+
Logger? logger,
5156
bool sourceMap = false,
5257
bool charset = true}) {
5358
indentWidth ??= 2;
@@ -57,6 +62,7 @@ SerializeResult serialize(CssNode node,
5762
useSpaces: useSpaces,
5863
indentWidth: indentWidth,
5964
lineFeed: lineFeed,
65+
logger: logger,
6066
sourceMap: sourceMap);
6167
node.accept(visitor);
6268
var css = visitor._buffer.toString();
@@ -128,6 +134,12 @@ final class _SerializeVisitor
128134
/// The characters to use for a line feed.
129135
final LineFeed _lineFeed;
130136

137+
/// The logger to use to print warnings.
138+
///
139+
/// This should only be used for statement-level serialization. It's not
140+
/// guaranteed to be the main user-provided logger for expressions.
141+
final Logger _logger;
142+
131143
/// Whether we're emitting compressed output.
132144
bool get _isCompressed => _style == OutputStyle.compressed;
133145

@@ -138,14 +150,16 @@ final class _SerializeVisitor
138150
bool useSpaces = true,
139151
int? indentWidth,
140152
LineFeed? lineFeed,
153+
Logger? logger,
141154
bool sourceMap = true})
142155
: _buffer = sourceMap ? SourceMapBuffer() : NoSourceMapBuffer(),
143156
_style = style ?? OutputStyle.expanded,
144157
_inspect = inspect,
145158
_quote = quote,
146159
_indentCharacter = useSpaces ? $space : $tab,
147160
_indentWidth = indentWidth ?? 2,
148-
_lineFeed = lineFeed ?? LineFeed.lf {
161+
_lineFeed = lineFeed ?? LineFeed.lf,
162+
_logger = logger ?? const Logger.stderr() {
149163
RangeError.checkValueInInterval(_indentWidth, 0, 10, "indentWidth");
150164
}
151165

@@ -329,6 +343,33 @@ final class _SerializeVisitor
329343
}
330344

331345
void visitCssDeclaration(CssDeclaration node) {
346+
if (node.interleavedRules.isNotEmpty) {
347+
var declSpecificities = _specificities(node.parent!);
348+
for (var rule in node.interleavedRules) {
349+
var ruleSpecificities = _specificities(rule);
350+
351+
// If the declaration can never match with the same specificity as one
352+
// of its sibling rules, then ordering will never matter and there's no
353+
// need to warn about the declaration being re-ordered.
354+
if (!declSpecificities.any(ruleSpecificities.contains)) continue;
355+
356+
_logger.warnForDeprecation(
357+
Deprecation.mixedDecls,
358+
"Sass's behavior for declarations that appear after nested\n"
359+
"rules will be changing to match the behavior specified by CSS in an "
360+
"upcoming\n"
361+
"version. To keep the existing behavior, move the declaration above "
362+
"the nested\n"
363+
"rule. To opt into the new behavior, wrap the declaration in `& "
364+
"{}`.\n"
365+
"\n"
366+
"More info: https://sass-lang.com/d/mixed-decls",
367+
span:
368+
MultiSpan(node.span, 'declaration', {rule.span: 'nested rule'}),
369+
trace: node.trace);
370+
}
371+
}
372+
332373
_writeIndentation();
333374

334375
_write(node.name);
@@ -363,6 +404,22 @@ final class _SerializeVisitor
363404
}
364405
}
365406

407+
/// Returns the set of possible specificities which which [node] might match.
408+
Set<int> _specificities(CssParentNode node) {
409+
if (node case CssStyleRule rule) {
410+
// Plain CSS style rule nesting implicitly wraps parent selectors in
411+
// `:is()`, so they all match with the highest specificity among any of
412+
// them.
413+
var parent = node.parent.andThen(_specificities)?.max ?? 0;
414+
return {
415+
for (var selector in rule.selector.components)
416+
parent + selector.specificity
417+
};
418+
} else {
419+
return node.parent.andThen(_specificities) ?? const {0};
420+
}
421+
}
422+
366423
/// Emits the value of [node], with all newlines followed by whitespace
367424
void _writeFoldedValue(CssDeclaration node) {
368425
var scanner = StringScanner((node.value.value as SassString).text);

0 commit comments

Comments
 (0)