Skip to content

Commit 96db135

Browse files
author
Andy Hanson
committed
Add a new diagnostic for each range of unreachable statements
1 parent 8bc4554 commit 96db135

10 files changed

+96
-54
lines changed

‎src/compiler/binder.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2799,14 +2799,46 @@ namespace ts {
27992799
node.declarationList.declarations.some(d => !!d.initializer)
28002800
);
28012801

2802-
errorOrSuggestionOnRange(isError, node, isBlock(node.parent) ? last(node.parent.statements) : node, Diagnostics.Unreachable_code_detected);
2802+
eachUnreachableRange(node, (start, end) => errorOrSuggestionOnRange(isError, start, end, Diagnostics.Unreachable_code_detected));
28032803
}
28042804
}
28052805
}
28062806
return true;
28072807
}
28082808
}
28092809

2810+
function eachUnreachableRange(node: Node, cb: (start: Node, last: Node) => void): void {
2811+
if (isStatement(node) && isExecutableStatement(node) && isBlock(node.parent)) {
2812+
const { statements } = node.parent;
2813+
const slice = sliceAfter(statements, node);
2814+
getRangesWhere(slice, isExecutableStatement, (start, afterEnd) => cb(slice[start], slice[afterEnd - 1]));
2815+
}
2816+
else {
2817+
cb(node, node);
2818+
}
2819+
}
2820+
// As opposed to a pure declaration like an `interface`
2821+
function isExecutableStatement(s: Statement): boolean {
2822+
// Don't remove statements that can validly be used before they appear.
2823+
return !isFunctionDeclaration(s) && !isPurelyTypeDeclaration(s) &&
2824+
// `var x;` may declare a variable used above
2825+
!(isVariableStatement(s) && !(getCombinedNodeFlags(s) & (NodeFlags.Let | NodeFlags.Const)) && s.declarationList.declarations.some(d => !d.initializer));
2826+
}
2827+
2828+
function isPurelyTypeDeclaration(s: Statement): boolean {
2829+
switch (s.kind) {
2830+
case SyntaxKind.InterfaceDeclaration:
2831+
case SyntaxKind.TypeAliasDeclaration:
2832+
return true;
2833+
case SyntaxKind.ModuleDeclaration:
2834+
return getModuleInstanceState(s as ModuleDeclaration) !== ModuleInstanceState.Instantiated;
2835+
case SyntaxKind.EnumDeclaration:
2836+
return hasModifier(s, ModifierFlags.Const);
2837+
default:
2838+
return false;
2839+
}
2840+
}
2841+
28102842
/* @internal */
28112843
export function isExportsOrModuleExportsOrAlias(sourceFile: SourceFile, node: Expression): boolean {
28122844
return isExportsIdentifier(node) ||

‎src/compiler/utilities.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8105,4 +8105,10 @@ namespace ts {
81058105

81068106
return findBestPatternMatch(patterns, _ => _, candidate);
81078107
}
8108+
8109+
export function sliceAfter<T>(arr: ReadonlyArray<T>, value: T): ReadonlyArray<T> {
8110+
const index = arr.indexOf(value);
8111+
Debug.assert(index !== -1);
8112+
return arr.slice(index);
8113+
}
81088114
}

‎src/services/codefixes/fixUnreachableCode.ts

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ namespace ts.codefix {
55
registerCodeFix({
66
errorCodes,
77
getCodeActions(context) {
8-
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, context.span.start));
8+
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, context.span.start, context.span.length));
99
return [createCodeFixAction(fixId, changes, Diagnostics.Remove_unreachable_code, fixId, Diagnostics.Remove_all_unreachable_code)];
1010
},
1111
fixIds: [fixId],
12-
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => doChange(changes, diag.file, diag.start)),
12+
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => doChange(changes, diag.file, diag.start, diag.length)),
1313
});
1414

15-
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, start: number): void {
15+
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, start: number, length: number): void {
1616
const token = getTokenAtPosition(sourceFile, start);
1717
const statement = findAncestor(token, isStatement)!;
1818
Debug.assert(statement.getStart(sourceFile) === token.getStart(sourceFile));
@@ -36,43 +36,22 @@ namespace ts.codefix {
3636
break;
3737
default:
3838
if (isBlock(statement.parent)) {
39-
split(sliceAfter(statement.parent.statements, statement), shouldRemove, (start, end) => changes.deleteNodeRange(sourceFile, start, end));
39+
const end = start + length;
40+
const lastStatement = Debug.assertDefined(lastWhere(sliceAfter(statement.parent.statements, statement), s => s.pos < end));
41+
changes.deleteNodeRange(sourceFile, statement, lastStatement);
4042
}
4143
else {
4244
changes.delete(sourceFile, statement);
4345
}
4446
}
4547
}
4648

