From e6ce2cc2edb27d1eb654bf82ded2795c998d8b33 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 17 Aug 2022 18:14:08 -0700 Subject: [PATCH 1/2] Update specificity calculation for selector pseudos This is very close to invisible to the user and actually making it visible would require a complex and hard-to-read test, so I'm electing to avoid testing it. Closes #2528 --- CHANGELOG.md | 2 + lib/src/ast/selector/complex.dart | 38 +++----------- lib/src/ast/selector/compound.dart | 38 +++----------- lib/src/ast/selector/id.dart | 2 +- lib/src/ast/selector/pseudo.dart | 77 ++++++++++------------------- lib/src/ast/selector/simple.dart | 13 +---- lib/src/ast/selector/type.dart | 2 +- lib/src/ast/selector/universal.dart | 2 +- lib/src/extend/extender.dart | 4 +- lib/src/extend/extension.dart | 6 +-- lib/src/extend/extension_store.dart | 6 +-- lib/src/extend/functions.dart | 6 +-- pkg/sass_api/CHANGELOG.md | 5 ++ pkg/sass_api/pubspec.yaml | 2 +- pubspec.yaml | 2 +- 15 files changed, 60 insertions(+), 145 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c6c613b0..38e5e84c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ * Properly consider `b > c` to be a superselector of `a > b > c`, and similarly for other combinators. +* Properly calculate specificity for selector pseudoclasses. + ## 1.54.4 * Improve error messages when passing incorrect units that are also diff --git a/lib/src/ast/selector/complex.dart b/lib/src/ast/selector/complex.dart index ac4312ff6..e5eb6cd25 100644 --- a/lib/src/ast/selector/complex.dart +++ b/lib/src/ast/selector/complex.dart @@ -45,27 +45,13 @@ class ComplexSelector extends Selector { @internal final bool lineBreak; - /// The minimum possible specificity that this selector can have. + /// This selector's specificity. /// - /// Pseudo selectors that contain selectors, like `:not()` and `:matches()`, - /// can have a range of possible specificities. - int get minSpecificity { - if (_minSpecificity == null) _computeSpecificity(); - return _minSpecificity!; - } - - int? _minSpecificity; - - /// The maximum possible specificity that this selector can have. - /// - /// Pseudo selectors that contain selectors, like `:not()` and `:matches()`, - /// can have a range of possible specificities. - int get maxSpecificity { - if (_maxSpecificity == null) _computeSpecificity(); - return _maxSpecificity!; - } - - int? _maxSpecificity; + /// Specificity is represented in base 1000. The spec says this should be + /// "sufficiently high"; it's extremely unlikely that any single selector + /// sequence will contain 1000 simple selectors. + late final int specificity = components.fold( + 0, (sum, component) => sum + component.selector.specificity); /// If this compound selector is composed of a single compound selector with /// no combinators, returns it. @@ -115,18 +101,6 @@ class ComplexSelector extends Selector { other.leadingCombinators.isEmpty && complexIsSuperselector(components, other.components); - /// Computes [_minSpecificity] and [_maxSpecificity]. - void _computeSpecificity() { - var minSpecificity = 0; - var maxSpecificity = 0; - for (var component in components) { - minSpecificity += component.selector.minSpecificity; - maxSpecificity += component.selector.maxSpecificity; - } - _minSpecificity = minSpecificity; - _maxSpecificity = maxSpecificity; - } - /// Returns a copy of `this` with [combinators] added to the end of the final /// component in [components]. /// diff --git a/lib/src/ast/selector/compound.dart b/lib/src/ast/selector/compound.dart index d4361a465..1c3905154 100644 --- a/lib/src/ast/selector/compound.dart +++ b/lib/src/ast/selector/compound.dart @@ -25,27 +25,13 @@ class CompoundSelector extends Selector { /// This is never empty. final List components; - /// The minimum possible specificity that this selector can have. + /// This selector's specificity. /// - /// Pseudo selectors that contain selectors, like `:not()` and `:matches()`, - /// can have a range of possible specificities. - int get minSpecificity { - if (_minSpecificity == null) _computeSpecificity(); - return _minSpecificity!; - } - - int? _minSpecificity; - - /// The maximum possible specificity that this selector can have. - /// - /// Pseudo selectors that contain selectors, like `:not()` and `:matches()`, - /// can have a range of possible specificities. - int get maxSpecificity { - if (_maxSpecificity == null) _computeSpecificity(); - return _maxSpecificity!; - } - - int? _maxSpecificity; + /// Specificity is represented in base 1000. The spec says this should be + /// "sufficiently high"; it's extremely unlikely that any single selector + /// sequence will contain 1000 simple selectors. + late final int specificity = + components.fold(0, (sum, component) => sum + component.specificity); /// If this compound selector is composed of a single simple selector, returns /// it. @@ -87,18 +73,6 @@ class CompoundSelector extends Selector { bool isSuperselector(CompoundSelector other) => compoundIsSuperselector(this, other); - /// Computes [_minSpecificity] and [_maxSpecificity]. - void _computeSpecificity() { - var minSpecificity = 0; - var maxSpecificity = 0; - for (var simple in components) { - minSpecificity += simple.minSpecificity; - maxSpecificity += simple.maxSpecificity; - } - _minSpecificity = minSpecificity; - _maxSpecificity = maxSpecificity; - } - int get hashCode => listHash(components); bool operator ==(Object other) => diff --git a/lib/src/ast/selector/id.dart b/lib/src/ast/selector/id.dart index 1045f48ad..010bd2161 100644 --- a/lib/src/ast/selector/id.dart +++ b/lib/src/ast/selector/id.dart @@ -19,7 +19,7 @@ class IDSelector extends SimpleSelector { /// The ID name this selects for. final String name; - int get minSpecificity => math.pow(super.minSpecificity, 2) as int; + int get specificity => math.pow(super.specificity, 2) as int; IDSelector(this.name); diff --git a/lib/src/ast/selector/pseudo.dart b/lib/src/ast/selector/pseudo.dart index dc9b36287..7840eccab 100644 --- a/lib/src/ast/selector/pseudo.dart +++ b/lib/src/ast/selector/pseudo.dart @@ -2,9 +2,8 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import 'dart:math' as math; - import 'package:charcode/charcode.dart'; +import 'package:collection/collection.dart'; import 'package:meta/meta.dart'; import '../../utils.dart'; @@ -80,19 +79,30 @@ class PseudoSelector extends SimpleSelector { /// both non-`null`, the selector follows the argument. final SelectorList? selector; - int get minSpecificity { - if (_minSpecificity == null) _computeSpecificity(); - return _minSpecificity!; - } - - int? _minSpecificity; - - int get maxSpecificity { - if (_maxSpecificity == null) _computeSpecificity(); - return _maxSpecificity!; - } - - int? _maxSpecificity; + late final int specificity = () { + if (isElement) return 1; + var selector = this.selector; + if (selector == null) return super.specificity; + + // https://drafts.csswg.org/selectors/#specificity-rules + switch (normalizedName) { + case 'where': + return 0; + case 'is': + case 'not': + case 'has': + case 'matches': + return selector.components + .map((component) => component.specificity) + .max; + case 'nth-child': + case 'nth-last-child': + return super.specificity + + selector.components.map((component) => component.specificity).max; + default: + return super.specificity; + } + }(); PseudoSelector(this.name, {bool element = false, this.argument, this.selector}) @@ -193,43 +203,6 @@ class PseudoSelector extends SimpleSelector { return CompoundSelector([this]).isSuperselector(CompoundSelector([other])); } - /// Computes [_minSpecificity] and [_maxSpecificity]. - void _computeSpecificity() { - if (isElement) { - _minSpecificity = 1; - _maxSpecificity = 1; - return; - } - - var selector = this.selector; - if (selector == null) { - _minSpecificity = super.minSpecificity; - _maxSpecificity = super.maxSpecificity; - return; - } - - if (name == 'not') { - var minSpecificity = 0; - var maxSpecificity = 0; - for (var complex in selector.components) { - minSpecificity = math.max(minSpecificity, complex.minSpecificity); - maxSpecificity = math.max(maxSpecificity, complex.maxSpecificity); - } - _minSpecificity = minSpecificity; - _maxSpecificity = maxSpecificity; - } else { - // This is higher than any selector's specificity can actually be. - var minSpecificity = math.pow(super.minSpecificity, 3) as int; - var maxSpecificity = 0; - for (var complex in selector.components) { - minSpecificity = math.min(minSpecificity, complex.minSpecificity); - maxSpecificity = math.max(maxSpecificity, complex.maxSpecificity); - } - _minSpecificity = minSpecificity; - _maxSpecificity = maxSpecificity; - } - } - T accept(SelectorVisitor visitor) => visitor.visitPseudoSelector(this); // This intentionally uses identity for the selector list, if one is available. diff --git a/lib/src/ast/selector/simple.dart b/lib/src/ast/selector/simple.dart index 45b8d4de1..599b43c8e 100644 --- a/lib/src/ast/selector/simple.dart +++ b/lib/src/ast/selector/simple.dart @@ -27,21 +27,12 @@ final _subselectorPseudos = { /// {@category AST} /// {@category Parsing} abstract class SimpleSelector extends Selector { - /// The minimum possible specificity that this selector can have. - /// - /// Pseudo selectors that contain selectors, like `:not()` and `:matches()`, - /// can have a range of possible specificities. + /// This selector's specificity. /// /// Specificity is represented in base 1000. The spec says this should be /// "sufficiently high"; it's extremely unlikely that any single selector /// sequence will contain 1000 simple selectors. - int get minSpecificity => 1000; - - /// The maximum possible specificity that this selector can have. - /// - /// Pseudo selectors that contain selectors, like `:not()` and `:matches()`, - /// can have a range of possible specificities. - int get maxSpecificity => minSpecificity; + int get specificity => 1000; SimpleSelector(); diff --git a/lib/src/ast/selector/type.dart b/lib/src/ast/selector/type.dart index 5f075f431..0430de768 100644 --- a/lib/src/ast/selector/type.dart +++ b/lib/src/ast/selector/type.dart @@ -18,7 +18,7 @@ class TypeSelector extends SimpleSelector { /// The element name being selected. final QualifiedName name; - int get minSpecificity => 1; + int get specificity => 1; TypeSelector(this.name); diff --git a/lib/src/ast/selector/universal.dart b/lib/src/ast/selector/universal.dart index c283ca236..2937a78f6 100644 --- a/lib/src/ast/selector/universal.dart +++ b/lib/src/ast/selector/universal.dart @@ -21,7 +21,7 @@ class UniversalSelector extends SimpleSelector { /// Otherwise, it matches all elements in the given namespace. final String? namespace; - int get minSpecificity => 0; + int get specificity => 0; UniversalSelector({this.namespace}); diff --git a/lib/src/extend/extender.dart b/lib/src/extend/extender.dart index 5f2b48ae7..441ca5fd7 100644 --- a/lib/src/extend/extender.dart +++ b/lib/src/extend/extender.dart @@ -32,10 +32,10 @@ class Extender { /// Creates a new extender. /// - /// If [specificity] isn't passed, it defaults to `extender.maxSpecificity`. + /// If [specificity] isn't passed, it defaults to `extender.specificity`. Extender(this.selector, this.span, {this.mediaContext, int? specificity, bool original = false}) - : specificity = specificity ?? selector.maxSpecificity, + : specificity = specificity ?? selector.specificity, isOriginal = original; /// Asserts that the [mediaContext] for a selector is compatible with the diff --git a/lib/src/extend/extension.dart b/lib/src/extend/extension.dart index 0c3afe61a..96a901e1f 100644 --- a/lib/src/extend/extension.dart +++ b/lib/src/extend/extension.dart @@ -34,8 +34,6 @@ class Extension { final FileSpan span; /// Creates a new extension. - /// - /// If [specificity] isn't passed, it defaults to `extender.maxSpecificity`. Extension( ComplexSelector extender, FileSpan extenderSpan, this.target, this.span, {this.mediaContext, bool optional = false}) @@ -77,9 +75,9 @@ class Extender { /// Creates a new extender. /// - /// If [specificity] isn't passed, it defaults to `extender.maxSpecificity`. + /// If [specificity] isn't passed, it defaults to `extender.specificity`. Extender(this.selector, this.span, {int? specificity, bool original = false}) - : specificity = specificity ?? selector.maxSpecificity, + : specificity = specificity ?? selector.specificity, isOriginal = original; /// Asserts that the [mediaContext] for a selector is compatible with the diff --git a/lib/src/extend/extension_store.dart b/lib/src/extend/extension_store.dart index 82d6a9f1a..cfeae78a4 100644 --- a/lib/src/extend/extension_store.dart +++ b/lib/src/extend/extension_store.dart @@ -261,7 +261,7 @@ class ExtensionStore { _extensionsByExtender.putIfAbsent(simple, () => []).add(extension); // Only source specificity for the original selector is relevant. // Selectors generated by `@extend` don't get new specificity. - _sourceSpecificity.putIfAbsent(simple, () => complex.maxSpecificity); + _sourceSpecificity.putIfAbsent(simple, () => complex.specificity); } if (selectors != null || existingExtensions != null) { @@ -970,13 +970,13 @@ class ExtensionStore { // trimmed, and thus that if there are two identical selectors only one is // trimmed. if (result.any((complex2) => - complex2.minSpecificity >= maxSpecificity && + complex2.specificity >= maxSpecificity && complex2.isSuperselector(complex1))) { continue; } if (selectors.take(i).any((complex2) => - complex2.minSpecificity >= maxSpecificity && + complex2.specificity >= maxSpecificity && complex2.isSuperselector(complex1))) { continue; } diff --git a/lib/src/extend/functions.dart b/lib/src/extend/functions.dart index a1d392f45..ee1ca99b8 100644 --- a/lib/src/extend/functions.dart +++ b/lib/src/extend/functions.dart @@ -657,10 +657,8 @@ bool complexIsSuperselector(List complex1, if (combinator1 == Combinator.followingSibling) { // The selector `.foo ~ .bar` is only a superselector of selectors that // *exclusively* contain subcombinators of `~`. - if (!complex2 - .take(complex2.length - 1) - .skip(i2) - .every((component) => _isSupercombinator( + if (!complex2.take(complex2.length - 1).skip(i2).every((component) => + _isSupercombinator( combinator1, component.combinators.firstOrNull))) { return false; } diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 1cc917aac..6dcd08952 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,8 @@ +## 3.0.0 + +* Replace the `minSpecificity` and `maxSpecificity` fields on `ComplexSelector`, + `CompoundSelector`, and `SimpleSelector` with a single `specificity` field. + ## 2.0.5 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index 0e21ebbc9..313036779 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ 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: 2.0.5 +version: 3.0.0 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass diff --git a/pubspec.yaml b/pubspec.yaml index 26652ac7a..6b0dfeea0 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -15,7 +15,7 @@ dependencies: async: ^2.5.0 charcode: ^1.2.0 cli_repl: ^0.2.1 - collection: ^1.15.0 + collection: ^1.16.0 meta: ^1.3.0 node_interop: ^2.1.0 js: ^0.6.3 From 895f15c12e62596b00233864d135999186bd8895 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 18 Aug 2022 14:27:23 -0700 Subject: [PATCH 2/2] Code review --- pkg/sass_api/CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 6dcd08952..3ea3bfb4d 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -3,10 +3,6 @@ * Replace the `minSpecificity` and `maxSpecificity` fields on `ComplexSelector`, `CompoundSelector`, and `SimpleSelector` with a single `specificity` field. -## 2.0.5 - -* No user-visible changes. - ## 2.0.4 * No user-visible changes.