diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index b571530c5d9d7..20183785dfd10 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -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 @@ -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); } @@ -2792,7 +2799,7 @@ 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)); } } } @@ -2800,6 +2807,38 @@ namespace ts { } } + 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) || diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index e68f24536dec9..0c788ce79bbb1 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -8108,4 +8108,10 @@ namespace ts { } export type Mutable = { -readonly [K in keyof T]: T[K] }; + + export function sliceAfter(arr: ReadonlyArray, value: T): ReadonlyArray { + const index = arr.indexOf(value); + Debug.assert(index !== -1); + return arr.slice(index); + } } diff --git a/src/services/codefixes/fixUnreachableCode.ts b/src/services/codefixes/fixUnreachableCode.ts index 1c6c53efe8e60..4fdb2c2016fd5 100644 --- a/src/services/codefixes/fixUnreachableCode.ts +++ b/src/services/codefixes/fixUnreachableCode.ts @@ -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)); @@ -36,7 +36,9 @@ 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); @@ -44,35 +46,12 @@ namespace ts.codefix { } } - 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(a: ReadonlyArray, pred: (value: T) => boolean): T | undefined { + let last: T | undefined; + for (const value of a) { + if (!pred(value)) break; + last = value; } - } - - function sliceAfter(arr: ReadonlyArray, value: T): ReadonlyArray { - 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(arr: ReadonlyArray, pred: (t: T) => boolean, cb: (start: T, end: T) => void): void { - getRangesWhere(arr, pred, (start, afterEnd) => cb(arr[start], arr[afterEnd - 1])); + return last; } } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index bb441132862ac..dd6aaf2708ead 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -7390,6 +7390,7 @@ declare namespace ts { type Mutable = { -readonly [K in keyof T]: T[K]; }; + function sliceAfter(arr: ReadonlyArray, value: T): ReadonlyArray; } declare namespace ts { function createNode(kind: SyntaxKind, pos?: number, end?: number): Node; diff --git a/tests/baselines/reference/cf.errors.txt b/tests/baselines/reference/cf.errors.txt index 8f5f82f816962..910ffe5d88d37 100644 --- a/tests/baselines/reference/cf.errors.txt +++ b/tests/baselines/reference/cf.errors.txt @@ -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) { @@ -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); @@ -41,13 +41,13 @@ tests/cases/compiler/cf.ts(36,13): error TS7027: Unreachable code detected. L3: if (xunreachable : Symbol(unreachable, Decl(unreachable.js, 0, 0)) - return 1; + return f(); +>f : Symbol(f, Decl(unreachable.js, 3, 13)) + return 2; + return 3; + function f() {} +>f : Symbol(f, Decl(unreachable.js, 3, 13)) + + return 4; } + diff --git a/tests/baselines/reference/unreachableJavascriptChecked.types b/tests/baselines/reference/unreachableJavascriptChecked.types index 266a9038875cb..d27af1d33d88d 100644 --- a/tests/baselines/reference/unreachableJavascriptChecked.types +++ b/tests/baselines/reference/unreachableJavascriptChecked.types @@ -1,10 +1,21 @@ === tests/cases/compiler/unreachable.js === function unreachable() { ->unreachable : () => 1 | 2 +>unreachable : () => void | 2 | 3 | 4 - return 1; ->1 : 1 + return f(); +>f() : void +>f : () => void return 2; >2 : 2 + + return 3; +>3 : 3 + + function f() {} +>f : () => void + + return 4; +>4 : 4 } + diff --git a/tests/cases/compiler/unreachableJavascriptChecked.ts b/tests/cases/compiler/unreachableJavascriptChecked.ts index afddcba6a23b1..3d9c906dc05e2 100644 --- a/tests/cases/compiler/unreachableJavascriptChecked.ts +++ b/tests/cases/compiler/unreachableJavascriptChecked.ts @@ -4,6 +4,9 @@ // @outDir: out // @allowUnreachableCode: false function unreachable() { - return 1; + return f(); return 2; -} \ No newline at end of file + return 3; + function f() {} + return 4; +} diff --git a/tests/cases/fourslash/codeFixUnreachableCode.ts b/tests/cases/fourslash/codeFixUnreachableCode.ts index 5f70d51c1ee10..9c9eea6dae40c 100644 --- a/tests/cases/fourslash/codeFixUnreachableCode.ts +++ b/tests/cases/fourslash/codeFixUnreachableCode.ts @@ -2,29 +2,30 @@ ////function f() { //// return f(); -//// [|return|] 1; +//// [|return 1;|] //// function f() {} -//// return 2; +//// [|return 2;|] //// type T = number; //// interface I {} //// const enum E {} -//// enum E {} +//// [|enum E {}|] //// namespace N { export type T = number; } -//// namespace N { export const x: T = 0; } +//// [|namespace N { export const x: T = 0; }|] //// var x: I; -//// var y: T = 0; -//// E; N; x; y; +//// [|var y: T = 0; +//// E; N; x; y;|] ////} -verify.getSuggestionDiagnostics([{ +verify.getSuggestionDiagnostics(test.ranges().map((range): FourSlashInterface.Diagnostic => ({ message: "Unreachable code detected.", code: 7027, reportsUnnecessary: true, -}]); + range, +}))); -verify.codeFix({ - description: "Remove unreachable code", - index: 0, +verify.codeFixAll({ + fixId: "fixUnreachableCode", + fixAllDescription: "Remove all unreachable code", newFileContent: `function f() { return f();