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
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
bece581
Created rule and test files
jmoore914 f95678e
Working rules and tests
jmoore914 dd9d66d
Working for decimals and scientific notation
jmoore914 a02acc2
Check all digits match instead of just most precise
jmoore914 7258cbd
Created rule and test files
jmoore914 479767b
Working rules and tests
jmoore914 f4864e9
Working for decimals and scientific notation
jmoore914 64a61bf
Check all digits match instead of just most precise
jmoore914 d8edf52
Expanded rule to non-base ten numbers
jmoore914 1f4b1ea
New: noLossOfPrecision (fixes #11279)
jmoore914 3a9be96
Added docs and updated config files
jmoore914 e7ca7b7
Update docs/rules/no-loss-of-precision.md
jmoore914 6238203
Update lib/rules/no-loss-of-precision.js
jmoore914 56f1aae
Removed rule from recommended
jmoore914 ef8ec88
Merge branch 'no-loss-of-precision' of https://github.com/jmoore914/e…
jmoore914 5fcd473
Renamed functions; fixed function description
jmoore914 a6e3df8
Update lib/rules/no-loss-of-precision.js
jmoore914 bb40fb4
Removed always-true conditional
jmoore914 3a0d251
Removed rule from recommended
jmoore914 4fb6481
Fixing octal cases
jmoore914 f293b93
Working with octals
jmoore914 ff1e610
Merge branch 'no-loss-of-precision' of https://github.com/jmoore914/e…
jmoore914 9eccda2
Changed isNotBaseTen to isBaseTen
jmoore914 ec9b14e
Simplify isBaseTen test
jmoore914 76179a2
Merge remote-tracking branch 'upstream/master' into no-loss-of-precision
jmoore914 d39d8e4
Merge remote-tracking branch 'upstream/master' into no-loss-of-precision
jmoore914 e82aaf5
Added regression tests
jmoore914 4398188
Additional regression test
jmoore914 File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,198 @@ | ||
/** | ||
* @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"; | ||
} | ||
|
||
|
||
/** | ||
* Checks whether the number is base ten | ||
* @param {ASTNode} node the node being evaluated | ||
* @returns {boolean} true if the node is in base ten | ||
*/ | ||
function isBaseTen(node) { | ||
const prefixes = ["0x", "0X", "0b", "0B", "0o", "0O"]; | ||
|
||
return prefixes.every(prefix => !node.raw.startsWith(prefix)) && | ||
!/^0[0-7]+$/u.test(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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using Can you add some tests that use leading zeros on non-base-10 literals (e.g. |
||
} | ||
|
||
/** | ||
* 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 float being converted | ||
* @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 isBaseTen(node) ? baseTenLosesPrecision(node) : notBaseTenLosesPrecision(node); | ||
} | ||
|
||
|
||
return { | ||
Literal(node) { | ||
if (node.value && isNumber(node) && losesPrecision(node)) { | ||
context.report({ | ||
messageId: "noLossOfPrecision", | ||
node | ||
}); | ||
} | ||
} | ||
}; | ||
} | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,159 @@ | ||
/** | ||
*@fileoverview Tests for no-loss-of-precision rule. | ||
*@author Jacob Moore | ||
*/ | ||
|
||
"use strict"; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Requirements | ||
//------------------------------------------------------------------------------ | ||
|
||
const rule = require("../../../lib/rules/no-loss-of-precision"), | ||
{ RuleTester } = require("../../../lib/rule-tester"); | ||
|
||
//------------------------------------------------------------------------------ | ||
// Helpers | ||
//------------------------------------------------------------------------------ | ||
|
||
const ruleTester = new RuleTester(); | ||
|
||
ruleTester.run("no-loss-of-precision", rule, { | ||
valid: [ | ||
"var x = 12345", | ||
kaicataldo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"var x = 123.456", | ||
"var x = -123.456", | ||
"var x = -123456", | ||
"var x = 123e34", | ||
"var x = 123.0e34", | ||
"var x = 123e-34", | ||
"var x = -123e34", | ||
"var x = -123e-34", | ||
"var x = 12.3e34", | ||
"var x = 12.3e-34", | ||
"var x = -12.3e34", | ||
"var x = -12.3e-34", | ||
"var x = 12300000000000000000000000", | ||
"var x = -12300000000000000000000000", | ||
"var x = 0.00000000000000000000000123", | ||
"var x = -0.00000000000000000000000123", | ||
"var x = 9007199254740991", | ||
"var x = 0", | ||
"var x = 0.0", | ||
"var x = 0.000000000000000000000000000000000000000000000000000000000000000000000000000000", | ||
"var x = -0", | ||
"var x = 123.0000000000000000000000", | ||
"var x = 019.5", | ||
"var x = 0195", | ||
"var x = 0e5", | ||
|
||
|
||
{ code: "var x = 0b11111111111111111111111111111111111111111111111111111", parserOptions: { ecmaVersion: 6 } }, | ||
{ code: "var x = 0B11111111111111111111111111111111111111111111111111111", parserOptions: { ecmaVersion: 6 } }, | ||
|
||
{ code: "var x = 0o377777777777777777", parserOptions: { ecmaVersion: 6 } }, | ||
{ code: "var x = 0O377777777777777777", parserOptions: { ecmaVersion: 6 } }, | ||
"var x = 0377777777777777777", | ||
|
||
"var x = 0x1FFFFFFFFFFFFF", | ||
"var x = 0X1FFFFFFFFFFFFF", | ||
"var x = true", | ||
"var x = 'abc'", | ||
"var x = ''", | ||
"var x = null", | ||
"var x = undefined", | ||
"var x = {}", | ||
"var x = ['a', 'b']", | ||
"var x = new Date()", | ||
"var x = '9007199254740993'" | ||
|
||
], | ||
invalid: [ | ||
{ | ||
code: "var x = 9007199254740993", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 9007199254740.993e3", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 9.007199254740993e15", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = -9007199254740993", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 900719.9254740994", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = -900719.9254740994", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
|
||
{ | ||
code: "var x = 5123000000000000000000000000001", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = -5123000000000000000000000000001", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 1230000000000000000000000.0", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 1.0000000000000000000000123", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 17498005798264095394980017816940970922825355447145699491406164851279623993595007385788105416184430592", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 2e999", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = .1230000000000000000000000", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 0b100000000000000000000000000000000000000000000000000001", | ||
parserOptions: { ecmaVersion: 6 }, | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 0B100000000000000000000000000000000000000000000000000001", | ||
parserOptions: { ecmaVersion: 6 }, | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 0o400000000000000001", | ||
parserOptions: { ecmaVersion: 6 }, | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 0O400000000000000001", | ||
parserOptions: { ecmaVersion: 6 }, | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 0400000000000000001", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 0x20000000000001", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
}, | ||
{ | ||
code: "var x = 0X20000000000001", | ||
errors: [{ messageId: "noLossOfPrecision" }] | ||
} | ||
|
||
] | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will treat the literal
0
as octal, which is incorrect although it doesn't affect the result of the precision check.