From 33139bef6ca35d817348720a3ac65a6965db5cab Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Sat, 28 Jan 2023 09:30:10 -0800 Subject: [PATCH 1/2] fix the reported span for expressions --- lib/parser.dart | 30 +++++++++---------- lib/src/preprocessor_options.dart | 23 +++++++------- lib/src/tree.dart | 50 +++++++++++++++---------------- test/declaration_test.dart | 26 ++++++++-------- test/error_test.dart | 4 +-- 5 files changed, 68 insertions(+), 65 deletions(-) diff --git a/lib/parser.dart b/lib/parser.dart index be74a06..f630240 100644 --- a/lib/parser.dart +++ b/lib/parser.dart @@ -321,7 +321,7 @@ class _Parser { messages.warning(message, location); } - SourceSpan _makeSpan(FileSpan start) { + FileSpan _makeSpan(FileSpan start) { // TODO(terry): there are places where we are creating spans before we eat // the tokens, so using _previousToken is not always valid. // TODO(nweiz): use < rather than compareTo when SourceSpan supports it. @@ -930,7 +930,7 @@ class _Parser { dynamic expr; var keepGoing = true; while (keepGoing && (expr = processTerm()) != null) { - // VarUsage is returns as a list + // VarUsage is returned as a list terms.add((expr is List ? expr[0] : expr) as Expression); keepGoing = !_peekKind(TokenKind.RPAREN); if (keepGoing) { @@ -978,7 +978,7 @@ class _Parser { _eat(TokenKind.RPAREN); - var arguments = Expressions(_makeSpan(argumentSpan as FileSpan)) + var arguments = Expressions(_makeSpan(argumentSpan)) ..add(LiteralTerm(argument, argument, argumentSpan)); function = FunctionTerm(ident.name, ident.name, arguments, _makeSpan(ident.span as FileSpan)); @@ -2406,12 +2406,12 @@ class _Parser { switch (unitType) { case TokenKind.UNIT_EM: + span = span.union(_next().span); term = EmTerm(value, t!.text, span); - _next(); // Skip the unit break; case TokenKind.UNIT_EX: + span = span.union(_next().span); term = ExTerm(value, t!.text, span); - _next(); // Skip the unit break; case TokenKind.UNIT_LENGTH_PX: case TokenKind.UNIT_LENGTH_CM: @@ -2419,54 +2419,54 @@ class _Parser { case TokenKind.UNIT_LENGTH_IN: case TokenKind.UNIT_LENGTH_PT: case TokenKind.UNIT_LENGTH_PC: + span = span.union(_next().span); term = LengthTerm(value, t!.text, span, unitType); - _next(); // Skip the unit break; case TokenKind.UNIT_ANGLE_DEG: case TokenKind.UNIT_ANGLE_RAD: case TokenKind.UNIT_ANGLE_GRAD: case TokenKind.UNIT_ANGLE_TURN: + span = span.union(_next().span); term = AngleTerm(value, t!.text, span, unitType); - _next(); // Skip the unit break; case TokenKind.UNIT_TIME_MS: case TokenKind.UNIT_TIME_S: + span = span.union(_next().span); term = TimeTerm(value, t!.text, span, unitType); - _next(); // Skip the unit break; case TokenKind.UNIT_FREQ_HZ: case TokenKind.UNIT_FREQ_KHZ: + span = span.union(_next().span); term = FreqTerm(value, t!.text, span, unitType); - _next(); // Skip the unit break; case TokenKind.PERCENT: + span = span.union(_next().span); term = PercentageTerm(value, t!.text, span); - _next(); // Skip the % break; case TokenKind.UNIT_FRACTION: + span = span.union(_next().span); term = FractionTerm(value, t!.text, span); - _next(); // Skip the unit break; case TokenKind.UNIT_RESOLUTION_DPI: case TokenKind.UNIT_RESOLUTION_DPCM: case TokenKind.UNIT_RESOLUTION_DPPX: + span = span.union(_next().span); term = ResolutionTerm(value, t!.text, span, unitType); - _next(); // Skip the unit break; case TokenKind.UNIT_CH: + span = span.union(_next().span); term = ChTerm(value, t!.text, span, unitType); - _next(); // Skip the unit break; case TokenKind.UNIT_REM: + span = span.union(_next().span); term = RemTerm(value, t!.text, span, unitType); - _next(); // Skip the unit break; case TokenKind.UNIT_VIEWPORT_VW: case TokenKind.UNIT_VIEWPORT_VH: case TokenKind.UNIT_VIEWPORT_VMIN: case TokenKind.UNIT_VIEWPORT_VMAX: + span = span.union(_next().span); term = ViewportTerm(value, t!.text, span, unitType); - _next(); // Skip the unit break; default: if (value is Identifier) { diff --git a/lib/src/preprocessor_options.dart b/lib/src/preprocessor_options.dart index 2ca67dc..3879ead 100644 --- a/lib/src/preprocessor_options.dart +++ b/lib/src/preprocessor_options.dart @@ -34,14 +34,17 @@ class PreprocessorOptions { /// File to process by the compiler. final String? inputFile; - const PreprocessorOptions( - {this.verbose = false, - this.checked = false, - this.lessSupport = true, - this.warningsAsErrors = false, - this.throwOnErrors = false, - this.throwOnWarnings = false, - this.useColors = true, - this.polyfill = false, - this.inputFile}); + // TODO: Should less support really default to being enabled? + + const PreprocessorOptions({ + this.verbose = false, + this.checked = false, + this.lessSupport = true, + this.warningsAsErrors = false, + this.throwOnErrors = false, + this.throwOnWarnings = false, + this.useColors = true, + this.polyfill = false, + this.inputFile, + }); } diff --git a/lib/src/tree.dart b/lib/src/tree.dart index 7e99426..fb1241d 100644 --- a/lib/src/tree.dart +++ b/lib/src/tree.dart @@ -64,8 +64,8 @@ class Negation extends TreeNode { class CalcTerm extends LiteralTerm { final LiteralTerm expr; - CalcTerm(Object value, String t, this.expr, SourceSpan? span) - : super(value, t, span); + CalcTerm(Object value, String text, this.expr, SourceSpan? span) + : super(value, text, span); @override CalcTerm clone() => CalcTerm(value, text, expr.clone(), span); @@ -1242,7 +1242,7 @@ class LiteralTerm extends Expression { } class NumberTerm extends LiteralTerm { - NumberTerm(super.value, super.t, super.span); + NumberTerm(super.value, super.text, super.span); @override NumberTerm clone() => NumberTerm(value, text, span); @override @@ -1252,7 +1252,7 @@ class NumberTerm extends LiteralTerm { class UnitTerm extends LiteralTerm { final int unit; - UnitTerm(super.value, super.t, super.span, this.unit); + UnitTerm(super.value, super.text, super.span, this.unit); @override UnitTerm clone() => UnitTerm(value, text, span, unit); @@ -1267,7 +1267,7 @@ class UnitTerm extends LiteralTerm { } class LengthTerm extends UnitTerm { - LengthTerm(super.value, super.t, super.span, + LengthTerm(super.value, super.text, super.span, [super.unit = TokenKind.UNIT_LENGTH_PX]) { assert(unit == TokenKind.UNIT_LENGTH_PX || unit == TokenKind.UNIT_LENGTH_CM || @@ -1283,7 +1283,7 @@ class LengthTerm extends UnitTerm { } class PercentageTerm extends LiteralTerm { - PercentageTerm(super.value, super.t, super.span); + PercentageTerm(super.value, super.text, super.span); @override PercentageTerm clone() => PercentageTerm(value, text, span); @override @@ -1291,7 +1291,7 @@ class PercentageTerm extends LiteralTerm { } class EmTerm extends LiteralTerm { - EmTerm(super.value, super.t, super.span); + EmTerm(super.value, super.text, super.span); @override EmTerm clone() => EmTerm(value, text, span); @override @@ -1299,7 +1299,7 @@ class EmTerm extends LiteralTerm { } class ExTerm extends LiteralTerm { - ExTerm(super.value, super.t, super.span); + ExTerm(super.value, super.text, super.span); @override ExTerm clone() => ExTerm(value, text, span); @override @@ -1307,7 +1307,7 @@ class ExTerm extends LiteralTerm { } class AngleTerm extends UnitTerm { - AngleTerm(super.value, super.t, super.span, + AngleTerm(super.value, super.text, super.span, [super.unit = TokenKind.UNIT_LENGTH_PX]) { assert(unit == TokenKind.UNIT_ANGLE_DEG || unit == TokenKind.UNIT_ANGLE_RAD || @@ -1322,7 +1322,7 @@ class AngleTerm extends UnitTerm { } class TimeTerm extends UnitTerm { - TimeTerm(super.value, super.t, super.span, + TimeTerm(super.value, super.text, super.span, [super.unit = TokenKind.UNIT_LENGTH_PX]) { assert(unit == TokenKind.UNIT_ANGLE_DEG || unit == TokenKind.UNIT_TIME_MS || @@ -1336,9 +1336,9 @@ class TimeTerm extends UnitTerm { } class FreqTerm extends UnitTerm { - FreqTerm(Object value, String t, SourceSpan? span, + FreqTerm(Object value, String text, SourceSpan? span, [int unit = TokenKind.UNIT_LENGTH_PX]) - : super(value, t, span, unit) { + : super(value, text, span, unit) { assert(unit == TokenKind.UNIT_FREQ_HZ || unit == TokenKind.UNIT_FREQ_KHZ); } @@ -1349,7 +1349,7 @@ class FreqTerm extends UnitTerm { } class FractionTerm extends LiteralTerm { - FractionTerm(super.value, super.t, super.span); + FractionTerm(super.value, super.text, super.span); @override FractionTerm clone() => FractionTerm(value, text, span); @@ -1367,9 +1367,9 @@ class UriTerm extends LiteralTerm { } class ResolutionTerm extends UnitTerm { - ResolutionTerm(Object value, String t, SourceSpan? span, + ResolutionTerm(Object value, String text, SourceSpan? span, [int unit = TokenKind.UNIT_LENGTH_PX]) - : super(value, t, span, unit) { + : super(value, text, span, unit) { assert(unit == TokenKind.UNIT_RESOLUTION_DPI || unit == TokenKind.UNIT_RESOLUTION_DPCM || unit == TokenKind.UNIT_RESOLUTION_DPPX); @@ -1382,9 +1382,9 @@ class ResolutionTerm extends UnitTerm { } class ChTerm extends UnitTerm { - ChTerm(Object value, String t, SourceSpan? span, + ChTerm(Object value, String text, SourceSpan? span, [int unit = TokenKind.UNIT_LENGTH_PX]) - : super(value, t, span, unit) { + : super(value, text, span, unit) { assert(unit == TokenKind.UNIT_CH); } @@ -1395,9 +1395,9 @@ class ChTerm extends UnitTerm { } class RemTerm extends UnitTerm { - RemTerm(Object value, String t, SourceSpan? span, + RemTerm(Object value, String text, SourceSpan? span, [int unit = TokenKind.UNIT_LENGTH_PX]) - : super(value, t, span, unit) { + : super(value, text, span, unit) { assert(unit == TokenKind.UNIT_REM); } @@ -1408,9 +1408,9 @@ class RemTerm extends UnitTerm { } class ViewportTerm extends UnitTerm { - ViewportTerm(Object value, String t, SourceSpan? span, + ViewportTerm(Object value, String text, SourceSpan? span, [int unit = TokenKind.UNIT_LENGTH_PX]) - : super(value, t, span, unit) { + : super(value, text, span, unit) { assert(unit == TokenKind.UNIT_VIEWPORT_VW || unit == TokenKind.UNIT_VIEWPORT_VH || unit == TokenKind.UNIT_VIEWPORT_VMIN || @@ -1428,7 +1428,7 @@ class ViewportTerm extends UnitTerm { class BAD_HEX_VALUE {} class HexColorTerm extends LiteralTerm { - HexColorTerm(super.value, super.t, super.span); + HexColorTerm(super.value, super.text, super.span); @override HexColorTerm clone() => HexColorTerm(value, text, span); @@ -1439,8 +1439,8 @@ class HexColorTerm extends LiteralTerm { class FunctionTerm extends LiteralTerm { final Expressions _params; - FunctionTerm(Object value, String t, this._params, SourceSpan? span) - : super(value, t, span); + FunctionTerm(Object value, String text, this._params, SourceSpan? span) + : super(value, text, span); @override FunctionTerm clone() => FunctionTerm(value, text, _params.clone(), span); @@ -1475,7 +1475,7 @@ class GroupTerm extends Expression { } class ItemTerm extends NumberTerm { - ItemTerm(super.value, super.t, super.span); + ItemTerm(super.value, super.text, super.span); @override ItemTerm clone() => ItemTerm(value, text, span); diff --git a/test/declaration_test.dart b/test/declaration_test.dart index 6cccc9c..8e0b8dd 100644 --- a/test/declaration_test.dart +++ b/test/declaration_test.dart @@ -719,14 +719,16 @@ src: url(ideal-sans-serif.woff) format("woff"), expect(errors.isEmpty, true, reason: errors.toString()); expect(prettyPrint(stylesheet), generated2); - final input3 = '''@font-face { + final input3 = ''' +@font-face { font-family: MyGentium Text Ornaments; src: local(Gentium Bold), /* full font name */ local(Gentium-Bold), /* Postscript name */ url(GentiumBold.ttf); /* otherwise, download it */ font-weight: bold; }'''; - final generated3 = '''@font-face { + final generated3 = ''' +@font-face { font-family: MyGentium Text Ornaments; src: local(Gentium Bold), local(Gentium-Bold), url("GentiumBold.ttf"); font-weight: bold; @@ -1270,15 +1272,15 @@ void testHangs() { void testExpressionSpans() { final input = r'''.foo { width: 50px; }'''; + var stylesheet = parseCss(input); - var decl = (stylesheet.topLevels.single as RuleSet) - .declarationGroup - .declarations - .single; - // This passes - expect(decl.span!.text, 'width: 50px'); - // This currently fails - expect((decl as Declaration).expression!.span!.text, '50px'); + var ruleSet = stylesheet.topLevels.single as RuleSet; + + var declaration = ruleSet.declarationGroup.declarations.single as Declaration; + expect(declaration.span.text, 'width: 50px'); + + var expressions = declaration.expression as Expressions; + expect(expressions.expressions.first.span!.text, '50px'); } void testComments() { @@ -1378,9 +1380,7 @@ void main() { test('IE stuff', testIE); test('IE declaration syntax', testIEDeclaration); test('Hanging bugs', testHangs); - test('Expression spans', testExpressionSpans, - skip: 'expression spans are broken' - ' (https://github.com/dart-lang/csslib/issues/15)'); + test('Expression spans', testExpressionSpans); test('Comments', testComments); group('calc function', () { test('simple calc', simpleCalc); diff --git a/test/error_test.dart b/test/error_test.dart index b6be913..cc169b4 100644 --- a/test/error_test.dart +++ b/test/error_test.dart @@ -81,7 +81,7 @@ void testUnsupportedLineHeights() { error on line 1, column 24: Unexpected value for line-height , 1 | .foobar { line-height: 120%; } - | ^^^ + | ^^^^ \''''); expect(prettyPrint(stylesheet), r''' .foobar { @@ -98,7 +98,7 @@ error on line 1, column 24: Unexpected value for line-height error on line 1, column 24: Unexpected unit for line-height , 1 | .foobar { line-height: 20cm; } - | ^^ + | ^^^^ \''''); expect(prettyPrint(stylesheet), r''' .foobar { From 885f0f328b5a2c97e3671a8ababd0032453b5670 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Sat, 28 Jan 2023 15:31:26 -0800 Subject: [PATCH 2/2] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b1b84c..419df9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - Add markdown badges to the readme. - Adopted `package:dart_flutter_team_lints` linting rules. +- Fixed the reported span for `Expression` nodes. ## 0.17.2