Add no-fn-reference-in-iterator rule - fixes #91 (#97)

docs/rules/no-fn-reference-in-iterator.md

@@ -0,0 +1,167 @@
+# Prevents passing a function reference directly to iterator methods
+
+Suppose you have a `unicorn` module:
+
+```js
+module.exports = x => x + 1;
+```
+
+You can then use it like this:
+
+```js
+const unicorn = require('unicorn');
+
+[1, 2, 3].map(unicorn);
+//=> [2, 3, 4]
+```
+
+The `unicorn` module now does a minor version that adds another argument:
+
+```js
+module.exports = (x, y) => x + (y ? y : 1);
+```
+
+Your code will now return something different and probably break for users because it is now passing the index of the item as second argument.
+
+```js
+const unicorn = require('unicorn');
+
+[1, 2, 3].map(unicorn);
+//=> [2, 3, 5]
+```
+
+
+## Fail
+
+```js
+const fn = x => x + 1;
+
+[1, 2, 3].map(fn);
+```
+
+```js
+const fn = x => console.log(x);
+
+[1, 2, 3].forEach(fn);
+```
+
+```js
+const fn = x => x < 10;
+
+[1, 2, 3].every(fn);
+```
+
+```js
+const fn = x => x % 2;
+
+[1, 2, 3].filter(fn);
+```
+
+```js
+const fn = x => x === 1;
+
+[1, 2, 3].find(fn);
+```
+
+```js
+const fn = x => x === 1;
+
+[1, 2, 3].findIndex(fn);
+```
+
+```js
+const fn = x => x === 2;
+
+[1, 2, 3].some(fn);
+```
+
+```js
+const fn = (a, b) => a + b;
+
+[1, 2, 3].reduce(fn, 0);
+```
+
+```js
+const fn = (a, b) => a.concat(b);
+
+[1, 2, 3].reduceRight(fn, []);
+```
+
+```js
+const fn = x => x === 2;
+
+[1, 2, 3].map(m({foo: 'bar'}));
+```
+
+
+## Pass
+
+```js
+const fn = x => x + 1;
+
+[1, 2, 3].map(x => fn(x));
+```
+
+```js
+const fn = x => console.log(x);
+
+[1, 2, 3].forEach(x => fn(x));
+```
+
+```js
+const fn = x => x < 10;
+
+[1, 2, 3].every(x => fn(x));
+```
+
+```js
+const fn = x => x % 2;
+
+[1, 2, 3].filter(x => fn(x));
+```
+
+```js
+[undefined, 2, 3].filter(Boolean);
+```
+
+```js
+const fn = x => x === 1;
+
+[1, 2, 3].find(x => fn(x));
+```
+
+```js
+const fn = x => x === 1;
+
+[1, 2, 3].findIndex(x => fn(x));
+```
+
+```js
+const fn = x => x === 2;
+
+[1, 2, 3].some(x => fn(x));
+```
+
+```js
+const fn = (a, b) => a + b;
+
+[1, 2, 3].reduce((a, b) => fn(a, b), 0);
+```
+
+```js
+const fn = (a, b) => a.concat(b);
+
+[1, 2, 3].reduceRight((a, b) => fn(a, b), []);
+```
+
+```js
+const fn = (a, b) => a.concat(b);
+
+[1, 2, 3].reduceRight(fn, []);
+```
+
+```js
+const fn = x => x === 2;
+
+[1, 2, 3].map(x => m({foo: 'bar'})(x));
+```

index.js

@@ -27,7 +27,8 @@ module.exports = {
 				'unicorn/no-hex-escape': 'off',
 				'unicorn/custom-error-definition': 'error',
 				'unicorn/prefer-starts-ends-with': 'error',
-				'unicorn/prefer-type-error': 'error'
+				'unicorn/prefer-type-error': 'error',
+				'unicorn/no-fn-reference-in-iterator': 'error'
 			}
 		}
 	}

readme.md

@@ -69,6 +69,7 @@ Configure it in `package.json`.
 - [custom-error-definition](docs/rules/custom-error-definition.md) - Enforce correct `Error` subclassing. *(fixable)*
 - [prefer-starts-ends-with](docs/rules/prefer-starts-ends-with.md) - Prefer `String#startsWith` & `String#endsWith` over more complex alternatives.
 - [prefer-type-error](docs/rules/prefer-type-error.md) - Enforce throwing `TypeError` in type checking conditions. *(fixable)*
+- [no-fn-reference-in-iterator](docs/rules/no-fn-reference-in-iterator.md) - Prevents passing a function reference directly to iterator methods. *(fixable)*
 
 
 ## Recommended config

rules/no-fn-reference-in-iterator.js

