Skip to content

Increase span of unreachable code error (#25388) #25555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,7 @@ namespace ts {
bind(node.statement);
popActiveLabel();
if (!activeLabel.referenced && !options.allowUnusedLabels) {
errorOrSuggestionOnFirstToken(unusedLabelIsError(options), node, Diagnostics.Unused_label);
errorOrSuggestionOnNode(unusedLabelIsError(options), node.label, Diagnostics.Unused_label);
}
if (!node.statement || node.statement.kind !== SyntaxKind.DoStatement) {
// do statement sets current flow inside bindDoStatement
Expand Down Expand Up @@ -1918,9 +1918,16 @@ namespace ts {
file.bindDiagnostics.push(createFileDiagnostic(file, span.start, span.length, message, arg0, arg1, arg2));
}

function errorOrSuggestionOnFirstToken(isError: boolean, node: Node, message: DiagnosticMessage, arg0?: any, arg1?: any, arg2?: any) {
const span = getSpanOfTokenAtPosition(file, node.pos);
const diag = createFileDiagnostic(file, span.start, span.length, message, arg0, arg1, arg2);
function errorOrSuggestionOnNode(isError: boolean, node: Node, message: DiagnosticMessage): void {
errorOrSuggestionOnRange(isError, node, node, message);
}

function errorOrSuggestionOnRange(isError: boolean, startNode: Node, endNode: Node, message: DiagnosticMessage): void {
addErrorOrSuggestionDiagnostic(isError, { pos: getTokenPosOfNode(startNode, file), end: endNode.end }, message);
}

function addErrorOrSuggestionDiagnostic(isError: boolean, range: TextRange, message: DiagnosticMessage): void {
const diag = createFileDiagnostic(file, range.pos, range.end - range.pos, message);
if (isError) {
file.bindDiagnostics.push(diag);
}
Expand Down Expand Up @@ -2792,14 +2799,46 @@ namespace ts {
node.declarationList.declarations.some(d => !!d.initializer)
);

errorOrSuggestionOnFirstToken(isError, node, Diagnostics.Unreachable_code_detected);
eachUnreachableRange(node, (start, end) => errorOrSuggestionOnRange(isError, start, end, Diagnostics.Unreachable_code_detected));
}
}
}
return true;
}
}

function eachUnreachableRange(node: Node, cb: (start: Node, last: Node) => void): void {
if (isStatement(node) && isExecutableStatement(node) && isBlock(node.parent)) {
const { statements } = node.parent;
const slice = sliceAfter(statements, node);
getRangesWhere(slice, isExecutableStatement, (start, afterEnd) => cb(slice[start], slice[afterEnd - 1]));
}
else {
cb(node, node);
}
}
// As opposed to a pure declaration like an `interface`
function isExecutableStatement(s: Statement): boolean {
// Don't remove statements that can validly be used before they appear.
return !isFunctionDeclaration(s) && !isPurelyTypeDeclaration(s) &&
// `var x;` may declare a variable used above
!(isVariableStatement(s) && !(getCombinedNodeFlags(s) & (NodeFlags.Let | NodeFlags.Const)) && s.declarationList.declarations.some(d => !d.initializer));
}

function isPurelyTypeDeclaration(s: Statement): boolean {
switch (s.kind) {
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
return true;
case SyntaxKind.ModuleDeclaration:
return getModuleInstanceState(s as ModuleDeclaration) !== ModuleInstanceState.Instantiated;
case SyntaxKind.EnumDeclaration:
return hasModifier(s, ModifierFlags.Const);
default:
return false;
}
}

/* @internal */
export function isExportsOrModuleExportsOrAlias(sourceFile: SourceFile, node: Expression): boolean {
return isExportsIdentifier(node) ||
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8108,4 +8108,10 @@ namespace ts {
}

export type Mutable<T extends object> = { -readonly [K in keyof T]: T[K] };

export function sliceAfter<T>(arr: ReadonlyArray<T>, value: T): ReadonlyArray<T> {
const index = arr.indexOf(value);
Debug.assert(index !== -1);
return arr.slice(index);
}
}
45 changes: 12 additions & 33 deletions src/services/codefixes/fixUnreachableCode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ namespace ts.codefix {
registerCodeFix({
errorCodes,
getCodeActions(context) {
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, context.span.start));
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, context.span.start, context.span.length));
return [createCodeFixAction(fixId, changes, Diagnostics.Remove_unreachable_code, fixId, Diagnostics.Remove_all_unreachable_code)];
},
fixIds: [fixId],
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => doChange(changes, diag.file, diag.start)),
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => doChange(changes, diag.file, diag.start, diag.length)),
});

