Skip to content

Commit

Permalink
Parse silent comments in _interpolatedDeclarationValue() (#2266)
Browse files Browse the repository at this point in the history
Closes #2263
  • Loading branch information
nex3 committed Jun 20, 2024
1 parent 860eb5a commit 04b6251
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 22 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/src/parse/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
86 changes: 66 additions & 20 deletions lib/src/parse/stylesheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -1550,7 +1552,7 @@ abstract class StylesheetParser extends Parser {

Interpolation? value;
if (scanner.peekChar() != $exclamation && !atEndOfStatement()) {
value = almostAnyValue();
value = _interpolatedDeclarationValue(allowOpenBrace: false);
}

AtRule rule;
Expand All @@ -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));
}

Expand Down Expand Up @@ -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}) {
Expand All @@ -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:
Expand All @@ -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;
Expand Down Expand Up @@ -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.

Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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

Expand Down

0 comments on commit 04b6251

Please sign in to comment.