Add no-array-method-this-argument rule

docs/rules/no-array-method-this-argument.md

@@ -0,0 +1,49 @@
+# Forbid using this argument in array methods
+
+Forbid using the `thisArg` argument in array methods:
+
+- If the `callback` is an arrow function or a bound function, it won't effect `this` in callback
+- If you intent to use a custom `this` in callback, it's better to use the variable directly or use `callback.bind(thisArg)`
+
+This rule checks following array methods accepts `thisArg`:
+
+- [`Array#every()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_objects/Array/every)
+- [`Array#filter()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_objects/Array/filter)
+- [`Array#find()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_objects/Array/find)
+- [`Array#findIndex()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_objects/Array/findIndex)
+- [`Array#flatMap()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_objects/Array/flatMap)
+- [`Array#forEach()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_objects/Array/forEach)
+- [`Array#map()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_objects/Array/map)
+- [`Array#some()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_objects/Array/some)
+
+This rule is fixable when the `callback` is an arrow function and the `thisArg` argument has no side effect.
+
+## Fail
+
+```js
+const foo = bar.find(element => isUnicorn(element), baz);
+```
+
+```js
+const foo = bar.map(function (element) => {
+	return this.unicorn(element);
+}, baz);
+```
+
+## Pass
+
+```js
+const foo = bar.find(element => isUnicorn(element));
+```
+
+```js
+const foo = bar.map(function (element) => {
+	return baz.unicorn(element);
+});
+```
+
+```js
+const foo = bar.map(function (element) => {
+	return this.unicorn(element);
+}.bind(baz));
+```

index.js

@@ -55,6 +55,7 @@ module.exports = {
 				'unicorn/no-abusive-eslint-disable': 'error',
 				'unicorn/no-array-callback-reference': 'error',
 				'unicorn/no-array-for-each': 'error',
+				'unicorn/no-array-method-this-argument': 'error',
 				'unicorn/no-array-push-push': 'error',
 				'unicorn/no-array-reduce': 'error',
 				'unicorn/no-console-spaces': 'error',

readme.md

@@ -52,6 +52,7 @@ Configure it in `package.json`.
 			"unicorn/no-abusive-eslint-disable": "error",
 			"unicorn/no-array-callback-reference": "error",
 			"unicorn/no-array-for-each": "error",
+			"unicorn/no-array-method-this-argument": "error",
 			"unicorn/no-array-push-push": "error",
 			"unicorn/no-array-reduce": "error",
 			"unicorn/no-console-spaces": "error",
@@ -156,6 +157,7 @@ Each rule has emojis denoting:
 | [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) | Enforce specifying rules to disable in `eslint-disable` comments. | ✅ |  |  |
 | [no-array-callback-reference](docs/rules/no-array-callback-reference.md) | Prevent passing a function reference directly to iterator methods. | ✅ |  | 💡 |
 | [no-array-for-each](docs/rules/no-array-for-each.md) | Prefer `for…of` over `Array#forEach(…)`. | ✅ | 🔧 |  |
+| [no-array-method-this-argument](docs/rules/no-array-method-this-argument.md) | Forbid use this argument in array methods. | ✅ | 🔧 | 💡 |
 | [no-array-push-push](docs/rules/no-array-push-push.md) | Enforce combining multiple `Array#push()` into one call. | ✅ | 🔧 | 💡 |
 | [no-array-reduce](docs/rules/no-array-reduce.md) | Disallow `Array#reduce()` and `Array#reduceRight()`. | ✅ |  |  |
 | [no-console-spaces](docs/rules/no-console-spaces.md) | Do not use leading/trailing space between `console.log` parameters. | ✅ | 🔧 |  |

rules/fix/index.js

@@ -2,5 +2,6 @@
 
 module.exports = {
 	appendArgument: require('./append-argument.js'),
+	removeArgument: require('./remove-argument.js'),
 	switchNewExpressionToCallExpression: require('./switch-new-expression-to-call-expression.js')
 };

rules/fix/remove-argument.js

@@ -0,0 +1,29 @@
+'use strict';
+const {isCommaToken} = require('eslint-utils');
+const {getParentheses} = require('../utils/parentheses.js');
+
+function removeArgument(fixer, node, sourceCode) {
+	const callExpression = node.parent;
+	const index = callExpression.arguments.indexOf(node);
+	const parentheses = getParentheses(node, sourceCode);
+	const firstToken = parentheses[0] || node;
+	const lastToken = parentheses[parentheses.length - 1] || node;
+
+	let [start] = firstToken.range;
+	let [, end] = lastToken.range;
+
+	if (index !== 0) {
+		start = sourceCode.getTokenBefore(firstToken).range[0];
+	}
+
+	if (callExpression.arguments.length === 1) {
+		const tokenAfter = sourceCode.getTokenBefore(lastToken);
+		if (isCommaToken(tokenAfter)) {
+			end = tokenAfter[1];
+		}
+	}
+
+	return fixer.replaceTextRange([start, end], '');
+}
+
+module.exports = removeArgument;

rules/no-array-method-this-argument.js

@@ -0,0 +1,120 @@
+'use strict';
+const {hasSideEffect} = require('eslint-utils');
+const {methodCallSelector} = require('./selectors/index.js');
+const {removeArgument} = require('./fix/index.js');
+const {getParentheses, getParenthesizedText} = require('./utils/parentheses.js');
+const shouldAddParenthesesToMemberExpressionObject = require('./utils/should-add-parentheses-to-member-expression-object.js');
+
+const ERROR = 'error';
+const SUGGESTION_BIND = 'suggestion-bind';
+const SUGGESTION_REMOVE = 'suggestion-remove';
+const messages = {
+	[ERROR]: 'Do not use this argument in `Array#{{method}}()`.',
+	[SUGGESTION_REMOVE]: 'Remove the second argument.',
+	[SUGGESTION_BIND]: 'Use bound function.'
+};
+
+const selector = methodCallSelector({
+	names: [
+		'every',
+		'filter',
+		'find',
+		'findIndex',
+		'flatMap',
+		'forEach',
+		'map',
+		'some'
+	],
+	length: 2
+});
+
+function removeThisArgument(callExpression, sourceCode) {
+	return fixer => removeArgument(fixer, callExpression.arguments[1], sourceCode);
+}
+
+function useBoundFunction(callExpression, sourceCode) {
+	return function * (fixer) {
+		yield removeThisArgument(callExpression, sourceCode)(fixer);
+
+		const [callback, thisArgument] = callExpression.arguments;
+
+		const callbackParentheses = getParentheses(callback, sourceCode);
+		const isParenthesized = callbackParentheses.length > 0;
+		const callbackLastToken = isParenthesized ?
+			callbackParentheses[callbackParentheses.length - 1] :
+			callback;
+		if (
+			!isParenthesized &&
+			shouldAddParenthesesToMemberExpressionObject(callback, sourceCode)
+		) {
+			yield fixer.insertTextBefore(callbackLastToken, '(');
+			yield fixer.insertTextAfter(callbackLastToken, ')');
+		}
+
+		const thisArgumentText = getParenthesizedText(thisArgument, sourceCode);
+		// `thisArgument` was a argument, no need add extra parentheses
+		yield fixer.insertTextAfter(callbackLastToken, `.bind(${thisArgumentText})`);
+	};
+}
+
+/** @param {import('eslint').Rule.RuleContext} context */
+const create = context => {
+	const sourceCode = context.getSourceCode();
+
+	return {
+		[selector](callExpression) {
+			const method = callExpression.callee.property.name;
+			const [callback, thisArgument] = callExpression.arguments;
+
+			const problem = {
+				node: thisArgument,
+				messageId: ERROR,
+				data: {method}
+			};
+
+			const thisArgumentHasSideEffect = hasSideEffect(thisArgument, sourceCode);
+			const isArrowCallback = callback.type === 'ArrowFunctionExpression';
+
+			if (isArrowCallback) {
+				if (thisArgumentHasSideEffect) {
+					problem.suggest = [
+						{
+							messageId: SUGGESTION_REMOVE,
+							fix: removeThisArgument(callExpression, sourceCode)
+						}
+					];
+				} else {
+					problem.fix = removeThisArgument(callExpression, sourceCode);
+				}
+
+				return problem;
+			}
+
+			problem.suggest = [
+				{
+					messageId: SUGGESTION_REMOVE,
+					fix: removeThisArgument(callExpression, sourceCode)
+				},
+				{
+					messageId: SUGGESTION_BIND,
+					fix: useBoundFunction(callExpression, sourceCode)
+				}
+			];
+
+			return problem;
+		}
+	};
+};
+
+module.exports = {
+	create,
+	meta: {
+		type: 'suggestion',
+		docs: {
+			description: 'Forbid use this argument in array methods.'
+		},
+		fixable: 'code',
+		messages,
+		hasSuggestions: true
+	}
+};

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

@@ -21,6 +21,7 @@ function shouldAddParenthesesToMemberExpressionObject(node, sourceCode) {
 		case 'TemplateLiteral':
 		case 'ThisExpression':
 		case 'ArrayExpression':
+		case 'FunctionExpression':
 			return false;
 		case 'NewExpression':
 			return !isNewExpressionWithParentheses(node, sourceCode);

test/no-array-method-this-argument.mjs

@@ -0,0 +1,54 @@
+import {getTester} from './utils/test.mjs';
+
+const {test} = getTester(import.meta);
+
+// Arrow functions
+test.snapshot({
+	valid: [
+		'array.unknownMethod(() => {}, thisArgument)',
+		'new array.map(() => {}, thisArgument)',
+		'array.map?.(() => {}, thisArgument)',
+		'array?.map(() => {}, thisArgument)',
+		// More or less arguments
+		'array.map()',
+		'array.map(() => {},)',
+		'array.map(() => {}, ...thisArgument)',
+		'array.map(...() => {}, thisArgument)',
+		'array.map(() => {}, thisArgument, extraArgument)'
+	],
+	invalid: [
+		'array.every(() => {}, thisArgument)',
+		'array.filter(() => {}, thisArgument)',
+		'array.find(() => {}, thisArgument)',
+		'array.findIndex(() => {}, thisArgument)',
+		'array.flatMap(() => {}, thisArgument)',
+		'array.forEach(() => {}, thisArgument)',
+		'array.map(() => {}, thisArgument)',
+		// Comma
+		'array.map(() => {}, thisArgument,)',
+		'array.map(() => {}, (0, thisArgument),)',
+		// Side effect
+		'array.map(() => {}, thisArgumentHasSideEffect())'
+	]
+});
+
+// Non-arrow functions
+test.snapshot({
+	valid: [],
+	invalid: [
+		'array.map(callback, thisArgument)',
+		'array.map(callback, (0, thisArgument))',
+		'array.map(function () {}, thisArgument)',
+		'array.map(function callback () {}, thisArgument)',
+		'array.map(new Callback, thisArgument)',
+		'array.map(1, thisArgument)',
+		'async () => array.map(await callback, thisArgument)',
+		'array.map((new Callback), thisArgument)',
+		'array.map(new Callback(), thisArgument)',
+		'array.map( (( callback )), (( thisArgument )),)',
+		// This callback is actually arrow function, but we don't know
+		'array.map((0, () => {}), thisArgument)',
+		// This callback is a bound function, but we don't know
+		'array.map(callback.bind(foo), thisArgument)'
+	]
+});