Deprecate bogus combinators (#1740)

See sass/sass#3340
See #1727

Author: nex3

CHANGELOG.md

@@ -1,5 +1,13 @@
 ## 1.54.0
 
+* Deprecate selectors with leading or trailing combinators, or with multiple
+  combinators in a row. If they're included in style rules after nesting is
+  resolved, Sass will now produce a deprecation warning and, in most cases, omit
+  the selector. Leading and trailing combinators can still be freely used for
+  nesting purposes.
+
+  See https://sass-lang.com/d/bogus-combinators for more details.
+
 ### JS API
 
 * Add a `charset` option that controls whether or not Sass emits a

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

@@ -5,9 +5,7 @@
 import 'dart:collection';
 
 import '../../../visitor/interface/modifiable_css.dart';
-import '../at_rule.dart';
 import '../node.dart';
-import '../style_rule.dart';
 
 /// A modifiable version of [CssNode].
 ///
@@ -27,36 +25,11 @@ abstract class ModifiableCssNode extends CssNode {
   var isGroupEnd = false;
 
   /// Whether this node has a visible sibling after it.
-  bool get hasFollowingSibling {
-    var parent = _parent;
-    if (parent == null) return false;
-    var siblings = parent.children;
-    for (var i = _indexInParent! + 1; i < siblings.length; i++) {
-      var sibling = siblings[i];
-      if (!_isInvisible(sibling)) return true;
-    }
-    return false;
-  }
-
-  /// Returns whether [node] is invisible for the purposes of
-  /// [hasFollowingSibling].
-  ///
-  /// This can return a false negative for a comment node in compressed mode,
-  /// since the AST doesn't know the output style, but that's an extremely
-  /// narrow edge case so we don't worry about it.
-  bool _isInvisible(CssNode node) {
-    if (node is CssParentNode) {
-      // An unknown at-rule is never invisible. Because we don't know the
-      // semantics of unknown rules, we can't guarantee that (for example)
-      // `@foo {}` isn't meaningful.
-      if (node is CssAtRule) return false;
-
-      if (node is CssStyleRule && node.selector.value.isInvisible) return true;
-      return node.children.every(_isInvisible);
-    } else {
-      return false;
-    }
-  }
+  bool get hasFollowingSibling =>
+      _parent?.children
+          .skip(_indexInParent! + 1)
+          .any((sibling) => !sibling.isInvisible) ??
+      false;
 
   T accept<T>(ModifiableCssVisitor<T> visitor);
 

lib/src/ast/css/node.dart

@@ -2,9 +2,15 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'package:meta/meta.dart';
+
+import '../../visitor/every_css.dart';
 import '../../visitor/interface/css.dart';
 import '../../visitor/serialize.dart';
 import '../node.dart';
+import 'at_rule.dart';
+import 'comment.dart';
+import 'style_rule.dart';
 
 /// A statement in a plain CSS syntax tree.
 abstract class CssNode extends AstNode {
@@ -15,6 +21,28 @@ abstract class CssNode extends AstNode {
   /// Calls the appropriate visit method on [visitor].
   T accept<T>(CssVisitor<T> visitor);
 
+  /// Whether this is invisible and won't be emitted to the compiled stylesheet.
+  ///
+  /// Note that this doesn't consider nodes that contain loud comments to be
+  /// invisible even though they're omitted in compressed mode.
+  @internal
+  bool get isInvisible => accept(
+      const _IsInvisibleVisitor(includeBogus: true, includeComments: false));
+
+  // Whether this node would be invisible even if style rule selectors within it
+  // didn't have bogus combinators.
+  ///
+  /// Note that this doesn't consider nodes that contain loud comments to be
+  /// invisible even though they're omitted in compressed mode.
+  @internal
+  bool get isInvisibleOtherThanBogusCombinators => accept(
+      const _IsInvisibleVisitor(includeBogus: false, includeComments: false));
+
+  // Whether this node will be invisible when loud comments are stripped.
+  @internal
+  bool get isInvisibleHidingComments => accept(
+      const _IsInvisibleVisitor(includeBogus: true, includeComments: true));
+
   String toString() => serialize(this, inspect: true).css;
 }
 
@@ -32,3 +60,29 @@ abstract class CssParentNode extends CssNode {
   /// like `@foo {}`, [children] is empty but [isChildless] is `false`.
   bool get isChildless;
 }
+
+/// The visitor used to implement [CssNode.isInvisible]
+class _IsInvisibleVisitor extends EveryCssVisitor {
+  /// Whether to consider selectors with bogus combinators invisible.
+  final bool includeBogus;
+
+  /// Whether to consider comments invisible.
+  final bool includeComments;
+
+  const _IsInvisibleVisitor(
+      {required this.includeBogus, required this.includeComments});
+
+  // An unknown at-rule is never invisible. Because we don't know the semantics
+  // of unknown rules, we can't guarantee that (for example) `@foo {}` isn't
+  // meaningful.
+  bool visitCssAtRule(CssAtRule rule) => false;
+
+  bool visitCssComment(CssComment comment) =>
+      includeComments && !comment.isPreserved;
+
+  bool visitCssStyleRule(CssStyleRule rule) =>
+      (includeBogus
+          ? rule.selector.value.isInvisible
+          : rule.selector.value.isInvisibleOtherThanBogusCombinators) ||
+      super.visitCssStyleRule(rule);
+}

lib/src/ast/selector.dart

@@ -4,12 +4,21 @@
 
 import 'package:meta/meta.dart';
 
+import '../evaluation_context.dart';
+import '../exception.dart';
+import '../visitor/any_selector.dart';
 import '../visitor/interface/selector.dart';
 import '../visitor/serialize.dart';
+import 'selector/complex.dart';
+import 'selector/list.dart';
+import 'selector/placeholder.dart';
+import 'selector/pseudo.dart';
 
 export 'selector/attribute.dart';
 export 'selector/class.dart';
+export 'selector/combinator.dart';
 export 'selector/complex.dart';
+export 'selector/complex_component.dart';
 export 'selector/compound.dart';
 export 'selector/id.dart';
 export 'selector/list.dart';
@@ -32,11 +41,131 @@ export 'selector/universal.dart';
 abstract class Selector {
   /// Whether this selector, and complex selectors containing it, should not be
   /// emitted.
+  ///
+  /// @nodoc
   @internal
-  bool get isInvisible => false;
+  bool get isInvisible => accept(const _IsInvisibleVisitor(includeBogus: true));
+
+  // Whether this selector would be invisible even if it didn't have bogus
+  // combinators.
+  ///
+  /// @nodoc
+  @internal
+  bool get isInvisibleOtherThanBogusCombinators =>
+      accept(const _IsInvisibleVisitor(includeBogus: false));
+
+  /// Whether this selector is not valid CSS.
+  ///
+  /// This includes both selectors that are useful exclusively for build-time
+  /// nesting (`> .foo)` and selectors with invalid combiantors that are still
+  /// supported for backwards-compatibility reasons (`.foo + ~ .bar`).
+  bool get isBogus =>
+      accept(const _IsBogusVisitor(includeLeadingCombinator: true));
+
+  /// Whether this selector is bogus other than having a leading combinator.
+  ///
+  /// @nodoc
+  @internal
+  bool get isBogusOtherThanLeadingCombinator =>
+      accept(const _IsBogusVisitor(includeLeadingCombinator: false));
+
+  /// Whether this is a useless selector (that is, it's bogus _and_ it can't be
+  /// transformed into valid CSS by `@extend` or nesting).
+  ///
+  /// @nodoc
+  @internal
+  bool get isUseless => accept(const _IsUselessVisitor());
+
+  /// Prints a warning if [this] is a bogus selector.
+  ///
+  /// This may only be called from within a custom Sass function. This will
+  /// throw a [SassScriptException] in Dart Sass 2.0.0.
+  void assertNotBogus({String? name}) {
+    if (!isBogus) return;
+    warn(
+        (name == null ? '' : '\$$name: ') +
+            '$this is not valid CSS.\n'
+                'This will be an error in Dart Sass 2.0.0.\n'
+                '\n'
+                'More info: https://sass-lang.com/d/bogus-combinators',
+        deprecation: true);
+  }
 
   /// Calls the appropriate visit method on [visitor].
   T accept<T>(SelectorVisitor<T> visitor);
 
   String toString() => serializeSelector(this, inspect: true);
 }
+
+/// The visitor used to implement [Selector.isInvisible].
+class _IsInvisibleVisitor extends AnySelectorVisitor {
+  /// Whether to consider selectors with bogus combinators invisible.
+  final bool includeBogus;
+
+  const _IsInvisibleVisitor({required this.includeBogus});
+
+  bool visitSelectorList(SelectorList list) =>
+      list.components.every(visitComplexSelector);
+
+  bool visitComplexSelector(ComplexSelector complex) =>
+      super.visitComplexSelector(complex) ||
+      (includeBogus && complex.isBogusOtherThanLeadingCombinator);
+
+  bool visitPlaceholderSelector(PlaceholderSelector placeholder) => true;
+
+  bool visitPseudoSelector(PseudoSelector pseudo) {
+    var selector = pseudo.selector;
+    if (selector == null) return false;
+
+    // We don't consider `:not(%foo)` to be invisible because, semantically, it
+    // means "doesn't match this selector that matches nothing", so it's
+    // equivalent to *. If the entire compound selector is composed of `:not`s
+    // with invisible lists, the serializer emits it as `*`.
+    return pseudo.name == 'not'
+        ? (includeBogus && selector.isBogus)
+        : selector.accept(this);
+  }
+}
+
+/// The visitor used to implement [Selector.isBogus].
+class _IsBogusVisitor extends AnySelectorVisitor {
+  /// Whether to consider selectors with leading combinators as bogus.
+  final bool includeLeadingCombinator;
+
+  const _IsBogusVisitor({required this.includeLeadingCombinator});
+
+  bool visitComplexSelector(ComplexSelector complex) {
+    if (complex.components.isEmpty) {
+      return complex.leadingCombinators.isNotEmpty;
+    } else {
+      return complex.leadingCombinators.length >
+              (includeLeadingCombinator ? 0 : 1) ||
+          complex.components.last.combinators.isNotEmpty ||
+          complex.components.any((component) =>
+              component.combinators.length > 1 ||
+              component.selector.accept(this));
+    }
+  }
+
+  bool visitPseudoSelector(PseudoSelector pseudo) {
+    var selector = pseudo.selector;
+    if (selector == null) return false;
+
+    // The CSS spec specifically allows leading combinators in `:has()`.
+    return pseudo.name == 'has'
+        ? selector.isBogusOtherThanLeadingCombinator
+        : selector.isBogus;
+  }
+}
+
+/// The visitor used to implement [Selector.isUseless]
+class _IsUselessVisitor extends AnySelectorVisitor {
+  const _IsUselessVisitor();
+
+  bool visitComplexSelector(ComplexSelector complex) =>
+      complex.leadingCombinators.length > 1 ||
+      complex.components.any((component) =>
+          component.combinators.length > 1 || component.selector.accept(this));
+
+  bool visitPseudoSelector(PseudoSelector pseudo) => pseudo.isBogus;
+}

lib/src/ast/selector/combinator.dart

@@ -0,0 +1,31 @@
+// Copyright 2022 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 'package:meta/meta.dart';
+
+/// A combinator that defines the relationship between selectors in a
+/// [ComplexSelector].
+///
+/// {@category Selector}
+@sealed
+class Combinator {
+  /// Matches the right-hand selector if it's immediately adjacent to the
+  /// left-hand selector in the DOM tree.
+  static const nextSibling = Combinator._("+");
+
+  /// Matches the right-hand selector if it's a direct child of the left-hand
+  /// selector in the DOM tree.
+  static const child = Combinator._(">");
+
+  /// Matches the right-hand selector if it comes after the left-hand selector
+  /// in the DOM tree.
+  static const followingSibling = Combinator._("~");
+
+  /// The combinator's token text.
+  final String _text;
+
+  const Combinator._(this._text);
+
+  String toString() => _text;
+}