Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Update: Add no-magic-numbers 'ignoreDefaultValues' option (#12611)
* Fix: Add no-magic-numbers 'ignoreDefaultValues' option

* Test: Add cases for `no-magic-numbers` array destructuring

* Chore: consistent function parameters for `no-magic-numbers`

* Test: default option `no-magic-numbers`

* Docs: add `no-magic-numbers` array destructuring example

* Fix: add `no-magic-numbers` additional check

* Test: add `no-magic-numbers` test

* Docs: fix typo

* Chore: move ignoreDefaultValues option check

* Docs: add line-break

* Test: Add test for default function parameter values

* Chore: rename function

* docs: update JSDoc
  • Loading branch information
moeriki committed Aug 20, 2020
1 parent b487164 commit 66442a9
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 3 deletions.
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) { /***/ }
```

```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 }) => {}",
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 }
]
},
{
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 }
]
},
{
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 }
]
}
]
});

0 comments on commit 66442a9

Please sign in to comment.