Skip to content

Commit f33b934

Browse files
authored
Fix some edge cases when serializing edge-case numbers (#1312)
Also pin static analysis to Dart 2.12 to work around dart-lang/sdk#45488.
1 parent 3849165 commit f33b934

File tree

4 files changed

+119
-55
lines changed

4 files changed

+119
-55
lines changed

.github/workflows/ci.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ jobs:
121121
steps:
122122
- uses: actions/checkout@v2
123123
- uses: dart-lang/setup-dart@v1
124+
# TODO(nweiz): Use the latest Dart when dart-lang/sdk#45488
125+
with: {sdk: 2.12.4}
124126
- run: dart pub get
125127
- name: Analyze dart
126128
run: dartanalyzer --fatal-warnings --fatal-infos lib tool test

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@
1515
made slash-free in both cases. This is a behavioral change, but it's unlikely
1616
to affect any real-world stylesheets.
1717

18+
* Fix a bug where non-integer numbers that were very close to integer
19+
values would be incorrectly formatted in CSS.
20+
21+
* Fix a bug where very small number and very large negative numbers would be
22+
incorrectly formatted in CSS.
23+
1824
### JS API
1925

2026
* The `this` context for importers now has a `fromImport` field, which is `true`

lib/src/util/character.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ int asDecimal(int character) {
103103
///
104104
/// Assumes that [number] is less than 10.
105105
int decimalCharFor(int number) {
106-
assert(number < 10);
106+
assert(number < 10, "Expected $number to be a digit from 0 to 9.");
107107
return $0 + number;
108108
}
109109

lib/src/visitor/serialize.dart

Lines changed: 110 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import '../ast/node.dart';
1616
import '../ast/selector.dart';
1717
import '../color_names.dart';
1818
import '../exception.dart';
19-
import '../io.dart';
2019
import '../parse/parser.dart';
2120
import '../utils.dart';
2221
import '../util/character.dart';
@@ -682,31 +681,26 @@ class _SerializeVisitor
682681
// have to do is clamp doubles that are close to being integers.
683682
var integer = fuzzyAsInt(number);
684683
if (integer != null) {
685-
// Node.js prints integers at least 1e21 using exponential notation.
686-
_buffer.write(isNode && integer >= 1e21
687-
? _removeExponent(integer.toString())
688-
: integer.toString());
684+
// Node.js still uses exponential notation for integers, so we have to
685+
// handle it here.
686+
_buffer.write(_removeExponent(integer.toString()));
689687
return;
690688
}
691689

692-
// Dart and Node both print doubles at least 1e21 using exponential
693-
// notation.
694-
var text =
695-
number >= 1e21 ? _removeExponent(number.toString()) : number.toString();
690+
var text = _removeExponent(number.toString());
696691

697692
// Any double that's less than `SassNumber.precision + 2` digits long is
698693
// guaranteed to be safe to emit directly, since it'll contain at most `0.`
699694
// followed by [SassNumber.precision] digits.
700695
var canWriteDirectly = text.length < SassNumber.precision + 2;
701696

702-
if (_isCompressed && text.codeUnitAt(0) == $0) text = text.substring(1);
703-
704697
if (canWriteDirectly) {
698+
if (_isCompressed && text.codeUnitAt(0) == $0) text = text.substring(1);
705699
_buffer.write(text);
706700
return;
707701
}
708702

709-
_writeDecimal(text);
703+
_writeRounded(text);
710704
}
711705

712706
/// If [text] is written in exponent notation, returns a string representation
@@ -716,6 +710,8 @@ class _SerializeVisitor
716710
String _removeExponent(String text) {
717711
// Don't allocate this until we know [text] contains exponent notation.
718712
StringBuffer? buffer;
713+
var negative = text.codeUnitAt(0) == $minus;
714+
719715
late int exponent;
720716
for (var i = 0; i < text.length; i++) {
721717
var codeUnit = text.codeUnitAt(i);
@@ -727,7 +723,12 @@ class _SerializeVisitor
727723
// If the number has more than one significant digit, the second
728724
// character will be a decimal point that we don't want to include in
729725
// the generated number.
730-
if (i > 2) buffer.write(text.substring(2, i));
726+
if (negative) {
727+
buffer.writeCharCode(text.codeUnitAt(1));
728+
if (i > 3) buffer.write(text.substring(3, i));
729+
} else {
730+
if (i > 2) buffer.write(text.substring(2, i));
731+
}
731732

732733
exponent = int.parse(text.substring(i + 1, text.length));
733734
break;
@@ -737,8 +738,11 @@ class _SerializeVisitor
737738
if (exponent > 0) {
738739
// Write an additional zero for each exponent digits other than those
739740
// already written to the buffer. We subtract 1 from `buffer.length`
740-
// because the first digit doesn't count towards the exponent.
741-
var additionalZeroes = exponent - (buffer.length - 1);
741+
// because the first digit doesn't count towards the exponent. Subtract 1
742+
// more for negative numbers because of the `-` written to the buffer.
743+
var additionalZeroes =
744+
exponent - (buffer.length - 1 - (negative ? 1 : 0));
745+
742746
for (var i = 0; i < additionalZeroes; i++) {
743747
buffer.writeCharCode($0);
744748
}
@@ -756,62 +760,114 @@ class _SerializeVisitor
756760
}
757761
}
758762

759-
/// Assuming [text] is a double written without exponent notation, writes it
760-
/// to [_buffer] with at most [SassNumber.precision] digits after the decimal.
761-
void _writeDecimal(String text) {
762-
// Write up until the decimal point, or to the end of [text] if it has no
763-
// decimal point.
764-
var textIndex = 0;
765-
for (; textIndex < text.length; textIndex++) {
766-
var codeUnit = text.codeUnitAt(textIndex);
767-
if (codeUnit == $dot) {
768-
// Most integer-value doubles will have been converted to ints using
769-
// [fuzzyAsInt] in [_writeNumber]. However, that logic isn't rock-solid
770-
// for very large doubles due to floating-point imprecision, so we
771-
// handle that case here as well.
772-
if (textIndex == text.length - 2 &&
773-
text.codeUnitAt(text.length - 1) == $0) {
774-
return;
775-
}
763+
/// Assuming [text] is a number written without exponent notation, rounds it
764+
/// to [SassNumber.precision] digits after the decimal and writes the result
765+
/// to [_buffer].
766+
void _writeRounded(String text) {
767+
assert(RegExp(r"^-?\d+(\.\d+)?$").hasMatch(text),
768+
'"$text" should be a number written without exponent notation.');
769+
770+
// Dart serializes all doubles with a trailing `.0`, even if they have
771+
// integer values. In that case we definitely don't need to adjust for
772+
// precision, so we can just write the number as-is without the `.0`.
773+
if (text.endsWith(".0")) {
774+
_buffer.write(text.substring(0, text.length - 2));
775+
return;
776+
}
776777

777-
_buffer.writeCharCode(codeUnit);
778-
textIndex++;
779-
break;
778+
// We need to ensure that we write at most [SassNumber.precision] digits
779+
// after the decimal point, and that we round appropriately if necessary. To
780+
// do this, we maintain an intermediate buffer of digits (both before and
781+
// after the decimal point), which we then write to [_buffer] as text. We
782+
// start writing after the first digit to give us room to round up to a
783+
// higher decimal place than was represented in the original number.
784+
var digits = Uint8List(text.length + 1);
785+
var digitsIndex = 1;
786+
787+
// Write the digits before the decimal to [digits].
788+
var textIndex = 0;
789+
var negative = text.codeUnitAt(0) == $minus;
790+
if (negative) textIndex++;
791+
while (true) {
792+
if (textIndex == text.length) {
793+
// If we get here, [text] has no decmial point. It definitely doesn't
794+
// need to be rounded; we can write it as-is.
795+
_buffer.write(text);
796+
return;
780797
}
781798

782-
_buffer.writeCharCode(codeUnit);
799+
var codeUnit = text.codeUnitAt(textIndex++);
800+
if (codeUnit == $dot) break;
801+
digits[digitsIndex++] = asDecimal(codeUnit);
783802
}
784-
if (textIndex == text.length) return;
803+
var firstFractionalDigit = digitsIndex;
785804

786-
// We need to ensure that we write at most [SassNumber.precision] digits
787-
// after the decimal point, and that we round appropriately if necessary. To
788-
// do this, we maintain an intermediate buffer of decimal digits, which we
789-
// then convert to text.
790-
var digits = Uint8List(SassNumber.precision);
791-
var digitsIndex = 0;
792-
while (textIndex < text.length && digitsIndex < digits.length) {
805+
// Only write at most [precision] digits after the decimal. If there aren't
806+
// that many digits left in the number, write it as-is since no rounding or
807+
// truncation is needed.
808+
var indexAfterPrecision = textIndex + SassNumber.precision;
809+
if (indexAfterPrecision >= text.length) {
810+
_buffer.write(text);
811+
return;
812+
}
813+
814+
// Write the digits after the decimal to [digits].
815+
while (textIndex < indexAfterPrecision) {
793816
digits[digitsIndex++] = asDecimal(text.codeUnitAt(textIndex++));
794817
}
795818

796-
// Round the trailing digits in [digits] up if necessary. We can be
797-
// confident this won't cause us to need to round anything before the
798-
// decimal, because otherwise the number would be [fuzzyIsInt].
799-
if (textIndex != text.length &&
800-
asDecimal(text.codeUnitAt(textIndex)) >= 5) {
801-
while (digitsIndex >= 0) {
819+
// Round the trailing digits in [digits] up if necessary.
820+
if (asDecimal(text.codeUnitAt(textIndex)) >= 5) {
821+
while (true) {
822+
// [digitsIndex] is guaranteed to be >0 here because we added a leading
823+
// 0 to [digits] when we constructed it, so even if we round everything
824+
// up [newDigit] will always be 1 when digitsIndex is 1.
802825
var newDigit = ++digits[digitsIndex - 1];
803826
if (newDigit != 10) break;
804827
digitsIndex--;
805828
}
806829
}
807830

808-
// Remove trailing zeros.
809-
while (digitsIndex > 0 && digits[digitsIndex - 1] == 0) {
831+
// At most one of the following loops will actually execute. If we rounded
832+
// digits up before the decimal point, the first loop will set those digits
833+
// to 0 (rather than 10, which is not a valid decimal digit). On the other
834+
// hand, if we have trailing zeros left after the decimal point, the second
835+
// loop will move [digitsIndex] before them and cause them not to be
836+
// written. Either way, [digitsIndex] will end up >= [firstFractionalDigit].
837+
for (; digitsIndex < firstFractionalDigit; digitsIndex++) {
838+
digits[digitsIndex] = 0;
839+
}
840+
while (digitsIndex > firstFractionalDigit && digits[digitsIndex - 1] == 0) {
810841
digitsIndex--;
811842
}
812843

813-
for (var i = 0; i < digitsIndex; i++) {
814-
_buffer.writeCharCode(decimalCharFor(digits[i]));
844+
// Omit the minus sign if the number ended up being rounded to exactly zero,
845+
// write "0" explicit to avoid adding a minus sign or omitting the number
846+
// entirely in compressed mode.
847+
if (digitsIndex == 2 && digits[0] == 0 && digits[1] == 0) {
848+
_buffer.writeCharCode($0);
849+
return;
850+
}
851+
852+
if (negative) _buffer.writeCharCode($minus);
853+
854+
// Write the digits before the decimal point to [_buffer]. Omit the leading
855+
// 0 that's added to [digits] to accommodate rounding, and in compressed
856+
// mode omit the 0 before the decimal point as well.
857+
var writtenIndex = 0;
858+
if (digits[0] == 0) {
859+
writtenIndex++;
860+
if (_isCompressed && digits[1] == 0) writtenIndex++;
861+
}
862+
for (; writtenIndex < firstFractionalDigit; writtenIndex++) {
863+
_buffer.writeCharCode(decimalCharFor(digits[writtenIndex]));
864+
}
865+
866+
if (digitsIndex > firstFractionalDigit) {
867+
_buffer.writeCharCode($dot);
868+
for (; writtenIndex < digitsIndex; writtenIndex++) {
869+
_buffer.writeCharCode(decimalCharFor(digits[writtenIndex]));
870+
}
815871
}
816872
}
817873

0 commit comments

Comments
 (0)