Merge pull request #159 from mikegreiling/add-allow-finally

Add allowFinally option to catch-or-return rule

Author: xjamundx

__tests__/catch-or-return.js

@@ -89,6 +89,21 @@ ruleTester.run('catch-or-return', rule, {
       options: [{ allowThen: true }]
     },
 
+    // allowFinally - .finally(fn)
+    {
+      code: 'frank().then(go).catch(doIt).finally(fn)',
+      options: [{ allowFinally: true }]
+    },
+    {
+      code: 'frank().then(go).then().then().then().catch(doIt).finally(fn)',
+      options: [{ allowFinally: true }]
+    },
+    {
+      code:
+        'frank().then(go).then().catch(function() { /* why bother */ }).finally(fn)',
+      options: [{ allowFinally: true }]
+    },
+
     // terminationMethod=done - .done(null, fn)
     {
       code: 'frank().then(go).done()',
@@ -129,7 +144,28 @@ ruleTester.run('catch-or-return', rule, {
       errors: [{ message: catchMessage }]
     },
     {
-      code: 'frank.then(to).finally(fn)',
+      code: 'frank().then(to).catch(fn).then(foo)',
+      errors: [{ message: catchMessage }]
+    },
+    {
+      code: 'frank().finally(fn)',
+      errors: [{ message: catchMessage }]
+    },
+    {
+      code: 'frank().then(to).finally(fn)',
+      errors: [{ message: catchMessage }]
+    },
+    {
+      code: 'frank().then(go).catch(doIt).finally(fn)',
+      errors: [{ message: catchMessage }]
+    },
+    {
+      code: 'frank().then(go).then().then().then().catch(doIt).finally(fn)',
+      errors: [{ message: catchMessage }]
+    },
+    {
+      code:
+        'frank().then(go).then().catch(function() { /* why bother */ }).finally(fn)',
       errors: [{ message: catchMessage }]
     },
 
@@ -151,6 +187,18 @@ ruleTester.run('catch-or-return', rule, {
       errors: [{ message: catchMessage }]
     },
 
+    // allowFinally=true failures
+    {
+      code: 'frank().then(go).catch(doIt).finally(fn).then(foo)',
+      options: [{ allowFinally: true }],
+      errors: [{ message: catchMessage }]
+    },
+    {
+      code: 'frank().then(go).catch(doIt).finally(fn).foobar(foo)',
+      options: [{ allowFinally: true }],
+      errors: [{ message: catchMessage }]
+    },
+
     // terminationMethod=done - .done(null, fn)
     {
       code: 'frank().then(go)',
@@ -161,6 +209,12 @@ ruleTester.run('catch-or-return', rule, {
       code: 'frank().catch(go)',
       options: [{ terminationMethod: 'done' }],
       errors: [{ message: doneMessage }]
+    },
+
+    // assume somePromise.ANYTHING() is a new promise
+    {
+      code: 'frank().catch(go).someOtherMethod()',
+      errors: [{ message: catchMessage }]
     }
   ]
 })

docs/rules/catch-or-return.md

@@ -34,6 +34,13 @@ 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.
 
+##### `allowFinally`
+
+You can pass an `{ allowFinally: true }` as an option to this rule to allow for
+`.finally(fn)` to be used after `catch()` at the end of the promise chain. This
+is different from adding `'finally'` as a `terminationMethod` because it will
+still require the Promise chain to be "caught" beforehand.
+
 ##### `terminationMethod`
 
 You can pass a `{ terminationMethod: 'done' }` as an option to this rule to

package-lock.json

@@ -21,45 +21,66 @@ module.exports = {
   create(context) {
     const options = context.options[0] || {}
     const allowThen = options.allowThen
+    const allowFinally = options.allowFinally
     let terminationMethod = options.terminationMethod || 'catch'
 
     if (typeof terminationMethod === 'string') {
       terminationMethod = [terminationMethod]
     }
 
+    function isAllowedPromiseTermination(expression) {
+      // somePromise.then(a, b)
+      if (
+        allowThen &&
+        expression.type === 'CallExpression' &&
+        expression.callee.type === 'MemberExpression' &&
+        expression.callee.property.name === 'then' &&
+        expression.arguments.length === 2
+      ) {
+        return true
+      }
+
+      // somePromise.catch().finally(fn)
+      if (
+        allowFinally &&
+        expression.type === 'CallExpression' &&
+        expression.callee.type === 'MemberExpression' &&
+        expression.callee.property.name === 'finally' &&
+        isPromise(expression.callee.object) &&
+        isAllowedPromiseTermination(expression.callee.object)
+      ) {
+        return true
+      }
+
+      // somePromise.catch()
+      if (
+        expression.type === 'CallExpression' &&
+        expression.callee.type === 'MemberExpression' &&
+        terminationMethod.indexOf(expression.callee.property.name) !== -1
+      ) {
+        return true
+      }
+
+      // somePromise['catch']()
+      if (
+        expression.type === 'CallExpression' &&
+        expression.callee.type === 'MemberExpression' &&
+        expression.callee.property.type === 'Literal' &&
+        expression.callee.property.value === 'catch'
+      ) {
+        return true
+      }
+
+      return false
+    }
+
     return {
       ExpressionStatement(node) {
         if (!isPromise(node.expression)) {
           return
         }
 
-        // somePromise.then(a, b)
-        if (
-          allowThen &&
-          node.expression.type === 'CallExpression' &&
-          node.expression.callee.type === 'MemberExpression' &&
-          node.expression.callee.property.name === 'then' &&
-          node.expression.arguments.length === 2
-        ) {
-          return
-        }
-
-        // somePromise.catch()
-        if (
-          node.expression.type === 'CallExpression' &&
-          node.expression.callee.type === 'MemberExpression' &&
-          terminationMethod.indexOf(node.expression.callee.property.name) !== -1
-        ) {
-          return
-        }
-
-        // somePromise['catch']()
-        if (
-          node.expression.type === 'CallExpression' &&
-          node.expression.callee.type === 'MemberExpression' &&
-          node.expression.callee.property.type === 'Literal' &&
-          node.expression.callee.property.value === 'catch'
-        ) {
+        if (isAllowedPromiseTermination(node.expression)) {
           return
         }