From f2a34408ec45f0373ace9504295055c8435c9dea Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 7 Feb 2019 16:09:40 -0800 Subject: [PATCH] Verify mandatory extensions --- lib/src/extend/extender.dart | 50 ++++++++++++++------ lib/src/extend/extension.dart | 54 +++++++-------------- lib/src/extend/merged_extension.dart | 70 ++++++++++++++++++++++++++++ lib/src/utils.dart | 6 +++ lib/src/visitor/async_evaluate.dart | 49 ++++++++++++++++++- lib/src/visitor/evaluate.dart | 51 +++++++++++++++++++- 6 files changed, 225 insertions(+), 55 deletions(-) create mode 100644 lib/src/extend/merged_extension.dart diff --git a/lib/src/extend/extender.dart b/lib/src/extend/extender.dart index 5efffce6..50984ab6 100644 --- a/lib/src/extend/extender.dart +++ b/lib/src/extend/extender.dart @@ -14,6 +14,7 @@ import '../ast/sass.dart'; import '../exception.dart'; import '../utils.dart'; import 'extension.dart'; +import 'merged_extension.dart'; import 'functions.dart'; import 'mode.dart'; @@ -106,10 +107,37 @@ class Extender { return selector; } + /// The set of all simple selectors in style rules handled by this extender. + /// + /// This includes simple selectors that were added because of downstream + /// extensions. + Set get simpleSelectors => MapKeySet(_selectors); + Extender() : _mode = ExtendMode.normal; Extender._(this._mode); + /// Returns all mandatory extensions in this extender for whose targets + /// [callback] returns `true`. + /// + /// This un-merges any [MergedExtension] so only base [Extension]s are + /// returned. + Iterable extensionsWhereTarget( + bool callback(SimpleSelector target)) sync* { + for (var target in _extensions.keys) { + if (!callback(target)) continue; + for (var extension in _extensions[target].values) { + if (extension is MergedExtension) { + yield* extension + .unmerge() + .where((extension) => !extension.isOptional); + } else if (!extension.isOptional) { + yield extension; + } + } + } + } + /// Adds [selector] to this extender, with [selectorSpan] as the span covering /// the selector and [ruleSpan] as the span covering the entire style rule. /// @@ -184,19 +212,19 @@ class Extender { Map newExtensions; var sources = _extensions.putIfAbsent(target, () => {}); for (var complex in extender.value.components) { + var state = Extension( + complex, target, extender.span, extend.span, mediaContext, + optional: extend.isOptional); + var existingState = sources[complex]; if (existingState != null) { // 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 // mandatory, though. - existingState.addSource(extend.span, mediaContext, - optional: extend.isOptional); + sources[complex] = MergedExtension.merge(existingState, state); continue; } - var state = Extension( - complex, target, extender.span, extend.span, mediaContext, - optional: extend.isOptional); sources[complex] = state; for (var component in complex.components) { @@ -279,12 +307,12 @@ class Extender { continue; } + var withExtender = extension.withExtender(complex); var existingExtension = sources[complex]; if (existingExtension != null) { - existingExtension.addSource(extension.span, extension.mediaContext, - optional: extension.isOptional); + sources[complex] = + MergedExtension.merge(existingExtension, withExtender); } else { - var withExtender = extension.withExtender(complex); sources[complex] = withExtender; for (var component in complex.components) { @@ -412,12 +440,6 @@ class Extender { } } - /// Throws a [SassException] if any (non-optional) extensions failed to match - /// any selectors. - void finalize() { - // TODO(nweiz): Do this across modules. - } - /// Extends [list] using [extensions]. SelectorList _extendList( SelectorList list, diff --git a/lib/src/extend/extension.dart b/lib/src/extend/extension.dart index 6ad33d37..7f289f83 100644 --- a/lib/src/extend/extension.dart +++ b/lib/src/extend/extension.dart @@ -27,8 +27,7 @@ class Extension { final int specificity; /// Whether this extension is optional. - bool get isOptional => _isOptional; - bool _isOptional; + final bool isOptional; /// Whether this is a one-off extender representing a selector that was /// originally in the document, rather than one defined with `@extend`. @@ -36,8 +35,7 @@ class Extension { /// The media query context to which this extend is restricted, or `null` if /// it can apply within any context. - List get mediaContext => _mediaContext; - List _mediaContext; + final List mediaContext; /// The span in which [extender] was defined. /// @@ -48,18 +46,17 @@ class Extension { /// /// If any extend rule for this is extension is mandatory, this is guaranteed /// to be a span for a mandatory rule. - FileSpan get span => _span; - FileSpan _span; + final FileSpan span; /// Creates a new extension. /// /// If [specificity] isn't passed, it defaults to `extender.maxSpecificity`. - Extension(ComplexSelector extender, this.target, this.extenderSpan, - this._span, this._mediaContext, + Extension(ComplexSelector extender, this.target, this.extenderSpan, this.span, + this.mediaContext, {int specificity, bool optional = false}) : extender = extender, specificity = specificity ?? extender.maxSpecificity, - _isOptional = optional, + isOptional = optional, isOriginal = false; /// Creates a one-off extension that's not intended to be modified over time. @@ -71,44 +68,25 @@ class Extension { target = null, extenderSpan = null, specificity = specificity ?? extender.maxSpecificity, - _isOptional = true, - _mediaContext = null, - _span = null; + isOptional = true, + mediaContext = null, + span = null; /// Asserts that the [mediaContext] for a selector is compatible with the /// query context for this extender. void assertCompatibleMediaContext(List mediaContext) { - if (_mediaContext == null) return; - if (mediaContext != null && listEquals(_mediaContext, mediaContext)) return; + if (this.mediaContext == null) return; + if (mediaContext != null && listEquals(this.mediaContext, mediaContext)) + return; throw SassException( - "You may not @extend selectors across media queries.", _span); - } - - /// Indicates that the stylesheet contains another `@extend` with the same - /// source and target selectors, and the given [span] and [mediaContext]. - void addSource(FileSpan span, List mediaContext, - {bool optional = false}) { - if (mediaContext != null) { - if (_mediaContext == null) { - _mediaContext = mediaContext; - } else if (!listEquals(_mediaContext, mediaContext)) { - throw SassException( - "From ${_span.message('')}\n" - "You may not @extend the same selector from within different media " - "queries.", - span); - } - } - - if (optional || !_isOptional) return; - _span = span; - _isOptional = false; + "You may not @extend selectors across media queries.", span); } Extension withExtender(ComplexSelector newExtender) => - Extension(newExtender, target, extenderSpan, _span, _mediaContext, + Extension(newExtender, target, extenderSpan, span, mediaContext, specificity: specificity, optional: isOptional); - String toString() => extender.toString(); + String toString() => + "$extender {@extend $target${isOptional ? ' !optional' : ''}}"; } diff --git a/lib/src/extend/merged_extension.dart b/lib/src/extend/merged_extension.dart new file mode 100644 index 00000000..05fbbb21 --- /dev/null +++ b/lib/src/extend/merged_extension.dart @@ -0,0 +1,70 @@ +// Copyright 2019 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import '../exception.dart'; +import '../utils.dart'; +import 'extension.dart'; + +/// An [Extension] created by merging two [Extension]s with the same extender +/// and target. +/// +/// This is used when multiple mandatory extensions exist to ensure that both of +/// them are marked as resolved. +class MergedExtension extends Extension { + /// One of the merged extensions. + final Extension left; + + /// The other merged extension. + final Extension right; + + /// Returns an extension that combines [left] and [right]. + /// + /// Throws a [SassException] if [left] and [right] have incompatible media + /// contexts. + /// + /// Throws an [ArgumentError] if [left] and [right] don't have the same + /// extender and target. + static Extension merge(Extension left, Extension right) { + if (left.extender != right.extender || left.target != right.target) { + throw ArgumentError("$left and $right aren't the same extension."); + } + + if (left.mediaContext != null && + right.mediaContext != null && + !listEquals(left.mediaContext, right.mediaContext)) { + throw SassException( + "From ${left.span.message('')}\n" + "You may not @extend the same selector from within different media " + "queries.", + right.span); + } + + // If one extension is optional and doesn't add a special media context, it + // doesn't need to be merged. + if (right.isOptional && right.mediaContext == null) return left; + if (left.isOptional && left.mediaContext == null) return right; + + return MergedExtension._(left, right); + } + + MergedExtension._(this.left, this.right) + : super(left.extender, left.target, left.extenderSpan, left.span, + left.mediaContext ?? right.mediaContext, + specificity: left.specificity, optional: true); + + /// Returns all leaf-node [Extension]s in the tree or [MergedExtension]s. + Iterable unmerge() sync* { + if (left is MergedExtension) { + yield* (left as MergedExtension).unmerge(); + } else { + yield left; + } + + if (right is MergedExtension) { + yield* (right as MergedExtension).unmerge(); + } else { + yield right; + } + } +} diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 8b8075d9..bd4dcf9a 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -133,6 +133,12 @@ List flattenVertically(Iterable> iterable) { return result; } +/// Returns the first element of [iterable], or `null` if the iterable is empty. +T firstOrNull(Iterable iterable) { + var iterator = iterable.iterator; + return iterator.moveNext() ? iterator.current : null; +} + /// Converts [codepointIndex] to a code unit index, relative to [string]. /// /// A codepoint index is the index in pure Unicode codepoints; a code unit index diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index 3e904701..42a747ea 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -7,6 +7,7 @@ import 'dart:math' as math; import 'package:charcode/charcode.dart'; import 'package:collection/collection.dart'; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:source_span/source_span.dart'; import 'package:stack_trace/stack_trace.dart'; @@ -24,6 +25,7 @@ import '../callable.dart'; import '../color_names.dart'; import '../exception.dart'; import '../extend/extender.dart'; +import '../extend/extension.dart'; import '../importer.dart'; import '../importer/node.dart'; import '../importer/utils.dart'; @@ -415,7 +417,18 @@ class _EvaluateVisitor /// /// This also applies each module's extensions to its upstream modules. CssStylesheet _combineCss(AsyncModule root) { - if (root.upstream.isEmpty) return root.css; + // TODO(nweiz): short-circuit if no upstream modules (transitively) include + // any CSS. + if (root.upstream.isEmpty) { + var selectors = root.extender.simpleSelectors; + var unsatisfiedExtension = firstOrNull(root.extender + .extensionsWhereTarget((target) => !selectors.contains(target))); + if (unsatisfiedExtension != null) { + _throwForUnsatisfiedExtension(unsatisfiedExtension); + } + + return root.css; + } var sortedModules = _topologicalModules(root); _extendModules(sortedModules); @@ -446,7 +459,22 @@ class _EvaluateVisitor // and we can use them to extend that module. var downstreamExtenders = >{}; + /// Extensions that haven't yet been satisfied by some upstream module. This + /// adds extensions when they're defined but not satisfied, and removes them + /// when they're satisfied by any module. + var unsatisfiedExtensions = Set.identity(); + for (var module in sortedModules) { + // Create a snapshot of the simple selectors currently in the extender so + // that we don't consider an extension "satisfied" below because of a + // simple selector added by another (sibling) extension. + var originalSelectors = module.extender.simpleSelectors.toSet(); + + // Add all as-yet-unsatisfied extensions before adding downstream + // extenders, because those are all in [unsatisfiedExtensions] already. + unsatisfiedExtensions.addAll(module.extender.extensionsWhereTarget( + (target) => !originalSelectors.contains(target))); + var extenders = downstreamExtenders[module]; if (extenders != null) module.extender.addExtensions(extenders); if (module.extender.isEmpty) continue; @@ -456,7 +484,26 @@ class _EvaluateVisitor .putIfAbsent(upstream, () => []) .add(module.extender); } + + // Remove all extensions that are now satisfied after adding downstream + // extenders so it counts any downstream extensions that have been newly + // satisfied. + unsatisfiedExtensions.removeAll( + module.extender.extensionsWhereTarget(originalSelectors.contains)); } + + if (unsatisfiedExtensions.isNotEmpty) { + _throwForUnsatisfiedExtension(unsatisfiedExtensions.first); + } + } + + /// Throws an exception indicating that [extension] is unsatisfied. + @alwaysThrows + void _throwForUnsatisfiedExtension(Extension extension) { + throw SassException( + 'The target selector was not found.\n' + 'Use "@extend ${extension.target} !optional" to avoid this error.', + extension.span); } /// Returns all modules transitively used by [root] in topological order, diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 145ae2d4..2bbf6947 100644 --- a/lib/src/visitor/evaluate.dart +++ b/lib/src/visitor/evaluate.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_evaluate.dart. // See tool/synchronize.dart for details. // -// Checksum: d60a4b3395a8abc00190edb5380481914680a281 +// Checksum: f2cf0b954a83f5a986d490155ef77c6a6b3e63f5 // // ignore_for_file: unused_import @@ -16,6 +16,7 @@ import 'dart:math' as math; import 'package:charcode/charcode.dart'; import 'package:collection/collection.dart'; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:source_span/source_span.dart'; import 'package:stack_trace/stack_trace.dart'; @@ -33,6 +34,7 @@ import '../callable.dart'; import '../color_names.dart'; import '../exception.dart'; import '../extend/extender.dart'; +import '../extend/extension.dart'; import '../importer.dart'; import '../importer/node.dart'; import '../importer/utils.dart'; @@ -421,7 +423,18 @@ class _EvaluateVisitor /// /// This also applies each module's extensions to its upstream modules. CssStylesheet _combineCss(Module root) { - if (root.upstream.isEmpty) return root.css; + // TODO(nweiz): short-circuit if no upstream modules (transitively) include + // any CSS. + if (root.upstream.isEmpty) { + var selectors = root.extender.simpleSelectors; + var unsatisfiedExtension = firstOrNull(root.extender + .extensionsWhereTarget((target) => !selectors.contains(target))); + if (unsatisfiedExtension != null) { + _throwForUnsatisfiedExtension(unsatisfiedExtension); + } + + return root.css; + } var sortedModules = _topologicalModules(root); _extendModules(sortedModules); @@ -452,7 +465,22 @@ class _EvaluateVisitor // and we can use them to extend that module. var downstreamExtenders = >{}; + /// Extensions that haven't yet been satisfied by some upstream module. This + /// adds extensions when they're defined but not satisfied, and removes them + /// when they're satisfied by any module. + var unsatisfiedExtensions = Set.identity(); + for (var module in sortedModules) { + // Create a snapshot of the simple selectors currently in the extender so + // that we don't consider an extension "satisfied" below because of a + // simple selector added by another (sibling) extension. + var originalSelectors = module.extender.simpleSelectors.toSet(); + + // Add all as-yet-unsatisfied extensions before adding downstream + // extenders, because those are all in [unsatisfiedExtensions] already. + unsatisfiedExtensions.addAll(module.extender.extensionsWhereTarget( + (target) => !originalSelectors.contains(target))); + var extenders = downstreamExtenders[module]; if (extenders != null) module.extender.addExtensions(extenders); if (module.extender.isEmpty) continue; @@ -462,7 +490,26 @@ class _EvaluateVisitor .putIfAbsent(upstream, () => []) .add(module.extender); } + + // Remove all extensions that are now satisfied after adding downstream + // extenders so it counts any downstream extensions that have been newly + // satisfied. + unsatisfiedExtensions.removeAll( + module.extender.extensionsWhereTarget(originalSelectors.contains)); } + + if (unsatisfiedExtensions.isNotEmpty) { + _throwForUnsatisfiedExtension(unsatisfiedExtensions.first); + } + } + + /// Throws an exception indicating that [extension] is unsatisfied. + @alwaysThrows + void _throwForUnsatisfiedExtension(Extension extension) { + throw SassException( + 'The target selector was not found.\n' + 'Use "@extend ${extension.target} !optional" to avoid this error.', + extension.span); } /// Returns all modules transitively used by [root] in topological order,