diff --git a/README.md b/README.md index 9fa9344e..32a3f111 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ Enforce best practices for JavaScript promises. * [`always-return`](#always-return) * [`no-native`](#no-native) * [`no-nesting`](#no-nesting) + * [`no-identity-handlers`](#no-identity-handlers) * [`no-promise-in-callback`](#no-promise-in-callback) * [`no-callback-in-promise`](#no-callback-in-promise) * [`avoid-new`](#avoid-new) @@ -70,6 +71,7 @@ Then configure the rules you want to use under the rules section. "promise/catch-or-return": "error", "promise/no-native": "off", "promise/no-nesting": "warn", + "promise/no-identity-handlers": "warn", "promise/no-promise-in-callback": "warn", "promise/no-callback-in-promise": "warn", "promise/avoid-new": "warn", @@ -96,6 +98,7 @@ or start with the recommended rule set | `always-return` | Return inside each `then()` to create readable and reusable Promise chains. | :bangbang: | | | `no-native` | In an ES5 environment, make sure to create a `Promise` constructor before using. | | | | `no-nesting` | Avoid nested `then()` or `catch()` statements | :warning: | | +| `no-identity-handlers` | Avoid unnecessary identity functions in `then()` or `catch()` | :warning: | | | `no-promise-in-callback` | Avoid using promises inside of callbacks | :warning: | | | `no-callback-in-promise` | Avoid calling `cb()` inside of a `then()` (use [nodeify][] instead) | :warning: | | | `avoid-new` | Avoid creating `new` promises outside of utility libs (use [pify][] instead) | :warning: | | @@ -297,6 +300,10 @@ var x = Promise.resolve('bad') Avoid nested `then()` or `catch()` statements +### `no-identity-handlers` + +Avoid unnecessary identity functions in `then()` or `catch()` + ### `no-promise-in-callback` Avoid using promises inside of callbacks diff --git a/index.js b/index.js index 79e2c8b2..3318c180 100644 --- a/index.js +++ b/index.js @@ -12,6 +12,7 @@ module.exports = { 'no-callback-in-promise': require('./rules/no-callback-in-promise'), 'no-promise-in-callback': require('./rules/no-promise-in-callback'), 'no-nesting': require('./rules/no-nesting'), + 'no-identity-handlers': require('./rules/no-identity-handlers'), 'avoid-new': require('./rules/avoid-new'), 'no-return-in-finally': require('./rules/no-return-in-finally') }, @@ -31,6 +32,7 @@ module.exports = { 'promise/catch-or-return': 'error', 'promise/no-native': 'off', 'promise/no-nesting': 'warn', + 'promise/no-identity-handlers': 'warn', 'promise/no-promise-in-callback': 'warn', 'promise/no-callback-in-promise': 'warn', 'promise/avoid-new': 'warn', diff --git a/rules/no-identity-handlers.js b/rules/no-identity-handlers.js new file mode 100644 index 00000000..9bbe984c --- /dev/null +++ b/rules/no-identity-handlers.js @@ -0,0 +1,76 @@ +'use strict' + +const isPromise = require('./lib/is-promise') + +function isFunctionExpression(node) { + return ( + node.type === 'FunctionExpression' || + node.type === 'ArrowFunctionExpression' + ) +} + +function getFirstParamName(node) { + const firstParam = node.params[0] + return firstParam && firstParam.type === 'Identifier' ? firstParam.name : null +} + +function getBodyValueName(node) { + const body = node.body || {} + if (body.type === 'Identifier') { + return body.name + } + if (body.type === 'BlockStatement') { + const firstBodyStatement = body.body[0] || { type: '', argument: {} } + return (firstBodyStatement.type === 'ReturnStatement' || + firstBodyStatement.type === 'ThrowStatement') && + firstBodyStatement.argument.type === 'Identifier' + ? firstBodyStatement.argument.name + : null + } + return null +} + +function isIdentityFunction(node) { + return node.params.length === 1 + ? getFirstParamName(node) === getBodyValueName(node) + : false +} + +module.exports = { + meta: { + docs: { + url: + 'https://github.com/xjamundx/eslint-plugin-promise#no-identity-handlers' + } + }, + create(context) { + function checkIdentity(node) { + if (node && isFunctionExpression(node) && isIdentityFunction(node)) { + context.report({ + node, + message: 'No identity handlers' + }) + } + } + + return { + CallExpression(node) { + if (!isPromise(node)) { + return + } + + switch (node.callee.property.name) { + case 'then': + checkIdentity(node.arguments[0]) + checkIdentity(node.arguments[1]) + break + case 'catch': + checkIdentity(node.arguments[0]) + break + default: + break + } + } + } + } +} diff --git a/test/no-identity-handlers.js b/test/no-identity-handlers.js new file mode 100644 index 00000000..90f6eadc --- /dev/null +++ b/test/no-identity-handlers.js @@ -0,0 +1,60 @@ +'use strict' + +const rule = require('../rules/no-identity-handlers') +const RuleTester = require('eslint').RuleTester +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 6 + } +}) + +ruleTester.run('no-identity-handlers', rule, { + valid: [ + 'Promise.resolve(2).then(value => value * 2)', + 'Promise.resolve(2).then(value => { return value + 1 })', + 'Promise.resolve(2).then(() => null)', + 'Promise.resolve(2).then(() => doSomethingTotallyDifferent())', + 'somePromise().then(value => doSomethingWith(value))', + 'somePromise().then(handler)', + 'Promise.reject(Error()).catch(err => { console.error(err); throw err; })', + 'Promise.reject(Error()).catch(err => { throw doSomethingTo(err) })', + 'somePromise().catch(handler)', + 'somePromise().catch(createHandler())', + 'somePromise().then(value => value * 2, err => { console.error(err); throw err; })', + 'somePromise().then(func, func)', + + // edge cases that aren't really valid but shouldn't throw or report + 'Promise.resolve(2).then()', + 'Promise.reject(Error()).catch()' + ], + invalid: [ + { + code: 'Promise.resolve(2).then(_ => _)', + errors: [{ message: 'No identity handlers' }] + }, + { + code: 'Promise.resolve(2).then(val => { return val })', + errors: [{ message: 'No identity handlers' }] + }, + { + code: 'Promise.resolve(2).then(function (value) { return value })', + errors: [{ message: 'No identity handlers' }] + }, + { + code: 'Promise.reject(Error()).catch(err => { throw err })', + errors: [{ message: 'No identity handlers' }] + }, + { + code: 'Promise.reject(Error()).catch(function (e) { throw e })', + errors: [{ message: 'No identity handlers' }] + }, + { + code: 'Promise.reject(Error()).then(null, error => { throw error })', + errors: [{ message: 'No identity handlers' }] + }, + { + code: 'Promise.reject(Error()).then(null, function (e) { throw e })', + errors: [{ message: 'No identity handlers' }] + } + ] +})