Skip to content

Commit bc02341

Browse files
committed
addressed PR feedback, updated tests to suppress reachability errors where they are not needed
1 parent 17716fb commit bc02341

268 files changed

Lines changed: 12446 additions & 2425 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/compiler/binder.ts

Lines changed: 43 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -374,18 +374,17 @@ namespace ts {
374374
blockScopeContainer = savedBlockScopeContainer;
375375
}
376376

377-
function shouldSaveReachabilityState(n: Node): boolean {
378-
return n.kind === SyntaxKind.SourceFile || n.kind === SyntaxKind.ModuleBlock || isFunctionLike(n);
379-
}
380-
381377
function bindWithReachabilityChecks(node: Node): void {
382378
let savedReachabilityState: Reachability;
383379
let savedLabelStack: Reachability[];
384380
let savedLabels: Map<number>;
385381
let savedImplicitLabels: number[];
386382
let savedHasExplicitReturn: boolean;
387383

388-
let saveState = shouldSaveReachabilityState(node);
384+
// reset all reachability check related flags on node (for incremental scenarios)
385+
node.flags &= ~NodeFlags.ReachabilityCheckFlags;
386+
387+
let saveState = node.kind === SyntaxKind.SourceFile || node.kind === SyntaxKind.ModuleBlock || isFunctionLike(node);
389388
if (saveState) {
390389
savedReachabilityState = currentReachabilityState;
391390
savedLabelStack = labelStack;
@@ -398,9 +397,7 @@ namespace ts {
398397
labelStack = labelIndexMap = implicitLabels = undefined;
399398
}
400399

401-
if (!bindReachableStatement(node)) {
402-
forEachChild(node, bind);
403-
}
400+
bindReachableStatement(node);
404401

405402
if (currentReachabilityState === Reachability.Reachable && isFunctionLike(node) && nodeIsPresent((<FunctionLikeDeclaration>node).body)) {
406403
node.flags |= NodeFlags.HasImplicitReturn;
@@ -422,43 +419,56 @@ namespace ts {
422419
* Returns true if node and its subnodes were successfully traversed.
423420
* Returning false means that node was not examined and caller needs to dive into the node himself.
424421
*/
425-
function bindReachableStatement(node: Node): boolean {
422+
function bindReachableStatement(node: Node): void {
426423
if (checkUnreachable(node)) {
427-
return false;
424+
forEachChild(node, bind);
425+
return;
428426
}
429427

430428
switch (node.kind) {
431429
case SyntaxKind.WhileStatement:
432-
return bindWhileStatement(<WhileStatement>node);
430+
bindWhileStatement(<WhileStatement>node);
431+
break;
433432
case SyntaxKind.DoStatement:
434-
return bindDoStatement(<DoStatement>node);
433+
bindDoStatement(<DoStatement>node);
434+
break;
435435
case SyntaxKind.ForStatement:
436-
return bindForStatement(<ForStatement>node);
436+
bindForStatement(<ForStatement>node);
437+
break;
437438
case SyntaxKind.ForInStatement:
438439
case SyntaxKind.ForOfStatement:
439-
return bindForInOrForOfStatement(<ForInStatement | ForOfStatement>node);
440+
bindForInOrForOfStatement(<ForInStatement | ForOfStatement>node);
441+
break;
440442
case SyntaxKind.IfStatement:
441-
return bindIfStatement(<IfStatement>node);
443+
bindIfStatement(<IfStatement>node);
444+
break;
442445
case SyntaxKind.ReturnStatement:
443446
case SyntaxKind.ThrowStatement:
444-
return bindReturnOrThrow(<ReturnStatement | ThrowStatement>node);
447+
bindReturnOrThrow(<ReturnStatement | ThrowStatement>node);
448+
break;
445449
case SyntaxKind.BreakStatement:
446450
case SyntaxKind.ContinueStatement:
447-
return bindBreakOrContinueStatement(<BreakOrContinueStatement>node);
451+
bindBreakOrContinueStatement(<BreakOrContinueStatement>node);
452+
break;
448453
case SyntaxKind.TryStatement:
449-
return bindTryStatement(<TryStatement>node);
454+
bindTryStatement(<TryStatement>node);
455+
break;
450456
case SyntaxKind.SwitchStatement:
451-
return bindSwitchStatement(<SwitchStatement>node);
457+
bindSwitchStatement(<SwitchStatement>node);
458+
break;
452459
case SyntaxKind.CaseBlock:
453-
return bindCaseBlock(<CaseBlock>node);
460+
bindCaseBlock(<CaseBlock>node);
461+
break;
454462
case SyntaxKind.LabeledStatement:
455-
return bindLabeledStatement(<LabeledStatement>node);
463+
bindLabeledStatement(<LabeledStatement>node);
464+
break;
456465
default:
457-
return false;
466+
forEachChild(node, bind);
467+
break;
458468
}
459469
}
460470

461-
function bindWhileStatement(n: WhileStatement): boolean {
471+
function bindWhileStatement(n: WhileStatement): void {
462472
const preWhileState =
463473
n.expression.kind === SyntaxKind.FalseKeyword ? Reachability.Unreachable : currentReachabilityState;
464474
const postWhileState =
@@ -471,11 +481,9 @@ namespace ts {
471481
const postWhileLabel = pushImplicitLabel();
472482
bind(n.statement);
473483
popImplicitLabel(postWhileLabel, postWhileState);
474-
475-
return true;
476484
}
477485

478-
function bindDoStatement(n: DoStatement): boolean {
486+
function bindDoStatement(n: DoStatement): void {
479487
const preDoState = currentReachabilityState;
480488

481489
const postDoLabel = pushImplicitLabel();
@@ -485,11 +493,9 @@ namespace ts {
485493

486494
// bind expressions (don't affect reachability)
487495
bind(n.expression);
488-
489-
return true;
490496
}
491497

492-
function bindForStatement(n: ForStatement): boolean {
498+
function bindForStatement(n: ForStatement): void {
493499
const preForState = currentReachabilityState;
494500
const postForLabel = pushImplicitLabel();
495501

@@ -506,11 +512,9 @@ namespace ts {
506512
const isInfiniteLoop = (!n.condition || n.condition.kind === SyntaxKind.TrueKeyword);
507513
const postForState = isInfiniteLoop ? Reachability.Unreachable : preForState;
508514
popImplicitLabel(postForLabel, postForState);
509-
510-
return true;
511515
}
512516

513-
function bindForInOrForOfStatement(n: ForInStatement | ForOfStatement): boolean {
517+
function bindForInOrForOfStatement(n: ForInStatement | ForOfStatement): void {
514518
const preStatementState = currentReachabilityState;
515519
const postStatementLabel = pushImplicitLabel();
516520

@@ -520,11 +524,9 @@ namespace ts {
520524

521525
bind(n.statement);
522526
popImplicitLabel(postStatementLabel, preStatementState);
523-
524-
return true;
525527
}
526528

527-
function bindIfStatement(n: IfStatement): boolean {
529+
function bindIfStatement(n: IfStatement): void {
528530
// denotes reachability state when entering 'thenStatement' part of the if statement:
529531
// i.e. if condition is false then thenStatement is unreachable
530532
const ifTrueState = n.expression.kind === SyntaxKind.FalseKeyword ? Reachability.Unreachable : currentReachabilityState;
@@ -547,33 +549,28 @@ namespace ts {
547549
else {
548550
currentReachabilityState = or(currentReachabilityState, ifFalseState);
549551
}
550-
551-
return true;
552552
}
553553

554-
function bindReturnOrThrow(n: ReturnStatement | ThrowStatement): boolean {
554+
function bindReturnOrThrow(n: ReturnStatement | ThrowStatement): void {
555555
// bind expression (don't affect reachability)
556556
bind(n.expression);
557557
if (n.kind === SyntaxKind.ReturnStatement) {
558558
hasExplicitReturn = true;
559559
}
560560
currentReachabilityState = Reachability.Unreachable;
561-
562-
return true;
563561
}
564562

565-
function bindBreakOrContinueStatement(n: BreakOrContinueStatement): boolean {
563+
function bindBreakOrContinueStatement(n: BreakOrContinueStatement): void {
566564
// call bind on label (don't affect reachability)
567565
bind(n.label);
568566
// for continue case touch label so it will be marked a used
569567
const isValidJump = jumpToLabel(n.label, n.kind === SyntaxKind.BreakStatement ? currentReachabilityState : Reachability.Unreachable);
570568
if (isValidJump) {
571569
currentReachabilityState = Reachability.Unreachable;
572570
}
573-
return true;
574571
}
575572

576-
function bindTryStatement(n: TryStatement): boolean {
573+
function bindTryStatement(n: TryStatement): void {
577574
// catch\finally blocks has the same reachability as try block
578575
const preTryState = currentReachabilityState;
579576
bind(n.tryBlock);
@@ -590,11 +587,9 @@ namespace ts {
590587
// - post try state is reachable - control flow can fall out of try block
591588
// - post catch state is reachable - control flow can fall out of catch block
592589
currentReachabilityState = or(postTryState, postCatchState);
593-
594-
return true;
595590
}
596591

597-
function bindSwitchStatement(n: SwitchStatement): boolean {
592+
function bindSwitchStatement(n: SwitchStatement): void {
598593
const preSwitchState = currentReachabilityState;
599594
const postSwitchLabel = pushImplicitLabel();
600595

@@ -609,11 +604,9 @@ namespace ts {
609604
const postSwitchState = hasDefault && currentReachabilityState !== Reachability.Reachable ? Reachability.Unreachable : preSwitchState;
610605

611606
popImplicitLabel(postSwitchLabel, postSwitchState);
612-
613-
return true;
614607
}
615608

616-
function bindCaseBlock(n: CaseBlock): boolean {
609+
function bindCaseBlock(n: CaseBlock): void {
617610
const startState = currentReachabilityState;
618611

619612
for (let clause of n.clauses) {
@@ -623,11 +616,9 @@ namespace ts {
623616
errorOnFirstToken(clause, Diagnostics.Fallthrough_case_in_switch);
624617
}
625618
}
626-
627-
return true;
628619
}
629620

630-
function bindLabeledStatement(n: LabeledStatement): boolean {
621+
function bindLabeledStatement(n: LabeledStatement): void {
631622
// call bind on label (don't affect reachability)
632623
bind(n.label);
633624

@@ -636,8 +627,6 @@ namespace ts {
636627
if (ok) {
637628
popNamedLabel(n.label, currentReachabilityState);
638629
}
639-
640-
return true;
641630
}
642631

643632
function getContainerFlags(node: Node): ContainerFlags {
@@ -1387,8 +1376,6 @@ namespace ts {
13871376
}
13881377

13891378
function popNamedLabel(label: Identifier, outerState: Reachability): void {
1390-
initializeReachabilityStateIfNecessary();
1391-
13921379
let index = labelIndexMap[label.text];
13931380
Debug.assert(index !== undefined);
13941381
Debug.assert(labelStack.length == index + 1);
@@ -1399,17 +1386,13 @@ namespace ts {
13991386
}
14001387

14011388
function popImplicitLabel(implicitLabelIndex: number, outerState: Reachability): void {
1402-
initializeReachabilityStateIfNecessary();
1403-
14041389
Debug.assert(labelStack.length === implicitLabelIndex + 1, `Label stack: ${labelStack.length}, index:${implicitLabelIndex}`);
14051390
let i = implicitLabels.pop();
14061391
Debug.assert(implicitLabelIndex === i, `i: ${i}, index: ${implicitLabelIndex}`);
14071392
setCurrentStateAtLabel(labelStack.pop(), outerState, /*name*/ undefined);
14081393
}
14091394

14101395
function setCurrentStateAtLabel(innerMergedState: Reachability, outerState: Reachability, label: Identifier): void {
1411-
initializeReachabilityStateIfNecessary();
1412-
14131396
if (innerMergedState === Reachability.Unintialized) {
14141397
if (label && !options.allowUnusedLabels) {
14151398
file.bindDiagnostics.push(createDiagnosticForNode(label, Diagnostics.Unused_label));

src/compiler/types.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,9 @@ namespace ts {
384384

385385
Modifier = Export | Ambient | Public | Private | Protected | Static | Abstract | Default | Async,
386386
AccessibilityModifier = Public | Private | Protected,
387-
BlockScoped = Let | Const
387+
BlockScoped = Let | Const,
388+
389+
ReachabilityCheckFlags = HasImplicitReturn | HasExplicitReturn
388390
}
389391

390392
/* @internal */

tests/baselines/reference/bestCommonTypeReturnStatement.errors.txt

Lines changed: 0 additions & 18 deletions
This file was deleted.

tests/baselines/reference/bestCommonTypeReturnStatement.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//// [bestCommonTypeReturnStatement.ts]
2+
23
interface IPromise<T> {
34
then(successCallback: (promiseValue: T) => any, errorCallback?: (reason: any) => any): IPromise<any>;
45
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
=== tests/cases/compiler/bestCommonTypeReturnStatement.ts ===
2+
3+
interface IPromise<T> {
4+
>IPromise : Symbol(IPromise, Decl(bestCommonTypeReturnStatement.ts, 0, 0))
5+
>T : Symbol(T, Decl(bestCommonTypeReturnStatement.ts, 1, 19))
6+
7+
then(successCallback: (promiseValue: T) => any, errorCallback?: (reason: any) => any): IPromise<any>;
8+
>then : Symbol(then, Decl(bestCommonTypeReturnStatement.ts, 1, 23))
9+
>successCallback : Symbol(successCallback, Decl(bestCommonTypeReturnStatement.ts, 2, 9))
10+
>promiseValue : Symbol(promiseValue, Decl(bestCommonTypeReturnStatement.ts, 2, 27))
11+
>T : Symbol(T, Decl(bestCommonTypeReturnStatement.ts, 1, 19))
12+
>errorCallback : Symbol(errorCallback, Decl(bestCommonTypeReturnStatement.ts, 2, 51))
13+
>reason : Symbol(reason, Decl(bestCommonTypeReturnStatement.ts, 2, 69))
14+
>IPromise : Symbol(IPromise, Decl(bestCommonTypeReturnStatement.ts, 0, 0))
15+
}
16+
17+
function f() {
18+
>f : Symbol(f, Decl(bestCommonTypeReturnStatement.ts, 3, 1))
19+
20+
if (true) return b();
21+
>b : Symbol(b, Decl(bestCommonTypeReturnStatement.ts, 8, 1))
22+
23+
return d();
24+
>d : Symbol(d, Decl(bestCommonTypeReturnStatement.ts, 11, 45))
25+
}
26+
27+
28+
function b(): IPromise<void> { return null; }
29+
>b : Symbol(b, Decl(bestCommonTypeReturnStatement.ts, 8, 1))
30+
>IPromise : Symbol(IPromise, Decl(bestCommonTypeReturnStatement.ts, 0, 0))
31+
32+
function d(): IPromise<any> { return null; }
33+
>d : Symbol(d, Decl(bestCommonTypeReturnStatement.ts, 11, 45))
34+
>IPromise : Symbol(IPromise, Decl(bestCommonTypeReturnStatement.ts, 0, 0))
35+
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
=== tests/cases/compiler/bestCommonTypeReturnStatement.ts ===
2+
3+
interface IPromise<T> {
4+
>IPromise : IPromise<T>
5+
>T : T
6+
7+
then(successCallback: (promiseValue: T) => any, errorCallback?: (reason: any) => any): IPromise<any>;
8+
>then : (successCallback: (promiseValue: T) => any, errorCallback?: (reason: any) => any) => IPromise<any>
9+
>successCallback : (promiseValue: T) => any
10+
>promiseValue : T
11+
>T : T
12+
>errorCallback : (reason: any) => any
13+
>reason : any
14+
>IPromise : IPromise<T>
15+
}
16+
17+
function f() {
18+
>f : () => IPromise<void>
19+
20+
if (true) return b();
21+
>true : boolean
22+
>b() : IPromise<void>
23+
>b : () => IPromise<void>
24+
25+
return d();
26+
>d() : IPromise<any>
27+
>d : () => IPromise<any>
28+
}
29+
30+
31+
function b(): IPromise<void> { return null; }
32+
>b : () => IPromise<void>
33+
>IPromise : IPromise<T>
34+
>null : null
35+
36+
function d(): IPromise<any> { return null; }
37+
>d : () => IPromise<any>
38+
>IPromise : IPromise<T>
39+
>null : null
40+

tests/baselines/reference/breakTarget3.errors.txt

Lines changed: 0 additions & 11 deletions
This file was deleted.

0 commit comments

Comments
 (0)