@@ -0,0 +1,51 @@
+'use strict';
+const iteratorMethods = new Map([
+	['map', 1],
+	['forEach', 1],
+	['every', 1],
+	['filter', 1],
+	['find', 1],
+	['findIndex', 1],
+	['some', 1],
+	['reduce', 2],
+	['reduceRight', 2]
+]);
+
+const whitelist = new Set([
+	'Boolean'
+]);
+
+const isIteratorMethod = node => node.callee.property && iteratorMethods.has(node.callee.property.name);
+const hasFunctionArgument = node => node.arguments.length > 0 && (node.arguments[0].type === 'Identifier' || node.arguments[0].type === 'CallExpression') && !whitelist.has(node.arguments[0].name);
+
+const getNumberOfArguments = node => node.callee.property && iteratorMethods.get(node.callee.property.name);
+const parseArgument = (context, arg) => arg.type === 'Identifier' ? arg.name : context.getSourceCode().getText(arg);
+
+const fix = (context, node) => {
+	const numberOfArgs = getNumberOfArguments(node);
+	const arg = node.arguments[0];
+	const argString = numberOfArgs === 1 ? 'x' : 'a, b';
+
+	return fixer => fixer.replaceText(arg, `${numberOfArgs === 1 ? argString : `(${argString})`} => ${parseArgument(context, arg)}(${argString})`);
+};
+
+const create = context => ({
+	CallExpression: node => {
+		if (isIteratorMethod(node) && hasFunctionArgument(node)) {
+			const arg = node.arguments[0];
+
+			context.report({
+				node: arg,
+				message: 'Do not pass a function reference directly to an iterator method.',
+				fix: fix(context, node)
+			});
+		}
+	}
+});
+
+module.exports = {
+	create,
+	meta: {
+		fixable: 'code'
+	}
+};

test/no-fn-reference-in-iterator.js

@@ -0,0 +1,106 @@
+import test from 'ava';
+import avaRuleTester from 'eslint-ava-rule-tester';
+import rule from '../rules/no-fn-reference-in-iterator';
+
+const ruleTester = avaRuleTester(test, {
+	env: {
+		es6: true
+	}
+});
+
+const errors = [
+	{
+		ruleId: 'no-fn-reference-in-iterator',
+		message: 'Do not pass a function reference directly to an iterator method.'
+	}
+];
+
+ruleTester.run('no-fn-reference-in-iterator', rule, {
+	valid: [
+		'foo.map(x => fn(x))',
+		'foo.forEach(x => fn(x))',
+		'foo.every(x => fn(x))',
+		'foo.filter(x => fn(x))',
+		'foo.find(x => fn(x))',
+		'foo.findIndex(x => fn(x))',
+		'foo.some(x => fn(x))',
+		'foo.filter(x => Boolean(x))',
+		'foo.filter(Boolean)',
+		'foo.map(x => parseInt(x, 10))',
+		'foo.map(x => m({foo: true})(x))',
+		'foo.reduce((a, b) => a + b, 0)',
+		'foo.reduceRight((a, b) => a.concat(b), [])'
+	],
+	invalid: [
+		{
+			code: 'foo.map(fn)',
+			errors,
+			output: 'foo.map(x => fn(x))'
+		},
+		{
+			code: 'foo.forEach(fn)',
+			errors,
+			output: 'foo.forEach(x => fn(x))'
+		},
+		{
+			code: 'foo.every(fn)',
+			errors,
+			output: 'foo.every(x => fn(x))'
+		},
+		{
+			code: 'foo.filter(fn)',
+			errors,
+			output: 'foo.filter(x => fn(x))'
+		},
+		{
+			code: 'foo.find(fn)',
+			errors,
+			output: 'foo.find(x => fn(x))'
+		},
+		{
+			code: 'foo.findIndex(fn)',
+			errors,
+			output: 'foo.findIndex(x => fn(x))'
+		},
+		{
+			code: 'foo.some(fn)',
+			errors,
+			output: 'foo.some(x => fn(x))'
+		},
+		{
+			code: 'foo.filter(fn)',
+			errors,
+			output: 'foo.filter(x => fn(x))'
+		},
+		{
+			code: 'foo.map(parseInt)',
+			errors,
+			output: 'foo.map(x => parseInt(x))'
+		},
+		{
+			code: 'foo.map(m({foo: true}))',
+			errors,
+			output: 'foo.map(x => m({foo: true})(x))'
+		},
+		{
+			code: 'foo.reduce(m)',
+			errors,
+			output: 'foo.reduce((a, b) => m(a, b))'
+		},
+		{
+			code: 'foo.reduce(m, 0)',
+			errors,
+			output: 'foo.reduce((a, b) => m(a, b), 0)'
+		},
+		{
+			code: 'foo.reduce(m({foo: true}), 0)',
+			errors,
+			output: 'foo.reduce((a, b) => m({foo: true})(a, b), 0)'
+		},
+		{
+			code: 'foo.reduceRight(m, [])',
+			errors,
+			output: 'foo.reduceRight((a, b) => m(a, b), [])'
+		}
+	]
+});