diff --git a/docs/rules/no-new-array.md b/docs/rules/no-new-array.md new file mode 100644 index 0000000000..224df5af08 --- /dev/null +++ b/docs/rules/no-new-array.md @@ -0,0 +1,35 @@ +# Disallow `new Array()` + +The ESLint built-in rule [`no-array-constructor`](https://eslint.org/docs/rules/no-array-constructor) enforces using an array literal instead of the `Array` constructor, but it still allows using the `Array` constructor with **one** argument. This rule fills that gap. + +When using the `Array` constructor with one argument, it's not clear whether the argument is meant to be the length of the array or the only element. + +This rule is fixable if the value type of the argument is known. + +## Fail + +```js +const array = new Array(length); +``` + +```js +const array = new Array(onlyElement); +``` + +```js +const array = new Array(...unknownArgumentsList); +``` + +## Pass + +```js +const array = Array.from({length}); +``` + +```js +const array = [onlyElement]; +``` + +```js +const array = [...items]; +``` diff --git a/index.js b/index.js index 54ba729426..cd36141653 100644 --- a/index.js +++ b/index.js @@ -64,6 +64,7 @@ module.exports = { 'unicorn/no-lonely-if': 'error', 'no-nested-ternary': 'off', 'unicorn/no-nested-ternary': 'error', + 'unicorn/no-new-array': 'error', 'unicorn/no-new-buffer': 'error', 'unicorn/no-null': 'error', 'unicorn/no-object-as-default-parameter': 'error', diff --git a/readme.md b/readme.md index 10302cafe4..06eb5d2573 100644 --- a/readme.md +++ b/readme.md @@ -58,6 +58,7 @@ Configure it in `package.json`. "unicorn/no-lonely-if": "error", "no-nested-ternary": "off", "unicorn/no-nested-ternary": "error", + "unicorn/no-new-array": "error", "unicorn/no-new-buffer": "error", "unicorn/no-null": "error", "unicorn/no-object-as-default-parameter": "error", @@ -130,6 +131,7 @@ Configure it in `package.json`. - [no-keyword-prefix](docs/rules/no-keyword-prefix.md) - Disallow identifiers starting with `new` or `class`. - [no-lonely-if](docs/rules/no-lonely-if.md) - Disallow `if` statements as the only statement in `if` blocks without `else`. *(fixable)* - [no-nested-ternary](docs/rules/no-nested-ternary.md) - Disallow nested ternary expressions. *(partly fixable)* +- [no-new-array](docs/rules/no-new-array.md) - Disallow `new Array()`. *(partly fixable)* - [no-new-buffer](docs/rules/no-new-buffer.md) - Enforce the use of `Buffer.from()` and `Buffer.alloc()` instead of the deprecated `new Buffer()`. *(fixable)* - [no-null](docs/rules/no-null.md) - Disallow the use of the `null` literal. - [no-object-as-default-parameter](docs/rules/no-object-as-default-parameter.md) - Disallow the use of objects as default parameters. diff --git a/rules/no-new-array.js b/rules/no-new-array.js new file mode 100644 index 0000000000..9e45f246e1 --- /dev/null +++ b/rules/no-new-array.js @@ -0,0 +1,95 @@ +'use strict'; +const {isParenthesized, getStaticValue} = require('eslint-utils'); +const getDocumentationUrl = require('./utils/get-documentation-url'); +const needsSemicolon = require('./utils/needs-semicolon'); + +const MESSAGE_ID_ERROR = 'error'; +const MESSAGE_ID_LENGTH = 'array-length'; +const MESSAGE_ID_ONLY_ELEMENT = 'only-element'; +const MESSAGE_ID_SPREAD = 'spread'; +const messages = { + [MESSAGE_ID_ERROR]: 'Do not use `new Array()`.', + [MESSAGE_ID_LENGTH]: 'The argument is the length of array.', + [MESSAGE_ID_ONLY_ELEMENT]: 'The argument is the only element of array.', + [MESSAGE_ID_SPREAD]: 'Spread the argument.' +}; +const newArraySelector = [ + 'NewExpression', + '[callee.type="Identifier"]', + '[callee.name="Array"]', + '[arguments.length=1]' +].join(''); + +function getProblem(context, node) { + const problem = { + node, + messageId: MESSAGE_ID_ERROR + }; + + const [argumentNode] = node.arguments; + + const sourceCode = context.getSourceCode(); + let text = sourceCode.getText(argumentNode); + if (isParenthesized(argumentNode, sourceCode)) { + text = `(${text})`; + } + + const maybeSemiColon = needsSemicolon(sourceCode.getTokenBefore(node), sourceCode, '[') ? + ';' : + ''; + + // We are not sure how many `arguments` passed + if (argumentNode.type === 'SpreadElement') { + problem.suggest = [ + { + messageId: MESSAGE_ID_SPREAD, + fix: fixer => fixer.replaceText(node, `${maybeSemiColon}[${text}]`) + } + ]; + return problem; + } + + const result = getStaticValue(argumentNode, context.getScope()); + const fromLengthText = `Array.from(${text === 'length' ? '{length}' : `{length: ${text}}`})`; + const onlyElementText = `${maybeSemiColon}[${text}]`; + + // We don't know the argument is number or not + if (result === null) { + problem.suggest = [ + { + messageId: MESSAGE_ID_LENGTH, + fix: fixer => fixer.replaceText(node, fromLengthText) + }, + { + messageId: MESSAGE_ID_ONLY_ELEMENT, + fix: fixer => fixer.replaceText(node, onlyElementText) + } + ]; + return problem; + } + + problem.fix = fixer => fixer.replaceText( + node, + typeof result.value === 'number' ? fromLengthText : onlyElementText + ); + + return problem; +} + +const create = context => ({ + [newArraySelector](node) { + context.report(getProblem(context, node)); + } +}); + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + url: getDocumentationUrl(__filename) + }, + fixable: 'code', + messages + } +}; diff --git a/test/no-new-array.js b/test/no-new-array.js new file mode 100644 index 0000000000..ae5ef700f5 --- /dev/null +++ b/test/no-new-array.js @@ -0,0 +1,148 @@ +import {outdent} from 'outdent'; +import {test} from './utils/test'; + +const MESSAGE_ID_ERROR = 'error'; +const MESSAGE_ID_LENGTH = 'array-length'; +const MESSAGE_ID_ONLY_ELEMENT = 'only-element'; +const MESSAGE_ID_SPREAD = 'spread'; + +const suggestionCase = ({code, suggestions}) => ({ + code, + output: code, + errors: [ + { + messageId: MESSAGE_ID_ERROR, + suggestions + } + ] +}); + +test({ + valid: [ + 'const array = Array.from({length: 1})', + + // ESLint builtin rule `no-array-constructor` cases + 'const array = new Array()', + 'const array = new Array', + 'const array = new Array(1, 2)', + 'const array = Array(1, 2)', + + // `unicorn/new-for-builtins` cases + 'const array = Array(1)' + ], + invalid: [ + suggestionCase({ + code: 'const array = new Array(foo)', + suggestions: [ + { + messageId: MESSAGE_ID_LENGTH, + output: 'const array = Array.from({length: foo})' + }, + { + messageId: MESSAGE_ID_ONLY_ELEMENT, + output: 'const array = [foo]' + } + ] + }), + suggestionCase({ + code: 'const array = new Array(length)', + suggestions: [ + { + messageId: MESSAGE_ID_LENGTH, + output: 'const array = Array.from({length})' + }, + { + messageId: MESSAGE_ID_ONLY_ELEMENT, + output: 'const array = [length]' + } + ] + }), + suggestionCase({ + code: outdent` + const foo = [] + new Array(bar).forEach(baz) + `, + suggestions: [ + { + messageId: MESSAGE_ID_LENGTH, + output: outdent` + const foo = [] + Array.from({length: bar}).forEach(baz) + ` + }, + { + messageId: MESSAGE_ID_ONLY_ELEMENT, + output: outdent` + const foo = [] + ;[bar].forEach(baz) + ` + } + ] + }), + ...[ + '...[foo]', + '...foo', + '...[...foo]', + // The following cases we can know the result, but we are not auto-fixing them + '...[1]', + '...["1"]', + '...[1, "1"]' + ].map(argumentText => { + const code = `const array = new Array(${argumentText})`; + return { + code, + output: code, + errors: [ + { + messageId: MESSAGE_ID_ERROR, + suggestions: [ + { + messageId: MESSAGE_ID_SPREAD, + output: `const array = [${argumentText}]` + } + ] + } + ] + }; + }), + suggestionCase({ + code: outdent` + const foo = [] + new Array(...bar).forEach(baz) + `, + suggestions: [ + { + messageId: MESSAGE_ID_SPREAD, + output: outdent` + const foo = [] + ;[...bar].forEach(baz) + ` + } + ] + }) + ] +}); + +test.visualize([ + 'const array = new Array(1)', + // This is actually `[]`, but we fix to `Array.from({length: zero})` + outdent` + const zero = 0; + const array = new Array(zero); + `, + // Use shorthand + outdent` + const length = 1; + const array = new Array(length); + `, + 'const array = new Array(1.5)', + 'const array = new Array(Number("1"))', + 'const array = new Array("1")', + 'const array = new Array(null)', + 'const array = new Array(("1"))', + 'const array = new Array((0, 1))', + outdent` + const foo = [] + new Array("bar").forEach(baz) + ` +]); diff --git a/test/snapshots/no-new-array.js.md b/test/snapshots/no-new-array.js.md new file mode 100644 index 0000000000..010749c47c --- /dev/null +++ b/test/snapshots/no-new-array.js.md @@ -0,0 +1,174 @@ +# Snapshot report for `test/no-new-array.js` + +The actual snapshot is saved in `no-new-array.js.snap`. + +Generated by [AVA](https://avajs.dev). + +## no-new-array - #1 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const array = new Array(1)␊ + ␊ + Output:␊ + 1 | const array = Array.from({length: 1})␊ + ␊ + Error 1/1:␊ + > 1 | const array = new Array(1)␊ + | ^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #2 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const zero = 0;␊ + 2 | const array = new Array(zero);␊ + ␊ + Output:␊ + 1 | const zero = 0;␊ + 2 | const array = Array.from({length: zero});␊ + ␊ + Error 1/1:␊ + 1 | const zero = 0;␊ + > 2 | const array = new Array(zero);␊ + | ^^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #3 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const length = 1;␊ + 2 | const array = new Array(length);␊ + ␊ + Output:␊ + 1 | const length = 1;␊ + 2 | const array = Array.from({length});␊ + ␊ + Error 1/1:␊ + 1 | const length = 1;␊ + > 2 | const array = new Array(length);␊ + | ^^^^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #4 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const array = new Array(1.5)␊ + ␊ + Output:␊ + 1 | const array = Array.from({length: 1.5})␊ + ␊ + Error 1/1:␊ + > 1 | const array = new Array(1.5)␊ + | ^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #5 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const array = new Array(Number("1"))␊ + ␊ + Output:␊ + 1 | const array = Array.from({length: Number("1")})␊ + ␊ + Error 1/1:␊ + > 1 | const array = new Array(Number("1"))␊ + | ^^^^^^^^^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #6 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const array = new Array("1")␊ + ␊ + Output:␊ + 1 | const array = ["1"]␊ + ␊ + Error 1/1:␊ + > 1 | const array = new Array("1")␊ + | ^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #7 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const array = new Array(null)␊ + ␊ + Output:␊ + 1 | const array = [null]␊ + ␊ + Error 1/1:␊ + > 1 | const array = new Array(null)␊ + | ^^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #8 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const array = new Array(("1"))␊ + ␊ + Output:␊ + 1 | const array = [("1")]␊ + ␊ + Error 1/1:␊ + > 1 | const array = new Array(("1"))␊ + | ^^^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #9 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const array = new Array((0, 1))␊ + ␊ + Output:␊ + 1 | const array = Array.from({length: (0, 1)})␊ + ␊ + Error 1/1:␊ + > 1 | const array = new Array((0, 1))␊ + | ^^^^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` + +## no-new-array - #10 + +> Snapshot 1 + + `␊ + Input:␊ + 1 | const foo = []␊ + 2 | new Array("bar").forEach(baz)␊ + ␊ + Output:␊ + 1 | const foo = []␊ + 2 | ;["bar"].forEach(baz)␊ + ␊ + Error 1/1:␊ + 1 | const foo = []␊ + > 2 | new Array("bar").forEach(baz)␊ + | ^^^^^^^^^^^^^^^^ Do not use `new Array()`.␊ + ` diff --git a/test/snapshots/no-new-array.js.snap b/test/snapshots/no-new-array.js.snap new file mode 100644 index 0000000000..c9057a40f4 Binary files /dev/null and b/test/snapshots/no-new-array.js.snap differ