Parse silent comments in _interpolatedDeclarationValue() (#2266)

Closes #2263

Author: nex3

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 1.77.7
+
+* **Potentially breaking bug fix:** `//` in certain places such as unknown
+  at-rule values was being preserved in the CSS output, leading to potentially
+  invalid CSS. It's now properly parsed as a silent comment and omitted from the
+  CSS output.
+
 ## 1.77.6
 
 * Fix a few cases where comments and occasionally even whitespace wasn't allowed

lib/src/parse/parser.dart

@@ -133,7 +133,8 @@ class Parser {
     whitespace();
   }
 
-  /// Consumes and ignores a silent (Sass-style) comment.
+  /// Consumes and ignores a single silent (Sass-style) comment, not including
+  /// the trailing newline.
   ///
   /// Returns whether the comment was consumed.
   @protected

lib/src/parse/stylesheet.dart

@@ -380,7 +380,8 @@ abstract class StylesheetParser extends Parser {
     // Parse custom properties as declarations no matter what.
     var name = nameBuffer.interpolation(scanner.spanFrom(start, beforeColon));
     if (name.initialPlain.startsWith('--')) {
-      var value = StringExpression(_interpolatedDeclarationValue());
+      var value = StringExpression(
+          _interpolatedDeclarationValue(silentComments: false));
       expectStatementSeparator("custom property");
       return Declaration(name, value, scanner.spanFrom(start));
     }
@@ -532,7 +533,8 @@ abstract class StylesheetParser extends Parser {
     scanner.expectChar($colon);
 
     if (parseCustomProperties && name.initialPlain.startsWith('--')) {
-      var value = StringExpression(_interpolatedDeclarationValue());
+      var value = StringExpression(
+          _interpolatedDeclarationValue(silentComments: false));
       expectStatementSeparator("custom property");
       return Declaration(name, value, scanner.spanFrom(start));
     }
@@ -1550,7 +1552,7 @@ abstract class StylesheetParser extends Parser {
 
     Interpolation? value;
     if (scanner.peekChar() != $exclamation && !atEndOfStatement()) {
-      value = almostAnyValue();
+      value = _interpolatedDeclarationValue(allowOpenBrace: false);
     }
 
     AtRule rule;
@@ -1575,7 +1577,7 @@ abstract class StylesheetParser extends Parser {
   /// This declares a return type of [Statement] so that it can be returned
   /// within case statements.
   Statement _disallowedAtRule(LineScannerState start) {
-    almostAnyValue();
+    _interpolatedDeclarationValue(allowEmpty: true, allowOpenBrace: false);
     error("This at-rule is not allowed here.", scanner.spanFrom(start));
   }
 
@@ -2748,13 +2750,11 @@ abstract class StylesheetParser extends Parser {
   ///
   /// Differences from [_interpolatedDeclarationValue] include:
   ///
-  /// * This does not balance brackets.
+  /// * This always stops at curly braces.
   ///
   /// * This does not interpret backslashes, since the text is expected to be
   ///   re-parsed.
   ///
-  /// * This supports Sass-style single-line comments.
-  ///
   /// * This does not compress adjacent whitespace characters.
   @protected
   Interpolation almostAnyValue({bool omitComments = false}) {
@@ -2773,11 +2773,21 @@ abstract class StylesheetParser extends Parser {
           buffer.addInterpolation(interpolatedString().asInterpolation());
 
         case $slash:
-          var commentStart = scanner.position;
-          if (scanComment()) {
-            if (!omitComments) buffer.write(scanner.substring(commentStart));
-          } else {
-            buffer.writeCharCode(scanner.readChar());
+          switch (scanner.peekChar(1)) {
+            case $asterisk when !omitComments:
+              buffer.write(rawText(loudComment));
+
+            case $asterisk:
+              loudComment();
+
+            case $slash when !omitComments:
+              buffer.write(rawText(silentComment));
+
+            case $slash:
+              silentComment();
+
+            case _:
+              buffer.writeCharCode(scanner.readChar());
           }
 
         case $hash when scanner.peekChar(1) == $lbrace:
@@ -2794,12 +2804,17 @@ abstract class StylesheetParser extends Parser {
 
         case $u || $U:
           var beforeUrl = scanner.state;
-          if (!scanIdentifier("url")) {
-            buffer.writeCharCode(scanner.readChar());
+          var identifier = this.identifier();
+          if (identifier != "url" &&
+              // This isn't actually a standard CSS feature, but it was
+              // supported by the old `@document` rule so we continue to support
+              // it for backwards-compatibility.
+              identifier != "url-prefix") {
+            buffer.write(identifier);
             continue loop;
           }
 
-          if (_tryUrlContents(beforeUrl) case var contents?) {
+          if (_tryUrlContents(beforeUrl, name: identifier) case var contents?) {
             buffer.addInterpolation(contents);
           } else {
             scanner.state = beforeUrl;
@@ -2830,11 +2845,19 @@ abstract class StylesheetParser extends Parser {
   ///
   /// If [allowColon] is `false`, this stops at top-level colons.
   ///
+  /// If [allowOpenBrace] is `false`, this stops at top-level colons.
+  ///
+  /// If [silentComments] is `true`, this will parse silent comments as
+  /// comments. Otherwise, it will preserve two adjacent slashes and emit them
+  /// to CSS.
+  ///
   /// Unlike [declarationValue], this allows interpolation.
   Interpolation _interpolatedDeclarationValue(
       {bool allowEmpty = false,
       bool allowSemicolon = false,
-      bool allowColon = true}) {
+      bool allowColon = true,
+      bool allowOpenBrace = true,
+      bool silentComments = true}) {
     // NOTE: this logic is largely duplicated in Parser.declarationValue. Most
     // changes here should be mirrored there.
 
@@ -2854,7 +2877,22 @@ abstract class StylesheetParser extends Parser {
           buffer.addInterpolation(interpolatedString().asInterpolation());
           wroteNewline = false;
 
-        case $slash when scanner.peekChar(1) == $asterisk:
+        case $slash:
+          switch (scanner.peekChar(1)) {
+            case $asterisk:
+              buffer.write(rawText(loudComment));
+              wroteNewline = false;
+
+            case $slash when silentComments:
+              silentComment();
+              wroteNewline = false;
+
+            case _:
+              buffer.writeCharCode(scanner.readChar());
+              wroteNewline = false;
+          }
+
+        case $slash when silentComments && scanner.peekChar(1) == $slash:
           buffer.write(rawText(loudComment));
           wroteNewline = false;
 
@@ -2882,6 +2920,9 @@ abstract class StylesheetParser extends Parser {
           scanner.readChar();
           wroteNewline = true;
 
+        case $lbrace when !allowOpenBrace:
+          break loop;
+
         case $lparen || $lbrace || $lbracket:
           var bracket = scanner.readChar();
           buffer.writeCharCode(bracket);
@@ -2907,13 +2948,18 @@ abstract class StylesheetParser extends Parser {
 
         case $u || $U:
           var beforeUrl = scanner.state;
-          if (!scanIdentifier("url")) {
-            buffer.writeCharCode(scanner.readChar());
+          var identifier = this.identifier();
+          if (identifier != "url" &&
+              // This isn't actually a standard CSS feature, but it was
+              // supported by the old `@document` rule so we continue to support
+              // it for backwards-compatibility.
+              identifier != "url-prefix") {
+            buffer.write(identifier);
             wroteNewline = false;
             continue loop;
           }
 
-          if (_tryUrlContents(beforeUrl) case var contents?) {
+          if (_tryUrlContents(beforeUrl, name: identifier) case var contents?) {
             buffer.addInterpolation(contents);
           } else {
             scanner.state = beforeUrl;

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass
-version: 1.77.6
+version: 1.77.7-dev
 description: A Sass implementation in Dart.
 homepage: https://github.com/sass/dart-sass