From 54bc2dedda72bc45edb355106ac35b5aa2833bb5 Mon Sep 17 00:00:00 2001 From: Macklin Underdown Date: Tue, 6 Feb 2018 11:52:34 -0500 Subject: [PATCH] feat(rule): add valid-params rule Resolves #47 --- README.md | 54 +++++++++++++- index.js | 6 +- rules/valid-params.js | 57 +++++++++++++++ test/valid-params.js | 161 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 275 insertions(+), 3 deletions(-) create mode 100644 rules/valid-params.js create mode 100644 test/valid-params.js diff --git a/README.md b/README.md index e43dc6c8..9b4c7bfb 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ Enforce best practices for JavaScript promises. - [`no-callback-in-promise`](#no-callback-in-promise) - [`avoid-new`](#avoid-new) - [`no-return-in-finally`](#no-return-in-finally) + - [`valid-params`](#valid-params) - [`prefer-await-to-then`](#prefer-await-to-then) - [`prefer-await-to-callbacks`](#prefer-await-to-callbacks) - [License](#license) @@ -72,7 +73,8 @@ Then configure the rules you want to use under the rules section. "promise/no-promise-in-callback": "warn", "promise/no-callback-in-promise": "warn", "promise/avoid-new": "warn", - "promise/no-return-in-finally": "warn" + "promise/no-return-in-finally": "warn", + "promise/valid-params": "warn" } } ``` @@ -101,6 +103,7 @@ or start with the recommended rule set | :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: | `no-return-in-finally` | Disallow return statements in `finally()` | +| :warning: | `valid-params` | Ensures the proper number of arguments are passed to Promise functions | | :seven: | `prefer-await-to-then` | Prefer `await` to `then()` for reading Promise values | | :seven: | `prefer-await-to-callbacks` | Prefer async/await to the callback pattern | @@ -285,6 +288,55 @@ myPromise.finally(function(val) { }) ``` +### `valid-params` + +Ensures the proper number of arguments are passed to Promise functions + +#### Valid + +```js +// Promise.all() requires 1 argument +Promise.all([p1, p2, p3]) +Promise.all(iterable) + +// Promise.race() requires 1 argument +Promise.race([p1, p2, p3]) +Promise.race(iterable) + +// Promise.resolve() requires 0 or 1 arguments +Promise.resolve() +Promise.resolve({}) +Promise.resolve([1, 2, 3]) +Promise.resolve(referenceToObject) + +// Promise.reject() requires 0 or 1 arguments +Promise.reject() +Promise.reject(Error()) +Promise.reject(referenceToError) + +// Promise.then() requires 1 or 2 arguments +somePromise().then(value => doSomething(value)) +somePromise().then(successCallback, errorCallback) + +// Promise.catch() requires 1 argument +somePromise().catch(error => { handleError(error) }) +somePromise().catch(console.error) + +// Promise.finally() requires 1 argument +somePromise().finally(() => { console.log('done!') }) +somePromise().finally(console.log) +``` + +#### Invalid + +* `Promise.all()` is called with 0 or 2+ arguments +* `Promise.race()` is called with 0 or 2+ arguments +* `Promise.resolve()` is called with 2+ arguments +* `Promise.reject()` is called with 2+ arguments +* `Promise.then()` is called with 0 or 3+ arguments +* `Promise.catch()` is called with 0 or 2+ arguments +* `Promise.finally()` is called with 0 or 2+ arguments + ### `prefer-await-to-then` Prefer `await` to `then()` for reading Promise values diff --git a/index.js b/index.js index 79e2c8b2..ab71bb38 100644 --- a/index.js +++ b/index.js @@ -13,7 +13,8 @@ module.exports = { 'no-promise-in-callback': require('./rules/no-promise-in-callback'), 'no-nesting': require('./rules/no-nesting'), 'avoid-new': require('./rules/avoid-new'), - 'no-return-in-finally': require('./rules/no-return-in-finally') + 'no-return-in-finally': require('./rules/no-return-in-finally'), + 'valid-params': require('./rules/valid-params') }, rulesConfig: { 'param-names': 1, @@ -34,7 +35,8 @@ module.exports = { 'promise/no-promise-in-callback': 'warn', 'promise/no-callback-in-promise': 'warn', 'promise/avoid-new': 'warn', - 'promise/no-return-in-finally': 'warn' + 'promise/no-return-in-finally': 'warn', + 'promise/valid-params': 'warn' } } } diff --git a/rules/valid-params.js b/rules/valid-params.js new file mode 100644 index 00000000..cecc689c --- /dev/null +++ b/rules/valid-params.js @@ -0,0 +1,57 @@ +'use strict' + +const isPromise = require('./lib/is-promise') + +module.exports = { + meta: { + docs: { + description: 'Ensures the proper number of arguments are passed to Promise functions', + url: 'https://github.com/xjamundx/eslint-plugin-promise#valid-params' + } + }, + create (context) { + return { + CallExpression (node) { + if (!isPromise(node)) { + return + } + + const name = node.callee.property.name + const numArgs = node.arguments.length + + switch (name) { + case 'resolve': + case 'reject': + if (numArgs > 1) { + context.report({ + node, + message: `Promise.${name}() requires 0 or 1 arguments, but received ${numArgs}` + }) + } + break + case 'then': + if (numArgs < 1 || numArgs > 2) { + context.report({ + node, + message: `Promise.${name}() requires 1 or 2 arguments, but received ${numArgs}` + }) + } + break + case 'race': + case 'all': + case 'catch': + case 'finally': + if (numArgs !== 1) { + context.report({ + node, + message: `Promise.${name}() requires 1 argument, but received ${numArgs}` + }) + } + break + default: + break + } + } + } + } +} diff --git a/test/valid-params.js b/test/valid-params.js new file mode 100644 index 00000000..a0f54fd6 --- /dev/null +++ b/test/valid-params.js @@ -0,0 +1,161 @@ +'use strict' + +const rule = require('../rules/valid-params') +const RuleTester = require('eslint').RuleTester +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 6 + } +}) + +ruleTester.run('valid-params', rule, { + valid: [ + // valid Promise.resolve() + 'Promise.resolve()', + 'Promise.resolve(1)', + 'Promise.resolve({})', + 'Promise.resolve(referenceToSomething)', + + // valid Promise.reject() + 'Promise.reject()', + 'Promise.reject(1)', + 'Promise.reject({})', + 'Promise.reject(referenceToSomething)', + 'Promise.reject(Error())', + + // valid Promise.race() + 'Promise.race([])', + 'Promise.race(iterable)', + 'Promise.race([one, two, three])', + + // valid Promise.all() + 'Promise.all([])', + 'Promise.all(iterable)', + 'Promise.all([one, two, three])', + + // valid Promise.then() + 'somePromise().then(success)', + 'somePromise().then(success, failure)', + 'promiseReference.then(() => {})', + 'promiseReference.then(() => {}, () => {})', + + // valid Promise.catch() + 'somePromise().catch(callback)', + 'somePromise().catch(err => {})', + 'promiseReference.catch(callback)', + 'promiseReference.catch(err => {})', + + // valid Promise.finally() + 'somePromise().finally(callback)', + 'somePromise().finally(() => {})', + 'promiseReference.finally(callback)', + 'promiseReference.finally(() => {})', + + // integration test + [ + 'Promise.all([', + ' Promise.resolve(1),', + ' Promise.resolve(2),', + ' Promise.reject(Error()),', + '])', + ' .then(console.log)', + ' .catch(console.error)', + ' .finally(console.log)' + ].join('\n') + ], + invalid: [ + // invalid Promise.resolve() + { + code: 'Promise.resolve(1, 2)', + errors: [{ message: 'Promise.resolve() requires 0 or 1 arguments, but received 2' }] + }, + { + code: 'Promise.resolve({}, function() {}, 1, 2, 3)', + errors: [{ message: 'Promise.resolve() requires 0 or 1 arguments, but received 5' }] + }, + + // invalid Promise.reject() + { + code: 'Promise.reject(1, 2, 3)', + errors: [{ message: 'Promise.reject() requires 0 or 1 arguments, but received 3' }] + }, + { + code: 'Promise.reject({}, function() {}, 1, 2)', + errors: [{ message: 'Promise.reject() requires 0 or 1 arguments, but received 4' }] + }, + + // invalid Promise.race() + { + code: 'Promise.race(1, 2)', + errors: [{ message: 'Promise.race() requires 1 argument, but received 2' }] + }, + { + code: 'Promise.race({}, function() {}, 1, 2, 3)', + errors: [{ message: 'Promise.race() requires 1 argument, but received 5' }] + }, + + // invalid Promise.all() + { + code: 'Promise.all(1, 2, 3)', + errors: [{ message: 'Promise.all() requires 1 argument, but received 3' }] + }, + { + code: 'Promise.all({}, function() {}, 1, 2)', + errors: [{ message: 'Promise.all() requires 1 argument, but received 4' }] + }, + + // invalid Promise.then() + { + code: 'somePromise().then()', + errors: [{ message: 'Promise.then() requires 1 or 2 arguments, but received 0' }] + }, + { + code: 'somePromise().then(() => {}, () => {}, () => {})', + errors: [{ message: 'Promise.then() requires 1 or 2 arguments, but received 3' }] + }, + { + code: 'promiseReference.then()', + errors: [{ message: 'Promise.then() requires 1 or 2 arguments, but received 0' }] + }, + { + code: 'promiseReference.then(() => {}, () => {}, () => {})', + errors: [{ message: 'Promise.then() requires 1 or 2 arguments, but received 3' }] + }, + + // invalid Promise.catch() + { + code: 'somePromise().catch()', + errors: [{ message: 'Promise.catch() requires 1 argument, but received 0' }] + }, + { + code: 'somePromise().catch(() => {}, () => {})', + errors: [{ message: 'Promise.catch() requires 1 argument, but received 2' }] + }, + { + code: 'promiseReference.catch()', + errors: [{ message: 'Promise.catch() requires 1 argument, but received 0' }] + }, + { + code: 'promiseReference.catch(() => {}, () => {})', + errors: [{ message: 'Promise.catch() requires 1 argument, but received 2' }] + }, + + // invalid Promise.finally() + { + code: 'somePromise().finally()', + errors: [{ message: 'Promise.finally() requires 1 argument, but received 0' }] + }, + { + code: 'somePromise().finally(() => {}, () => {})', + errors: [{ message: 'Promise.finally() requires 1 argument, but received 2' }] + }, + { + code: 'promiseReference.finally()', + errors: [{ message: 'Promise.finally() requires 1 argument, but received 0' }] + }, + { + code: 'promiseReference.finally(() => {}, () => {})', + errors: [{ message: 'Promise.finally() requires 1 argument, but received 2' }] + } + ] +})