diff --git a/docs/rules/no-fn-reference-in-iterator.md b/docs/rules/no-fn-reference-in-iterator.md new file mode 100644 index 0000000000..f9a2718c07 --- /dev/null +++ b/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)); +``` diff --git a/index.js b/index.js index 8184e0ee7c..9c8cd3bfc3 100644 --- a/index.js +++ b/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' } } } diff --git a/readme.md b/readme.md index 1410758743..dfd92d05f4 100644 --- a/readme.md +++ b/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 diff --git a/rules/no-fn-reference-in-iterator.js b/rules/no-fn-reference-in-iterator.js new file mode 100644 index 0000000000..cf681526a4 --- /dev/null +++ b/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' + } +}; diff --git a/test/no-fn-reference-in-iterator.js b/test/no-fn-reference-in-iterator.js new file mode 100644 index 0000000000..88f738f7ef --- /dev/null +++ b/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), [])' + } + ] +});