Merge pull request palantir#106 from gscshoyru/hoisted-functions-unre…

…achable

Fixes palantir#100 -- unreachable rule no longer triggers for function declarations

CHANGELOG.md

@@ -1,6 +1,7 @@
 Change Log
 ===
 
+* [bug] hoisted functions no longer cause false positives for the `no-unreachable` rule
 * [bug] the rule loader no longer transforms/ignores the leading and trailing underscores and dashes of rule names in the config file
 * [feature] the `check-operator` option for the `whitespace` rule now checks whitespace around the => token
 * [bug] `export import` statements no longer false positives for `no-unused-variable-rule`

src/rules/noUnreachableRule.ts

@@ -33,41 +33,48 @@ class UnreachableWalker extends Lint.RuleWalker {
     }
 
     public visitNode(node: TypeScript.SyntaxNode): void {
+        var previousReturned = this.hasReturned;
+        // function declarations can be hoisted -- so set hasReturned to false until we're done with the function
+        if (node.kind() === TypeScript.SyntaxKind.FunctionDeclaration) {
+            this.hasReturned = false;
+        }
+
         if (this.hasReturned) {
             this.hasReturned = false;
             var position = this.position() + node.leadingTriviaWidth();
             this.addFailure(this.createFailure(position, node.width(), Rule.FAILURE_STRING));
         }
 
         super.visitNode(node);
+
+        // if there is further code after the hoisted function and we returned before that code is unreachable
+        // so reset hasReturned to its previous state to check for that
+        if (node.kind() === TypeScript.SyntaxKind.FunctionDeclaration) {
+            this.hasReturned = previousReturned;
+        }
     }
 
     public visitBlock(node: TypeScript.BlockSyntax): void {
-        this.hasReturned = false;
         super.visitBlock(node);
         this.hasReturned = false;
     }
 
     public visitCaseSwitchClause(node: TypeScript.CaseSwitchClauseSyntax): void {
-        this.hasReturned = false;
         super.visitCaseSwitchClause(node);
         this.hasReturned = false;
     }
 
     public visitDefaultSwitchClause(node: TypeScript.DefaultSwitchClauseSyntax): void {
-        this.hasReturned = false;
         super.visitDefaultSwitchClause(node);
         this.hasReturned = false;
     }
 
     public visitIfStatement(node: TypeScript.IfStatementSyntax): void {
-        this.hasReturned = false;
         super.visitIfStatement(node);
         this.hasReturned = false;
     }
 
     public visitElseClause(node: TypeScript.ElseClauseSyntax): void {
-        this.hasReturned = false;
         super.visitElseClause(node);
         this.hasReturned = false;
     }

test/files/rules/nounreachable.test.ts

@@ -66,3 +66,24 @@ function f5() {
     var y = 7;
 }
 
+function f6() {
+    hoisted();
+    return 0;
+
+    function hoisted() {
+        return 0;
+    }
+}
+
+// more invalid code
+
+function f7() {
+    hoisted();
+    return 0;
+
+    function hoisted() {
+        return 0;
+    }
+
+    var y = 7;
+}

test/rules/noUnreachableRuleTests.ts

@@ -25,7 +25,8 @@ describe("<no-unreachable>", () => {
             createFailure([6, 5], [6, 11]),
             createFailure([13, 9], [13, 15]),
             createFailure([25, 9], [25, 27]),
-            createFailure([28, 9], [28, 15])
+            createFailure([28, 9], [28, 15]),
+            createFailure([88, 5], [88, 15])
         ];
         var actualFailures = Lint.Test.applyRuleOnFile(fileName, NoUnreachableRule);