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

Update: Add no-magic-numbers 'ignoreDefaultValues' option #12611

Merged
merged 14 commits into from Aug 20, 2020
Merged
21 changes: 21 additions & 0 deletions docs/rules/no-magic-numbers.md
Expand Up @@ -128,6 +128,27 @@ a = data[4294967295]; // above the max array index
a = data[1e500]; // same as data["Infinity"]
```

### ignoreDefaultValues

A boolean to specify if numbers used in default value assignments are considered okay. `false` by default.

Examples of **correct** code for the `{ "ignoreDefaultValues": true }` option:

```js
/*eslint no-magic-numbers: ["error", { "ignoreDefaultValues": true }]*/

const { tax = 0.25 } = accountancy;

function mapParallel(concurrency = 3) { /***/ }
```
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved

```js
/*eslint no-magic-numbers: ["error", { "ignoreDefaultValues": true }]*/

let head;
[head = 100] = []
```

### enforceConst

A boolean to specify if we should check for the const keyword in variable declaration of numbers. `false` by default.
Expand Down
24 changes: 21 additions & 3 deletions lib/rules/no-magic-numbers.js
Expand Up @@ -61,6 +61,10 @@ module.exports = {
ignoreArrayIndexes: {
type: "boolean",
default: false
},
ignoreDefaultValues: {
type: "boolean",
default: false
}
},
additionalProperties: false
Expand All @@ -77,7 +81,8 @@ module.exports = {
detectObjects = !!config.detectObjects,
enforceConst = !!config.enforceConst,
ignore = (config.ignore || []).map(normalizeIgnoreValue),
ignoreArrayIndexes = !!config.ignoreArrayIndexes;
ignoreArrayIndexes = !!config.ignoreArrayIndexes,
ignoreDefaultValues = !!config.ignoreDefaultValues;

const okTypes = detectObjects ? [] : ["ObjectExpression", "Property", "AssignmentExpression"];

Expand All @@ -90,6 +95,18 @@ module.exports = {
return ignore.indexOf(value) !== -1;
}

/**
* Returns whether the number should be ignored when used as a default
* value assignment.
moeriki marked this conversation as resolved.
Show resolved Hide resolved
* @param {ASTNode} fullNumberNode `Literal` or `UnaryExpression` full number node
* @returns {boolean} true if the number should be ignored
moeriki marked this conversation as resolved.
Show resolved Hide resolved
*/
function isDefaultValue(fullNumberNode) {
const parent = fullNumberNode.parent;

return parent.type === "AssignmentPattern" && parent.right === fullNumberNode;
}

/**
* Returns whether the given node is used as a radix within parseInt() or Number.parseInt()
* @param {ASTNode} fullNumberNode `Literal` or `UnaryExpression` full number node
Expand Down Expand Up @@ -176,18 +193,19 @@ module.exports = {
raw = node.raw;
}

const parent = fullNumberNode.parent;

// Always allow radix arguments and JSX props
if (
isIgnoredValue(value) ||
(ignoreDefaultValues && isDefaultValue(fullNumberNode)) ||
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({
Expand Down
74 changes: 74 additions & 0 deletions tests/lib/rules/no-magic-numbers.js
Expand Up @@ -213,6 +213,31 @@ ruleTester.run("no-magic-numbers", rule, {
code: "f(-100n)",
options: [{ ignore: ["-100n"] }],
parserOptions: { ecmaVersion: 2020 }
},
{
code: "const { param = 123 } = sourceObject;",
options: [{ ignoreDefaultValues: true }],
env: { es6: true }
},
{
code: "const func = (param = 123) => {}",
options: [{ ignoreDefaultValues: true }],
env: { es6: true }
},
{
code: "const func = ({ param = 123 }) => {}",
moeriki marked this conversation as resolved.
Show resolved Hide resolved
options: [{ ignoreDefaultValues: true }],
env: { es6: true }
},
{
code: "const [one = 1, two = 2] = []",
options: [{ ignoreDefaultValues: true }],
env: { es6: true }
},
{
code: "var one, two; [one = 1, two = 2] = []",
options: [{ ignoreDefaultValues: true }],
env: { es6: true }
}
],
invalid: [
Expand Down Expand Up @@ -719,6 +744,55 @@ ruleTester.run("no-magic-numbers", rule, {
errors: [
{ messageId: "noMagic", data: { raw: "100" }, line: 1 }
]
},
{
code: "const func = (param = 123) => {}",
options: [{ ignoreDefaultValues: false }],
env: { es6: true },
errors: [
{ messageId: "noMagic", data: { raw: "123" }, line: 1 }
]
},
{
code: "const { param = 123 } = sourceObject;",
options: [{}],
env: { es6: true },
errors: [
{ messageId: "noMagic", data: { raw: "123" }, line: 1 }
]
},
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
{
code: "const { param = 123 } = sourceObject;",
env: { es6: true },
errors: [
{ messageId: "noMagic", data: { raw: "123" }, line: 1 }
]
},
{
code: "const { param = 123 } = sourceObject;",
options: [{ ignoreDefaultValues: false }],
env: { es6: true },
errors: [
{ messageId: "noMagic", data: { raw: "123" }, line: 1 }
]
moeriki marked this conversation as resolved.
Show resolved Hide resolved
},
{
code: "const [one = 1, two = 2] = []",
options: [{ ignoreDefaultValues: false }],
env: { es6: true },
errors: [
{ messageId: "noMagic", data: { raw: "1" }, line: 1 },
{ messageId: "noMagic", data: { raw: "2" }, line: 1 }
]
},
{
code: "var one, two; [one = 1, two = 2] = []",
options: [{ ignoreDefaultValues: false }],
env: { es6: true },
errors: [
{ messageId: "noMagic", data: { raw: "1" }, line: 1 },
{ messageId: "noMagic", data: { raw: "2" }, line: 1 }
]
}
]
});