Merge pull request #34212 from microsoft/fix34021

ahejlsberg · web-flow · commit a685ac426c16 · 2019-10-15T12:10:53.000-07:00
Fix control flow analysis for --noFallthroughCasesInSwitch
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -1341,7 +1341,7 @@ namespace ts {
                 bind(clause);
                 fallthroughFlow = currentFlow;
                 if (!(currentFlow.flags & FlowFlags.Unreachable) && i !== clauses.length - 1 && options.noFallthroughCasesInSwitch) {
-                    errorOnFirstToken(clause, Diagnostics.Fallthrough_case_in_switch);
+                    clause.fallthroughFlowNode = currentFlow;
                 }
             }
             clauses.transformFlags = subtreeTransformFlags | TransformFlags.HasComputedFlags;
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -31054,10 +31054,7 @@ namespace ts {
                         firstDefaultClause = clause;
                     }
                     else {
-                        const sourceFile = getSourceFileOfNode(node);
-                        const start = skipTrivia(sourceFile.text, clause.pos);
-                        const end = clause.statements.length > 0 ? clause.statements[0].pos : clause.end;
-                        grammarErrorAtPos(sourceFile, start, end - start, Diagnostics.A_default_clause_cannot_appear_more_than_once_in_a_switch_statement);
+                        grammarErrorOnNode(clause, Diagnostics.A_default_clause_cannot_appear_more_than_once_in_a_switch_statement);
                         hasDuplicateDefaultClause = true;
                     }
                 }
@@ -31079,6 +31076,9 @@ namespace ts {
                     }
                 }
                 forEach(clause.statements, checkSourceElement);
