Fix: no-useless-return false positive on conditionals (fixes #7477) (…

…#7482)
eslint · Oct 31, 2016 · 8a71d4a · 8a71d4a
1 parent 56a662b
commit 8a71d4a
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 23 deletions.
diff --git a/lib/rules/no-useless-return.js b/lib/rules/no-useless-return.js
@@ -254,6 +254,9 @@ module.exports = {
 
             // Adds ReturnStatement node to check whether it's useless or not.
             ReturnStatement(node) {
+                if (node.argument) {
+                    markReturnStatementsOnCurrentSegmentsAsUsed();
+                }
                 if (node.argument || isInLoop(node) || isInFinally(node)) {
                     return;
                 }

diff --git a/tests/lib/rules/no-useless-return.js b/tests/lib/rules/no-useless-return.js
@@ -125,7 +125,30 @@ ruleTester.run("no-useless-return", rule, {
         {
             code: "if (foo) { return; } doSomething();",
             parserOptions: {ecmaFeatures: {globalReturn: true}}
-        }
+        },
+
+        // https://github.com/eslint/eslint/issues/7477
+        `
+          function foo() {
+            if (bar) return;
+            return baz;
+          }
+        `,
+        `
+          function foo() {
+            if (bar) {
+              return;
+            }
+            return baz;
+          }
+        `,
+        `
+          function foo() {
+            if (bar) baz();
+            else return;
+            return 5;
+          }
+        `
     ],
 
     invalid: [
@@ -155,6 +178,28 @@ ruleTester.run("no-useless-return", rule, {
             output: "if (foo) { bar();  } else { baz(); }",
             parserOptions: {ecmaFeatures: {globalReturn: true}}
         },
+        {
+            code: `
+              function foo() {
+                if (foo) {
+                  return;
+                }
+                return;
+              }
+            `,
+            output: `
+              function foo() {
+                if (foo) {
+                  
+                }
+                
+              }
+            `,
+            errors: [
+                {message: "Unnecessary return statement.", type: "ReturnStatement"},
+                {message: "Unnecessary return statement.", type: "ReturnStatement"},
+            ]
+        },
         {
             code: `
               function foo() {
@@ -305,28 +350,10 @@ ruleTester.run("no-useless-return", rule, {
               }
             `
         },
-        {
-            code: `
-              function foo() {
-                try {
-                  foo();
-                  return;
-                } catch (err) {
-                  return 5;
-                }
-              }
-            `,
-            output: `
-              function foo() {
-                try {
-                  foo();
-                  
-                } catch (err) {
-                  return 5;
-                }
-              }
-            `
-        },
+
+        // FIXME: Re-add this case (removed due to https://github.com/eslint/eslint/issues/7481):
+        // https://github.com/eslint/eslint/blob/261d7287820253408ec87c344beccdba2fe829a4/tests/lib/rules/no-useless-return.js#L308-L329
+
         {
             code: `
               function foo() {