diff --git a/docs/rules/no-magic-numbers.md b/docs/rules/no-magic-numbers.md
index f6e1bacbd51..ee6d3de7ee7 100644
--- a/docs/rules/no-magic-numbers.md
+++ b/docs/rules/no-magic-numbers.md
@@ -78,15 +78,54 @@ foo(1n);
### ignoreArrayIndexes
-A boolean to specify if numbers used as array indexes are considered okay. `false` by default.
+A boolean to specify if numbers used in the context of array indexes (e.g., `data[2]`) are considered okay. `false` by default.
+
+This option allows only valid array indexes: numbers that will be coerced to one of `"0"`, `"1"`, `"2"` ... `"4294967294"`.
+
+Arrays are objects, so they can have property names such as `"-1"` or `"2.5"`. However, those are just "normal" object properties that don't represent array elements. They don't influence the array's `length`, and they are ignored by array methods like `.map` or `.forEach`.
+
+Additionally, since the maximum [array length](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/length) is 232 - 1, all values above 232 - 2 also represent just normal property names and are thus not considered to be array indexes.
Examples of **correct** code for the `{ "ignoreArrayIndexes": true }` option:
```js
/*eslint no-magic-numbers: ["error", { "ignoreArrayIndexes": true }]*/
-var data = ['foo', 'bar', 'baz'];
-var dataLast = data[2];
+var item = data[2];
+
+data[100] = a;
+
+f(data[0]);
+
+a = data[-0]; // same as data[0], -0 will be coerced to "0"
+
+a = data[0xAB];
+
+a = data[5.6e1];
+
+a = data[10n]; // same as data[10], 10n will be coerced to "10"
+
+a = data[4294967294]; // max array index
+```
+
+Examples of **incorrect** code for the `{ "ignoreArrayIndexes": true }` option:
+
+```js
+/*eslint no-magic-numbers: ["error", { "ignoreArrayIndexes": true }]*/
+
+f(2); // not used as array index
+
+a = data[-1];
+
+a = data[2.5];
+
+a = data[5.67e1];
+
+a = data[-10n];
+
+a = data[4294967295]; // above the max array index
+
+a = data[1e500]; // same as data["Infinity"]
```
### enforceConst
diff --git a/lib/rules/no-magic-numbers.js b/lib/rules/no-magic-numbers.js
index 5dd6feaab0d..cd07f5c3bda 100644
--- a/lib/rules/no-magic-numbers.js
+++ b/lib/rules/no-magic-numbers.js
@@ -7,6 +7,9 @@
const { isNumericLiteral } = require("./utils/ast-utils");
+// Maximum array length by the ECMAScript Specification.
+const MAX_ARRAY_LENGTH = 2 ** 32 - 1;
+
//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
@@ -76,83 +79,115 @@ module.exports = {
ignore = (config.ignore || []).map(normalizeIgnoreValue),
ignoreArrayIndexes = !!config.ignoreArrayIndexes;
+ const okTypes = detectObjects ? [] : ["ObjectExpression", "Property", "AssignmentExpression"];
+
/**
- * Returns whether the number should be ignored
- * @param {number} num the number
- * @returns {boolean} true if the number should be ignored
+ * Returns whether the rule is configured to ignore the given value
+ * @param {bigint|number} value The value to check
+ * @returns {boolean} true if the value is ignored
*/
- function shouldIgnoreNumber(num) {
- return ignore.indexOf(num) !== -1;
+ function isIgnoredValue(value) {
+ return ignore.indexOf(value) !== -1;
}
/**
- * Returns whether the number should be ignored when used as a radix within parseInt() or Number.parseInt()
- * @param {ASTNode} parent the non-"UnaryExpression" parent
- * @param {ASTNode} node the node literal being evaluated
- * @returns {boolean} true if the number should be ignored
+ * Returns whether the given node is used as a radix within parseInt() or Number.parseInt()
+ * @param {ASTNode} fullNumberNode `Literal` or `UnaryExpression` full number node
+ * @returns {boolean} true if the node is radix
*/
- function shouldIgnoreParseInt(parent, node) {
- return parent.type === "CallExpression" && node === parent.arguments[1] &&
- (parent.callee.name === "parseInt" ||
- parent.callee.type === "MemberExpression" &&
- parent.callee.object.name === "Number" &&
- parent.callee.property.name === "parseInt");
+ function isParseIntRadix(fullNumberNode) {
+ const parent = fullNumberNode.parent;
+
+ return parent.type === "CallExpression" && fullNumberNode === parent.arguments[1] &&
+ (
+ parent.callee.name === "parseInt" ||
+ (
+ parent.callee.type === "MemberExpression" &&
+ parent.callee.object.name === "Number" &&
+ parent.callee.property.name === "parseInt"
+ )
+ );
}
/**
- * Returns whether the number should be ignored when used to define a JSX prop
- * @param {ASTNode} parent the non-"UnaryExpression" parent
- * @returns {boolean} true if the number should be ignored
+ * Returns whether the given node is a direct child of a JSX node.
+ * In particular, it aims to detect numbers used as prop values in JSX tags.
+ * Example:
+ * @param {ASTNode} fullNumberNode `Literal` or `UnaryExpression` full number node
+ * @returns {boolean} true if the node is a JSX number
*/
- function shouldIgnoreJSXNumbers(parent) {
- return parent.type.indexOf("JSX") === 0;
+ function isJSXNumber(fullNumberNode) {
+ return fullNumberNode.parent.type.indexOf("JSX") === 0;
}
/**
- * Returns whether the number should be ignored when used as an array index with enabled 'ignoreArrayIndexes' option.
- * @param {ASTNode} node Node to check
- * @returns {boolean} true if the number should be ignored
+ * Returns whether the given node is used as an array index.
+ * Value must coerce to a valid array index name: "0", "1", "2" ... "4294967294".
+ *
+ * All other values, like "-1", "2.5", or "4294967295", are just "normal" object properties,
+ * which can be created and accessed on an array in addition to the array index properties,
+ * but they don't affect array's length and are not considered by methods such as .map(), .forEach() etc.
+ *
+ * The maximum array length by the specification is 2 ** 32 - 1 = 4294967295,
+ * thus the maximum valid index is 2 ** 32 - 2 = 4294967294.
+ *
+ * All notations are allowed, as long as the value coerces to one of "0", "1", "2" ... "4294967294".
+ *
+ * Valid examples:
+ * a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n]
+ * a[-0] (same as a[0] because -0 coerces to "0")
+ * a[-0n] (-0n evaluates to 0n)
+ *
+ * Invalid examples:
+ * a[-1], a[-0xAB], a[-1n], a[2.5], a[1.23e1], a[12e-1]
+ * a[4294967295] (above the max index, it's an access to a regular property a["4294967295"])
+ * a[999999999999999999999] (even if it wasn't above the max index, it would be a["1e+21"])
+ * a[1e310] (same as a["Infinity"])
+ * @param {ASTNode} fullNumberNode `Literal` or `UnaryExpression` full number node
+ * @param {bigint|number} value Value expressed by the fullNumberNode
+ * @returns {boolean} true if the node is a valid array index
*/
- function shouldIgnoreArrayIndexes(node) {
- const parent = node.parent;
+ function isArrayIndex(fullNumberNode, value) {
+ const parent = fullNumberNode.parent;
- return ignoreArrayIndexes &&
- parent.type === "MemberExpression" && parent.property === node;
+ return parent.type === "MemberExpression" && parent.property === fullNumberNode &&
+ (Number.isInteger(value) || typeof value === "bigint") &&
+ value >= 0 && value < MAX_ARRAY_LENGTH;
}
return {
Literal(node) {
- const okTypes = detectObjects ? [] : ["ObjectExpression", "Property", "AssignmentExpression"];
-
if (!isNumericLiteral(node)) {
return;
}
let fullNumberNode;
- let parent;
let value;
let raw;
- // For negative magic numbers: update the value and parent node
+ // Treat unary minus as a part of the number
if (node.parent.type === "UnaryExpression" && node.parent.operator === "-") {
fullNumberNode = node.parent;
- parent = fullNumberNode.parent;
value = -node.value;
raw = `-${node.raw}`;
} else {
fullNumberNode = node;
- parent = node.parent;
value = node.value;
raw = node.raw;
}
- if (shouldIgnoreNumber(value) ||
- shouldIgnoreParseInt(parent, fullNumberNode) ||
- shouldIgnoreArrayIndexes(fullNumberNode) ||
- shouldIgnoreJSXNumbers(parent)) {
+ // Always allow radix arguments and JSX props
+ if (
+ isIgnoredValue(value) ||
+ isParseIntRadix(fullNumberNode) ||
+ isJSXNumber(fullNumberNode) ||
+ (ignoreArrayIndexes && isArrayIndex(fullNumberNode, value))
+ ) {
return;
}
+ const parent = fullNumberNode.parent;
+
if (parent.type === "VariableDeclarator") {
if (enforceConst && parent.parent.kind !== "const") {
context.report({
diff --git a/tests/lib/rules/no-magic-numbers.js b/tests/lib/rules/no-magic-numbers.js
index 36c0302d137..da75571ec28 100644
--- a/tests/lib/rules/no-magic-numbers.js
+++ b/tests/lib/rules/no-magic-numbers.js
@@ -60,6 +60,134 @@ ruleTester.run("no-magic-numbers", rule, {
ignoreArrayIndexes: true
}]
},
+ {
+ code: "foo[0]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[-0]", // Allowed. -0 is not the same as 0 but it will be coerced to "0", so foo[-0] refers to the element at index 0.
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[1]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[100]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[200.00]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[3e4]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[1.23e2]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[230e-1]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[0b110]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2015 }
+ },
+ {
+ code: "foo[0o71]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2015 }
+ },
+ {
+ code: "foo[0xABC]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[0123]",
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[5.0000000000000001]", // loses precision and evaluates to 5
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[4294967294]", // max array index
+ options: [{
+ ignoreArrayIndexes: true
+ }]
+ },
+ {
+ code: "foo[0n]", // Allowed. 0n will be coerced to "0", so foo[0n] refers to the element at index 0.
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 }
+ },
+ {
+ code: "foo[-0n]", // Allowed. -0n evaluates to 0n which will be coerced to "0", so foo[-0n] refers to the element at index 0.
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 }
+ },
+ {
+ code: "foo[1n]", // Allowed. 1n will be coerced to "1", so foo[1n] refers to the element at index 1.
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 }
+ },
+ {
+ code: "foo[100n]", // Allowed. 100n will be coerced to "100", so foo[100n] refers to the element at index 100.
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 }
+ },
+ {
+ code: "foo[0xABn]", // Allowed. 0xABn is evaluated to 171n and will be coerced to "171", so foo[0xABn] refers to the element at index 171.
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 }
+ },
+ {
+ code: "foo[4294967294n]", // max array index
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 }
+ },
{
code: "var a = ;",
parserOptions: {
@@ -85,27 +213,6 @@ ruleTester.run("no-magic-numbers", rule, {
code: "f(-100n)",
options: [{ ignore: ["-100n"] }],
parserOptions: { ecmaVersion: 2020 }
- },
-
- // Regression tests to preserve the behavior of ignoreArrayIndexes.
- {
- code: "var foo = bar[-100];",
- options: [{
- ignoreArrayIndexes: true
- }]
- },
- {
- code: "var foo = bar[1.5];",
- options: [{
- ignoreArrayIndexes: true
- }]
- },
- {
- code: "var foo = bar[100n];",
- options: [{
- ignoreArrayIndexes: true
- }],
- parserOptions: { ecmaVersion: 2020 }
}
],
invalid: [
@@ -250,6 +357,12 @@ ruleTester.run("no-magic-numbers", rule, {
{ messageId: "noMagic", data: { raw: "10" }, line: 22 }
]
},
+ {
+ code: "var data = ['foo', 'bar', 'baz']; var third = data[3];",
+ errors: [{
+ messageId: "noMagic", data: { raw: "3" }, line: 1
+ }]
+ },
{
code: "var data = ['foo', 'bar', 'baz']; var third = data[3];",
options: [{}],
@@ -257,6 +370,285 @@ ruleTester.run("no-magic-numbers", rule, {
messageId: "noMagic", data: { raw: "3" }, line: 1
}]
},
+ {
+ code: "var data = ['foo', 'bar', 'baz']; var third = data[3];",
+ options: [{
+ ignoreArrayIndexes: false
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "3" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-100]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "-100" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-1.5]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "-1.5" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-1]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "-1" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-0.1]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "-0.1" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-0b110]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2015 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "-0b110" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-0o71]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2015 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "-0o71" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-0x12]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "-0x12" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-012]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "-012" }, line: 1
+ }]
+ },
+ {
+ code: "foo[0.1]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "0.1" }, line: 1
+ }]
+ },
+ {
+ code: "foo[0.12e1]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "0.12e1" }, line: 1
+ }]
+ },
+ {
+ code: "foo[1.5]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "1.5" }, line: 1
+ }]
+ },
+ {
+ code: "foo[1.678e2]", // 167.8
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "1.678e2" }, line: 1
+ }]
+ },
+ {
+ code: "foo[56e-1]", // 5.6
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "56e-1" }, line: 1
+ }]
+ },
+ {
+ code: "foo[5.000000000000001]", // doesn't lose precision
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "5.000000000000001" }, line: 1
+ }]
+ },
+ {
+ code: "foo[100.9]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "100.9" }, line: 1
+ }]
+ },
+ {
+ code: "foo[4294967295]", // first above the max index
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "4294967295" }, line: 1
+ }]
+ },
+ {
+ code: "foo[1e300]", // Above the max, and also coerces to "1e+300"
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "1e300" }, line: 1
+ }]
+ },
+ {
+ code: "foo[1e310]", // refers to property "Infinity"
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "1e310" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-1e310]", // refers to property "-Infinity"
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "-1e310" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-1n]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "-1n" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-100n]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "-100n" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-0x12n]",
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "-0x12n" }, line: 1
+ }]
+ },
+ {
+ code: "foo[4294967295n]", // first above the max index
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "4294967295n" }, line: 1
+ }]
+ },
+ {
+ code: "foo[+0]", // Consistent with the default behavior, which doesn't allow: var foo = +0
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "0" }, line: 1
+ }]
+ },
+ {
+ code: "foo[+1]", // Consistent with the default behavior, which doesn't allow: var foo = +1
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "1" }, line: 1
+ }]
+ },
+ {
+ code: "foo[-(-1)]", // Consistent with the default behavior, which doesn't allow: var foo = -(-1)
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ errors: [{
+ messageId: "noMagic", data: { raw: "-1" }, line: 1
+ }]
+ },
+ {
+ code: "foo[+0n]", // Consistent with the default behavior, which doesn't allow: var foo = +0n
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "0n" }, line: 1
+ }]
+ },
+ {
+ code: "foo[+1n]", // Consistent with the default behavior, which doesn't allow: var foo = +1n
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "1n" }, line: 1
+ }]
+ },
+ {
+ code: "foo[- -1n]", // Consistent with the default behavior, which doesn't allow: var foo = - -1n
+ options: [{
+ ignoreArrayIndexes: true
+ }],
+ parserOptions: { ecmaVersion: 2020 },
+ errors: [{
+ messageId: "noMagic", data: { raw: "-1n" }, line: 1
+ }]
+ },
{
code: "100 .toString()",
options: [{