+                if (compilerOptions.noFallthroughCasesInSwitch && clause.fallthroughFlowNode && isReachableFlowNode(clause.fallthroughFlowNode)) {
+                    error(clause, Diagnostics.Fallthrough_case_in_switch);
+                }
             });
             if (node.caseBlock.locals) {
                 registerForUnusedIdentifiersCheck(node.caseBlock);
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -2250,12 +2250,14 @@ namespace ts {
         parent: CaseBlock;
         expression: Expression;
         statements: NodeArray<Statement>;
+        /* @internal */ fallthroughFlowNode?: FlowNode;
     }
 
     export interface DefaultClause extends Node {
         kind: SyntaxKind.DefaultClause;
         parent: CaseBlock;
         statements: NodeArray<Statement>;
+        /* @internal */ fallthroughFlowNode?: FlowNode;
     }
 
     export type CaseOrDefaultClause = CaseClause | DefaultClause;
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -961,6 +961,11 @@ namespace ts {
                 break;
             case SyntaxKind.ArrowFunction:
                 return getErrorSpanForArrowFunction(sourceFile, <ArrowFunction>node);
+            case SyntaxKind.CaseClause:
+            case SyntaxKind.DefaultClause:
+                const start = skipTrivia(sourceFile.text, (<CaseOrDefaultClause>node).pos);
+                const end = (<CaseOrDefaultClause>node).statements.length > 0 ? (<CaseOrDefaultClause>node).statements[0].pos : (<CaseOrDefaultClause>node).end;
+                return createTextSpanFromBounds(start, end);
         }
 
         if (errorNode === undefined) {
diff --git a/tests/baselines/reference/fallFromLastCase2.errors.txt b/tests/baselines/reference/fallFromLastCase2.errors.txt
@@ -11,7 +11,7 @@ tests/cases/compiler/fallFromLastCase2.ts(21,9): error TS7029: Fallthrough case
                 use("1");
                 break;
             case 2:
-            ~~~~
+            ~~~~~~~
 !!! error TS7029: Fallthrough case in switch.
                 use("2");
             case 3:
@@ -26,7 +26,7 @@ tests/cases/compiler/fallFromLastCase2.ts(21,9): error TS7029: Fallthrough case
                 use("1");
                 break;
             default:
-            ~~~~~~~
+            ~~~~~~~~
 !!! error TS7029: Fallthrough case in switch.
                 use("2");
             case 2:
diff --git a/tests/baselines/reference/jsFileCompilationBindReachabilityErrors.errors.txt b/tests/baselines/reference/jsFileCompilationBindReachabilityErrors.errors.txt
@@ -7,7 +7,7 @@ tests/cases/compiler/a.js(19,1): error TS7028: Unused label.
     function foo(a, b) {
         switch (a) {
             case 10:
-            ~~~~
+            ~~~~~~~~
 !!! error TS7029: Fallthrough case in switch.
                 if (b) {
                     return b;
diff --git a/tests/baselines/reference/reachabilityChecks4.errors.txt b/tests/baselines/reference/reachabilityChecks4.errors.txt
@@ -1,19 +1,74 @@
 tests/cases/compiler/reachabilityChecks4.ts(6,9): error TS7029: Fallthrough case in switch.
+tests/cases/compiler/reachabilityChecks4.ts(22,9): error TS7029: Fallthrough case in switch.
 
 
-==== tests/cases/compiler/reachabilityChecks4.ts (1 errors) ====
+==== tests/cases/compiler/reachabilityChecks4.ts (2 errors) ====
     function foo(x, y) {
         switch (x) {
             case 1:
             case 2:
                 return 1;
             case 3:
-            ~~~~
+            ~~~~~~~
 !!! error TS7029: Fallthrough case in switch.
                 if (y) {
                     return 2;
                 }
             case 4:
                 return 3;
         }
-    }
+    }
+    
+    declare function noop(): void;
+    declare function fail(): never;
+    
+    function f1(x: 0 | 1 | 2) {
+        switch (x) {
+            case 0:
+                fail();
+            case 1:
+            ~~~~~~~
+!!! error TS7029: Fallthrough case in switch.
+                noop();
+            case 2:
+                return;
+        }
+    }
+    
+    // Repro from #34021
+    
+    type Behavior = 'SLIDE' | 'SLIDE_OUT'
+    type Direction = 'LEFT' | 'RIGHT' | 'TOP' | 'BOTTOM'
+    
+    interface Transition {
+      behavior: Behavior
+      direction: Direction
+    }
+    
+    function f2(transition: Transition): any {
+        switch (transition.behavior) {
+            case 'SLIDE':
+                switch (transition.direction) {
+                    case 'LEFT':
+                        return []
+                    case 'RIGHT':
+                        return []
+                    case 'TOP':
+                        return []
+                    case 'BOTTOM':
+                        return []
+                }
+            case 'SLIDE_OUT':
+                switch (transition.direction) {
+                    case 'LEFT':
+                        return []
+                    case 'RIGHT':
+                        return []
+                    case 'TOP':
+                        return []
+                    case 'BOTTOM':
+                        return []
+                }
+        }
+    }
+    
diff --git a/tests/baselines/reference/reachabilityChecks4.js b/tests/baselines/reference/reachabilityChecks4.js
@@ -11,7 +11,59 @@ function foo(x, y) {
         case 4:
             return 3;
     }
-}
+}
+
+declare function noop(): void;
+declare function fail(): never;
+
+function f1(x: 0 | 1 | 2) {
+    switch (x) {
+        case 0:
+            fail();
+        case 1:
+            noop();
+        case 2:
+            return;
+    }
+}
+
+// Repro from #34021
+
+type Behavior = 'SLIDE' | 'SLIDE_OUT'
+type Direction = 'LEFT' | 'RIGHT' | 'TOP' | 'BOTTOM'
+
+interface Transition {
+  behavior: Behavior
+  direction: Direction
+}
+
+function f2(transition: Transition): any {
+    switch (transition.behavior) {
+        case 'SLIDE':
+            switch (transition.direction) {
+                case 'LEFT':
+                    return []
+                case 'RIGHT':
+                    return []
+                case 'TOP':
+                    return []
+                case 'BOTTOM':
+                    return []
+            }
+        case 'SLIDE_OUT':
+            switch (transition.direction) {
+                case 'LEFT':
+                    return []
+                case 'RIGHT':
+                    return []
+                case 'TOP':
+                    return []
+                case 'BOTTOM':
+                    return []
+            }
+    }
+}
+
 
 //// [reachabilityChecks4.js]
 function foo(x, y) {
@@ -27,3 +79,39 @@ function foo(x, y) {
             return 3;
     }
 }
+function f1(x) {
+    switch (x) {
+        case 0:
+            fail();
+        case 1:
+            noop();
+        case 2:
+            return;
+    }
+}
+function f2(transition) {
+    switch (transition.behavior) {
+        case 'SLIDE':
+            switch (transition.direction) {
+                case 'LEFT':
+                    return [];
+                case 'RIGHT':
+                    return [];
+                case 'TOP':
+                    return [];
+                case 'BOTTOM':
+                    return [];
+            }
+        case 'SLIDE_OUT':
+            switch (transition.direction) {
+                case 'LEFT':
+                    return [];
+                case 'RIGHT':
+                    return [];
+                case 'TOP':
+                    return [];
+                case 'BOTTOM':
+                    return [];
+            }
+    }
+}
diff --git a/tests/baselines/reference/reachabilityChecks4.symbols b/tests/baselines/reference/reachabilityChecks4.symbols
@@ -20,3 +20,93 @@ function foo(x, y) {
             return 3;
     }
 }
+
+declare function noop(): void;
+>noop : Symbol(noop, Decl(reachabilityChecks4.ts, 12, 1))
+
+declare function fail(): never;
+>fail : Symbol(fail, Decl(reachabilityChecks4.ts, 14, 30))
+
+function f1(x: 0 | 1 | 2) {
+>f1 : Symbol(f1, Decl(reachabilityChecks4.ts, 15, 31))
+>x : Symbol(x, Decl(reachabilityChecks4.ts, 17, 12))
+
+    switch (x) {
+>x : Symbol(x, Decl(reachabilityChecks4.ts, 17, 12))
+
+        case 0:
+            fail();
+>fail : Symbol(fail, Decl(reachabilityChecks4.ts, 14, 30))
+
+        case 1:
+            noop();
+>noop : Symbol(noop, Decl(reachabilityChecks4.ts, 12, 1))
+
+        case 2:
+            return;
+    }
+}
+
+// Repro from #34021
+
+type Behavior = 'SLIDE' | 'SLIDE_OUT'
+>Behavior : Symbol(Behavior, Decl(reachabilityChecks4.ts, 26, 1))
+
+type Direction = 'LEFT' | 'RIGHT' | 'TOP' | 'BOTTOM'
+>Direction : Symbol(Direction, Decl(reachabilityChecks4.ts, 30, 37))
+
+interface Transition {
+>Transition : Symbol(Transition, Decl(reachabilityChecks4.ts, 31, 52))
+
+  behavior: Behavior
+>behavior : Symbol(Transition.behavior, Decl(reachabilityChecks4.ts, 33, 22))
+>Behavior : Symbol(Behavior, Decl(reachabilityChecks4.ts, 26, 1))
+
+  direction: Direction
+>direction : Symbol(Transition.direction, Decl(reachabilityChecks4.ts, 34, 20))
+>Direction : Symbol(Direction, Decl(reachabilityChecks4.ts, 30, 37))
+}
+
+function f2(transition: Transition): any {
+>f2 : Symbol(f2, Decl(reachabilityChecks4.ts, 36, 1))
+>transition : Symbol(transition, Decl(reachabilityChecks4.ts, 38, 12))
+>Transition : Symbol(Transition, Decl(reachabilityChecks4.ts, 31, 52))
+
+    switch (transition.behavior) {
+>transition.behavior : Symbol(Transition.behavior, Decl(reachabilityChecks4.ts, 33, 22))
+>transition : Symbol(transition, Decl(reachabilityChecks4.ts, 38, 12))
+>behavior : Symbol(Transition.behavior, Decl(reachabilityChecks4.ts, 33, 22))
+
+        case 'SLIDE':
+            switch (transition.direction) {
+>transition.direction : Symbol(Transition.direction, Decl(reachabilityChecks4.ts, 34, 20))
+>transition : Symbol(transition, Decl(reachabilityChecks4.ts, 38, 12))
+>direction : Symbol(Transition.direction, Decl(reachabilityChecks4.ts, 34, 20))
+
+                case 'LEFT':
+                    return []
+                case 'RIGHT':
+                    return []
+                case 'TOP':
+                    return []
+                case 'BOTTOM':
+                    return []
+            }
+        case 'SLIDE_OUT':
+            switch (transition.direction) {
+>transition.direction : Symbol(Transition.direction, Decl(reachabilityChecks4.ts, 34, 20))
+>transition : Symbol(transition, Decl(reachabilityChecks4.ts, 38, 12))
+>direction : Symbol(Transition.direction, Decl(reachabilityChecks4.ts, 34, 20))
+
+                case 'LEFT':
+                    return []
+                case 'RIGHT':
+                    return []
+                case 'TOP':
+                    return []
+                case 'BOTTOM':
+                    return []
+            }
+    }
+}
+
diff --git a/tests/baselines/reference/reachabilityChecks4.types b/tests/baselines/reference/reachabilityChecks4.types
diff --git a/tests/cases/compiler/reachabilityChecks4.ts b/tests/cases/compiler/reachabilityChecks4.ts

Original file line number	Diff line number	Diff line change
`@@ -1341,7 +1341,7 @@ namespace ts {`
`1341`	`1341`	`bind(clause);`
`1342`	`1342`	`fallthroughFlow = currentFlow;`
`1343`	`1343`	`if (!(currentFlow.flags & FlowFlags.Unreachable) && i !== clauses.length - 1 && options.noFallthroughCasesInSwitch) {`
`1344`		`- errorOnFirstToken(clause, Diagnostics.Fallthrough_case_in_switch);`
	`1344`	`+ clause.fallthroughFlowNode = currentFlow;`
`1345`	`1345`	`}`
`1346`	`1346`	`}`
`1347`	`1347`	`clauses.transformFlags = subtreeTransformFlags \| TransformFlags.HasComputedFlags;`
Original file line number	Diff line number	Diff line change
`@@ -31054,10 +31054,7 @@ namespace ts {`
`31054`	`31054`	`firstDefaultClause = clause;`
`31055`	`31055`	`}`
`31056`	`31056`	`else {`
`31057`		`- const sourceFile = getSourceFileOfNode(node);`
`31058`		`- const start = skipTrivia(sourceFile.text, clause.pos);`
`31059`		`- const end = clause.statements.length > 0 ? clause.statements[0].pos : clause.end;`
`31060`		`- grammarErrorAtPos(sourceFile, start, end - start, Diagnostics.A_default_clause_cannot_appear_more_than_once_in_a_switch_statement);`
	`31057`	`+ grammarErrorOnNode(clause, Diagnostics.A_default_clause_cannot_appear_more_than_once_in_a_switch_statement);`
`31061`	`31058`	`hasDuplicateDefaultClause = true;`
`31062`	`31059`	`}`
`31063`	`31060`	`}`
`@@ -31079,6 +31076,9 @@ namespace ts {`
`31079`	`31076`	`}`
`31080`	`31077`	`}`
`31081`	`31078`	`forEach(clause.statements, checkSourceElement);`
	`31079`	`+ if (compilerOptions.noFallthroughCasesInSwitch && clause.fallthroughFlowNode && isReachableFlowNode(clause.fallthroughFlowNode)) {`
	`31080`	`+ error(clause, Diagnostics.Fallthrough_case_in_switch);`
	`31081`	`+ }`
`31082`	`31082`	`});`
`31083`	`31083`	`if (node.caseBlock.locals) {`
`31084`	`31084`	`registerForUnusedIdentifiersCheck(node.caseBlock);`
Original file line number	Diff line number	Diff line change
`@@ -2250,12 +2250,14 @@ namespace ts {`
`2250`	`2250`	`parent: CaseBlock;`
`2251`	`2251`	`expression: Expression;`
`2252`	`2252`	`statements: NodeArray<Statement>;`
	`2253`	`+ /* @internal */ fallthroughFlowNode?: FlowNode;`
`2253`	`2254`	`}`
`2254`	`2255`
`2255`	`2256`	`export interface DefaultClause extends Node {`
`2256`	`2257`	`kind: SyntaxKind.DefaultClause;`
`2257`	`2258`	`parent: CaseBlock;`
`2258`	`2259`	`statements: NodeArray<Statement>;`
	`2260`	`+ /* @internal */ fallthroughFlowNode?: FlowNode;`
`2259`	`2261`	`}`
`2260`	`2262`
`2261`	`2263`	`export type CaseOrDefaultClause = CaseClause \| DefaultClause;`