Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rule): add valid-params rule #85

Merged
merged 2 commits into from Mar 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
}
}
}
}
}