Merge pull request #146 from sass/fixes

Fix more bugs.
This commit is contained in:
Natalie Weizenbaum 2017-05-29 16:37:53 -07:00 committed by GitHub
commit 6202908b06
8 changed files with 144 additions and 68 deletions

View File

@ -24,6 +24,9 @@
* Fix a number of `@extend` bugs:
* `selector-extend()` and `selector-replace()` now allow compound selector
extendees.
* Remove the universal selector `*` when unifying with other selectors.
* Properly unify the result of multiple simple selectors in the same compound
@ -38,6 +41,8 @@
* Allow extensions that match selectors but fail to unify.
* Partially-extended selectors are no longer used as parent selectors.
* Fix an edge case where both the extender and the extended selector
have invalid combinator sequences.

View File

@ -183,6 +183,10 @@ Sass to update the reference behavior.
15. `@extend` doesn't produce an error if it matches but fails to unify. See
[issue 2250][].
16. Dart Sass currently only supports UTF-8 documents. We'd like to support
more, but Dart currently doesn't support them. See [dart-lang/sdk#11744][],
for example.
[issue 1599]: https://github.com/sass/sass/issues/1599
[issue 1126]: https://github.com/sass/sass/issues/1126
[issue 2120]: https://github.com/sass/sass/issues/2120
@ -198,5 +202,6 @@ Sass to update the reference behavior.
[issue 303]: https://github.com/sass/sass/issues/303
[issue 2247]: https://github.com/sass/sass/issues/2247
[issue 2250]: https://github.com/sass/sass/issues/2250
[dart-lang/sdk#11744]: https://github.com/dart-lang/sdk/issues/11744
Disclaimer: this is not an official Google product.

View File

@ -18,6 +18,9 @@ class CssStyleRule extends CssParentNode {
/// The selector for this rule.
final CssValue<SelectorList> selector;
/// The selector for this rule, before any extensions are applied.
final SelectorList originalSelector;
final FileSpan span;
/// A style rule is invisible if it's empty, if all its children are
@ -28,9 +31,16 @@ class CssStyleRule extends CssParentNode {
return selector.value.isInvisible;
}
CssStyleRule(this.selector, this.span);
/// Creates a new [CssStyleRule].
///
/// If [originalSelector] isn't passed, it defaults to [selector.value].
CssStyleRule(CssValue<SelectorList> selector, this.span,
{SelectorList originalSelector})
: selector = selector,
originalSelector = originalSelector ?? selector.value;
T accept<T>(CssVisitor<T> visitor) => visitor.visitStyleRule(this);
CssStyleRule copyWithoutChildren() => new CssStyleRule(selector, span);
CssStyleRule copyWithoutChildren() =>
new CssStyleRule(selector, span, originalSelector: originalSelector);
}

View File

@ -14,6 +14,7 @@ import '../exception.dart';
import '../utils.dart';
import 'extension.dart';
import 'functions.dart';
import 'mode.dart';
/// Tracks style rules and extensions, and applies the latter to the former.
class Extender {
@ -59,27 +60,52 @@ class Extender {
/// [first law of extend]: https://github.com/sass/sass/issues/324#issuecomment-4607184
final _originals = new Set<ComplexSelector>.identity();
/// Extends [selector] with [source] extender and [target] the extendee.
/// The mode that controls this extender's behavior.
final ExtendMode _mode;
/// Extends [selector] with [source] extender and [targets] extendees.
///
/// This works as though `source {@extend target}` were written in
/// the stylesheet.
/// This works as though `source {@extend target}` were written in the
/// stylesheet, with the exception that [target] can contain compound
/// selectors which must be extended as a unit.
static SelectorList extend(
SelectorList selector, SelectorList source, SimpleSelector target) {
SelectorList selector, SelectorList source, SelectorList targets) =>
_extendOrReplace(selector, source, targets, ExtendMode.allTargets);
/// Returns a copy of [selector] with [targets] replaced by [source].
static SelectorList replace(
SelectorList selector, SelectorList source, SelectorList targets) =>
_extendOrReplace(selector, source, targets, ExtendMode.replace);
/// A helper function for [extend] and [replace].
static SelectorList _extendOrReplace(SelectorList selector,
SelectorList source, SelectorList targets, ExtendMode mode) {
var extenders = new Map<ComplexSelector, Extension>.fromIterable(
source.components,
value: (ComplexSelector complex) => new Extension.oneOff(complex));
return new Extender()._extendList(selector, {target: extenders}, null);
for (var complex in targets.components) {
if (complex.components.length != 1) {
throw new SassScriptException(
"Can't extend complex selector $complex.");
}
var extensions = <SimpleSelector, Map<ComplexSelector, Extension>>{};
var compound = complex.components.first as CompoundSelector;
for (var simple in compound.components) {
extensions[simple] = extenders;
}
var extender = new Extender._(mode);
extender._originals.addAll(selector.components);
selector = extender._extendList(selector, extensions, null);
}
return selector;
}
/// Returns a copy of [selector] with [source] replaced by [target].
static SelectorList replace(
SelectorList selector, SelectorList source, SimpleSelector target) {
var extenders = new Map<ComplexSelector, Extension>.fromIterable(
source.components,
value: (ComplexSelector complex) => new Extension.oneOff(complex));
return new Extender()
._extendList(selector, {target: extenders}, null, replace: true);
}
Extender() : _mode = ExtendMode.normal;
Extender._(this._mode);
/// Adds [selector] to this extender, associated with [span].
///
@ -91,14 +117,15 @@ class Extender {
/// defined, or `null` if it was defined at the top level of the document.
CssStyleRule addSelector(CssValue<SelectorList> selector, FileSpan span,
[List<CssMediaQuery> mediaContext]) {
for (var complex in selector.value.components) {
var originalSelector = selector.value;
for (var complex in originalSelector.components) {
_originals.add(complex);
}
if (_extensions.isNotEmpty) {
try {
selector = new CssValue(
_extendList(selector.value, _extensions, mediaContext),
_extendList(originalSelector, _extensions, mediaContext),
selector.span);
} on SassException catch (error) {
throw new SassException(
@ -107,7 +134,8 @@ class Extender {
selector.span);
}
}
var rule = new CssStyleRule(selector, span);
var rule =
new CssStyleRule(selector, span, originalSelector: originalSelector);
if (mediaContext != null) _mediaContexts[rule] = mediaContext;
_registerSelector(selector.value, rule);
@ -308,20 +336,16 @@ class Extender {
}
/// Extends [list] using [extensions].
///
/// If [replace] is `true`, this doesn't preserve the original selectors.
SelectorList _extendList(
SelectorList list,
Map<SimpleSelector, Map<ComplexSelector, Extension>> extensions,
List<CssMediaQuery> mediaQueryContext,
{bool replace: false}) {
List<CssMediaQuery> mediaQueryContext) {
// This could be written more simply using [List.map], but we want to avoid
// any allocations in the common case where no extends apply.
List<List<ComplexSelector>> extended;
for (var i = 0; i < list.components.length; i++) {
var complex = list.components[i];
var result = _extendComplex(complex, extensions, mediaQueryContext,
replace: replace);
var result = _extendComplex(complex, extensions, mediaQueryContext);
if (result == null) {
if (extended != null) extended.add([complex]);
} else {
@ -337,13 +361,10 @@ class Extender {
/// Extends [complex] using [extensions], and returns the contents of a
/// [SelectorList].
///
/// If [replace] is `true`, this doesn't preserve the original selectors.
List<List<ComplexSelector>> _extendComplex(
ComplexSelector complex,
Map<SimpleSelector, Map<ComplexSelector, Extension>> extensions,
List<CssMediaQuery> mediaQueryContext,
{bool replace: false}) {
List<CssMediaQuery> mediaQueryContext) {
// The complex selectors that each compound selector in [complex.components]
// can expand to.
//
@ -367,7 +388,7 @@ class Extender {
var component = complex.components[i];
if (component is CompoundSelector) {
var extended = _extendCompound(component, extensions, mediaQueryContext,
inOriginal: isOriginal, replace: replace);
inOriginal: isOriginal);
if (extended == null) {
extendedNotExpanded?.add([
new ComplexSelector([component])
@ -416,20 +437,23 @@ class Extender {
///
/// The [inOriginal] parameter indicates whether this is in an original
/// complex selector, meaning that [compound] should not be trimmed out.
///
/// If [replace] is `true`, this doesn't preserve the original selectors.
List<ComplexSelector> _extendCompound(
CompoundSelector compound,
Map<SimpleSelector, Map<ComplexSelector, Extension>> extensions,
List<CssMediaQuery> mediaQueryContext,
{bool inOriginal,
bool replace: false}) {
{bool inOriginal}) {
// If there's more than one target and they all need to match, we track
// which targets are actually extended.
var targetsUsed = _mode == ExtendMode.normal || extensions.length < 2
? null
: new Set<SimpleSelector>();
// The complex selectors produced from each component of [compound].
List<List<Extension>> options;
for (var i = 0; i < compound.components.length; i++) {
var simple = compound.components[i];
var extended = _extendSimple(simple, extensions, mediaQueryContext,
replace: replace);
var extended =
_extendSimple(simple, extensions, mediaQueryContext, targetsUsed);
if (extended == null) {
options?.add([_extensionForSimple(simple)]);
} else {
@ -445,6 +469,12 @@ class Extender {
}
if (options == null) return null;
// If [_mode] isn't [ExtendMode.normal] and we didn't use all the targets in
// [extensions], extension fails for [compound].
if (targetsUsed != null && targetsUsed.length != extensions.length) {
return null;
}
// Optimize for the simple case of a single simple selector that doesn't
// need any unification.
if (options.length == 1) {
@ -478,7 +508,7 @@ class Extender {
// [.w .x.b],
// [.w .y .x.z, .y .w .x.z]
// ]
var first = !replace;
var first = _mode != ExtendMode.replace;
var unifiedPaths = paths(options).map((path) {
List<List<ComplexSelectorComponent>> complexes;
if (first) {
@ -534,7 +564,7 @@ class Extender {
// If we're preserving the original selector, mark the first unification as
// such so [_trim] doesn't get rid of it.
var isOriginal = (ComplexSelector _) => false;
if (inOriginal && !replace) {
if (inOriginal && _mode != ExtendMode.replace) {
var original = unifiedPaths.first.first;
isOriginal = (complex) => complex == original;
}
@ -547,11 +577,12 @@ class Extender {
SimpleSelector simple,
Map<SimpleSelector, Map<ComplexSelector, Extension>> extensions,
List<CssMediaQuery> mediaQueryContext,
{bool replace: false}) {
Set<SimpleSelector> targetsUsed) {
withoutPseudo(SimpleSelector simple) {
var extenders = extensions[simple];
if (extenders == null) return null;
if (replace) return extenders.values.toList();
targetsUsed?.add(simple);
if (_mode == ExtendMode.replace) return extenders.values.toList();
var extenderList = new List<Extension>(extenders.length + 1);
extenderList[0] = _extensionForSimple(simple);
@ -560,8 +591,7 @@ class Extender {
}
if (simple is PseudoSelector && simple.selector != null) {
var extended = _extendPseudo(simple, extensions, mediaQueryContext,
replace: replace);
var extended = _extendPseudo(simple, extensions, mediaQueryContext);
if (extended != null) {
return extended.map(
(pseudo) => withoutPseudo(pseudo) ?? [_extensionForSimple(pseudo)]);
@ -591,15 +621,11 @@ class Extender {
/// Extends [pseudo] using [extensions], and returns a list of resulting
/// pseudo selectors.
///
/// If [replace] is `true`, this doesn't preserve the original selectors.
List<PseudoSelector> _extendPseudo(
PseudoSelector pseudo,
Map<SimpleSelector, Map<ComplexSelector, Extension>> extensions,
List<CssMediaQuery> mediaQueryContext,
{bool replace: false}) {
var extended = _extendList(pseudo.selector, extensions, mediaQueryContext,
replace: replace);
List<CssMediaQuery> mediaQueryContext) {
var extended = _extendList(pseudo.selector, extensions, mediaQueryContext);
if (identical(extended, pseudo.selector)) return null;
// TODO: what do we do about placeholders in the selector? If we just

30
lib/src/extend/mode.dart Normal file
View File

@ -0,0 +1,30 @@
// Copyright 2017 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.
/// Different modes in which extension can run.
class ExtendMode {
/// Normal mode, used with the `@extend` rule.
///
/// This preserves existing selectors and extends each target individually.
static const normal = const ExtendMode._("normal");
/// Replace mode, used by the `selector-replace()` function.
///
/// This replaces existing selectors and requires every target to match to
/// extend a given compound selector.
static const replace = const ExtendMode._("replace");
/// All-targets mode, used by the `selector-extend()` function.
///
/// This preserves existing selectors but requires every target to match to
/// extend a given compound selector.
static const allTargets = const ExtendMode._("allTargets");
/// The name of the mode.
final String name;
const ExtendMode._(this.name);
String toString() => name;
}

View File

@ -827,7 +827,7 @@ void defineCoreFunctions(Environment environment) {
environment.defineFunction(
"selector-extend", r"$selector, $extendee, $extender", (arguments) {
var selector = arguments[0].assertSelector(name: "selector");
var target = arguments[1].assertSimpleSelector(name: "extendee");
var target = arguments[1].assertSelector(name: "extendee");
var source = arguments[2].assertSelector(name: "extender");
return Extender.extend(selector, source, target).asSassList;
@ -836,7 +836,7 @@ void defineCoreFunctions(Environment environment) {
environment.defineFunction(
"selector-replace", r"$selector, $original, $replacement", (arguments) {
var selector = arguments[0].assertSelector(name: "selector");
var target = arguments[1].assertSimpleSelector(name: "original");
var target = arguments[1].assertSelector(name: "original");
var source = arguments[2].assertSelector(name: "replacement");
return Extender.replace(selector, source, target).asSassList;

View File

@ -290,7 +290,7 @@ class SelectorParser extends Parser {
selector = _selectorList();
}
} else {
argument = declarationValue();
argument = declarationValue().trimRight();
}
scanner.expectChar($rparen);

View File

@ -65,8 +65,8 @@ class _PerformVisitor
/// The current lexical environment.
Environment _environment;
/// The current selector, if any.
CssValue<SelectorList> _selector;
/// The style rule that defines the current parent selector, if any.
CssStyleRule _styleRule;
/// The current media queries, if any.
List<CssMediaQuery> _mediaQueries;
@ -95,7 +95,7 @@ class _PerformVisitor
var _inUnknownAtRule = false;
/// Whether we're currently building the output of a style rule.
bool get _inStyleRule => _selector != null && !_atRootExcludingStyleRule;
bool get _inStyleRule => _styleRule != null && !_atRootExcludingStyleRule;
/// Whether we're directly within an `@at-root` rule that excludes style
/// rules.
@ -464,7 +464,7 @@ class _PerformVisitor
targetText.span,
() => new SimpleSelector.parse(targetText.value.trim(),
allowParent: false));
_extender.addExtension(_selector, target, node, _mediaQueries);
_extender.addExtension(_styleRule.selector, target, node, _mediaQueries);
return null;
}
@ -502,7 +502,7 @@ class _PerformVisitor
// declarations immediately inside it have somewhere to go.
//
// For example, "a {@foo {b: c}}" should produce "@foo {a {b: c}}".
_withParent(new CssStyleRule(_selector, _selector.span), () {
_withParent(_styleRule.copyWithoutChildren(), () {
for (var child in node.children) {
child.accept(this);
}
@ -766,7 +766,7 @@ class _PerformVisitor
//
// For example, "a {@media screen {b: c}}" should produce
// "@media screen {a {b: c}}".
_withParent(new CssStyleRule(_selector, _selector.span), () {
_withParent(_styleRule.copyWithoutChildren(), () {
for (var child in node.children) {
child.accept(this);
}
@ -826,7 +826,8 @@ class _PerformVisitor
node.selector.span, () => new SelectorList.parse(selectorText.value));
parsedSelector = _addExceptionSpan(
node.selector.span,
() => parsedSelector.resolveParentSelectors(_selector?.value,
() => parsedSelector.resolveParentSelectors(
_styleRule?.originalSelector,
implicitParent: !_atRootExcludingStyleRule));
var selector =
@ -836,7 +837,7 @@ class _PerformVisitor
var oldAtRootExcludingStyleRule = _atRootExcludingStyleRule;
_atRootExcludingStyleRule = false;
_withParent(rule, () {
_withSelector(rule.selector, () {
_withStyleRule(rule, () {
for (var child in node.children) {
child.accept(this);
}
@ -872,7 +873,7 @@ class _PerformVisitor
//
// For example, "a {@supports (a: b) {b: c}}" should produce "@supports
// (a: b) {a {b: c}}".
_withParent(new CssStyleRule(_selector, _selector.span), () {
_withParent(_styleRule.copyWithoutChildren(), () {
for (var child in node.children) {
child.accept(this);
}
@ -1443,9 +1444,8 @@ class _PerformVisitor
}
Value visitSelectorExpression(SelectorExpression node) {
var selector = _selector;
if (selector == null) return sassNull;
return selector.value.asSassList;
if (_styleRule == null) return sassNull;
return _styleRule.originalSelector.asSassList;
}
SassString visitStringExpression(StringExpression node) {
@ -1580,12 +1580,12 @@ class _PerformVisitor
return result;
}
/// Runs [callback] with [selector] as the current selector.
T _withSelector<T>(CssValue<SelectorList> selector, T callback()) {
var oldSelector = _selector;
_selector = selector;
/// Runs [callback] with [rule] as the current style rule.
T _withStyleRule<T>(CssStyleRule rule, T callback()) {
var oldRule = _styleRule;
_styleRule = rule;
var result = callback();
_selector = oldSelector;
_styleRule = oldRule;
return result;
}