From 04b6251cedca92323095d8b8f9e13fbc5a651502 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 20 Jun 2024 13:17:32 -0700 Subject: [PATCH] Parse silent comments in `_interpolatedDeclarationValue()` (#2266) Closes #2263 --- CHANGELOG.md | 7 +++ lib/src/parse/parser.dart | 3 +- lib/src/parse/stylesheet.dart | 86 +++++++++++++++++++++++++++-------- pubspec.yaml | 2 +- 4 files changed, 76 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5229a0b08..edce7c487 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/src/parse/parser.dart b/lib/src/parse/parser.dart index a16b871e7..49dd72fe4 100644 --- a/lib/src/parse/parser.dart +++ b/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 diff --git a/lib/src/parse/stylesheet.dart b/lib/src/parse/stylesheet.dart index 046c56e6f..eace75790 100644 --- a/lib/src/parse/stylesheet.dart +++ b/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; diff --git a/pubspec.yaml b/pubspec.yaml index 936b32092..fd753e22b 100644 --- a/pubspec.yaml +++ b/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