Add no-new-array rule (#992)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

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];
+```

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',

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.

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
+	}
+};

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)
+	`
+]);