fix(require-returns-check): allow implicit-return finally if other try-catch branches returning; fixes #926

Also:
- fix(`require-returns-check`): finally-only returns to be treated as returns

Author: brettz9

README.md

@@ -17568,18 +17568,6 @@ function quux () {
 }
 // Message: JSDoc @returns declaration present but return expression not available in function.
 
-/**
- * @returns {true}
- */
-function quux () {
-  try {
-  } catch (error) {
-  } finally {
-    return true;
-  }
-}
-// Message: JSDoc @returns declaration present but return expression not available in function.
-
 /**
  * @returns {true}
  */
@@ -18130,6 +18118,29 @@ function parseSipHeaders(logPrefix, sipMessage, headers) {
         return {};
     }
 }
+
+/**
+ * @returns {true}
+ */
+function quux () {
+  try {
+  } catch (error) {
+  } finally {
+    return true;
+  }
+}
+
+/** Returns true.
+ *
+ * @returns {boolean} true
+ */
+function getTrue() {
+  try {
+    return true;
+  } finally {
+    console.log('returning...');
+  }
+}
 ````
 
 

src/utils/hasReturnValue.js

@@ -162,11 +162,28 @@ const allBrancheshaveReturnValues = (node, promFilter) => {
   }
 
   case 'TryStatement': {
-    return allBrancheshaveReturnValues(node.block, promFilter) &&
-      (!node.handler ||
-        allBrancheshaveReturnValues(node.handler && node.handler.body, promFilter)) &&
-      (!node.finalizer ||
-        allBrancheshaveReturnValues(node.finalizer, promFilter));
+    // If `finally` returns, all return
+    return node.finalizer && allBrancheshaveReturnValues(node.finalizer, promFilter) ||
+      // Return in `try`/`catch` may still occur despite `finally`
+      allBrancheshaveReturnValues(node.block, promFilter) &&
+        (!node.handler ||
+          allBrancheshaveReturnValues(node.handler && node.handler.body, promFilter)) &&
+          (!node.finalizer || (() => {
+            try {
+              hasReturnValue(node.finalizer, true, promFilter);
+            } catch (error) {
+              // istanbul ignore else
+              if (error.message === 'Null return') {
+                return false;
+              }
+
+              // istanbul ignore next
+              throw error;
+            }
+
+            // As long as not an explicit empty return, then return true
+            return true;
+          })());
   }
 
   case 'SwitchStatement': {

test/rules/assertions/requireReturnsCheck.js

@@ -569,26 +569,6 @@ export default {
         },
       ],
     },
-    {
-      code: `
-          /**
-           * @returns {true}
-           */
-          function quux () {
-            try {
-            } catch (error) {
-            } finally {
-              return true;
-            }
-          }
-      `,
-      errors: [
-        {
-          line: 2,
-          message: 'JSDoc @returns declaration present but return expression not available in function.',
-        },
-      ],
-    },
     {
       code: `
           /**
@@ -1417,5 +1397,34 @@ export default {
       }
       `,
     },
+    {
+      code: `
+          /**
+           * @returns {true}
+           */
+          function quux () {
+            try {
+            } catch (error) {
+            } finally {
+              return true;
+            }
+          }
+      `,
+    },
+    {
+      code: `
+      /** Returns true.
+       *
+       * @returns {boolean} true
+       */
+      function getTrue() {
+        try {
+          return true;
+        } finally {
+          console.log('returning...');
+        }
+      }
+      `,
+    },
   ],
 };