no-else-after-return: use correct control flow analysis

Author: ajafff

package.json

@@ -36,7 +36,7 @@
   "dependencies": {
     "@fimbul/bifrost": "^0.6.0",
     "tslib": "^1.7.1",
-    "tsutils": "^2.22.2"
+    "tsutils": "^2.24.0"
   },
   "devDependencies": {
     "@fimbul/valtyr": "^0.6.0",
@@ -47,7 +47,7 @@
     "nyc": "^11.0.3",
     "rimraf": "^2.6.2",
     "tslint": "^5.8.0",
-    "typescript": "~2.7.1"
+    "typescript": "~2.8.1"
   },
   "peerDependencies": {
     "tslint": "^5.0.0",

rules/noElseAfterReturnRule.ts

@@ -1,20 +1,12 @@
 import * as ts from 'typescript';
 import * as Lint from 'tslint';
-import * as utils from 'tsutils';
 
 import { isElseIf } from '../src/utils';
 import { AbstractIfStatementWalker } from '../src/walker';
+import { getControlFlowEnd, isReturnStatement } from 'tsutils';
 
 const FAIL_MESSAGE = `unnecessary else after return`;
 
-// TODO respect break label in switch
-
-const enum StatementType {
-    None,
-    Break,
-    Return,
-}
-
 export class Rule extends Lint.Rules.AbstractRule {
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
         return this.applyWithWalker(new IfWalker(sourceFile, this.ruleName, undefined));
@@ -23,62 +15,16 @@ export class Rule extends Lint.Rules.AbstractRule {
 
 class IfWalker extends AbstractIfStatementWalker<void> {
     protected _checkIfStatement(node: ts.IfStatement) {
-        if (node.elseStatement !== undefined &&
+        if (
+            node.elseStatement !== undefined &&
             !isElseIf(node) &&
-            isLastStatementReturn(node.thenStatement))
+            endsWithReturnStatement(node.thenStatement)
+        )
             this.addFailureAtNode(node.getChildAt(5 /*else*/, this.sourceFile), FAIL_MESSAGE);
     }
 }
 
-function isLastStatementReturn(statement: ts.Statement | ts.BlockLike | ts.DefaultClause): boolean {
-    return endsControlFlow(statement) === StatementType.Return;
-}
-
-function endsControlFlow(statement: ts.Statement | ts.BlockLike | ts.DefaultClause): StatementType {
-    // recurse into nested blocks
-    while (utils.isBlockLike(statement)) {
-        if (statement.statements.length === 0)
-            return StatementType.None;
-
-        statement = statement.statements[statement.statements.length - 1];
-    }
-
-    return isDefinitelyReturned(statement);
-}
-
-function isDefinitelyReturned(statement: ts.Statement): StatementType {
-    if (statement.kind === ts.SyntaxKind.ReturnStatement)
-        return StatementType.Return;
-    if (statement.kind === ts.SyntaxKind.BreakStatement)
-        return StatementType.Break;
-
-    if (utils.isIfStatement(statement)) {
-        if (statement.elseStatement === undefined)
-            return StatementType.None;
-        const then = endsControlFlow(statement.thenStatement);
-        if (!then)
-            return then;
-        return Math.min(
-            then,
-            endsControlFlow(statement.elseStatement),
-        );
-    }
-
-    if (utils.isSwitchStatement(statement)) {
-        let hasDefault = false;
-        let fallthrough = false;
-        for (const clause of statement.caseBlock.clauses) {
-            const retVal = endsControlFlow(clause);
-            if (retVal === StatementType.None) {
-                fallthrough = true;
-            } else if (retVal === StatementType.Break) {
-                return StatementType.None;
-            } else {
-                fallthrough = false;
-            }
-            hasDefault = hasDefault || clause.kind === ts.SyntaxKind.DefaultClause;
-        }
-        return !fallthrough && hasDefault ? StatementType.Return : StatementType.None;
-    }
-    return StatementType.None;
+function endsWithReturnStatement(node: ts.Statement): boolean {
+    const end = getControlFlowEnd(node);
+    return end.end && end.statements.every(isReturnStatement);
 }

test/rules/no-else-after-return/default/test.ts.lint

@@ -1,21 +1,37 @@
-if (true) {
+if (condition) {
     return;
 } else {}
   ~~~~ [fail]
 
-if (true) return;
+if (condition) {
+    while (true)
+        return;
+} else {}
+  ~~~~ [fail]
+
+if (condition) {
+    while (false)
+        return;
+} else {}
+
+if (condition) {
+    while (condition)
+        return;
+} else {}
+
+if (condition) return;
 else ;
 ~~~~ [fail]
 
-if (true) {
+if (condition) {
     {
         return;
     }
 } else {}
   ~~~~ [fail]
 
-if (true) {
-    if (true) {
+if (condition) {
+    if (confition) {
         return;
     } else {
       ~~~~ [fail]
@@ -24,46 +40,46 @@ if (true) {
 } else {}
   ~~~~ [fail]
 
-if (true) {
-    if (true) {
+if (condition) {
+    if (condition) {
         return;
     }
 } else {}
 
-if (true) return;
+if (condition) return;
 
-if (true) foo();
+if (condition) foo();
 else bar();
 
-if (true) {
+if (condition) {
     return;
 }
 
-if (true) {
+if (condition) {
     foo();
 } else {
     bar();
 }
 
-if (true) {
+if (condition) {
     {
     }
 } else {}
 
-if (true) {
-    if (true) {
+if (condition) {
+    if (condition) {
         return;
     } else {
       ~~~~ [fail]
     }
 } else {}
 
-if (true) {
-    if (true) {
+if (condition) {
+    if (condition) {
     }
 } else {}
 
-if (true) {
+if (condition) {
     switch (foo) {
         case 'a':
             return;
@@ -75,7 +91,7 @@ if (true) {
 } else {}
   ~~~~ [fail]
 
-if (true) {
+if (condition) {
     switch (foo) {
         default:
             return;
@@ -85,7 +101,7 @@ if (true) {
 } else {}
   ~~~~ [fail]
 
-if (true) {
+if (condition) {
     switch (foo) {
         case 'a':
             return;
@@ -94,7 +110,7 @@ if (true) {
     }
 } else {}
 
-if (true) {
+if (condition) {
     switch (foo) {
         case 'a':
             return;
@@ -105,7 +121,7 @@ if (true) {
 } else {}
   ~~~~ [fail]
 
-if (true) {
+if (condition) {
     switch (foo) {
         case 'a':
             return;
@@ -118,7 +134,7 @@ if (true) {
 } else {}
   ~~~~ [fail]
 
-if (true) {
+if (condition) {
     switch (foo) {
         case 'a':
             return;
@@ -128,18 +144,67 @@ if (true) {
     }
 } else {}
 
-if (true) {
-}else if (true) {
+if (condition) {
+}else if (condition) {
     return;
 } else {}
 
-if (true) {
+if (condition) {
 }else{
-    if (true) {
+    if (condition) {
         return;
     } else {}
       ~~~~ [fail]
 }
 
+if (condition) {
+    throw null;
+} else {}
+
+if (condition) {
+    if (condition)
+        return;
+    else
+    ~~~~ [fail]
+        return;
+} else {}
+  ~~~~ [fail]
+
+switch (Boolean()) {
+    case true:
+        if (condition) {
+            if (condition)
+                break;
+            return;
+        } else {}
+        break;
+    case false:
+        if (condition) {
+            if (condition)
+                return;
+            return;
+        } else {}
+          ~~~~ [fail]
+}
+
+outer: for (;;) {
+    if (condition) {
+        switch (Boolean()) {
+            case true:
+                break;
+        }
+        return;
+    } else {}
+      ~~~~ [fail]
+
+    if (condition) {
+        switch (Boolean()) {
+            case true:
+                break outer;
+        }
+        return;
+    } else {}
+}
+
 
-[fail]: unnecessary else after return
+[fail]: unnecessary else after return