prefer-array-find: Add option to also prefer .findLast() (#1900)

sindresorhus · Sep 19, 2022 · 02252c7 · 02252c7
1 parent 1c457bb
commit 02252c7
Show file tree

Hide file tree

Showing 4 changed files with 299 additions and 5 deletions.
diff --git a/docs/rules/prefer-array-find.md b/docs/rules/prefer-array-find.md
@@ -1,4 +1,4 @@
-# Prefer `.find(…)` over the first element from `.filter(…)`
+# Prefer `.find(…)` and `.findLast(…)` over the first or last element from `.filter(…)`
 
 <!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
 <!-- RULE_NOTICE -->
@@ -7,7 +7,7 @@
 🔧💡 *This rule is [auto-fixable](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) and provides [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).*
 <!-- /RULE_NOTICE -->
 
-[`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) breaks the loop as soon as it finds a match and doesn't create a new array.
+[`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) and [`Array#findLast()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/findLast) breaks the loop as soon as it finds a match and doesn't create a new array.
 
 This rule is fixable unless default values are used in declaration or assignment.
 
@@ -38,3 +38,37 @@ const item = array.find(x => isUnicorn(x));
 ```js
 item = array.find(x => isUnicorn(x));
 ```
+
+```js
+const item = array.findLast(x => isUnicorn(x));
+```
+
+## Options
+
+Type: `object`
+
+### checkFromLast
+
+Type: `boolean`\
+Default: `false`
+
+Pass `checkFromLast: true` to check cases searching from last.
+
+#### Fail
+
+```js
+// eslint unicorn/prefer-array-find: ["error", {"checkFromLast": true}]
+const item = array.filter(x => isUnicorn(x)).at(-1);
+```
+
+```js
+// eslint unicorn/prefer-array-find: ["error", {"checkFromLast": true}]
+const item = array.filter(x => isUnicorn(x)).pop();
+```
+
+#### Pass
+
+```js
+// eslint unicorn/prefer-array-find: ["error", {"checkFromLast": true}]
+const item = array.findLast(x => isUnicorn(x));
+```
diff --git a/readme.md b/readme.md
@@ -105,7 +105,7 @@ Each rule has emojis denoting:
 | [number-literal-case](docs/rules/number-literal-case.md) | Enforce proper case for numeric literals. | ✅ | 🔧 |  |
 | [numeric-separators-style](docs/rules/numeric-separators-style.md) | Enforce the style of numeric separators by correctly grouping digits. | ✅ | 🔧 |  |
 | [prefer-add-event-listener](docs/rules/prefer-add-event-listener.md) | Prefer `.addEventListener()` and `.removeEventListener()` over `on`-functions. | ✅ | 🔧 |  |
-| [prefer-array-find](docs/rules/prefer-array-find.md) | Prefer `.find(…)` over the first element from `.filter(…)`. | ✅ | 🔧 | 💡 |
+| [prefer-array-find](docs/rules/prefer-array-find.md) | Prefer `.find(…)` and `.findLast(…)` over the first or last element from `.filter(…)`. | ✅ | 🔧 | 💡 |
 | [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,lastIndexOf}()` over `Array#{findIndex,findLastIndex}()` when looking for the index of an item. | ✅ | 🔧 | 💡 |

diff --git a/rules/prefer-array-find.js b/rules/prefer-array-find.js
@@ -18,6 +18,8 @@ const {
 
 const ERROR_ZERO_INDEX = 'error-zero-index';
 const ERROR_SHIFT = 'error-shift';
+const ERROR_POP = 'error-pop';
+const ERROR_AT_MINUS_ONE = 'error-at-minus-one';
 const ERROR_DESTRUCTURING_DECLARATION = 'error-destructuring-declaration';
 const ERROR_DESTRUCTURING_ASSIGNMENT = 'error-destructuring-assignment';
 const ERROR_DECLARATION = 'error-variable';
@@ -27,6 +29,8 @@ const messages = {
 	[ERROR_DECLARATION]: 'Prefer `.find(…)` over `.filter(…)`.',
 	[ERROR_ZERO_INDEX]: 'Prefer `.find(…)` over `.filter(…)[0]`.',
 	[ERROR_SHIFT]: 'Prefer `.find(…)` over `.filter(…).shift()`.',
+	[ERROR_POP]: 'Prefer `.findLast(…)` over `.filter(…).pop()`.',
+	[ERROR_AT_MINUS_ONE]: 'Prefer `.findLast(…)` over `.filter(…).at(-1)`.',
 	[ERROR_DESTRUCTURING_DECLARATION]: 'Prefer `.find(…)` over destructuring `.filter(…)`.',
 	// Same message as `ERROR_DESTRUCTURING_DECLARATION`, but different case
 	[ERROR_DESTRUCTURING_ASSIGNMENT]: 'Prefer `.find(…)` over destructuring `.filter(…)`.',
@@ -76,6 +80,33 @@ const shiftSelector = [
 	}),
 ].join('');
 
+const popSelector = [
+	methodCallSelector({
+		method: 'pop',
+		argumentsLength: 0,
+	}),
+	methodCallSelector({
+		...filterMethodSelectorOptions,
+		path: 'callee.object',
+	}),
+].join('');
+
+const atMinusOneSelector = [
+	methodCallSelector({
+		method: 'at',
+		argumentsLength: 1,
+	}),
+	'[arguments.0.type="UnaryExpression"]',
+	'[arguments.0.operator="-"]',
+	'[arguments.0.prefix]',
+	'[arguments.0.argument.type="Literal"]',
+	'[arguments.0.argument.raw=1]',
+	methodCallSelector({
+		...filterMethodSelectorOptions,
+		path: 'callee.object',
+	}),
+].join('');
+
 const destructuringDeclaratorSelector = [
 	'VariableDeclarator',
 	'[id.type="ArrayPattern"]',
@@ -229,8 +260,14 @@ const isDestructuringFirstElement = node => {
 /** @param {import('eslint').Rule.RuleContext} context */
 const create = context => {
 	const sourceCode = context.getSourceCode();
+	const {
+		checkFromLast,
+	} = {
+		checkFromLast: false,
+		...context.options[0],
+	};
 
-	return {
+	const listeners = {
 		[zeroIndexSelector](node) {
 			return {
 				node: node.object.callee.property,
@@ -319,18 +356,60 @@ const create = context => {
 			return problem;
 		},
 	};
+
+	if (!checkFromLast) {
+		return listeners;
+	}
+
+	return Object.assign(listeners, {
+		[popSelector](node) {
+			return {
+				node: node.callee.object.callee.property,
+				messageId: ERROR_POP,
+				fix: fixer => [
+					fixer.replaceText(node.callee.object.callee.property, 'findLast'),
+					...removeMethodCall(fixer, node, sourceCode),
+				],
+			};
+		},
+		[atMinusOneSelector](node) {
+			return {
+				node: node.callee.object.callee.property,
+				messageId: ERROR_AT_MINUS_ONE,
+				fix: fixer => [
+					fixer.replaceText(node.callee.object.callee.property, 'findLast'),
+					...removeMethodCall(fixer, node, sourceCode),
+				],
+			};
+		},
+	});
 };
 
+const schema = [
+	{
+		type: 'object',
+		additionalProperties: false,
+		properties: {
+			checkFromLast: {
+				type: 'boolean',
+				// TODO: Change default value to `true`, or remove the option when targeting Node.js 18.
+				default: false,
+			},
+		},
+	},
+];
+
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
 	create,
 	meta: {
 		type: 'suggestion',
 		docs: {
-			description: 'Prefer `.find(…)` over the first element from `.filter(…)`.',
+			description: 'Prefer `.find(…)` and `.findLast(…)` over the first or last element from `.filter(…)`.',
 		},
 		fixable: 'code',
 		hasSuggestions: true,
+		schema,
 		messages,
 	},
 };
diff --git a/test/prefer-array-find.mjs b/test/prefer-array-find.mjs
@@ -5,6 +5,8 @@ const {test} = getTester(import.meta);
 
 const ERROR_ZERO_INDEX = 'error-zero-index';
 const ERROR_SHIFT = 'error-shift';
+const ERROR_POP = 'error-pop';
+const ERROR_AT_MINUS_ONE = 'error-at-minus-one';
 const ERROR_DESTRUCTURING_DECLARATION = 'error-destructuring-declaration';
 const ERROR_DESTRUCTURING_ASSIGNMENT = 'error-destructuring-assignment';
 const ERROR_DECLARATION = 'error-variable';
@@ -914,3 +916,182 @@ test({
 		},
 	],
 });
+
+// Check from last
+const checkFromLastOptions = [{checkFromLast: true}];
+
+// Default to false
+test({
+	valid: [
+		'array.filter(foo).pop()',
+		'array.filter(foo).at(-1)',
+	],
+	invalid: [],
+});
+
+// `.pop()`
+test({
+	valid: [
+		// Test `.pop()`
+		// Not `CallExpression`
+		'array.filter(foo).pop',
+		// Not `MemberExpression`
+		'pop(array.filter(foo))',
+		// `callee.property` is not a `Identifier`
+		'array.filter(foo)["pop"]()',
+		// Computed
+		'array.filter(foo)[pop]()',
+		// Not `pop`
+		'array.filter(foo).notPop()',
+		// More or less argument(s)
+		'array.filter(foo).pop(extraArgument)',
+		'array.filter(foo).pop(...[])',
+
+		// Test `.filter()`
+		// Not `CallExpression`
+		'array.filter.pop()',
+		// Not `MemberExpression`
+		'filter(foo).pop()',
+		// `callee.property` is not a `Identifier`
+		'array["filter"](foo).pop()',
+		// Computed
+		'array[filter](foo).pop()',
+		// Not `filter`
+		'array.notFilter(foo).pop()',
+		// More or less argument(s)
+		'array.filter().pop()',
+		'array.filter(foo, thisArgument, extraArgument).pop()',
+		'array.filter(...foo).pop()',
+	].map(code => ({code, options: checkFromLastOptions})),
+	invalid: [
+		{
+			code: 'array.filter(foo).pop()',
+			output: 'array.findLast(foo)',
+			errors: [{messageId: ERROR_POP}],
+		},
+		{
+			code: 'array.filter(foo, thisArgument).pop()',
+			output: 'array.findLast(foo, thisArgument)',
+			errors: [{messageId: ERROR_POP}],
+		},
+		{
+			code: outdent`
+				const item = array
+					// comment 1
+					.filter(
+						// comment 2
+						x => x === '🦄'
+					)
+					// comment 3
+					.pop()
+					// comment 4
+					;
+			`,
+			output: outdent`
+				const item = array
+					// comment 1
+					.findLast(
+						// comment 2
+						x => x === '🦄'
+					)
+					// comment 4
+					;
+			`,
+			errors: [{messageId: ERROR_POP}],
+		},
+	].map(test => ({...test, options: checkFromLastOptions})),
+});
+
+// `.at(-1)`
+test({
+	valid: [
+		// Test `.at()`
+		// Not `CallExpression`
+		'array.filter(foo).at',
+		// Not `MemberExpression`
+		'at(array.filter(foo), -1)',
+		// `callee.property` is not a `Identifier`
+		'array.filter(foo)["at"](-1)',
+		// Computed
+		'array.filter(foo)[at](-1)',
+		// Not `at`
+		'array.filter(foo).notAt(-1)',
+		// More or less argument(s)
+		'array.filter(foo).at()',
+		'array.filter(foo).at(-1, extraArgument)',
+		'array.filter(foo).at(...[-1])',
+
+		// Test `-1`
+		'array.filter(foo).at(1)',
+		'array.filter(foo).at(+1)',
+		'const ONE = 1; array.filter(foo).at(-ONE)',
+		'const MINUS_ONE = 1; array.filter(foo).at(MINUS_ONE)',
+		'const a = {b: 1}; array.filter(foo).at(-a.b)',
+		'const a = {b: -1}; array.filter(foo).at(a.b)',
+		'array.filter(foo).at(-2)',
+		'array.filter(foo).at(-(-1))',
+		'array.filter(foo).at(-1.)',
+		'array.filter(foo).at(-0b1)',
+		'array.filter(foo).at(-"1")',
+		'array.filter(foo).at(-null)',
+		'array.filter(foo).at(-false)',
+		'array.filter(foo).at(-true)',
+
+		// Test `.filter()`
+		// Not `CallExpression`
+		'array.filter.at(-1)',
+		// Not `MemberExpression`
+		'filter(foo).at(-1)',
+		// `callee.property` is not a `Identifier`
+		'array["filter"](foo).at(-1)',
+		// Computed
+		'array[filter](foo).at(-1)',
+		// Not `filter`
+		'array.notFilter(foo).at(-1)',
+		// More or less argument(s)
+		'array.filter().at(-1)',
+		'array.filter(foo, thisArgument, extraArgument).at(-1)',
+		'array.filter(...foo).at(-1)',
+	].map(code => ({code, options: checkFromLastOptions})),
+	invalid: [
+		{
+			code: 'array.filter(foo).at(-1)',
+			output: 'array.findLast(foo)',
+			errors: [{messageId: ERROR_AT_MINUS_ONE}],
+		},
+		{
+			code: 'array.filter(foo, thisArgument).at(-1)',
+			output: 'array.findLast(foo, thisArgument)',
+			errors: [{messageId: ERROR_AT_MINUS_ONE}],
+		},
+		{
+			code: outdent`
+				const item = array
+					// comment 1
+					.filter(
+						// comment 2
+						x => x === '🦄'
+					)
+					// comment 3
+					.at(
+						// comment 4
+						-1
+						// comment 5
+					)
+					// comment 6
+					;
+			`,
+			output: outdent`
+				const item = array
+					// comment 1
+					.findLast(
+						// comment 2
+						x => x === '🦄'
+					)
+					// comment 6
+					;
+			`,
+			errors: [{messageId: ERROR_AT_MINUS_ONE}],
+		},
+	].map(test => ({...test, options: checkFromLastOptions})),
+});