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

New: no-loss-of-precision (fixes #11279) #12747

Merged
merged 28 commits into from May 20, 2020
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
bece581
Created rule and test files
jmoore914 Dec 29, 2019
f95678e
Working rules and tests
jmoore914 Dec 30, 2019
dd9d66d
Working for decimals and scientific notation
jmoore914 Dec 31, 2019
a02acc2
Check all digits match instead of just most precise
jmoore914 Dec 31, 2019
7258cbd
Created rule and test files
jmoore914 Dec 29, 2019
479767b
Working rules and tests
jmoore914 Dec 30, 2019
f4864e9
Working for decimals and scientific notation
jmoore914 Dec 31, 2019
64a61bf
Check all digits match instead of just most precise
jmoore914 Dec 31, 2019
d8edf52
Expanded rule to non-base ten numbers
jmoore914 Jan 5, 2020
1f4b1ea
New: noLossOfPrecision (fixes #11279)
jmoore914 Jan 5, 2020
3a9be96
Added docs and updated config files
jmoore914 Jan 5, 2020
e7ca7b7
Update docs/rules/no-loss-of-precision.md
jmoore914 Jan 31, 2020
6238203
Update lib/rules/no-loss-of-precision.js
jmoore914 Jan 31, 2020
56f1aae
Removed rule from recommended
jmoore914 Jan 31, 2020
ef8ec88
Merge branch 'no-loss-of-precision' of https://github.com/jmoore914/e…
jmoore914 Jan 31, 2020
5fcd473
Renamed functions; fixed function description
jmoore914 Jan 31, 2020
a6e3df8
Update lib/rules/no-loss-of-precision.js
jmoore914 Jan 31, 2020
bb40fb4
Removed always-true conditional
jmoore914 Jan 31, 2020
3a0d251
Removed rule from recommended
jmoore914 Jan 31, 2020
4fb6481
Fixing octal cases
jmoore914 Jan 31, 2020
f293b93
Working with octals
jmoore914 Jan 31, 2020
ff1e610
Merge branch 'no-loss-of-precision' of https://github.com/jmoore914/e…
jmoore914 Jan 31, 2020
9eccda2
Changed isNotBaseTen to isBaseTen
jmoore914 Feb 1, 2020
ec9b14e
Simplify isBaseTen test
jmoore914 Feb 7, 2020
76179a2
Merge remote-tracking branch 'upstream/master' into no-loss-of-precision
jmoore914 Feb 7, 2020
d39d8e4
Merge remote-tracking branch 'upstream/master' into no-loss-of-precision
jmoore914 Mar 20, 2020
e82aaf5
Added regression tests
jmoore914 Mar 20, 2020
4398188
Additional regression test
jmoore914 Mar 20, 2020
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
32 changes: 32 additions & 0 deletions docs/rules/no-loss-of-precision.md
@@ -0,0 +1,32 @@
# Disallow Number Literals That Lose Precision (no-loss-of-precision)

This rule would disallow the use of number literals that immediately lose precision at runtime when converted to a JS `Number` due to 64-bit floating-point rounding.

## Rule Details

In JS, `Number`s are stored as double-precision floating-point numbers according to the [IEEE 754 standard](https://en.wikipedia.org/wiki/IEEE_754). Because of this, numbers can only retain accuracy up to a certain amount of digits. If the programmer enters additional digits, those digits will be lost in the conversion to the `Number` type and will result in unexpected behavior.

Examples of **incorrect** code for this rule:

```js
/*eslint no-loss-of-precision: "error"*/

const x = 9007199254740993
const x = 5123000000000000000000000000001
const x = 1230000000000000000000000.0
const x = .1230000000000000000000000
const x = 0X20000000000001
```

Examples of **correct** code for this rule:

```js
/*eslint no-loss-of-precision: "error"*/

const x = 12345
const x = 123.456
const x = 123e34
const x = 12300000000000000000000000
const x = 0x1FFFFFFFFFFFFF
const x = 9007199254740991
```
1 change: 1 addition & 0 deletions lib/rules/index.js
Expand Up @@ -147,6 +147,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
"no-lone-blocks": () => require("./no-lone-blocks"),
"no-lonely-if": () => require("./no-lonely-if"),
"no-loop-func": () => require("./no-loop-func"),
"no-loss-of-precision": () => require("./no-loss-of-precision"),
"no-magic-numbers": () => require("./no-magic-numbers"),
"no-misleading-character-class": () => require("./no-misleading-character-class"),
"no-mixed-operators": () => require("./no-mixed-operators"),
Expand Down
210 changes: 210 additions & 0 deletions lib/rules/no-loss-of-precision.js
@@ -0,0 +1,210 @@
/**
* @fileoverview Rule to flag numbers that will lose significant figure precision at runtime
* @author Jacob Moore
*/

"use strict";

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
type: "problem",

docs: {
description: "disallow literal numbers that lose precision",
category: "Possible Errors",
recommended: false,
url: "https://eslint.org/docs/rules/no-loss-of-precision"
},
schema: [],
messages: {
noLossOfPrecision: "This number literal will lose precision at runtime."
}
},

create(context) {

/**
* Returns whether the node is number literal
* @param {Node} node the node literal being evaluated
* @returns {boolean} true if the node is a number literal
*/
function isNumber(node) {
return typeof node.value === "number";
}


/**
* Returns whether the string only contains octal digits
* @param {string} rawString the string representation of the number being evaluated
* @returns {boolean} true if the string contains only octal digits
*/
function isOctalDigitsOnly(rawString) {
return rawString.split().every(digit => parseInt(digit, 10) < 8);
}

/**
* Checks whether the number is not base ten
* @param {ASTNode} node the node being evaluated
* @returns {boolean} true if the node is not in base ten
*/
function isNotBaseTen(node) {
jmoore914 marked this conversation as resolved.
Show resolved Hide resolved
const prefixes = ["0x", "0X", "0b", "0B", "0o", "0O"];

return prefixes.some(prefix => node.raw.startsWith(prefix)) ||
(node.raw.startsWith("0") &&
!node.raw.startsWith("0e") &&
!node.raw.startsWith("0.") &&
isOctalDigitsOnly(node.raw));
}

/**
* Checks that the user-intended non-base ten number equals the actual number after is has been converted to the Number type
* @param {Node} node the node being evaluated
* @returns {boolean} true if they do not match
*/
function notBaseTenLosesPrecision(node) {
const rawString = node.raw.toUpperCase();
let base = 0;

if (rawString.startsWith("0B")) {
base = 2;
} else if (rawString.startsWith("0X")) {
base = 16;
} else {
base = 8;
}

return !rawString.endsWith(node.value.toString(base).toUpperCase());
Copy link
Member

Choose a reason for hiding this comment

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

Using .endsWith here seems a bit weird, although I can't think of any cases where it would be incorrect.

Can you add some tests that use leading zeros on non-base-10 literals (e.g. 0x005f) to make sure the rule doesn't report an error?

}

/**
* Adds a decimal point to the numeric string at index 1
* @param {string} stringNumber the numeric string without any decimal point
* @returns {string} the numeric string with a decimal point in the proper place
*/
function addDecimalPointToNumber(stringNumber) {
return `${stringNumber.slice(0, 1)}.${stringNumber.slice(1)}`;
}

/**
* Returns the number stripped of leading zeros
* @param {string} numberAsString the string representation of the number
* @returns {string} the stripped string
*/
function removeLeadingZeros(numberAsString) {
return numberAsString.replace(/^0*/u, "");
}

/**
* Returns the number stripped of trailing zeros
* @param {string} numberAsString the string representation of the number
* @returns {string} the stripped string
*/
function removeTrailingZeros(numberAsString) {
return numberAsString.replace(/0*$/u, "");
}

/**
* Converts an integer to to an object containing the the integer's coefficient and order of magnitude
* @param {string} stringInteger the string representation of the integer being converted
* @returns {Object} the object containing the the integer's coefficient and order of magnitude
*/
function normalizeInteger(stringInteger) {
const significantDigits = removeTrailingZeros(removeLeadingZeros(stringInteger));

return {
magnitude: stringInteger.startsWith("0") ? stringInteger.length - 2 : stringInteger.length - 1,
coefficient: addDecimalPointToNumber(significantDigits)
};
}

/**
*
* Converts a float to to an object containing the the floats's coefficient and order of magnitude
* @param {string} stringFloat the string representation of the floate being converted
jmoore914 marked this conversation as resolved.
Show resolved Hide resolved
* @returns {Object} the object containing the the integer's coefficient and order of magnitude
*/
function normalizeFloat(stringFloat) {
const trimmedFloat = removeLeadingZeros(stringFloat);

if (trimmedFloat.startsWith(".")) {
const decimalDigits = trimmedFloat.split(".").pop();
const significantDigits = removeLeadingZeros(decimalDigits);

return {
magnitude: significantDigits.length - decimalDigits.length - 1,
coefficient: addDecimalPointToNumber(significantDigits)
};

}
return {
magnitude: trimmedFloat.indexOf(".") - 1,
coefficient: addDecimalPointToNumber(trimmedFloat.replace(".", ""))

};
}


/**
* Converts a base ten number to proper scientific notation
* @param {string} stringNumber the string representation of the base ten number to be converted
* @returns {string} the number converted to scientific notation
*/
function convertNumberToScientificNotation(stringNumber) {
const splitNumber = stringNumber.replace("E", "e").split("e");
const originalCoefficient = splitNumber[0];
const normalizedNumber = stringNumber.includes(".") ? normalizeFloat(originalCoefficient)
: normalizeInteger(originalCoefficient);
const normalizedCoefficient = normalizedNumber.coefficient;
const magnitude = splitNumber.length > 1 ? (parseInt(splitNumber[1], 10) + normalizedNumber.magnitude)
: normalizedNumber.magnitude;

return `${normalizedCoefficient}e${magnitude}`;

}

/**
* Checks that the user-intended base ten number equals the actual number after is has been converted to the Number type
* @param {Node} node the node being evaluated
* @returns {boolean} true if they do not match
*/
function baseTenLosesPrecision(node) {
const normalizedRawNumber = convertNumberToScientificNotation(node.raw);
const requestedPrecision = normalizedRawNumber.split("e")[0].replace(".", "").length;

if (requestedPrecision > 100) {
return true;
}
const storedNumber = node.value.toPrecision(requestedPrecision);
const normalizedStoredNumber = convertNumberToScientificNotation(storedNumber);

return normalizedRawNumber !== normalizedStoredNumber;
}


/**
* Checks that the user-intended number equals the actual number after is has been converted to the Number type
* @param {Node} node the node being evaluated
* @returns {boolean} true if they do not match
*/
function losesPrecision(node) {
return isNotBaseTen(node) ? notBaseTenLosesPrecision(node) : baseTenLosesPrecision(node);
}


return {
Literal(node) {
if (node.value && isNumber(node) && losesPrecision(node)) {
context.report({
messageId: "noLossOfPrecision",
node
});
}
}
};
}
};