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
23 changes: 20 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,17 @@ module.exports = {
return ignore.indexOf(value) !== -1;
}

/**
* Returns whether the number is a default value assignment.
* @param {ASTNode} fullNumberNode `Literal` or `UnaryExpression` full number node
* @returns {boolean} true if the number is a default value
*/
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 @@ -172,18 +188,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 @@ -214,6 +214,31 @@ ruleTester.run("no-magic-numbers", rule, {
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 }
},

// Optional chaining
{
Expand Down Expand Up @@ -738,6 +763,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 }
]
}
]
});