Merge branch 'master' into prefer-regexp-test

.github/workflows/main.yml

@@ -1,12 +1,10 @@
 name: CI
-
 on:
   - push
   - pull_request
-
 jobs:
   test:
-    name: Node.js ${{ matrix.node_version }} on ${{ matrix.os }}
+    name: Node.js ${{ matrix.node-version }} on ${{ matrix.os }}
     runs-on: ${{ matrix.os }}
     strategy:
       fail-fast: false
@@ -22,17 +20,16 @@ jobs:
       - uses: actions/checkout@v2
       - uses: actions/setup-node@v1
         with:
-          node-version: ${{ matrix.node_version }}
+          node-version: ${{ matrix.node-version }}
       - run: npm install
       - run: npm test
         if: matrix.os != 'windows-latest'
       - run: npx ava
         if: matrix.os == 'windows-latest'
       - uses: codecov/codecov-action@v1
-        if: matrix.os == 'ubuntu-latest' && matrix.node_version == 12
+        if: matrix.os == 'ubuntu-latest' && matrix.node-version == 14
         with:
           fail_ci_if_error: true
-
   lint:
     runs-on: ubuntu-latest
     steps:
@@ -42,7 +39,6 @@ jobs:
           node-version: 14
       - run: npm install
       - run: npm run lint
-
   integration:
     runs-on: ubuntu-latest
     steps:

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

