prefer-array-some: Report non-zero check on array.filter().length (…

…#1337)
sindresorhus · Jun 4, 2021 · f8aaac2 · f8aaac2
1 parent 119615b
commit f8aaac2
Show file tree

Hide file tree

Showing 6 changed files with 284 additions and 31 deletions.
diff --git a/docs/rules/prefer-array-some.md b/docs/rules/prefer-array-some.md
@@ -1,27 +1,49 @@
-# Prefer `.some(…)` over `.find(…)`.
+# Prefer `.some(…)` over `.filter(…).length` check and `.find(…)`
 
-Prefer using [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) over [`Array#find`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) when ensuring at least one element in the array passes a given check.
+Prefer using [`Array#some`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) over:
+
+- Non-zero length check on the result of [`Array#filter()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter).
+- Using [`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) to ensure at least one element in the array passes a given check.
+
+This rule is fixable for `.filter(…).length` check and has a suggestion for `.find(…)`.
 
 ## Fail
 
 ```js
-if (array.find(element => element === '🦄')) {
+const hasUnicorn = array.filter(element => isUnicorn(element)).length > 0;
+```
+
+```js
+const hasUnicorn = array.filter(element => isUnicorn(element)).length != 0;
+```
+
+```js
+const hasUnicorn = array.filter(element => isUnicorn(element)).length >= 1;
+```
+
+```js
+if (array.find(element => isUnicorn(element))) {
 	// …
 }
 ```
 
 ```js
-const foo = array.find(element => element === '🦄') ? bar : baz;
+const foo = array.find(element => isUnicorn(element)) ? bar : baz;
 ```
 
 ## Pass
 
 ```js
-if (array.some(element => element === '🦄')) {
+const hasUnicorn = array.some(element => isUnicorn(element));
+```
+
+
+```js
+if (array.some(element => isUnicorn(element))) {
 	// …
 }
 ```
 
 ```js
-const foo = array.find(element => element === '🦄') || bar;
+const foo = array.find(element => isUnicorn(element)) || bar;
 ```
diff --git a/readme.md b/readme.md
@@ -185,7 +185,7 @@ Each rule has emojis denoting:
 | [prefer-array-flat](docs/rules/prefer-array-flat.md) | Prefer `Array#flat()` over legacy techniques to flatten arrays. | ✅ | 🔧 |  |
 | [prefer-array-flat-map](docs/rules/prefer-array-flat-map.md) | Prefer `.flatMap(…)` over `.map(…).flat()`. | ✅ | 🔧 |  |
 | [prefer-array-index-of](docs/rules/prefer-array-index-of.md) | Prefer `Array#indexOf()` over `Array#findIndex()` when looking for the index of an item. | ✅ | 🔧 | 💡 |
-| [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.find(…)`. | ✅ |  | 💡 |
+| [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.filter(…).length` check and `.find(…)`. | ✅ | 🔧 | 💡 |
 | [prefer-at](docs/rules/prefer-at.md) | Prefer `.at()` method for index access and `String#charAt()`. |  | 🔧 | 💡 |
 | [prefer-date-now](docs/rules/prefer-date-now.md) | Prefer `Date.now()` to get the number of milliseconds since the Unix Epoch. | ✅ | 🔧 |  |
 | [prefer-default-parameters](docs/rules/prefer-default-parameters.md) | Prefer default parameters over reassignment. | ✅ | 🔧 | 💡 |

diff --git a/rules/prefer-array-some.js b/rules/prefer-array-some.js
@@ -1,13 +1,16 @@
 'use strict';
 const getDocumentationUrl = require('./utils/get-documentation-url');
-const {methodCallSelector} = require('./selectors');
+const {methodCallSelector, matches, memberExpressionSelector} = require('./selectors');
 const {isBooleanNode} = require('./utils/boolean');
+const {getParenthesizedRange} = require('./utils/parentheses');
 
-const MESSAGE_ID_ERROR = 'error';
-const MESSAGE_ID_SUGGESTION = 'suggestion';
+const ERROR_ID_ARRAY_SOME = 'some';
+const SUGGESTION_ID_ARRAY_SOME = 'some-suggestion';
+const ERROR_ID_ARRAY_FILTER = 'filter';
 const messages = {
-	[MESSAGE_ID_ERROR]: 'Prefer `.some(…)` over `.find(…)`.',
-	[MESSAGE_ID_SUGGESTION]: 'Replace `.find(…)` with `.some(…)`.'
+	[ERROR_ID_ARRAY_SOME]: 'Prefer `.some(…)` over `.find(…)`.',
+	[SUGGESTION_ID_ARRAY_SOME]: 'Replace `.find(…)` with `.some(…)`.',
+	[ERROR_ID_ARRAY_FILTER]: 'Prefer `.some(…)` over non-zero length check from `.filter(…)`.'
 };
 
 const arrayFindCallSelector = methodCallSelector({
@@ -16,22 +19,75 @@ const arrayFindCallSelector = methodCallSelector({
 	max: 2
 });
 
+const arrayFilterCallSelector = [
+	'BinaryExpression',
+	'[right.type="Literal"]',
+	// We assume the user already follows `unicorn/explicit-length-check`, these are allowed in that rule
+	matches([
+		'[operator=">"][right.raw="0"]',
+		'[operator="!=="][right.raw="0"]',
+		'[operator=">="][right.raw="1"]'
+	]),
+	' > ',
+	`${memberExpressionSelector('length')}.left`,
+	' > ',
+	`${methodCallSelector('filter')}.object`
+].join('');
+
 const create = context => {
 	return {
-		[arrayFindCallSelector](node) {
-			if (isBooleanNode(node)) {
-				node = node.callee.property;
-				context.report({
-					node,
-					messageId: MESSAGE_ID_ERROR,
-					suggest: [
-						{
-							messageId: MESSAGE_ID_SUGGESTION,
-							fix: fixer => fixer.replaceText(node, 'some')
-						}
-					]
-				});
+		[arrayFindCallSelector](findCall) {
+			if (!isBooleanNode(findCall)) {
+				return;
 			}
+
+			const findProperty = findCall.callee.property;
+			context.report({
+				node: findProperty,
+				messageId: ERROR_ID_ARRAY_SOME,
+				suggest: [
+					{
+						messageId: SUGGESTION_ID_ARRAY_SOME,
+						fix: fixer => fixer.replaceText(findProperty, 'some')
+					}
+				]
+			});
+		},
+		[arrayFilterCallSelector](filterCall) {
+			const filterProperty = filterCall.callee.property;
+			context.report({
+				node: filterProperty,
+				messageId: ERROR_ID_ARRAY_FILTER,
+				* fix(fixer) {
+					// `.filter` to `.some`
+					yield fixer.replaceText(filterProperty, 'some');
+
+					const sourceCode = context.getSourceCode();
+					const lengthNode = filterCall.parent;
+					/*
+						Remove `.length`
+						`(( (( array.filter() )).length )) > (( 0 ))`
+						------------------------^^^^^^^
+					*/
+					yield fixer.removeRange([
+						getParenthesizedRange(filterCall, sourceCode)[1],
+						lengthNode.range[1]
+					]);
+
+					const compareNode = lengthNode.parent;
+					/*
+						Remove `> 0`
+						`(( (( array.filter() )).length )) > (( 0 ))`
+						----------------------------------^^^^^^^^^^
+					*/
+					yield fixer.removeRange([
+						getParenthesizedRange(lengthNode, sourceCode)[1],
+						compareNode.range[1]
+					]);
+
+					// The `BinaryExpression` always ends with a number or `)`, no need check for ASI
+				}
+			});
 		}
 	};
 };
@@ -41,10 +97,11 @@ module.exports = {
 	meta: {
 		type: 'suggestion',
 		docs: {
-			description: 'Prefer `.some(…)` over `.find(…)`.',
+			description: 'Prefer `.some(…)` over `.filter(…).length` check and `.find(…)`.',
 			url: getDocumentationUrl(__filename),
 			suggestion: true
 		},
+		fixable: 'code',
 		schema: [],
 		messages
 	}

diff --git a/test/prefer-array-some.mjs b/test/prefer-array-some.mjs
@@ -3,17 +3,16 @@ import {getTester} from './utils/test.mjs';
 
 const {test} = getTester(import.meta);
 
-const MESSAGE_ID_ERROR = 'error';
-const MESSAGE_ID_SUGGESTION = 'suggestion';
-
+const ERROR_ID_ARRAY_SOME = 'some';
+const SUGGESTION_ID_ARRAY_SOME = 'some-suggestion';
 const invalidCase = ({code, suggestionOutput}) => ({
 	code,
 	errors: [
 		{
-			messageId: MESSAGE_ID_ERROR,
+			messageId: ERROR_ID_ARRAY_SOME,
 			suggestions: [
 				{
-					messageId: MESSAGE_ID_SUGGESTION,
+					messageId: SUGGESTION_ID_ARRAY_SOME,
 					output: suggestionOutput
 				}
 			]
@@ -107,3 +106,75 @@ test.snapshot({
 		`
 	]
 });
+
+// - `.filter(…).length > 0`
+// - `.filter(…).length !== 0`
+// - `.filter(…).length >= 1`
+test.snapshot({
+	valid: [
+		// `> 0`
+		'array.filter(fn).length > 0.',
+		'array.filter(fn).length > .0',
+		'array.filter(fn).length > 0.0',
+		'array.filter(fn).length > 0x00',
+		'array.filter(fn).length < 0',
+		'array.filter(fn).length >= 0',
+		'0 > array.filter(fn).length',
+
+		// `!== 0`
+		'array.filter(fn).length !== 0.',
+		'array.filter(fn).length !== .0',
+		'array.filter(fn).length !== 0.0',
+		'array.filter(fn).length !== 0x00',
+		'array.filter(fn).length != 0',
+		'array.filter(fn).length === 0',
+		'array.filter(fn).length == 0',
+		'array.filter(fn).length = 0',
+		'0 !== array.filter(fn).length',
+
+		// `>= 1`
+		'array.filter(fn).length >= 1.',
+		'array.filter(fn).length >= 1.0',
+		'array.filter(fn).length >= 0x1',
+		'array.filter(fn).length > 1',
+		'array.filter(fn).length < 1',
+		'array.filter(fn).length = 1',
+		'array.filter(fn).length += 1',
+		'1 >= array.filter(fn).length',
+
+		// `.length`
+		'array.filter(fn)?.length > 0',
+		'array.filter(fn)[length] > 0',
+		'array.filter(fn).notLength > 0',
+		'array.filter(fn).length() > 0',
+		'+array.filter(fn).length >= 1',
+
+		// `.filter`
+		'array.filter?.(fn).length > 0',
+		'array?.filter(fn).length > 0',
+		'array.notFilter(fn).length > 0',
+		'array.filter.length > 0'
+	],
+	invalid: [
+		'array.filter(fn).length > 0',
+		'array.filter(fn).length !== 0',
+		'array.filter(fn).length >= 1',
+		outdent`
+			if (
+				((
+					((
+						((
+							((
+								array
+							))
+								.filter(what_ever_here)
+						))
+							.length
+					))
+					>
+					(( 0 ))
+				))
+			);
+		`
+	]
+});
diff --git a/test/snapshots/prefer-array-some.mjs.md b/test/snapshots/prefer-array-some.mjs.md
@@ -66,3 +66,106 @@ Generated by [AVA](https://avajs.dev).
       7 | ) {␊
       8 | }␊
     `
+
+## Invalid #1
+      1 | array.filter(fn).length > 0
+
+> Output
+
+    `␊
+      1 | array.some(fn)␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | array.filter(fn).length > 0␊
+        |       ^^^^^^ Prefer \`.some(…)\` over non-zero length check from \`.filter(…)\`.␊
+    `
+
+## Invalid #2
+      1 | array.filter(fn).length !== 0
+
+> Output
+
+    `␊
+      1 | array.some(fn)␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | array.filter(fn).length !== 0␊
+        |       ^^^^^^ Prefer \`.some(…)\` over non-zero length check from \`.filter(…)\`.␊
+    `
+
+## Invalid #3
+      1 | array.filter(fn).length >= 1
+
+> Output
+
+    `␊
+      1 | array.some(fn)␊
+    `
+
+> Error 1/1
+
+    `␊
+    > 1 | array.filter(fn).length >= 1␊
+        |       ^^^^^^ Prefer \`.some(…)\` over non-zero length check from \`.filter(…)\`.␊
+    `
+
+## Invalid #4
+       1 | if (
+       2 | 	((
+       3 | 		((
+       4 | 			((
+       5 | 				((
+       6 | 					array
+       7 | 				))
+       8 | 					.filter(what_ever_here)
+       9 | 			))
+      10 | 				.length
+      11 | 		))
+      12 | 		>
+      13 | 		(( 0 ))
+      14 | 	))
+      15 | );
+
+> Output
+
+    `␊
+       1 | if (␊
+       2 | 	((␊
+       3 | 		((␊
+       4 | 			((␊
+       5 | 				((␊
+       6 | 					array␊
+       7 | 				))␊
+       8 | 					.some(what_ever_here)␊
+       9 | 			))␊
+      10 | 		))␊
+      11 | 	))␊
+      12 | );␊
+    `
+
+> Error 1/1
+
+    `␊
+       1 | if (␊
+       2 | 	((␊
+       3 | 		((␊
+       4 | 			((␊
+       5 | 				((␊
+       6 | 					array␊
+       7 | 				))␊
+    >  8 | 					.filter(what_ever_here)␊
+         | 					 ^^^^^^ Prefer \`.some(…)\` over non-zero length check from \`.filter(…)\`.␊
+       9 | 			))␊
+      10 | 				.length␊
+      11 | 		))␊
+      12 | 		>␊
+      13 | 		(( 0 ))␊
+      14 | 	))␊
+      15 | );␊
+    `
diff --git a/test/snapshots/prefer-array-some.mjs.snap b/test/snapshots/prefer-array-some.mjs.snap