From 52d02504135a7830623fb0c849c092e111781556 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 17 May 2021 19:13:49 -0700 Subject: [PATCH 1/4] Fix some edge cases when serializing edge-case numbers --- CHANGELOG.md | 6 ++ lib/src/util/character.dart | 2 +- lib/src/visitor/serialize.dart | 151 ++++++++++++++++++++++----------- 3 files changed, 108 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39ec4ff4d..22dd57531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ made slash-free in both cases. This is a behavioral change, but it's unlikely to affect any real-world stylesheets. +* Fix a bug where non-integer numbers that were very close to integer + values would be incorrectly formatted in CSS. + +* Fix a bug where very small number and very large negative numbers would be + incorrectly formatted in CSS. + ### JS API * The `this` context for importers now has a `fromImport` field, which is `true` diff --git a/lib/src/util/character.dart b/lib/src/util/character.dart index a1c5b3846..e899f96b5 100644 --- a/lib/src/util/character.dart +++ b/lib/src/util/character.dart @@ -103,7 +103,7 @@ int asDecimal(int character) { /// /// Assumes that [number] is less than 10. int decimalCharFor(int number) { - assert(number < 10); + assert(number < 10, "Expected ${number} to be a digit from 0 to 9."); return $0 + number; } diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart index 3171dcc45..90c3f4cea 100644 --- a/lib/src/visitor/serialize.dart +++ b/lib/src/visitor/serialize.dart @@ -16,7 +16,6 @@ import '../ast/node.dart'; import '../ast/selector.dart'; import '../color_names.dart'; import '../exception.dart'; -import '../io.dart'; import '../parse/parser.dart'; import '../utils.dart'; import '../util/character.dart'; @@ -682,26 +681,19 @@ class _SerializeVisitor // have to do is clamp doubles that are close to being integers. var integer = fuzzyAsInt(number); if (integer != null) { - // Node.js prints integers at least 1e21 using exponential notation. - _buffer.write(isNode && integer >= 1e21 - ? _removeExponent(integer.toString()) - : integer.toString()); + _buffer.write(_removeExponent(integer.toString())); return; } - // Dart and Node both print doubles at least 1e21 using exponential - // notation. - var text = - number >= 1e21 ? _removeExponent(number.toString()) : number.toString(); + var text = _removeExponent(number.toString()); // Any double that's less than `SassNumber.precision + 2` digits long is // guaranteed to be safe to emit directly, since it'll contain at most `0.` // followed by [SassNumber.precision] digits. var canWriteDirectly = text.length < SassNumber.precision + 2; - if (_isCompressed && text.codeUnitAt(0) == $0) text = text.substring(1); - if (canWriteDirectly) { + if (_isCompressed && text.codeUnitAt(0) == $0) text = text.substring(1); _buffer.write(text); return; } @@ -716,6 +708,8 @@ class _SerializeVisitor String _removeExponent(String text) { // Don't allocate this until we know [text] contains exponent notation. StringBuffer? buffer; + var negative = text.codeUnitAt(0) == $minus; + late int exponent; for (var i = 0; i < text.length; i++) { var codeUnit = text.codeUnitAt(i); @@ -727,7 +721,12 @@ class _SerializeVisitor // If the number has more than one significant digit, the second // character will be a decimal point that we don't want to include in // the generated number. - if (i > 2) buffer.write(text.substring(2, i)); + if (negative) { + buffer.writeCharCode(text.codeUnitAt(1)); + if (i > 3) buffer.write(text.substring(3, i)); + } else { + if (i > 2) buffer.write(text.substring(2, i)); + } exponent = int.parse(text.substring(i + 1, text.length)); break; @@ -737,8 +736,11 @@ class _SerializeVisitor if (exponent > 0) { // Write an additional zero for each exponent digits other than those // already written to the buffer. We subtract 1 from `buffer.length` - // because the first digit doesn't count towards the exponent. - var additionalZeroes = exponent - (buffer.length - 1); + // because the first digit doesn't count towards the exponent. Subtract 1 + // more for negative numbers because of the `-` written to the buffer. + var additionalZeroes = + exponent - (buffer.length - 1 - (negative ? 1 : 0)); + for (var i = 0; i < additionalZeroes; i++) { buffer.writeCharCode($0); } @@ -759,59 +761,108 @@ class _SerializeVisitor /// Assuming [text] is a double written without exponent notation, writes it /// to [_buffer] with at most [SassNumber.precision] digits after the decimal. void _writeDecimal(String text) { - // Write up until the decimal point, or to the end of [text] if it has no - // decimal point. + assert(RegExp(r"-?\d+.\d+").hasMatch(text), + '"$text" should be a double written without exponent notation.'); + + // Dart serializes all doubles with a trailing `.0`, even if they have + // integer values. In that case we definitely don't need to adjust for + // precision, so we can just write the number as-is without the `.0`. + if (text.endsWith(".0")) { + _buffer.write(text.substring(0, text.length - 1)); + return; + } + + // Consume through the decimal point (which we assume to exist). Don't + // actually write to [_buffer] yet because, if the number starts with `-0`, + // we might not want to write the minus sign if the decimal portion turns + // out to be insignificant. var textIndex = 0; - for (; textIndex < text.length; textIndex++) { - var codeUnit = text.codeUnitAt(textIndex); - if (codeUnit == $dot) { - // Most integer-value doubles will have been converted to ints using - // [fuzzyAsInt] in [_writeNumber]. However, that logic isn't rock-solid - // for very large doubles due to floating-point imprecision, so we - // handle that case here as well. - if (textIndex == text.length - 2 && - text.codeUnitAt(text.length - 1) == $0) { - return; - } + var negative = text.codeUnitAt(0) == $minus; + if (negative) textIndex++; - _buffer.writeCharCode(codeUnit); - textIndex++; - break; - } + // We need to ensure that we write at most [SassNumber.precision] digits + // after the decimal point, and that we round appropriately if necessary. To + // do this, we maintain an intermediate buffer of digits (both before and + // after the decimal point), which we then write to [_buffer] as text. We + // start writing after the first digit to give us room to round up to a + // higher decimal place than was represented in the original number. + var digits = Uint8List(text.length); + var digitsIndex = 1; + + // Write the digits before the decimal to [digits]. + while (true) { + var codeUnit = text.codeUnitAt(textIndex++); + if (codeUnit == $dot) break; + digits[digitsIndex++] = asDecimal(codeUnit); + } + var firstFractionalDigit = digitsIndex; - _buffer.writeCharCode(codeUnit); + // Only write at most [precision] digits after the decimal. If there aren't + // that many digits left in the number, write it as-is since no rounding or + // truncation is needed. + var indexAfterPrecision = textIndex + SassNumber.precision; + if (indexAfterPrecision >= text.length) { + _buffer.write(text); + return; } - if (textIndex == text.length) return; - // We need to ensure that we write at most [SassNumber.precision] digits - // after the decimal point, and that we round appropriately if necessary. To - // do this, we maintain an intermediate buffer of decimal digits, which we - // then convert to text. - var digits = Uint8List(SassNumber.precision); - var digitsIndex = 0; - while (textIndex < text.length && digitsIndex < digits.length) { + // Write the digits after the decimal to [digits]. + while (textIndex < indexAfterPrecision) { digits[digitsIndex++] = asDecimal(text.codeUnitAt(textIndex++)); } - // Round the trailing digits in [digits] up if necessary. We can be - // confident this won't cause us to need to round anything before the - // decimal, because otherwise the number would be [fuzzyIsInt]. - if (textIndex != text.length && - asDecimal(text.codeUnitAt(textIndex)) >= 5) { - while (digitsIndex >= 0) { + // Round the trailing digits in [digits] up if necessary. + if (asDecimal(text.codeUnitAt(textIndex)) >= 5) { + while (true) { + // [digitsIndex] is guaranteed to be >0 here because we added a leading + // 0 to [digits] when we constructed it, so even if we round everything + // up [newDigit] will always be 1 when digitsIndex is 1. var newDigit = ++digits[digitsIndex - 1]; if (newDigit != 10) break; digitsIndex--; } } - // Remove trailing zeros. - while (digitsIndex > 0 && digits[digitsIndex - 1] == 0) { + // At most one of the following loops will actually execute. If we rounded + // digits up before the decimal point, the first loop will set those digits + // to 0 (rather than 10, which is not a valid decimal digit). On the other + // hand, if we have trailing zeros left after the decimal point, the second + // loop will move [digitsIndex] before them and cause them not to be + // written. Either way, [digitsIndex] will end up >= [firstFractionalDigit]. + for (; digitsIndex < firstFractionalDigit; digitsIndex++) { + digits[digitsIndex] = 0; + } + while (digitsIndex > firstFractionalDigit && digits[digitsIndex - 1] == 0) { digitsIndex--; } - for (var i = 0; i < digitsIndex; i++) { - _buffer.writeCharCode(decimalCharFor(digits[i])); + // Omit the minus sign if the number ended up being rounded to exactly zero, + // write "0" explicit to avoid adding a minus sign or omitting the number + // entirely in compressed mode. + if (digitsIndex == 2 && digits[0] == 0 && digits[1] == 0) { + _buffer.writeCharCode($0); + return; + } + + if (negative) _buffer.writeCharCode($minus); + + // Write the digits before the decimal point to [_buffer]. Omit the leading + // 0 that's added to [digits] to accommodate rounding, and in compressed + // mode omit the 0 before the decimal point as well. + var writtenIndex = 0; + if (digits[0] == 0) { + writtenIndex++; + if (_isCompressed && digits[1] == 0) writtenIndex++; + } + for (; writtenIndex < firstFractionalDigit; writtenIndex++) { + _buffer.writeCharCode(decimalCharFor(digits[writtenIndex])); + } + + if (digitsIndex > firstFractionalDigit) { + _buffer.writeCharCode($dot); + for (; writtenIndex < digitsIndex; writtenIndex++) { + _buffer.writeCharCode(decimalCharFor(digits[writtenIndex])); + } } } From b968d87e1c91b16a25d689c2abc1a3403d6edeae Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 18 May 2021 18:33:50 -0700 Subject: [PATCH 2/4] Fix test failures --- lib/src/util/character.dart | 2 +- lib/src/visitor/serialize.dart | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/src/util/character.dart b/lib/src/util/character.dart index e899f96b5..9c6181ef9 100644 --- a/lib/src/util/character.dart +++ b/lib/src/util/character.dart @@ -103,7 +103,7 @@ int asDecimal(int character) { /// /// Assumes that [number] is less than 10. int decimalCharFor(int number) { - assert(number < 10, "Expected ${number} to be a digit from 0 to 9."); + assert(number < 10, "Expected $number to be a digit from 0 to 9."); return $0 + number; } diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart index 90c3f4cea..7a84c7e20 100644 --- a/lib/src/visitor/serialize.dart +++ b/lib/src/visitor/serialize.dart @@ -681,6 +681,8 @@ class _SerializeVisitor // have to do is clamp doubles that are close to being integers. var integer = fuzzyAsInt(number); if (integer != null) { + // Node.js still uses exponential notation for integers, so we have to + // handle it here. _buffer.write(_removeExponent(integer.toString())); return; } @@ -744,6 +746,10 @@ class _SerializeVisitor for (var i = 0; i < additionalZeroes; i++) { buffer.writeCharCode($0); } + + // Format this like a double so we can still pass it to [_writeDecimal]. + buffer.writeCharCode($dot); + buffer.writeCharCode($0); return buffer.toString(); } else { var result = StringBuffer(); @@ -761,7 +767,7 @@ class _SerializeVisitor /// Assuming [text] is a double written without exponent notation, writes it /// to [_buffer] with at most [SassNumber.precision] digits after the decimal. void _writeDecimal(String text) { - assert(RegExp(r"-?\d+.\d+").hasMatch(text), + assert(RegExp(r"-?\d+\.\d+").hasMatch(text), '"$text" should be a double written without exponent notation.'); // Dart serializes all doubles with a trailing `.0`, even if they have From b67b47cfb9ca6e4ee215692ecfff764427f3debe Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 19 May 2021 13:48:40 -0700 Subject: [PATCH 3/4] Pin static analysis to Dart 2.12 This works around dart-lang/sdk#45488 --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8999037b5..03c5b2c27 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,6 +121,8 @@ jobs: steps: - uses: actions/checkout@v2 - uses: dart-lang/setup-dart@v1 + # TODO(nweiz): Use the latest Dart when dart-lang/sdk#45488 + with: {sdk: 2.12.4} - run: dart pub get - name: Analyze dart run: dartanalyzer --fatal-warnings --fatal-infos lib tool test From 1a5bd1e74dcfdfd264db0bf7e30992b660b3532d Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 19 May 2021 14:09:30 -0700 Subject: [PATCH 4/4] Make _writeDecimal able to handle numbers without decimal points --- lib/src/visitor/serialize.dart | 39 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart index 7a84c7e20..fb7e35240 100644 --- a/lib/src/visitor/serialize.dart +++ b/lib/src/visitor/serialize.dart @@ -700,7 +700,7 @@ class _SerializeVisitor return; } - _writeDecimal(text); + _writeRounded(text); } /// If [text] is written in exponent notation, returns a string representation @@ -746,10 +746,6 @@ class _SerializeVisitor for (var i = 0; i < additionalZeroes; i++) { buffer.writeCharCode($0); } - - // Format this like a double so we can still pass it to [_writeDecimal]. - buffer.writeCharCode($dot); - buffer.writeCharCode($0); return buffer.toString(); } else { var result = StringBuffer(); @@ -764,39 +760,42 @@ class _SerializeVisitor } } - /// Assuming [text] is a double written without exponent notation, writes it - /// to [_buffer] with at most [SassNumber.precision] digits after the decimal. - void _writeDecimal(String text) { - assert(RegExp(r"-?\d+\.\d+").hasMatch(text), - '"$text" should be a double written without exponent notation.'); + /// Assuming [text] is a number written without exponent notation, rounds it + /// to [SassNumber.precision] digits after the decimal and writes the result + /// to [_buffer]. + void _writeRounded(String text) { + assert(RegExp(r"^-?\d+(\.\d+)?$").hasMatch(text), + '"$text" should be a number written without exponent notation.'); // Dart serializes all doubles with a trailing `.0`, even if they have // integer values. In that case we definitely don't need to adjust for // precision, so we can just write the number as-is without the `.0`. if (text.endsWith(".0")) { - _buffer.write(text.substring(0, text.length - 1)); + _buffer.write(text.substring(0, text.length - 2)); return; } - // Consume through the decimal point (which we assume to exist). Don't - // actually write to [_buffer] yet because, if the number starts with `-0`, - // we might not want to write the minus sign if the decimal portion turns - // out to be insignificant. - var textIndex = 0; - var negative = text.codeUnitAt(0) == $minus; - if (negative) textIndex++; - // We need to ensure that we write at most [SassNumber.precision] digits // after the decimal point, and that we round appropriately if necessary. To // do this, we maintain an intermediate buffer of digits (both before and // after the decimal point), which we then write to [_buffer] as text. We // start writing after the first digit to give us room to round up to a // higher decimal place than was represented in the original number. - var digits = Uint8List(text.length); + var digits = Uint8List(text.length + 1); var digitsIndex = 1; // Write the digits before the decimal to [digits]. + var textIndex = 0; + var negative = text.codeUnitAt(0) == $minus; + if (negative) textIndex++; while (true) { + if (textIndex == text.length) { + // If we get here, [text] has no decmial point. It definitely doesn't + // need to be rounded; we can write it as-is. + _buffer.write(text); + return; + } + var codeUnit = text.codeUnitAt(textIndex++); if (codeUnit == $dot) break; digits[digitsIndex++] = asDecimal(codeUnit);