Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some edge cases when serializing edge-case numbers #1312

Merged
merged 4 commits into from May 20, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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`
Expand Down
2 changes: 1 addition & 1 deletion lib/src/util/character.dart
Expand Up @@ -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;
}

Expand Down
151 changes: 101 additions & 50 deletions lib/src/visitor/serialize.dart
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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);
}
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great docs throughout, they are super helpful.

// 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]));
}
}
}

Expand Down