function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, start: number): void {
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, start: number, length: number): void {
const token = getTokenAtPosition(sourceFile, start);
const statement = findAncestor(token, isStatement)!;
Debug.assert(statement.getStart(sourceFile) === token.getStart(sourceFile));
Expand All @@ -36,43 +36,22 @@ namespace ts.codefix {
break;
default:
if (isBlock(statement.parent)) {
split(sliceAfter(statement.parent.statements, statement), shouldRemove, (start, end) => changes.deleteNodeRange(sourceFile, start, end));
const end = start + length;
const lastStatement = Debug.assertDefined(lastWhere(sliceAfter(statement.parent.statements, statement), s => s.pos < end));
changes.deleteNodeRange(sourceFile, statement, lastStatement);
}
else {
changes.delete(sourceFile, statement);
}
}
}

function shouldRemove(s: Statement): boolean {
// Don't remove statements that can validly be used before they appear.
return !isFunctionDeclaration(s) && !isPurelyTypeDeclaration(s) &&
// `var x;` may declare a variable used above
!(isVariableStatement(s) && !(getCombinedNodeFlags(s) & (NodeFlags.Let | NodeFlags.Const)) && s.declarationList.declarations.some(d => !d.initializer));
}

function isPurelyTypeDeclaration(s: Statement): boolean {
switch (s.kind) {
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
return true;
case SyntaxKind.ModuleDeclaration:
return getModuleInstanceState(s as ModuleDeclaration) !== ModuleInstanceState.Instantiated;
case SyntaxKind.EnumDeclaration:
return hasModifier(s, ModifierFlags.Const);
default:
return false;
function lastWhere<T>(a: ReadonlyArray<T>, pred: (value: T) => boolean): T | undefined {
let last: T | undefined;
for (const value of a) {
if (!pred(value)) break;
last = value;
}
}

function sliceAfter<T>(arr: ReadonlyArray<T>, value: T): ReadonlyArray<T> {
const index = arr.indexOf(value);
Debug.assert(index !== -1);
return arr.slice(index);
}

// Calls 'cb' with the start and end of each range where 'pred' is true.
function split<T>(arr: ReadonlyArray<T>, pred: (t: T) => boolean, cb: (start: T, end: T) => void): void {
getRangesWhere(arr, pred, (start, afterEnd) => cb(arr[start], arr[afterEnd - 1]));
return last;
}
}
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7399,6 +7399,7 @@ declare namespace ts {
type Mutable<T extends object> = {
-readonly [K in keyof T]: T[K];
};
function sliceAfter<T>(arr: ReadonlyArray<T>, value: T): ReadonlyArray<T>;
}
declare namespace ts {
function createNode(kind: SyntaxKind, pos?: number, end?: number): Node;
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/cf.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ tests/cases/compiler/cf.ts(36,13): error TS7027: Unreachable code detected.
if (y==7) {
continue L1;
x=11;
~
~~~~~
!!! error TS7027: Unreachable code detected.
}
if (y==3) {
Expand All @@ -28,7 +28,7 @@ tests/cases/compiler/cf.ts(36,13): error TS7027: Unreachable code detected.
if (y==20) {
break;
x=12;
~
~~~~~
!!! error TS7027: Unreachable code detected.
}
} while (y<41);
Expand All @@ -41,13 +41,13 @@ tests/cases/compiler/cf.ts(36,13): error TS7027: Unreachable code detected.
L3: if (x<y) {
break L2;
x=13;
~
~~~~~
!!! error TS7027: Unreachable code detected.
}
else {
break L3;
x=14;
~
~~~~~
!!! error TS7027: Unreachable code detected.
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ tests/cases/compiler/a.js(11,9): error TS1100: Invalid use of 'arguments' in str
function f() {
return;
return; // Error: Unreachable code detected.
~~~~~~
~~~~~~~
!!! error TS7027: Unreachable code detected.
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ tests/cases/compiler/a.js(19,1): error TS7028: Unused label.
function bar2() {
}
var x = 10; // error
~~~
~~~~~~~~~~~
!!! error TS7027: Unreachable code detected.
}

Expand Down
33 changes: 21 additions & 12 deletions tests/baselines/reference/reachabilityChecks1.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ tests/cases/compiler/reachabilityChecks1.ts(69,5): error TS7027: Unreachable cod
==== tests/cases/compiler/reachabilityChecks1.ts (7 errors) ====
while (true);
var x = 1;
~~~
~~~~~~~~~~
!!! error TS7027: Unreachable code detected.

module A {
while (true);
let x;
~~~
~~~~~~
!!! error TS7027: Unreachable code detected.
}

Expand All @@ -30,10 +30,12 @@ tests/cases/compiler/reachabilityChecks1.ts(69,5): error TS7027: Unreachable cod
module A2 {
while (true);
module A {
~~~~~~
!!! error TS7027: Unreachable code detected.
~~~~~~~~~~
var x = 1;
~~~~~~~~~~~~~~~~~~
}
~~~~~
!!! error TS7027: Unreachable code detected.
}

module A3 {
Expand All @@ -44,10 +46,12 @@ tests/cases/compiler/reachabilityChecks1.ts(69,5): error TS7027: Unreachable cod
module A4 {
while (true);
module A {
~~~~~~
!!! error TS7027: Unreachable code detected.
~~~~~~~~~~
const enum E { X }
~~~~~~~~~~~~~~~~~~~~~~~~~~
}
~~~~~
!!! error TS7027: Unreachable code detected.
}

function f1(x) {
Expand All @@ -63,9 +67,10 @@ tests/cases/compiler/reachabilityChecks1.ts(69,5): error TS7027: Unreachable cod
function f2() {
return;
class A {
~~~~~
!!! error TS7027: Unreachable code detected.
~~~~~~~~~
}
~~~~~
!!! error TS7027: Unreachable code detected.
}

module B {
Expand All @@ -78,21 +83,25 @@ tests/cases/compiler/reachabilityChecks1.ts(69,5): error TS7027: Unreachable cod
do {
} while (true);
enum E {
~~~~
!!! error TS7027: Unreachable code detected.
~~~~~~~~
X = 1
~~~~~~~~~~~~~
}
~~~~~
!!! error TS7027: Unreachable code detected.
}

function f4() {
if (true) {
throw new Error();
}
const enum E {
~~~~~
!!! error TS7027: Unreachable code detected.
~~~~~~~~~~~~~~
X = 1
~~~~~~~~~~~~~
}
~~~~~
!!! error TS7027: Unreachable code detected.
}


9 changes: 7 additions & 2 deletions tests/baselines/reference/reachabilityChecks2.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@ tests/cases/compiler/reachabilityChecks2.ts(4,1): error TS7027: Unreachable code
const enum E { X }

module A4 {
~~~~~~
!!! error TS7027: Unreachable code detected.
~~~~~~~~~~~
while (true);
~~~~~~~~~~~~~~~~~
module A {
~~~~~~~~~~~~~~
const enum E { X }
~~~~~~~~~~~~~~~~~~~~~~~~~~
}
~~~~~
}
~
!!! error TS7027: Unreachable code detected.


6 changes: 3 additions & 3 deletions tests/baselines/reference/reachabilityChecks5.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ tests/cases/compiler/reachabilityChecks5.ts(122,13): error TS7027: Unreachable c
}
else {
return 1;
~~~~~~
~~~~~~~~~
!!! error TS7027: Unreachable code detected.
}
}
Expand All @@ -124,7 +124,7 @@ tests/cases/compiler/reachabilityChecks5.ts(122,13): error TS7027: Unreachable c
try {
while (false) {
return 1;
~~~~~~
~~~~~~~~~
!!! error TS7027: Unreachable code detected.
}
}
Expand Down Expand Up @@ -154,7 +154,7 @@ tests/cases/compiler/reachabilityChecks5.ts(122,13): error TS7027: Unreachable c
break test;
} while (true);
x++;
~
~~~~
!!! error TS7027: Unreachable code detected.
} while (true);
}
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/reachabilityChecks6.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ tests/cases/compiler/reachabilityChecks6.ts(122,13): error TS7027: Unreachable c
}
else {
return 1;
~~~~~~
~~~~~~~~~
!!! error TS7027: Unreachable code detected.
}
}
Expand All @@ -121,7 +121,7 @@ tests/cases/compiler/reachabilityChecks6.ts(122,13): error TS7027: Unreachable c
try {
while (false) {
return 1;
~~~~~~
~~~~~~~~~
!!! error TS7027: Unreachable code detected.
}
}
Expand Down Expand Up @@ -151,7 +151,7 @@ tests/cases/compiler/reachabilityChecks6.ts(122,13): error TS7027: Unreachable c
break test;
} while (true);
x++;
~
~~~~
!!! error TS7027: Unreachable code detected.
} while (true);
}
Expand Down
Loading