Skip to content

Add suggestion diagnostics for unused label and unreachable code #24261

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

Merged
2 commits merged into from
May 22, 2018
Merged
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
53 changes: 32 additions & 21 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ namespace ts {
bind(node.statement);
popActiveLabel();
if (!activeLabel.referenced && !options.allowUnusedLabels) {
file.bindDiagnostics.push(createDiagnosticForNode(node.label, Diagnostics.Unused_label));
errorOrSuggestionOnFirstToken(unusedLabelIsError(options), node, Diagnostics.Unused_label);
}
if (!node.statement || node.statement.kind !== SyntaxKind.DoStatement) {
// do statement sets current flow inside bindDoStatement
Expand Down Expand Up @@ -1914,6 +1914,17 @@ 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);
if (isError) {
file.bindDiagnostics.push(diag);
}
else {
file.bindSuggestionDiagnostics = append(file.bindSuggestionDiagnostics, { ...diag, category: DiagnosticCategory.Suggestion });
}
}

function bind(node: Node): void {
if (!node) {
return;
Expand Down Expand Up @@ -2730,26 +2741,26 @@ namespace ts {
if (reportError) {
currentFlow = reportedUnreachableFlow;

// unreachable code is reported if
// - user has explicitly asked about it AND
// - statement is in not ambient context (statements in ambient context is already an error
// so we should not report extras) AND
// - node is not variable statement OR
// - node is block scoped variable statement OR
// - node is not block scoped variable statement and at least one variable declaration has initializer
// Rationale: we don't want to report errors on non-initialized var's since they are hoisted
// On the other side we do want to report errors on non-initialized 'lets' because of TDZ
const reportUnreachableCode =
!options.allowUnreachableCode &&
!(node.flags & NodeFlags.Ambient) &&
(
node.kind !== SyntaxKind.VariableStatement ||
getCombinedNodeFlags((<VariableStatement>node).declarationList) & NodeFlags.BlockScoped ||
forEach((<VariableStatement>node).declarationList.declarations, d => d.initializer)
);

if (reportUnreachableCode) {
errorOnFirstToken(node, Diagnostics.Unreachable_code_detected);
if (!options.allowUnreachableCode) {
// unreachable code is reported if
// - user has explicitly asked about it AND
// - statement is in not ambient context (statements in ambient context is already an error
// so we should not report extras) AND
// - node is not variable statement OR
// - node is block scoped variable statement OR
// - node is not block scoped variable statement and at least one variable declaration has initializer
// Rationale: we don't want to report errors on non-initialized var's since they are hoisted
// On the other side we do want to report errors on non-initialized 'lets' because of TDZ
const isError =
unreachableCodeIsError(options) &&
!(node.flags & NodeFlags.Ambient) &&
(
!isVariableStatement(node) ||
!!(getCombinedNodeFlags(node.declarationList) & NodeFlags.BlockScoped) ||
node.declarationList.declarations.some(d => !!d.initializer)
);

errorOrSuggestionOnFirstToken(isError, node, Diagnostics.Unreachable_code_detected);
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1991,6 +1991,14 @@ namespace ts {
return moduleResolution;
}

export function unreachableCodeIsError(options: CompilerOptions): boolean {
return options.allowUnreachableCode === false;
}

export function unusedLabelIsError(options: CompilerOptions): boolean {
return options.allowUnusedLabels === false;
}

export function getAreDeclarationMapsEnabled(options: CompilerOptions) {
return !!(options.declaration && options.declarationMap);
}
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2238,7 +2238,8 @@
},
"Left side of comma operator is unused and has no side effects.": {
"category": "Error",
"code": 2695
"code": 2695,
"reportsUnnecessary": true
},
"The 'Object' type is assignable to very few other types. Did you mean to use the 'any' type instead?": {
"category": "Error",
Expand Down Expand Up @@ -3668,7 +3669,8 @@
},
"Unreachable code detected.": {
"category": "Error",
"code": 7027
"code": 7027,
"reportsUnnecessary": true
},
"Unused label.": {
"category": "Error",
Expand Down
1 change: 1 addition & 0 deletions src/compiler/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2463,6 +2463,7 @@ namespace ts {
if (node.symbolCount !== undefined) updated.symbolCount = node.symbolCount;
if (node.parseDiagnostics !== undefined) updated.parseDiagnostics = node.parseDiagnostics;
if (node.bindDiagnostics !== undefined) updated.bindDiagnostics = node.bindDiagnostics;
if (node.bindSuggestionDiagnostics !== undefined) updated.bindSuggestionDiagnostics = node.bindSuggestionDiagnostics;
if (node.lineMap !== undefined) updated.lineMap = node.lineMap;
if (node.classifiableNames !== undefined) updated.classifiableNames = node.classifiableNames;
if (node.resolvedModules !== undefined) updated.resolvedModules = node.resolvedModules;
Expand Down
1 change: 1 addition & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,7 @@ namespace ts {

sourceFile.text = sourceText;
sourceFile.bindDiagnostics = [];
sourceFile.bindSuggestionDiagnostics = undefined;
sourceFile.languageVersion = languageVersion;
sourceFile.fileName = normalizePath(fileName);
sourceFile.languageVariant = getLanguageVariant(scriptKind);
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2603,6 +2603,7 @@ namespace ts {

// File-level diagnostics reported by the binder.
/* @internal */ bindDiagnostics: Diagnostic[];
/* @internal */ bindSuggestionDiagnostics?: Diagnostic[];

// File-level JSDoc diagnostics reported by the JSDoc parser
/* @internal */ jsDocDiagnostics?: Diagnostic[];
Expand Down
16 changes: 11 additions & 5 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1317,8 +1317,13 @@ Actual: ${stringify(fullActual)}`);
}

private testDiagnostics(expected: ReadonlyArray<FourSlashInterface.Diagnostic>, diagnostics: ReadonlyArray<ts.Diagnostic>, category: string) {
assert.deepEqual(ts.realizeDiagnostics(diagnostics, ts.newLineCharacter), expected.map<ts.RealizedDiagnostic>(e => (
{ message: e.message, category, code: e.code, ...ts.createTextSpanFromRange(e.range || this.getRanges()[0]) })));
assert.deepEqual(ts.realizeDiagnostics(diagnostics, ts.newLineCharacter), expected.map((e): ts.RealizedDiagnostic => ({
message: e.message,
category,
code: e.code,
...ts.createTextSpanFromRange(e.range || this.getRanges()[0]),
reportsUnnecessary: e.reportsUnnecessary,
})));
}

public verifyQuickInfoAt(markerName: string, expectedText: string, expectedDocumentation?: string) {
Expand Down Expand Up @@ -4422,15 +4427,15 @@ namespace FourSlashInterface {
this.state.verifyQuickInfoDisplayParts(kind, kindModifiers, textSpan, displayParts, documentation, tags);
}

public getSyntacticDiagnostics(expected: ReadonlyArray<ts.RealizedDiagnostic>) {
public getSyntacticDiagnostics(expected: ReadonlyArray<Diagnostic>) {
this.state.getSyntacticDiagnostics(expected);
}

public getSemanticDiagnostics(expected: ReadonlyArray<ts.RealizedDiagnostic>) {
public getSemanticDiagnostics(expected: ReadonlyArray<Diagnostic>) {
this.state.getSemanticDiagnostics(expected);
}

public getSuggestionDiagnostics(expected: ReadonlyArray<ts.RealizedDiagnostic>) {
public getSuggestionDiagnostics(expected: ReadonlyArray<Diagnostic>) {
this.state.getSuggestionDiagnostics(expected);
}

Expand Down Expand Up @@ -4837,6 +4842,7 @@ namespace FourSlashInterface {
message: string;
range?: FourSlash.Range;
code: number;
reportsUnnecessary?: true;
}

export interface GetEditsForFileRenameOptions {
Expand Down
1 change: 1 addition & 0 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ namespace ts {
public syntacticDiagnostics: Diagnostic[];
public parseDiagnostics: Diagnostic[];
public bindDiagnostics: Diagnostic[];
public bindSuggestionDiagnostics?: Diagnostic[];

public isDeclarationFile: boolean;
public isDefaultLib: boolean;
Expand Down
5 changes: 3 additions & 2 deletions src/services/shims.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ namespace ts {
length: number;
category: string;
code: number;
unused?: {};
reportsUnnecessary?: {};
}
export function realizeDiagnostics(diagnostics: ReadonlyArray<Diagnostic>, newLine: string): RealizedDiagnostic[] {
return diagnostics.map(d => realizeDiagnostic(d, newLine));
Expand All @@ -598,7 +598,8 @@ namespace ts {
start: diagnostic.start,
length: diagnostic.length,
category: diagnosticCategoryName(diagnostic),
code: diagnostic.code
code: diagnostic.code,
reportsUnnecessary: diagnostic.reportsUnnecessary,
};
}

Expand Down
1 change: 1 addition & 0 deletions src/services/suggestionDiagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ namespace ts {
}
}

addRange(diags, sourceFile.bindSuggestionDiagnostics);
return diags.concat(checker.getSuggestionDiagnostics(sourceFile)).sort((d1, d2) => d1.start - d2.start);
}

Expand Down
5 changes: 1 addition & 4 deletions tests/baselines/reference/assignmentLHSIsValue.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(2
tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(30,1): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(31,1): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(32,1): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(35,3): error TS7028: Unused label.
tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(35,9): error TS1128: Declaration or statement expected.
tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(38,2): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(38,6): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
Expand All @@ -39,7 +38,7 @@ tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(6
tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(70,1): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.


==== tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts (39 errors) ====
==== tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts (38 errors) ====
// expected error for all the LHS of assignments
var value: any;

Expand Down Expand Up @@ -105,8 +104,6 @@ tests/cases/conformance/expressions/assignmentOperator/assignmentLHSIsValue.ts(7

// object literals
{ a: 0} = value;
~
!!! error TS7028: Unused label.
~
!!! error TS1128: Declaration or statement expected.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
tests/cases/compiler/blockScopedBindingUsedBeforeDef.ts(2,12): error TS2448: Block-scoped variable 'a' used before its declaration.
tests/cases/compiler/blockScopedBindingUsedBeforeDef.ts(5,12): error TS2448: Block-scoped variable 'a' used before its declaration.
tests/cases/compiler/blockScopedBindingUsedBeforeDef.ts(5,35): error TS7027: Unreachable code detected.
tests/cases/compiler/blockScopedBindingUsedBeforeDef.ts(8,7): error TS2448: Block-scoped variable 'b' used before its declaration.


==== tests/cases/compiler/blockScopedBindingUsedBeforeDef.ts (4 errors) ====
==== tests/cases/compiler/blockScopedBindingUsedBeforeDef.ts (3 errors) ====
// 1:
for (let {[a]: a} of [{ }]) continue;
~
Expand All @@ -14,8 +13,6 @@ tests/cases/compiler/blockScopedBindingUsedBeforeDef.ts(8,7): error TS2448: Bloc
for (let {[a]: a} = { }; false; ) continue;
~
!!! error TS2448: Block-scoped variable 'a' used before its declaration.
~~~~~~~~
!!! error TS7027: Unreachable code detected.

// 3:
let {[b]: b} = { };
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/cf.errors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ tests/cases/compiler/cf.ts(36,13): error TS7027: Unreachable code detected.
}
catch (e) {
x++;
}
}
finally {
x+=3;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/cf.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function f() {
}
catch (e) {
x++;
}
}
finally {
x+=3;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/cf.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function f() {

x++;
>x : Symbol(x, Decl(cf.ts, 2, 7))
}
}
finally {
x+=3;
>x : Symbol(x, Decl(cf.ts, 2, 7))
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/cf.types
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function f() {
x++;
>x++ : number
>x : number
}
}
finally {
x+=3;
>x+=3 : number
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignm
tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts(38,1): error TS2364: The left-hand side of an assignment expression must be a variable or a property access.
tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts(39,1): error TS2362: The left-hand side of an arithmetic operation must be of type 'any', 'number' or an enum type.
tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts(40,1): error TS2362: The left-hand side of an arithmetic operation must be of type 'any', 'number' or an enum type.
tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts(43,3): error TS7028: Unused label.
tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts(43,10): error TS1128: Declaration or statement expected.
tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts(46,1): error TS2362: The left-hand side of an arithmetic operation must be of type 'any', 'number' or an enum type.
tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts(52,15): error TS1034: 'super' must be followed by an argument list or member access.
Expand All @@ -40,7 +39,7 @@ tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignm
tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts(85,1): error TS2362: The left-hand side of an arithmetic operation must be of type 'any', 'number' or an enum type.


==== tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts (40 errors) ====
==== tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignmentLHSIsValue.ts (39 errors) ====
// expected error for all the LHS of compound assignments (arithmetic and addition)
var value: any;

Expand Down Expand Up @@ -116,8 +115,6 @@ tests/cases/conformance/es7/exponentiationOperator/compoundExponentiationAssignm

// object literals
{ a: 0 } **= value;
~
!!! error TS7028: Unused label.
~~~
!!! error TS1128: Declaration or statement expected.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
tests/cases/compiler/constDeclarations-scopes.ts(12,5): error TS7027: Unreachable code detected.
tests/cases/compiler/constDeclarations-scopes.ts(21,1): error TS7027: Unreachable code detected.
tests/cases/compiler/constDeclarations-scopes.ts(27,1): error TS2410: The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.


==== tests/cases/compiler/constDeclarations-scopes.ts (3 errors) ====
==== tests/cases/compiler/constDeclarations-scopes.ts (1 errors) ====
// global
const c = "string";

Expand All @@ -16,8 +14,6 @@ tests/cases/compiler/constDeclarations-scopes.ts(27,1): error TS2410: The 'with'
}
else {
const c = 0;
~~~~~
!!! error TS7027: Unreachable code detected.
n = c;
}

Expand All @@ -27,8 +23,6 @@ tests/cases/compiler/constDeclarations-scopes.ts(27,1): error TS2410: The 'with'
}

do {
~~
!!! error TS7027: Unreachable code detected.
const c = 0;
n = c;
} while (true);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
tests/cases/compiler/declarationEmitInvalidExport.ts(2,3): error TS7027: Unreachable code detected.
tests/cases/compiler/declarationEmitInvalidExport.ts(4,30): error TS4081: Exported type alias 'MyClass' has or is using private name 'myClass'.
tests/cases/compiler/declarationEmitInvalidExport.ts(5,1): error TS1128: Declaration or statement expected.


==== tests/cases/compiler/declarationEmitInvalidExport.ts (3 errors) ====
==== tests/cases/compiler/declarationEmitInvalidExport.ts (2 errors) ====
if (false) {
export var myClass = 0;
~~~~~~
!!! error TS7027: Unreachable code detected.
}
export type MyClass = typeof myClass;
~~~~~~~
Expand Down
Loading