Add no-useless-length-check rule (#1398)

docs/rules/no-useless-length-check.md

@@ -0,0 +1,52 @@
+# Disallow useless array length check
+
+- `Array#some()` returns `false` for an empty array. There is no need to check if the array is not empty.
+- `Array#every()` returns `true` for an empty array. There is no need to check if the array is empty.
+
+We only check `.length === 0`, `.length !== 0`, and `.length > 0`. These zero and non-zero length check styles are allowed in the [`unicorn/explicit-length-check`](./explicit-length-check.md#options) rule. It is recommended to use them together.
+
+This rule is fixable.
+
+## Fail
+
+```js
+if (array.length === 0 || array.every(Boolean));
+```
+
+```js
+if (array.length !== 0 && array.some(Boolean));
+```
+
+```js
+if (array.length > 0 && array.some(Boolean));
+```
+
+```js
+const isAllTrulyOrEmpty = array.length === 0 || array.every(Boolean);
+```
+
+## Pass
+
+```js
+if (array.every(Boolean));
+```
+
+```js
+if (array.some(Boolean));
+```
+
+```js
+const isAllTrulyOrEmpty = array.every(Boolean);
+```
+
+```js
+if (array.length === 0 || anotherCheck() || array.every(Boolean));
+```
+
+```js
+const isNonEmptyAllTrulyArray = array.length > 0 && array.every(Boolean);
+```
+
+```js
+const isEmptyArrayOrAllTruly = array.length === 0 || array.some(Boolean);
+```

index.js

@@ -77,6 +77,7 @@ module.exports = {
 				'unicorn/no-unreadable-array-destructuring': 'error',
 				'unicorn/no-unsafe-regex': 'off',
 				'unicorn/no-unused-properties': 'off',
+				'unicorn/no-useless-length-check': 'error',
 				'unicorn/no-useless-undefined': 'error',
 				'unicorn/no-zero-fractions': 'error',
 				'unicorn/number-literal-case': 'error',

readme.md

@@ -74,6 +74,7 @@ Configure it in `package.json`.
 			"unicorn/no-unreadable-array-destructuring": "error",
 			"unicorn/no-unsafe-regex": "off",
 			"unicorn/no-unused-properties": "off",
+			"unicorn/no-useless-length-check": "error",
 			"unicorn/no-useless-undefined": "error",
 			"unicorn/no-zero-fractions": "error",
 			"unicorn/number-literal-case": "error",
@@ -178,6 +179,7 @@ Each rule has emojis denoting:
 | [no-unreadable-array-destructuring](docs/rules/no-unreadable-array-destructuring.md) | Disallow unreadable array destructuring. | ✅ | 🔧 |  |
 | [no-unsafe-regex](docs/rules/no-unsafe-regex.md) | Disallow unsafe regular expressions. |  |  |  |
 | [no-unused-properties](docs/rules/no-unused-properties.md) | Disallow unused object properties. |  |  |  |
+| [no-useless-length-check](docs/rules/no-useless-length-check.md) | Disallow useless array length check. | ✅ | 🔧 |  |
 | [no-useless-undefined](docs/rules/no-useless-undefined.md) | Disallow useless `undefined`. | ✅ | 🔧 |  |
 | [no-zero-fractions](docs/rules/no-zero-fractions.md) | Disallow number literals with zero fractions or dangling dots. | ✅ | 🔧 |  |
 | [number-literal-case](docs/rules/number-literal-case.md) | Enforce proper case for numeric literals. | ✅ | 🔧 |  |

rules/no-useless-length-check.js

@@ -0,0 +1,148 @@
+'use strict';
+const {methodCallSelector, matches, memberExpressionSelector} = require('./selectors/index.js');
+const isSameReference = require('./utils/is-same-reference.js');
+const {getParenthesizedRange} = require('./utils/parentheses.js');
+
+const messages = {
+	'non-zero': 'The non-empty check is useless as `Array#some()` returns `false` for an empty array.',
+	zero: 'The empty check is useless as `Array#every()` returns `true` for an empty array.'
+};
+
+const logicalExpressionSelector = [
+	'LogicalExpression',
+	matches(['[operator="||"]', '[operator="&&"]'])
+].join('');
+// We assume the user already follows `unicorn/explicit-length-check`. These are allowed in that rule.
+const lengthCompareZeroSelector = [
+	logicalExpressionSelector,
+	' > ',
+	'BinaryExpression',
+	memberExpressionSelector({path: 'left', name: 'length'}),
+	'[right.type="Literal"]',
+	'[right.raw="0"]'
+].join('');
+const zeroLengthCheckSelector = [
+	lengthCompareZeroSelector,
+	'[operator="==="]'
+].join('');
+const nonZeroLengthCheckSelector = [
+	lengthCompareZeroSelector,
+	matches(['[operator=">"]', '[operator="!=="]'])
+].join('');
+const arraySomeCallSelector = methodCallSelector('some');
+const arrayEveryCallSelector = methodCallSelector('every');
+
+function flatLogicalExpression(node) {
+	return [node.left, node.right].flatMap(child =>
+		child.type === 'LogicalExpression' && child.operator === node.operator ?
+			flatLogicalExpression(child) :
+			[child]
+	);
+}
+
+/** @param {import('eslint').Rule.RuleContext} context */
+const create = context => {
+	const logicalExpressions = [];
+	const zeroLengthChecks = new Set();
+	const nonZeroLengthChecks = new Set();
+	const arraySomeCalls = new Set();
+	const arrayEveryCalls = new Set();
+
+	function isUselessLengthCheckNode({node, operator, siblings}) {
+		return (
+			(
+				operator === '||' &&
+				zeroLengthChecks.has(node) &&
+				siblings.some(condition =>
+					arrayEveryCalls.has(condition) &&
+					isSameReference(node.left.object, condition.callee.object)
+				)
+			) ||
+			(
+				operator === '&&' &&
+				nonZeroLengthChecks.has(node) &&
+				siblings.some(condition =>
+					arraySomeCalls.has(condition) &&
+					isSameReference(node.left.object, condition.callee.object)
+				)
+			)
+		);
+	}
+
+	function getUselessLengthCheckNode(logicalExpression) {
+		const {operator} = logicalExpression;
+		return flatLogicalExpression(logicalExpression)
+			.filter((node, index, conditions) => isUselessLengthCheckNode({
+				node,
+				operator,
+				siblings: [
+					conditions[index - 1],
+					conditions[index + 1]
+				].filter(Boolean)
+			}));
+	}
+
+	return {
+		[zeroLengthCheckSelector](node) {
+			zeroLengthChecks.add(node);
+		},
+		[nonZeroLengthCheckSelector](node) {
+			nonZeroLengthChecks.add(node);
+		},
+		[arraySomeCallSelector](node) {
+			arraySomeCalls.add(node);
+		},
+		[arrayEveryCallSelector](node) {
+			arrayEveryCalls.add(node);
+		},
+		[logicalExpressionSelector](node) {
+			logicalExpressions.push(node);
+		},
+		* 'Program:exit'() {
+			const nodes = new Set(
+				logicalExpressions.flatMap(logicalExpression =>
+					getUselessLengthCheckNode(logicalExpression)
+				)
+			);
+
+			for (const node of nodes) {
+				yield {
+					loc: {
+						start: node.left.property.loc.start,
+						end: node.loc.end
+					},
+					messageId: zeroLengthChecks.has(node) ? 'zero' : 'non-zero',
+					/** @param {import('eslint').Rule.RuleFixer} fixer */
+					fix(fixer) {
+						const sourceCode = context.getSourceCode();
+						const {left, right} = node.parent;
+						const leftRange = getParenthesizedRange(left, sourceCode);
+						const rightRange = getParenthesizedRange(right, sourceCode);
+						const range = [];
+						if (left === node) {
+							range[0] = leftRange[0];
+							range[1] = rightRange[0];
+						} else {
+							range[0] = leftRange[1];
+							range[1] = rightRange[1];
+						}
+
+						return fixer.removeRange(range);
+					}
+				};
+			}
+		}
+	};
+};
+
+module.exports = {
+	create,
+	meta: {
+		type: 'suggestion',
+		docs: {
+			description: 'Disallow useless array length check.'
+		},
+		fixable: 'code',
+		messages
+	}
+};

test/no-useless-length-check.mjs

@@ -0,0 +1,167 @@
+import outdent from 'outdent';
+import {getTester} from './utils/test.mjs';
+
+const {test} = getTester(import.meta);
+
+test.snapshot({
+	valid: [
+		// `.length === 0 || .every()`
+		'array.length === 0 ?? array.every(Boolean)',
+		'array.length === 0 && array.every(Boolean)',
+		'(array.length === 0) + (array.every(Boolean))',
+		'array.length === 1 || array.every(Boolean)',
+		'array.length === "0" || array.every(Boolean)',
+		'array.length === 0. || array.every(Boolean)',
+		'array.length === 0x0 || array.every(Boolean)',
+		'array.length !== 0 || array.every(Boolean)',
+		'array.length == 0 || array.every(Boolean)',
+		'0 === array.length || array.every(Boolean)',
+		'array?.length === 0 || array.every(Boolean)',
+		'array.notLength === 0 || array.every(Boolean)',
+		'array[length] === 0 || array.every(Boolean)',
+		'array.length === 0 || array.every?.(Boolean)',
+		'array.length === 0 || array?.every(Boolean)',
+		'array.length === 0 || array.every',
+		'array.length === 0 || array[every](Boolean)',
+		'array1.length === 0 || array2.every(Boolean)',
+
+		// `.length !== 0 && .some()`
+		'array.length !== 0 ?? array.some(Boolean)',
+		'array.length !== 0 || array.some(Boolean)',
+		'(array.length !== 0) - (array.some(Boolean))',
+		'array.length !== 1 && array.some(Boolean)',
+		'array.length !== "0" && array.some(Boolean)',
+		'array.length !== 0. && array.some(Boolean)',
+		'array.length !== 0x0 && array.some(Boolean)',
+		'array.length === 0 && array.some(Boolean)',
+		'array.length <= 0 && array.some(Boolean)',
+		'array.length != 0 && array.some(Boolean)',
+		'0 !== array.length && array.some(Boolean)',
+		'array?.length !== 0 && array.some(Boolean)',
+		'array.notLength !== 0 && array.some(Boolean)',
+		'array[length] !== 0 && array.some(Boolean)',
+		'array.length !== 0 && array.some?.(Boolean)',
+		'array.length !== 0 && array?.some(Boolean)',
+		'array.length !== 0 && array.some',
+		'array.length !== 0 && array.notSome(Boolean)',
+		'array.length !== 0 && array[some](Boolean)',
+		'array1.length !== 0 && array2.some(Boolean)',
+
+		// `.length > 0 && .some()`
+		'array.length > 0 ?? array.some(Boolean)',
+		'array.length > 0 || array.some(Boolean)',
+		'(array.length > 0) - (array.some(Boolean))',
+		'array.length > 1 && array.some(Boolean)',
+		'array.length > "0" && array.some(Boolean)',
+		'array.length > 0. && array.some(Boolean)',
+		'array.length > 0x0 && array.some(Boolean)',
+		'array.length >= 0 && array.some(Boolean)',
+		'0 > array.length && array.some(Boolean)',
+		'0 < array.length && array.some(Boolean)',
+		'array?.length > 0 && array.some(Boolean)',
+		'array.notLength > 0 && array.some(Boolean)',
+		'array.length > 0 && array.some?.(Boolean)',
+		'array.length > 0 && array?.some(Boolean)',
+		'array.length > 0 && array.some',
+		'array.length > 0 && array.notSome(Boolean)',
+		'array.length > 0 && array[some](Boolean)',
+		'array1.length > 0 && array2.some(Boolean)',
+		outdent`
+			if (
+				foo &&
+				array.length !== 0 &&
+				bar &&
+				array.some(Boolean)
+			) {
+				// ...
+			}
+		`,
+
+		'(foo && array.length === 0) || array.every(Boolean) && foo',
+		'array.length === 0 || (array.every(Boolean) && foo)',
+		'(foo || array.length > 0) && array.some(Boolean)',
+		'array.length > 0 && (array.some(Boolean) || foo)'
+	],
+	invalid: [
+		'array.length === 0 || array.every(Boolean)',
+		'array.length > 0 && array.some(Boolean)',
+		'array.length !== 0 && array.some(Boolean)',
+		outdent`
+			((
+				((
+					(( array )).length
+				)) === (( 0 ))
+				||
+				((
+					(( array )).every(Boolean)
+				))
+			))
+		`,
+		outdent`
+			((
+				((
+					(( array )).every(Boolean)
+				))
+				||
+				((
+					(( array )).length
+				)) === (( 0 ))
+			))
+		`,
+		'if ((( array.length > 0 )) && array.some(Boolean));',
+		outdent`
+			if (
+				array.length !== 0 &&
+				array.some(Boolean) &&
+				foo
+			) {
+				// ...
+			}
+		`,
+		'(array.length === 0 || array.every(Boolean)) || foo',
+		'foo || (array.length === 0 || array.every(Boolean))',
+		'(array.length > 0 && array.some(Boolean)) && foo',
+		'foo && (array.length > 0 && array.some(Boolean))',
+		'array.every(Boolean) || array.length === 0',
+		'array.some(Boolean) && array.length !== 0',
+		'array.some(Boolean) && array.length > 0',
+		'foo && array.length > 0 && array.some(Boolean)',
+		'foo || array.length === 0 || array.every(Boolean)',
+		'(foo || array.length === 0) || array.every(Boolean)',
+		'array.length === 0 || (array.every(Boolean) || foo)',
+		'(foo && array.length > 0) && array.some(Boolean)',
+		'array.length > 0 && (array.some(Boolean) && foo)',
+		'array.every(Boolean) || array.length === 0 || array.every(Boolean)',
+		'array.length === 0 || array.every(Boolean) || array.length === 0',
+		outdent`
+			array1.every(Boolean)
+			|| (( array1.length === 0 || array2.length === 0 )) // Both useless
+			|| array2.every(Boolean)
+		`,
+		// Real world case from this rule initial implementation, but added useless length check
+		outdent`
+			function isUselessLengthCheckNode({node, operator, siblings}) {
+				return (
+					(
+						operator === '||' &&
+						zeroLengthChecks.has(node) &&
+						siblings.length > 0 &&
+						siblings.some(condition =>
+							arrayEveryCalls.has(condition) &&
+							isSameReference(node.left.object, condition.callee.object)
+						)
+					) ||
+					(
+						operator === '&&' &&
+						nonZeroLengthChecks.has(node) &&
+						siblings.length > 0 &&
+						siblings.some(condition =>
+							arraySomeCalls.has(condition) &&
+							isSameReference(node.left.object, condition.callee.object)
+						)
+					)
+				);
+			}
+		`
+	]
+});