47-
function shouldRemove(s: Statement): boolean {
48-
// Don't remove statements that can validly be used before they appear.
49-
return !isFunctionDeclaration(s) && !isPurelyTypeDeclaration(s) &&
50-
// `var x;` may declare a variable used above
51-
!(isVariableStatement(s) && !(getCombinedNodeFlags(s) & (NodeFlags.Let | NodeFlags.Const)) && s.declarationList.declarations.some(d => !d.initializer));
52-
}
53-
54-
function isPurelyTypeDeclaration(s: Statement): boolean {
55-
switch (s.kind) {
56-
case SyntaxKind.InterfaceDeclaration:
57-
case SyntaxKind.TypeAliasDeclaration:
58-
return true;
59-
case SyntaxKind.ModuleDeclaration:
60-
return getModuleInstanceState(s as ModuleDeclaration) !== ModuleInstanceState.Instantiated;
61-
case SyntaxKind.EnumDeclaration:
62-
return hasModifier(s, ModifierFlags.Const);
63-
default:
64-
return false;
49+
function lastWhere<T>(a: ReadonlyArray<T>, pred: (value: T) => boolean): T | undefined {
50+
let last: T | undefined;
51+
for (const value of a) {
52+
if (!pred(value)) break;
53+
last = value;
6554
}
66-
}
67-
68-
function sliceAfter<T>(arr: ReadonlyArray<T>, value: T): ReadonlyArray<T> {
69-
const index = arr.indexOf(value);
70-
Debug.assert(index !== -1);
71-
return arr.slice(index);
72-
}
73-
74-
// Calls 'cb' with the start and end of each range where 'pred' is true.
75-
function split<T>(arr: ReadonlyArray<T>, pred: (t: T) => boolean, cb: (start: T, end: T) => void): void {
76-
getRangesWhere(arr, pred, (start, afterEnd) => cb(arr[start], arr[afterEnd - 1]));
55+
return last;
7756
}
7857
}

‎tests/baselines/reference/jsFileCompilationBindReachabilityErrors.errors.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ tests/cases/compiler/a.js(19,1): error TS7028: Unused label.
2222
function bar2() {
2323
}
2424
var x = 10; // error
25-
~~~
25+
~~~~~~~~~~~
2626
!!! error TS7027: Unreachable code detected.
2727
}
2828

‎tests/baselines/reference/unreachableJavascriptChecked.errors.txt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
tests/cases/compiler/unreachable.js(3,5): error TS7027: Unreachable code detected.
2+
tests/cases/compiler/unreachable.js(6,5): error TS7027: Unreachable code detected.
23

34

4-
==== tests/cases/compiler/unreachable.js (1 errors) ====
5+
==== tests/cases/compiler/unreachable.js (2 errors) ====
56
function unreachable() {
6-
return 1;
7+
return f();
78
return 2;
89
~~~~~~~~~
910
return 3;
1011
~~~~~~~~~~~~~
12+
!!! error TS7027: Unreachable code detected.
13+
function f() {}
14+
return 4;
15+
~~~~~~~~~
1116
!!! error TS7027: Unreachable code detected.
1217
}
1318

‎tests/baselines/reference/unreachableJavascriptChecked.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
//// [unreachable.js]
22
function unreachable() {
3-
return 1;
3+
return f();
44
return 2;
55
return 3;
6+
function f() {}
7+
return 4;
68
}
79

810

911
//// [unreachable.js]
1012
function unreachable() {
11-
return 1;
13+
return f();
1214
return 2;
1315
return 3;
16+
function f() { }
17+
return 4;
1418
}

‎tests/baselines/reference/unreachableJavascriptChecked.symbols

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@
22
function unreachable() {
33
>unreachable : Symbol(unreachable, Decl(unreachable.js, 0, 0))
44

5-
return 1;
5+
return f();
6+
>f : Symbol(f, Decl(unreachable.js, 3, 13))
7+
68
return 2;
79
return 3;
10+
function f() {}
11+
>f : Symbol(f, Decl(unreachable.js, 3, 13))
12+
13+
return 4;
814
}
915

‎tests/baselines/reference/unreachableJavascriptChecked.types

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,21 @@
11
=== tests/cases/compiler/unreachable.js ===
22
function unreachable() {
3-
>unreachable : () => 1 | 2 | 3
3+
>unreachable : () => void | 2 | 3 | 4
44

5-
return 1;
6-
>1 : 1
5+
return f();
6+
>f() : void
7+
>f : () => void
78

89
return 2;
910
>2 : 2
1011

1112
return 3;
1213
>3 : 3
14+
15+
function f() {}
16+
>f : () => void
17+
18+
return 4;
19+
>4 : 4
1320
}
1421

‎tests/cases/compiler/unreachableJavascriptChecked.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
// @outDir: out
55
// @allowUnreachableCode: false
66
function unreachable() {
7-
return 1;
7+
return f();
88
return 2;
99
return 3;
10+
function f() {}
11+
return 4;
1012
}

‎tests/cases/fourslash/codeFixUnreachableCode.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,30 @@
22

33
////function f() {
44
//// return f();
5-
//// [|return 1;
5+
//// [|return 1;|]
66
//// function f() {}
7-
//// return 2;
7+
//// [|return 2;|]
88
//// type T = number;
99
//// interface I {}
1010
//// const enum E {}
11-
//// enum E {}
11+
//// [|enum E {}|]
1212
//// namespace N { export type T = number; }
13-
//// namespace N { export const x: T = 0; }
13+
//// [|namespace N { export const x: T = 0; }|]
1414
//// var x: I;
15-
//// var y: T = 0;
15+
//// [|var y: T = 0;
1616
//// E; N; x; y;|]
1717
////}
1818

19-
verify.getSuggestionDiagnostics([{
19+
verify.getSuggestionDiagnostics(test.ranges().map((range): FourSlashInterface.Diagnostic => ({
2020
message: "Unreachable code detected.",
2121
code: 7027,
2222
reportsUnnecessary: true,
23-
}]);
23+
range,
24+
})));
2425

25-
verify.codeFix({
26-
description: "Remove unreachable code",
27-
index: 0,
26+
verify.codeFixAll({
27+
fixId: "fixUnreachableCode",
28+
fixAllDescription: "Remove all unreachable code",
2829
newFileContent:
2930
`function f() {
3031
return f();

0 commit comments

Comments
 (0)