@@ -50,7 +50,7 @@ module.exports = {
 				'unicorn/expiring-todo-comments': 'error',
 				'unicorn/explicit-length-check': 'error',
 				'unicorn/filename-case': 'error',
-				'unicorn/import-index': 'error',
+				'unicorn/import-index': 'off',
 				'unicorn/import-style': 'error',
 				'unicorn/new-for-builtins': 'error',
 				'unicorn/no-abusive-eslint-disable': 'error',
@@ -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",
@@ -131,6 +132,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/explicit-length-check.js

@@ -2,6 +2,7 @@
 const {isParenthesized} = require('eslint-utils');
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const isLiteralValue = require('./utils/is-literal-value');
+const isLogicalExpression = require('./utils/is-logical-expression');
 const {isBooleanNode, getBooleanAncestor} = require('./utils/boolean');
 
 const TYPE_NON_ZERO = 'non-zero';
@@ -165,7 +166,7 @@ function create(context) {
 				if (isBooleanNode(ancestor)) {
 					isZeroLengthCheck = isNegative;
 					node = ancestor;
-				} else if (lengthNode.parent.type === 'LogicalExpression') {
+				} else if (isLogicalExpression(lengthNode.parent)) {
 					isZeroLengthCheck = isNegative;
 					node = lengthNode;
 					autoFix = false;

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

rules/prefer-string-starts-ends-with.js

@@ -3,6 +3,7 @@ const {isParenthesized} = require('eslint-utils');
 const getDocumentationUrl = require('./utils/get-documentation-url');
 const methodSelector = require('./utils/method-selector');
 const quoteString = require('./utils/quote-string');
+const shouldAddParenthesesToMemberExpressionObject = require('./utils/should-add-parentheses-to-member-expression-object');
 
 const MESSAGE_STARTS_WITH = 'prefer-starts-with';
 const MESSAGE_ENDS_WITH = 'prefer-ends-with';
@@ -79,7 +80,7 @@ const create = context => {
 					if (
 						// If regex is parenthesized, we can use it, so we don't need add again
 						!isParenthesized(regexNode, sourceCode) &&
-						(isParenthesized(target, sourceCode) || target.type === 'AwaitExpression')
+						(isParenthesized(target, sourceCode) || shouldAddParenthesesToMemberExpressionObject(target, sourceCode))
 					) {
 						targetString = `(${targetString})`;
 					}

rules/utils/boolean.js

@@ -1,5 +1,7 @@
 'use strict';
 
+const isLogicalExpression = require('./is-logical-expression');
+
 const isLogicNot = node =>
 	node &&
 	node.type === 'UnaryExpression' &&
@@ -48,7 +50,7 @@ function isBooleanNode(node) {
 		return true;
 	}
 
-	if (parent.type === 'LogicalExpression') {
+	if (isLogicalExpression(parent)) {
 		return isBooleanNode(parent);
 	}
 

rules/utils/is-logical-expression.js

@@ -0,0 +1,17 @@
+'use strict';
+
+/**
+Check if the given node is a true logical expression or not.
+
+The three binary expressions logical-or (`||`), logical-and (`&&`), and coalesce (`??`) are known as `ShortCircuitExpression`, but ESTree represents these by the `LogicalExpression` node type. This function rejects coalesce expressions of `LogicalExpression` node type.
+
+@param {Node} node - The node to check.
+@returns {boolean} `true` if the node is `&&` or `||`.
+@see https://tc39.es/ecma262/#prod-ShortCircuitExpression
+*/
+const isLogicalExpression = node =>
+	node &&
+	node.type === 'LogicalExpression' &&
+	(node.operator === '&&' || node.operator === '||');
+
+module.exports = isLogicalExpression;

rules/utils/should-add-parentheses-to-member-expression-object.js

@@ -0,0 +1,67 @@
+'use strict';
+
+const {isOpeningParenToken, isClosingParenToken} = require('eslint-utils');
+
+// Determine whether this node is a decimal integer literal.
+// Copied from https://github.com/eslint/eslint/blob/cc4871369645c3409dc56ded7a555af8a9f63d51/lib/rules/utils/ast-utils.js#L1237
+const DECIMAL_INTEGER_PATTERN = /^(?:0|0[0-7]*[89]\d*|[1-9](?:_?\d)*)$/u;
+const isDecimalInteger = node =>
+	node.type === 'Literal' &&
+	typeof node.value === 'number' &&
+	DECIMAL_INTEGER_PATTERN.test(node.raw);
+
+/**
+Determine if a constructor function is newed-up with parens.
+
+@param {Node} node - The `NewExpression` node to be checked.
+@param {SourceCode} sourceCode - The source code object.
+@returns {boolean} True if the constructor is called with parens.
+
+Copied from https://github.com/eslint/eslint/blob/cc4871369645c3409dc56ded7a555af8a9f63d51/lib/rules/no-extra-parens.js#L252
+*/
+function isNewExpressionWithParentheses(node, sourceCode) {
+	if (node.arguments.length > 0) {
+		return true;
+	}
+
+	const [penultimateToken, lastToken] = sourceCode.getLastTokens(node, 2);
+	// The expression should end with its own parens, for example, `new new Foo()` is not a new expression with parens.
+	return isOpeningParenToken(penultimateToken) &&
+		isClosingParenToken(lastToken) &&
+		node.callee.range[1] < node.range[1];
+}
+
+/**
+Check if parentheses should to be added to a `node` when it's used as an `object` of `MemberExpression`.
+
+@param {Node} node - The AST node to check.
+@param {SourceCode} sourceCode - The source code object.
+@returns {boolean}
+*/
+function shouldAddParenthesesToMemberExpressionObject(node, sourceCode) {
+	switch (node.type) {
+		// This is not a full list. Some other nodes like `FunctionDeclaration` don't need parentheses,
+		// but it's not possible to be in the place we are checking at this point.
+		case 'Identifier':
+		case 'MemberExpression':
+		case 'CallExpression':
+		case 'ChainExpression':
+		case 'TemplateLiteral':
+			return false;
+		case 'NewExpression':
+			return !isNewExpressionWithParentheses(node, sourceCode);
+		case 'Literal': {
+			/* istanbul ignore next */
+			if (isDecimalInteger(node)) {
+				return true;
+			}
+
+			return false;
+		}
+
+		default:
+			return true;
+	}
+}
+
+module.exports = shouldAddParenthesesToMemberExpressionObject;

test/explicit-length-check.js

@@ -58,6 +58,8 @@ test({
 		'const x = Boolean(foo.length, foo.length)',
 		'const x = new Boolean(foo.length)',
 		'const x = NotBoolean(foo.length)',
+		'const length = foo.length ?? 0',
+		'if (foo.length ?? bar) {}',
 
 		// Checking 'non-zero'
 		'if (foo.length > 0) {}',