Skip to content

Commit

Permalink
feat(rule): add valid-params rule (#85)
Browse files Browse the repository at this point in the history
Resolves #47
  • Loading branch information
macklinu committed Mar 8, 2018
1 parent 3af50b7 commit 4782ae5
Show file tree
Hide file tree
Showing 5 changed files with 342 additions and 3 deletions.
5 changes: 4 additions & 1 deletion README.md
Expand Up @@ -62,7 +62,8 @@ Then configure the rules you want to use under the rules section.
"promise/no-callback-in-promise": "warn",
"promise/avoid-new": "warn",
"promise/no-new-statics": "error",
"promise/no-return-in-finally": "warn"
"promise/no-return-in-finally": "warn",
"promise/valid-params": "warn"
}
}
```
Expand Down Expand Up @@ -90,6 +91,7 @@ or start with the recommended rule set
| [`avoid-new` ][avoid-new] | Avoid creating `new` promises outside of utility libs (use [pify][] instead) | :warning: | |
| [`no-new-statics`][no-new-statics] | Avoid calling `new` on a Promise static method | :bangbang: | |
| [`no-return-in-finally`][no-return-in-finally] | Disallow return statements in `finally()` | :warning: | |
| [`valid-params`][valid-params] | Ensures the proper number of arguments are passed to Promise functions | :warning: | |
| [`prefer-await-to-then`][prefer-await-to-then] | Prefer `await` to `then()` for reading Promise values | :seven: | |
| [`prefer-await-to-callbacks`][prefer-await-to-callbacks] | Prefer async/await to the callback pattern | :seven: | |

Expand Down Expand Up @@ -123,6 +125,7 @@ or start with the recommended rule set
[avoid-new]: docs/rules/avoid-new.md
[no-new-statics]: docs/rules/no-new-statics.md
[no-return-in-finally]: docs/rules/no-return-in-finally.md
[valid-params]: docs/rules/valid-params.md
[prefer-await-to-then]: docs/rules/prefer-await-to-then.md
[prefer-await-to-callbacks]: docs/rules/prefer-await-to-callbacks.md
[nodeify]: https://www.npmjs.com/package/nodeify
Expand Down
205 changes: 205 additions & 0 deletions __tests__/valid-params.js
@@ -0,0 +1,205 @@
'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' }
]
}
]
})
64 changes: 64 additions & 0 deletions docs/rules/valid-params.md
@@ -0,0 +1,64 @@
# Ensures the proper number of arguments are passed to Promise functions (valid-params)

Calling a Promise function with the incorrect number of arguments can lead to
unexpected behavior or hard to spot bugs.

## Rule Details

This rule is aimed at flagging instances where a Promise function is called with
the improper number of arguments.

Examples of **incorrect** code for this rule:

* `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

Examples of **correct** code for this rule:

```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)
```

## When Not To Use It

If you do not want to be notified when passing an invalid number of arguments to
a Promise function (for example, when using a typechecker like Flow), you can
safely disable this rule.
6 changes: 4 additions & 2 deletions index.js
Expand Up @@ -14,7 +14,8 @@ module.exports = {
'no-nesting': require('./rules/no-nesting'),
'avoid-new': require('./rules/avoid-new'),
'no-new-statics': require('./rules/no-new-statics'),
'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,
Expand All @@ -36,7 +37,8 @@ module.exports = {
'promise/no-callback-in-promise': 'warn',
'promise/avoid-new': 'warn',
'promise/no-new-statics': 'error',
'promise/no-return-in-finally': 'warn'
'promise/no-return-in-finally': 'warn',
'promise/valid-params': 'warn'
}
}
}
Expand Down
65 changes: 65 additions & 0 deletions rules/valid-params.js
@@ -0,0 +1,65 @@
'use strict'

const getDocsUrl = require('./lib/get-docs-url')
const isPromise = require('./lib/is-promise')

module.exports = {
meta: {
docs: {
description:
'Ensures the proper number of arguments are passed to Promise functions',
url: getDocsUrl('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 }}',
data: { name, numArgs }
})
}
break
case 'then':
if (numArgs < 1 || numArgs > 2) {
context.report({
node,
message:
'Promise.{{ name }}() requires 1 or 2 arguments, but received {{ numArgs }}',
data: { name, 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 }}',
data: { name, numArgs }
})
}
break
default:
break
}
}
}
}
}

0 comments on commit 4782ae5

Please sign in to comment.