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
14 changes: 14 additions & 0 deletions docs/rules/no-magic-numbers.md
Expand Up @@ -128,6 +128,20 @@ 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

### enforceConst

A boolean to specify if we should check for the const keyword in variable declaration of numbers. `false` by default.
Expand Down
22 changes: 19 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,16 @@ module.exports = {
return ignore.indexOf(value) !== -1;
}

/**
* Returns whether the number should be ignore when used as a default
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
* value assignment.
* @param {ASTNode} parent the non-"UnaryExpression" parent.
* @returns {boolean} true if the number should be ignored
moeriki marked this conversation as resolved.
Show resolved Hide resolved
*/
function isIgnoredDefaultValue(parent) {
moeriki marked this conversation as resolved.
Show resolved Hide resolved
return parent.type === "AssignmentPattern" && ignoreDefaultValues;
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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 +191,19 @@ module.exports = {
raw = node.raw;
}

const parent = fullNumberNode.parent;

// Always allow radix arguments and JSX props
if (
isIgnoredValue(value) ||
isIgnoredDefaultValue(parent) ||
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
18 changes: 18 additions & 0 deletions tests/lib/rules/no-magic-numbers.js
Expand Up @@ -213,6 +213,16 @@ 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 }) => {}",
moeriki marked this conversation as resolved.
Show resolved Hide resolved
options: [{ ignoreDefaultValues: true }],
env: { es6: true }
}
],
invalid: [
Expand Down Expand Up @@ -719,6 +729,14 @@ ruleTester.run("no-magic-numbers", rule, {
errors: [
{ messageId: "noMagic", data: { raw: "100" }, 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
}
]
});