From 53be970e91023a104ce3ef2918b3ee80ef265f27 Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Wed, 16 Oct 2024 20:27:57 +0800 Subject: [PATCH] feat(catch-or-return): add `allowThenStrict` option (#522) --- __tests__/catch-or-return.js | 113 ++++++++++++++++++++++++++++++++-- docs/rules/catch-or-return.md | 53 +++++++++++++++- rules/catch-or-return.js | 12 +++- 3 files changed, 167 insertions(+), 11 deletions(-) diff --git a/__tests__/catch-or-return.js b/__tests__/catch-or-return.js index 324f1482..03b1fa3e 100644 --- a/__tests__/catch-or-return.js +++ b/__tests__/catch-or-return.js @@ -53,16 +53,10 @@ ruleTester.run('catch-or-return', rule, { options: [{ allowThen: true }], }, - // allowThen - .then(null, fn) - { code: 'frank().then(a, b)', options: [{ allowThen: true }] }, { code: 'frank().then(a).then(b).then(null, c)', options: [{ allowThen: true }], }, - { - code: 'frank().then(a).then(b).then(c, d)', - options: [{ allowThen: true }], - }, { code: 'frank().then(a).then(b).then().then().then(null, doIt)', options: [{ allowThen: true }], @@ -73,10 +67,15 @@ ruleTester.run('catch-or-return', rule, { }, // allowThen - .then(fn, fn) + { code: 'frank().then(a, b)', options: [{ allowThen: true }] }, { code: 'frank().then(go).then(zam, doIt)', options: [{ allowThen: true }], }, + { + code: 'frank().then(a).then(b).then(c, d)', + options: [{ allowThen: true }], + }, { code: 'frank().then(go).then().then().then().then(wham, doIt)', options: [{ allowThen: true }], @@ -90,6 +89,37 @@ ruleTester.run('catch-or-return', rule, { options: [{ allowThen: true }], }, + // allowThenStrict - .then(null, fn) + { + code: 'frank().then(go).then(null, doIt)', + options: [{ allowThenStrict: true }], + }, + { + code: 'frank().then(go).then().then().then().then(null, doIt)', + options: [{ allowThenStrict: true }], + }, + { + code: 'frank().then(go).then().then(null, function() { /* why bother */ })', + options: [{ allowThenStrict: true }], + }, + { + code: 'frank.then(go).then(to).then(null, jail)', + options: [{ allowThenStrict: true }], + }, + + { + code: 'frank().then(a).then(b).then(null, c)', + options: [{ allowThenStrict: true }], + }, + { + code: 'frank().then(a).then(b).then().then().then(null, doIt)', + options: [{ allowThenStrict: true }], + }, + { + code: 'frank().then(a).then(b).then(null, function() { /* why bother */ })', + options: [{ allowThenStrict: true }], + }, + // allowFinally - .finally(fn) { code: 'frank().then(go).catch(doIt).finally(fn)', @@ -234,5 +264,76 @@ ruleTester.run('catch-or-return', rule, { code: 'frank().catch(go).someOtherMethod()', errors: [{ message: catchMessage }], }, + + // .then(null, fn) + { + code: 'frank().then(a).then(b).then(null, c)', + errors: [{ message: catchMessage }], + }, + { + code: 'frank().then(a).then(b).then().then().then(null, doIt)', + errors: [{ message: catchMessage }], + }, + { + code: 'frank().then(a).then(b).then(null, function() { /* why bother */ })', + errors: [{ message: catchMessage }], + }, + + // .then(fn, fn) + { + code: 'frank().then(a, b)', + errors: [{ message: catchMessage }], + }, + { + code: 'frank().then(go).then(zam, doIt)', + errors: [{ message: catchMessage }], + }, + { + code: 'frank().then(a).then(b).then(c, d)', + errors: [{ message: catchMessage }], + }, + { + code: 'frank().then(go).then().then().then().then(wham, doIt)', + errors: [{ message: catchMessage }], + }, + { + code: 'frank().then(go).then().then(function() {}, function() { /* why bother */ })', + errors: [{ message: catchMessage }], + }, + { + code: 'frank.then(go).then(to).then(pewPew, jail)', + errors: [{ message: catchMessage }], + }, + + { + code: 'frank().then(a, b)', + errors: [{ message: catchMessage }], + options: [{ allowThenStrict: true }], + }, + { + code: 'frank().then(go).then(zam, doIt)', + errors: [{ message: catchMessage }], + options: [{ allowThenStrict: true }], + }, + { + code: 'frank().then(a).then(b).then(c, d)', + errors: [{ message: catchMessage }], + options: [{ allowThenStrict: true }], + }, + { + code: 'frank().then(go).then().then().then().then(wham, doIt)', + errors: [{ message: catchMessage }], + options: [{ allowThenStrict: true }], + }, + { + code: 'frank().then(go).then().then(function() {}, function() { /* why bother */ })', + errors: [{ message: catchMessage }], + options: [{ allowThenStrict: true }], + }, + { + code: 'frank.then(go).then(to).then(pewPew, jail)', + errors: [{ message: catchMessage }], + options: [{ allowThenStrict: true }], + }, ], }) diff --git a/docs/rules/catch-or-return.md b/docs/rules/catch-or-return.md index 1833ca57..2077d046 100644 --- a/docs/rules/catch-or-return.md +++ b/docs/rules/catch-or-return.md @@ -32,9 +32,56 @@ function doSomethingElse() { ##### `allowThen` -You can pass an `{ allowThen: true }` as an option to this rule to allow for -`.then(null, fn)` to be used instead of `catch()` at the end of the promise -chain. +The second argument to `then()` can also be used to handle a promise rejection, +but it won't catch any errors from the first argument callback. Because of this, +this rule reports usage of `then()` with two arguments without `catch()` by +default. + +However, you can use `{ allowThen: true }` to allow using `then()` with two +arguments instead of `catch()` to handle promise rejections. + +Examples of **incorrect** code for the default `{ allowThen: false }` option: + +```js +myPromise.then(doSomething, handleErrors) +myPromise.then(null, handleErrors) +``` + +Examples of **correct** code for the `{ allowThen: true }` option: + +```js +myPromise.then(doSomething, handleErrors) +myPromise.then(null, handleErrors) +myPromise.then(doSomething).catch(handleErrors) +``` + +##### `allowThenStrict` + +`allowThenStrict` is similar to `allowThen` but it only permits `then` when the +fulfillment handler is `null`. This option ensures that the final rejected +handler works like a `catch` and can handle any uncaught errors from the final +`then`. + +Examples of **incorrect** code for the default `{ allowThenStrict: false }` +option: + +```js +myPromise.then(doSomething, handleErrors) +myPromise.then(null, handleErrors) +``` + +Examples of **correct** code for the `{ allowThenStrict: true }` option: + +```js +myPromise.then(null, handleErrors) +myPromise.then(doSomething).catch(handleErrors) +``` + +Examples of **incorrect** code for the `{ allowThenStrict: true }` option: + +```js +myPromise.then(doSomething, handleErrors) +``` ##### `allowFinally` diff --git a/rules/catch-or-return.js b/rules/catch-or-return.js index c12b82fc..1baae113 100644 --- a/rules/catch-or-return.js +++ b/rules/catch-or-return.js @@ -30,6 +30,9 @@ module.exports = { allowThen: { type: 'boolean', }, + allowThenStrict: { + type: 'boolean', + }, terminationMethod: { oneOf: [ { type: 'string' }, @@ -49,6 +52,7 @@ module.exports = { create(context) { const options = context.options[0] || {} const allowThen = options.allowThen + const allowThenStrict = options.allowThenStrict const allowFinally = options.allowFinally let terminationMethod = options.terminationMethod || 'catch' @@ -59,11 +63,15 @@ module.exports = { function isAllowedPromiseTermination(expression) { // somePromise.then(a, b) if ( - allowThen && + (allowThen || allowThenStrict) && expression.type === 'CallExpression' && expression.callee.type === 'MemberExpression' && expression.callee.property.name === 'then' && - expression.arguments.length === 2 + expression.arguments.length === 2 && + // somePromise.then(null, b) + ((allowThen && !allowThenStrict) || + (expression.arguments[0].type === 'Literal' && + expression.arguments[0].value === null)) ) { return true }