diff --git a/lib/src/ast/css/comment.dart b/lib/src/ast/css/comment.dart index 04c8b8ebd..4724a83b2 100644 --- a/lib/src/ast/css/comment.dart +++ b/lib/src/ast/css/comment.dart @@ -16,9 +16,5 @@ abstract class CssComment extends CssNode { /// compressed mode. bool get isPreserved; - /// Whether this comment follows non-comment text on a line and should remain - /// attached to that non-comment text when being serialized. - bool get isTrailing; - T accept(CssVisitor visitor) => visitor.visitCssComment(this); } diff --git a/lib/src/ast/css/modifiable/comment.dart b/lib/src/ast/css/modifiable/comment.dart index a6b7749bd..40e7838c7 100644 --- a/lib/src/ast/css/modifiable/comment.dart +++ b/lib/src/ast/css/modifiable/comment.dart @@ -13,12 +13,10 @@ import 'node.dart'; class ModifiableCssComment extends ModifiableCssNode implements CssComment { final String text; final FileSpan span; - final bool _isTrailing; bool get isPreserved => text.codeUnitAt(2) == $exclamation; - bool get isTrailing => _isTrailing; - ModifiableCssComment(this.text, this.span, this._isTrailing); + ModifiableCssComment(this.text, this.span); T accept(ModifiableCssVisitor visitor) => visitor.visitCssComment(this); } diff --git a/lib/src/ast/sass/statement/loud_comment.dart b/lib/src/ast/sass/statement/loud_comment.dart index d9443f1b3..41e824645 100644 --- a/lib/src/ast/sass/statement/loud_comment.dart +++ b/lib/src/ast/sass/statement/loud_comment.dart @@ -15,11 +15,7 @@ class LoudComment implements Statement { FileSpan get span => text.span; - /// Whether this comment follows non-comment text on a line and should remain - /// attached to that non-comment text when being serialized. - final bool isTrailing; - - LoudComment(this.text, this.isTrailing); + LoudComment(this.text); T accept(StatementVisitor visitor) => visitor.visitLoudComment(this); diff --git a/lib/src/parse/sass.dart b/lib/src/parse/sass.dart index 0a4d93a90..8bc81ce2c 100644 --- a/lib/src/parse/sass.dart +++ b/lib/src/parse/sass.dart @@ -9,7 +9,6 @@ import '../ast/sass.dart'; import '../interpolation_buffer.dart'; import '../logger.dart'; import '../util/character.dart'; -import '../value.dart'; import 'stylesheet.dart'; /// A parser for the indented syntax. @@ -99,21 +98,9 @@ class SassParser extends StylesheetParser { scanner.readChar(); next = scanner.peekChar(); } - var url = scanner.substring(start.position); - var span = scanner.spanFrom(start); - - if (isPlainImportUrl(url)) { - // Serialize [url] as a Sass string because [StaticImport] expects it to - // include quotes. - return StaticImport( - Interpolation([SassString(url).toString()], span), span); - } else { - try { - return DynamicImport(parseImportUrl(url), span); - } on FormatException catch (innerError) { - error("Invalid URL: ${innerError.message}", span); - } - } + + return DynamicImport(parseImportUrl(scanner.substring(start.position)), + scanner.spanFrom(start)); } bool scanElse(int ifIndentation) { @@ -306,8 +293,7 @@ class SassParser extends StylesheetParser { } if (!buffer.trailingString.trimRight().endsWith("*/")) buffer.write(" */"); - // The sass syntax does not support trailing loud comments ==> isTrailing = false. - return LoudComment(buffer.interpolation(scanner.spanFrom(start)), false); + return LoudComment(buffer.interpolation(scanner.spanFrom(start))); } void whitespace() { diff --git a/lib/src/parse/scss.dart b/lib/src/parse/scss.dart index 8dd7e0320..aec633d57 100644 --- a/lib/src/parse/scss.dart +++ b/lib/src/parse/scss.dart @@ -61,8 +61,8 @@ class ScssParser extends StylesheetParser { List children(Statement child()) { scanner.expectChar($lbrace); + whitespaceWithoutComments(); var children = []; - _whitespaceAndOptionalTrailingLoudComment(children); while (true) { switch (scanner.peekChar()) { case $dollar: @@ -76,7 +76,7 @@ class ScssParser extends StylesheetParser { whitespaceWithoutComments(); break; case $asterisk: - children.add(_loudComment(/* isTrailing= */ false)); + children.add(_loudComment()); whitespaceWithoutComments(); break; default: @@ -87,7 +87,7 @@ class ScssParser extends StylesheetParser { case $semicolon: scanner.readChar(); - _whitespaceAndOptionalTrailingLoudComment(children); + whitespaceWithoutComments(); break; case $rbrace: @@ -103,7 +103,7 @@ class ScssParser extends StylesheetParser { List statements(Statement statement()) { var statements = []; - _whitespaceAndOptionalTrailingLoudComment(statements); + whitespaceWithoutComments(); while (!scanner.isDone) { switch (scanner.peekChar()) { case $dollar: @@ -117,7 +117,7 @@ class ScssParser extends StylesheetParser { whitespaceWithoutComments(); break; case $asterisk: - statements.add(_loudComment(/* isTrailing= */ false)); + statements.add(_loudComment()); whitespaceWithoutComments(); break; default: @@ -129,7 +129,7 @@ class ScssParser extends StylesheetParser { case $semicolon: scanner.readChar(); - _whitespaceAndOptionalTrailingLoudComment(statements); + whitespaceWithoutComments(); break; default: @@ -163,7 +163,7 @@ class ScssParser extends StylesheetParser { } /// Consumes a statement-level loud comment block. - LoudComment _loudComment(bool isTrailing) { + LoudComment _loudComment() { var start = scanner.state; scanner.expect("/*"); var buffer = InterpolationBuffer()..write("/*"); @@ -182,8 +182,7 @@ class ScssParser extends StylesheetParser { if (scanner.peekChar() != $slash) break; buffer.writeCharCode(scanner.readChar()); - return LoudComment( - buffer.interpolation(scanner.spanFrom(start)), isTrailing); + return LoudComment(buffer.interpolation(scanner.spanFrom(start))); case $cr: scanner.readChar(); @@ -201,26 +200,4 @@ class ScssParser extends StylesheetParser { } } } - - /// Consumes whitespace and optionally a trailing loud comment, adding the - /// trailing loud comment to [statements] if consumed. - /// - /// A trailing loud comment is only possible if the initially consumed - /// whitespace lacks any newlines. If that's the case and a loud comment - /// immediately follows, the loud comment will also be consumed. - void _whitespaceAndOptionalTrailingLoudComment(List statements) { - bool sawAnyNewlines = whitespaceWithoutComments(); - if (_peekForStartOfLoudComment()) { - bool isTrailing = !sawAnyNewlines; - statements.add(_loudComment(isTrailing)); - whitespaceWithoutComments(); - } - } - - /// Returns whether the scanner is pointing at the start of a loud comment. - bool _peekForStartOfLoudComment() { - return !scanner.isDone && - scanner.peekChar() == $slash && - scanner.peekChar(1) == $asterisk; - } } diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index edc2d02f1..2f5cb2e31 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -1500,7 +1500,7 @@ class _EvaluateVisitor } _parent.addChild(ModifiableCssComment( - await _performInterpolation(node.text), node.span, node.isTrailing)); + await _performInterpolation(node.text), node.span)); return null; } @@ -2424,8 +2424,7 @@ class _EvaluateVisitor _endOfImports++; } - _parent - .addChild(ModifiableCssComment(node.text, node.span, node.isTrailing)); + _parent.addChild(ModifiableCssComment(node.text, node.span)); } Future visitCssDeclaration(CssDeclaration node) async { diff --git a/lib/src/visitor/clone_css.dart b/lib/src/visitor/clone_css.dart index f317859d4..c5d9f855f 100644 --- a/lib/src/visitor/clone_css.dart +++ b/lib/src/visitor/clone_css.dart @@ -37,7 +37,7 @@ class _CloneCssVisitor implements CssVisitor { } ModifiableCssComment visitCssComment(CssComment node) => - ModifiableCssComment(node.text, node.span, node.isTrailing); + ModifiableCssComment(node.text, node.span); ModifiableCssDeclaration visitCssDeclaration(CssDeclaration node) => ModifiableCssDeclaration(node.name, node.value, node.span, diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 3bd895346..38031840d 100644 --- a/lib/src/visitor/evaluate.dart +++ b/lib/src/visitor/evaluate.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_evaluate.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: 56067ad7a31ee7adf5bb4e9d484fe95f38d3762c +// Checksum: f4f4c5d1cbc9894d14b6d8ce7c1a3c09146db9ba // // ignore_for_file: unused_import @@ -1498,8 +1498,8 @@ class _EvaluateVisitor _endOfImports++; } - _parent.addChild(ModifiableCssComment( - _performInterpolation(node.text), node.span, node.isTrailing)); + _parent.addChild( + ModifiableCssComment(_performInterpolation(node.text), node.span)); return null; } @@ -2409,8 +2409,7 @@ class _EvaluateVisitor _endOfImports++; } - _parent - .addChild(ModifiableCssComment(node.text, node.span, node.isTrailing)); + _parent.addChild(ModifiableCssComment(node.text, node.span)); } void visitCssDeclaration(CssDeclaration node) { diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart index 2377d2568..f7c31d30d 100644 --- a/lib/src/visitor/serialize.dart +++ b/lib/src/visitor/serialize.dart @@ -156,19 +156,16 @@ class _SerializeVisitor void visitCssStylesheet(CssStylesheet node) { CssNode previous; - for (var i = 0; i < node.children.length; i++) { - var child = node.children[i]; + for (CssNode child in node.children) { if (_isInvisible(child)) continue; - if (previous != null) { if (_requiresSemicolon(previous)) _buffer.writeCharCode($semicolon); - if (_shouldWriteLineFeedBeforeChild(child)) { - _writeLineFeed(); - } else { + if (_isTrailingComment(child, previous)) { _writeOptionalSpace(); + } else { + _writeLineFeed(); + if (previous.isGroupEnd) _writeLineFeed(); } - - if (previous.isGroupEnd) _writeLineFeed(); } previous = child; @@ -218,7 +215,7 @@ class _SerializeVisitor if (!node.isChildless) { _writeOptionalSpace(); - _visitChildren(node.children); + _visitChildren(node.children, node); } } @@ -236,7 +233,7 @@ class _SerializeVisitor }); _writeOptionalSpace(); - _visitChildren(node.children); + _visitChildren(node.children, node); } void visitCssImport(CssImport node) { @@ -287,7 +284,7 @@ class _SerializeVisitor () => _writeBetween(node.selector.value, _commaSeparator, _buffer.write)); _writeOptionalSpace(); - _visitChildren(node.children); + _visitChildren(node.children, node); } void _visitMediaQuery(CssMediaQuery query) { @@ -312,7 +309,7 @@ class _SerializeVisitor _for(node.selector, () => node.selector.value.accept(this)); _writeOptionalSpace(); - _visitChildren(node.children); + _visitChildren(node.children, node); } void visitCssSupportsRule(CssSupportsRule node) { @@ -329,7 +326,7 @@ class _SerializeVisitor }); _writeOptionalSpace(); - _visitChildren(node.children); + _visitChildren(node.children, node); } void visitCssDeclaration(CssDeclaration node) { @@ -1083,7 +1080,7 @@ class _SerializeVisitor _for(value, () => _buffer.write(value.value)); /// Emits [children] in a block. - void _visitChildren(List children) { + void _visitChildren(Iterable children, CssNode parent) { _buffer.writeCharCode($lbrace); if (children.every(_isInvisible)) { _buffer.writeCharCode($rbrace); @@ -1091,38 +1088,37 @@ class _SerializeVisitor } bool indentNextChild; - if (_shouldWriteLineFeedBeforeChildren(children)) { - _writeLineFeed(); - indentNextChild = true; - } else { + if (children.isNotEmpty && _isTrailingComment(children.first, parent)) { _writeOptionalSpace(); indentNextChild = false; + } else { + _writeLineFeed(); + indentNextChild = true; } CssNode previous; for (var child in children) { - var child = children[i]; if (_isInvisible(child)) continue; if (previous != null) { if (_requiresSemicolon(previous)) _buffer.writeCharCode($semicolon); - if (_shouldWriteLineFeedBeforeChild(child)) { - _writeLineFeed(); - indentNextChild = true; - } else { + if (_isTrailingComment(child, previous)) { _writeOptionalSpace(); indentNextChild = false; + } else { + _writeLineFeed(); + indentNextChild = true; } if (previous.isGroupEnd) _writeLineFeed(); } previous = child; if (indentNextChild) { - _indentation++; - } - child.accept(this); - if (indentNextChild) { - _indentation--; + _indent(() { + child.accept(this); + }); + } else { + child.accept(this); } } @@ -1138,15 +1134,21 @@ class _SerializeVisitor bool _requiresSemicolon(CssNode node) => node is CssParentNode ? node.isChildless : node is! CssComment; - /// Whether [child] should have a line feed written before the child is serialized. - bool _shouldWriteLineFeedBeforeChild(CssNode child) => - child is! CssComment || !(child as CssComment).isTrailing; + /// Whether [node] represents a trailing comment when it appears after + /// [previous] in a sequence of nodes. If there are no nodes before [node], + /// then [previous] should be the parent node. + bool _isTrailingComment(CssNode node, CssNode previous) { + if (node is CssComment) { + if (previous is CssParentNode) { + // TODO(nbehrens): Can we do better? + return node.span.start.line == previous.span.start.line || + node.span.start.line == previous.span.end.line; + } else { + return node.span.start.line == previous.span.end.line; + } + } - /// Whether [children] should have a line feed written before the children are serialized. - bool _shouldWriteLineFeedBeforeChildren(List children) { - return children.isEmpty || - !(children.first is CssComment) || - !((children.first as CssComment).isTrailing); + return false; } /// Writes a line feed, unless this emitting compressed CSS. @@ -1190,6 +1192,13 @@ class _SerializeVisitor /// Returns a comma used to separate values in lists. String get _commaSeparator => _isCompressed ? "," : ", "; + /// Runs [callback] with indentation increased one level. + void _indent(void callback()) { + _indentation++; + callback(); + _indentation--; + } + /// Returns whether [node] is considered invisible. bool _isInvisible(CssNode node) { if (_inspect) return false; diff --git a/test/output_test.dart b/test/output_test.dart index 484d5cb4c..f929011f9 100644 --- a/test/output_test.dart +++ b/test/output_test.dart @@ -60,33 +60,72 @@ void main() { }); }); - // Regression test for sass/dart-sass#417. - group("preserve trailing loud comments", () { - // No need for "in Sass" cases as it's not possible to have - // trailing loud comments in the Sass syntax. - group("in SCSS", () { - test("after open block", () { - expect(compileString(""" + // Tests for sass/dart-sass#417. + // + // Note there's no need for "in Sass" cases as it's not possible to have + // trailing loud comments in the Sass syntax. + group("preserve trailing loud comments in SCSS", () { + test("after open block", () { + expect(compileString(""" selector { /* please don't move me */ name: value; }"""), equals(""" selector { /* please don't move me */ name: value; }""")); - }); - test("after declaration", () { - expect(compileString(""" + }); + + test("after close block", () { + expect(compileString(""" selector { - name: value; /* please don't move me */ + name: value; +} /* please don't move me */"""), equals(""" +selector { + name: value; +} /* please don't move me */""")); + }); + + test("only content in block", () { + expect(compileString(""" +selector { + /* please don't move me */ }"""), equals(""" selector { - name: value; /* please don't move me */ + /* please don't move me */ }""")); - }); - test("after top-level statement", () { - expect(compileString("@rule; /* please don't move me */"), - equals("@rule; /* please don't move me */")); - }); + }); + + test("after property in block", () { + expect(compileString(""" +selector { + name1: value1; /* please don't move me 1 */ + name2: value2; /* please don't move me 2 */ + name3: value3; /* please don't move me 3 */ +}"""), equals(""" +selector { + name1: value1; /* please don't move me 1 */ + name2: value2; /* please don't move me 2 */ + name3: value3; /* please don't move me 3 */ +}""")); + }); + + test("after rule in block", () { + expect(compileString(""" +selector { + @rule1; /* please don't move me 1 */ + @rule2; /* please don't move me 2 */ + @rule3; /* please don't move me 3 */ +}"""), equals(""" +selector { + @rule1; /* please don't move me 1 */ + @rule2; /* please don't move me 2 */ + @rule3; /* please don't move me 3 */ +}""")); + }); + + test("after top-level statement", () { + expect(compileString("@rule; /* please don't move me */"), + equals("@rule; /* please don't move me */")); }); }); }