fix(require-returns-check): check child nodes of consequents; fixes #923

Author: brettz9

README.md

@@ -17572,6 +17572,34 @@ function quux () {
   throw new Error('def');
 }
 // Message: JSDoc @returns declaration present but return expression not available in function.
+
+/**
+ * @returns Baz.
+ */
+function foo() {
+    switch (true) {
+        default:
+            switch (false) {
+                default: return;
+            }
+            return "baz";
+    }
+};
+// Message: JSDoc @returns declaration present but return expression not available in function.
+
+/**
+ * @returns Baz.
+ */
+function foo() {
+    switch (true) {
+        default:
+            switch (false) {
+                default: return;
+            }
+            return "baz";
+    }
+};
+// Message: JSDoc @returns declaration present but return expression not available in function.
 ````
 
 The following patterns are not considered problems:
@@ -18034,6 +18062,19 @@ function quux () {
 
   throw new Error('Fail');
 }
+
+/**
+ * @returns Baz.
+ */
+function foo() {
+    switch (true) {
+        default:
+            switch (false) {
+                default: break;
+            }
+            return "baz";
+    }
+};
 ````
 
 

src/utils/hasReturnValue.js

@@ -23,11 +23,12 @@ const undefinedKeywords = new Set([
  * Checks if a node has a return statement. Void return does not count.
  *
  * @param {object} node
+ * @param {boolean} throwOnNullReturn
  * @param {PromiseFilter} promFilter
  * @returns {boolean|Node}
  */
 // eslint-disable-next-line complexity
-const allBrancheshaveReturnValues = (node, promFilter) => {
+const hasReturnValue = (node, throwOnNullReturn, promFilter) => {
   if (!node) {
     return false;
   }
@@ -40,18 +41,19 @@ const allBrancheshaveReturnValues = (node, promFilter) => {
     return type && !undefinedKeywords.has(type);
   }
 
-  // case 'MethodDefinition':
-  //   return allBrancheshaveReturnValues(node.value, promFilter);
+  case 'MethodDefinition':
+    return hasReturnValue(node.value, throwOnNullReturn, promFilter);
   case 'FunctionExpression':
   case 'FunctionDeclaration':
   case 'ArrowFunctionExpression': {
     return node.expression && (!isNewPromiseExpression(node.body) || !isVoidPromise(node.body)) ||
-      allBrancheshaveReturnValues(node.body, promFilter);
+      hasReturnValue(node.body, throwOnNullReturn, promFilter);
   }
 
   case 'BlockStatement': {
-    const lastBodyNode = node.body.slice(-1)[0];
-    return allBrancheshaveReturnValues(lastBodyNode, promFilter);
+    return node.body.some((bodyNode) => {
+      return bodyNode.type !== 'FunctionDeclaration' && hasReturnValue(bodyNode, throwOnNullReturn, promFilter);
+    });
   }
 
   case 'LabeledStatement':
@@ -61,36 +63,37 @@ const allBrancheshaveReturnValues = (node, promFilter) => {
   case 'ForInStatement':
   case 'ForOfStatement':
   case 'WithStatement': {
-    return allBrancheshaveReturnValues(node.body, promFilter);
+    return hasReturnValue(node.body, throwOnNullReturn, promFilter);
   }
 
   case 'IfStatement': {
-    return allBrancheshaveReturnValues(node.consequent, promFilter) && allBrancheshaveReturnValues(node.alternate, promFilter);
+    return hasReturnValue(node.consequent, throwOnNullReturn, promFilter) ||
+      hasReturnValue(node.alternate, throwOnNullReturn, promFilter);
   }
 
   case 'TryStatement': {
-    return allBrancheshaveReturnValues(node.block, promFilter) &&
-      allBrancheshaveReturnValues(node.handler && node.handler.body, promFilter) &&
-      allBrancheshaveReturnValues(node.finalizer, promFilter);
+    return hasReturnValue(node.block, throwOnNullReturn, promFilter) ||
+      hasReturnValue(node.handler && node.handler.body, throwOnNullReturn, promFilter) ||
+      hasReturnValue(node.finalizer, throwOnNullReturn, promFilter);
   }
 
   case 'SwitchStatement': {
-    return node.cases.every(
+    return node.cases.some(
       (someCase) => {
-        return someCase.consequent.every((nde) => {
-          return allBrancheshaveReturnValues(nde, promFilter);
+        return someCase.consequent.some((nde) => {
+          return hasReturnValue(nde, throwOnNullReturn, promFilter);
         });
       },
     );
   }
 
-  case 'ThrowStatement': {
-    return true;
-  }
-
   case 'ReturnStatement': {
     // void return does not count.
     if (node.argument === null) {
+      if (throwOnNullReturn) {
+        throw new Error('Null return');
+      }
+
       return false;
     }
 
@@ -109,22 +112,15 @@ const allBrancheshaveReturnValues = (node, promFilter) => {
   }
 };
 
-/**
- * @callback PromiseFilter
- * @param {object} node
- * @returns {boolean}
- */
-
 /**
  * Checks if a node has a return statement. Void return does not count.
  *
  * @param {object} node
- * @param {boolean} throwOnNullReturn
  * @param {PromiseFilter} promFilter
  * @returns {boolean|Node}
  */
 // eslint-disable-next-line complexity
-const hasReturnValue = (node, throwOnNullReturn, promFilter) => {
+const allBrancheshaveReturnValues = (node, promFilter) => {
   if (!node) {
     return false;
   }
@@ -137,19 +133,18 @@ const hasReturnValue = (node, throwOnNullReturn, promFilter) => {
     return type && !undefinedKeywords.has(type);
   }
 
-  case 'MethodDefinition':
-    return hasReturnValue(node.value, throwOnNullReturn, promFilter);
+  // case 'MethodDefinition':
+  //   return allBrancheshaveReturnValues(node.value, promFilter);
   case 'FunctionExpression':
   case 'FunctionDeclaration':
   case 'ArrowFunctionExpression': {
     return node.expression && (!isNewPromiseExpression(node.body) || !isVoidPromise(node.body)) ||
-      hasReturnValue(node.body, throwOnNullReturn, promFilter);
+      allBrancheshaveReturnValues(node.body, promFilter);
   }
 
   case 'BlockStatement': {
-    return node.body.some((bodyNode) => {
-      return bodyNode.type !== 'FunctionDeclaration' && hasReturnValue(bodyNode, throwOnNullReturn, promFilter);
-    });
+    const lastBodyNode = node.body.slice(-1)[0];
+    return allBrancheshaveReturnValues(lastBodyNode, promFilter);
   }
 
   case 'LabeledStatement':
@@ -159,37 +154,35 @@ const hasReturnValue = (node, throwOnNullReturn, promFilter) => {
   case 'ForInStatement':
   case 'ForOfStatement':
   case 'WithStatement': {
-    return hasReturnValue(node.body, throwOnNullReturn, promFilter);
+    return allBrancheshaveReturnValues(node.body, promFilter);
   }
 
   case 'IfStatement': {
-    return hasReturnValue(node.consequent, throwOnNullReturn, promFilter) ||
-      hasReturnValue(node.alternate, throwOnNullReturn, promFilter);
+    return allBrancheshaveReturnValues(node.consequent, promFilter) && allBrancheshaveReturnValues(node.alternate, promFilter);
   }
 
   case 'TryStatement': {
-    return hasReturnValue(node.block, throwOnNullReturn, promFilter) ||
-      hasReturnValue(node.handler && node.handler.body, throwOnNullReturn, promFilter) ||
-      hasReturnValue(node.finalizer, throwOnNullReturn, promFilter);
+    return allBrancheshaveReturnValues(node.block, promFilter) &&
+      allBrancheshaveReturnValues(node.handler && node.handler.body, promFilter) &&
+      allBrancheshaveReturnValues(node.finalizer, promFilter);
   }
 
   case 'SwitchStatement': {
-    return node.cases.some(
+    return node.cases.every(
       (someCase) => {
-        return someCase.consequent.some((nde) => {
-          return hasReturnValue(nde, throwOnNullReturn, promFilter);
-        });
+        const nde = someCase.consequent.slice(-1)[0];
+        return allBrancheshaveReturnValues(nde, promFilter);
       },
     );
   }
 
+  case 'ThrowStatement': {
+    return true;
+  }
+
   case 'ReturnStatement': {
     // void return does not count.
     if (node.argument === null) {
-      if (throwOnNullReturn) {
-        throw new Error('Null return');
-      }
-
       return false;
     }
 
@@ -208,6 +201,12 @@ const hasReturnValue = (node, throwOnNullReturn, promFilter) => {
   }
 };
 
+/**
+ * @callback PromiseFilter
+ * @param {object} node
+ * @returns {boolean}
+ */
+
 /**
  * Avoids further checking child nodes if a nested function shadows the
  * resolver, but otherwise, if name is used (by call or passed in as an

test/rules/assertions/requireReturnsCheck.js

@@ -630,6 +630,50 @@ export default {
       ],
       ignoreReadme: true,
     },
+    {
+      code: `
+      /**
+       * @returns Baz.
+       */
+      function foo() {
+          switch (true) {
+              default:
+                  switch (false) {
+                      default: return;
+                  }
+                  return "baz";
+          }
+      };
+      `,
+      errors: [
+        {
+          line: 2,
+          message: 'JSDoc @returns declaration present but return expression not available in function.',
+        },
+      ],
+    },
+    {
+      code: `
+      /**
+       * @returns Baz.
+       */
+      function foo() {
+          switch (true) {
+              default:
+                  switch (false) {
+                      default: return;
+                  }
+                  return "baz";
+          }
+      };
+      `,
+      errors: [
+        {
+          line: 2,
+          message: 'JSDoc @returns declaration present but return expression not available in function.',
+        },
+      ],
+    },
   ],
   valid: [
     {
@@ -1315,5 +1359,21 @@ export default {
           }
       `,
     },
+    {
+      code: `
+      /**
+       * @returns Baz.
+       */
+      function foo() {
+          switch (true) {
+              default:
+                  switch (false) {
+                      default: break;
+                  }
+                  return "baz";
+          }
+      };
+      `,
+    },
   ],
 };