Skip to content
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

Classify lintish diagnostics as warnings, add treatWarningsAsErrors #19126

Closed
wants to merge 7 commits 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
7 changes: 7 additions & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ namespace ts {
category: Diagnostics.Command_line_Options,
description: Diagnostics.Watch_input_files,
},
{
name: "treatWarningsAsErrors",
type: "boolean",
showInSimplifiedHelpView: true,
category: Diagnostics.Command_line_Options,
description: Diagnostics.Treat_warnings_as_errors
},

// Basic
{
Expand Down
14 changes: 9 additions & 5 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3115,7 +3115,7 @@
"code": 6132
},
"'{0}' is declared but its value is never read.": {
"category": "Error",
"category": "Warning",
"code": 6133
},
"Report errors on unused locals.": {
Expand Down Expand Up @@ -3399,19 +3399,19 @@
"code": 7026
},
"Unreachable code detected.": {
"category": "Error",
"category": "Warning",
"code": 7027
},
"Unused label.": {
"category": "Error",
"category": "Warning",
"code": 7028
},
"Fallthrough case in switch.": {
"category": "Error",
"category": "Warning",
"code": 7029
},
"Not all code paths return a value.": {
"category": "Error",
"category": "Warning",
"code": 7030
},
"Binding element '{0}' implicitly has an '{1}' type.": {
Expand All @@ -3438,6 +3438,10 @@
"category": "Error",
"code": 7036
},
"Treat warnings as errors": {
"category": "Warning",
"code": 7037
},

"You cannot rename this element.": {
"category": "Error",
Expand Down
21 changes: 17 additions & 4 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ namespace ts {
let output = "";
for (const diagnostic of diagnostics) {
let context = "";
const categoryColor = getCategoryFormat(diagnostic.category);
if (diagnostic.file) {
const { start, length, file } = diagnostic;
const { line: firstLine, character: firstLineChar } = getLineAndCharacterOfPosition(file, start);
Expand Down Expand Up @@ -313,7 +314,7 @@ namespace ts {

// Output the gutter and the error span for the line using tildes.
context += formatAndReset(padLeft("", gutterWidth), gutterStyleSequence) + gutterSeparator;
context += redForegroundEscapeSequence;
context += categoryColor;
if (i === firstLine) {
// If we're on the last line, then limit it to the last character of the last line.
// Otherwise, we'll just squiggle the rest of the line, giving 'slice' no end position.
Expand All @@ -336,7 +337,6 @@ namespace ts {
output += `${ relativeFileName }(${ firstLine + 1 },${ firstLineChar + 1 }): `;
}

const categoryColor = getCategoryFormat(diagnostic.category);
const category = DiagnosticCategory[diagnostic.category].toLowerCase();
output += `${ formatAndReset(category, categoryColor) } TS${ diagnostic.code }: ${ flattenDiagnosticMessageText(diagnostic.messageText, host.getNewLine()) }`;

Expand Down Expand Up @@ -1142,12 +1142,14 @@ namespace ts {
...program.getGlobalDiagnostics(cancellationToken),
...program.getSemanticDiagnostics(sourceFile, cancellationToken)
];
const hasError = some(diagnostics, d => d.category === DiagnosticCategory.Error);

if (diagnostics.length === 0 && program.getCompilerOptions().declaration) {
if (!hasError && program.getCompilerOptions().declaration) {
declarationDiagnostics = program.getDeclarationDiagnostics(/*sourceFile*/ undefined, cancellationToken);
}

if (diagnostics.length > 0 || declarationDiagnostics.length > 0) {
const hasDeclarationError = some(declarationDiagnostics, d => d.category === DiagnosticCategory.Error);
if (hasError || hasDeclarationError) {
return {
diagnostics: concatenate(diagnostics, declarationDiagnostics),
sourceMaps: undefined,
Expand Down Expand Up @@ -1292,10 +1294,21 @@ namespace ts {
});
}

function shouldTreatWarningsAsErrors() {
const setValue = program.getCompilerOptions().treatWarningsAsErrors;
if (typeof setValue === "undefined") {
return true;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice that it defaults to the safer setting (true); but how do you disable it via command-line arguments? --no-treatWarningsAsErrors?

Copy link
Member Author

@weswigham weswigham Oct 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--treatWarningsAsErrors false, same as our other boolean flags.

}
return setValue;
}

/**
* Skip errors if previous line start with '// @ts-ignore' comment, not counting non-empty non-comment lines
*/
function shouldReportDiagnostic(diagnostic: Diagnostic) {
if (diagnostic.category === DiagnosticCategory.Warning && shouldTreatWarningsAsErrors()) {
diagnostic.category = DiagnosticCategory.Error;
}
const { file, start } = diagnostic;
if (file) {
const lineStarts = getLineStarts(file);
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/tsc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,11 @@ namespace ts {

// If we didn't have any syntactic errors, then also try getting the global and
// semantic errors.
if (diagnostics.length === 0) {
diagnostics = program.getOptionsDiagnostics().concat(program.getGlobalDiagnostics());
if (!some(diagnostics, d => d.category === DiagnosticCategory.Error)) {
diagnostics = diagnostics.concat(program.getOptionsDiagnostics().concat(program.getGlobalDiagnostics()));

if (diagnostics.length === 0) {
diagnostics = program.getSemanticDiagnostics().slice();
if (!some(diagnostics, d => d.category === DiagnosticCategory.Error)) {
diagnostics = diagnostics.concat(program.getSemanticDiagnostics());
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3764,6 +3764,8 @@ namespace ts {
/*@internal*/ version?: boolean;
/*@internal*/ watch?: boolean;

treatWarningsAsErrors?: boolean;

[option: string]: CompilerOptionsValue | JsonSourceFile | undefined;
}

Expand Down
9 changes: 5 additions & 4 deletions src/compiler/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,12 @@ namespace ts {
});
}

if (emitSkipped && diagnostics.length > 0) {
const hasError = some(diagnostics, d => d.category === DiagnosticCategory.Error);
if (emitSkipped && hasError) {
// If the emitter didn't emit anything, then pass that value along.
return ExitStatus.DiagnosticsPresent_OutputsSkipped;
}
else if (diagnostics.length > 0) {
else if (hasError) {
// The emitter emitted something, inform the caller if that happened in the presence
// of diagnostics or not.
return ExitStatus.DiagnosticsPresent_OutputsGenerated;
Expand Down Expand Up @@ -149,11 +150,11 @@ namespace ts {

// If we didn't have any syntactic errors, then also try getting the global and
// semantic errors.
if (diagnostics.length === 0) {
if (!some(diagnostics, d => d.category === DiagnosticCategory.Error)) {
addRange(diagnostics, program.getOptionsDiagnostics());
addRange(diagnostics, program.getGlobalDiagnostics());

if (diagnostics.length === 0) {
if (!some(diagnostics, d => d.category === DiagnosticCategory.Error)) {
reportSemanticDiagnostics = true;
}
}
Expand Down
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 @@ -2269,6 +2269,7 @@ declare namespace ts {
types?: string[];
/** Paths used to compute primary types search locations */
typeRoots?: string[];
treatWarningsAsErrors?: boolean;
[option: string]: CompilerOptionsValue | JsonSourceFile | undefined;
}
interface TypeAcquisition {
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2269,6 +2269,7 @@ declare namespace ts {
types?: string[];
/** Paths used to compute primary types search locations */
typeRoots?: string[];
treatWarningsAsErrors?: boolean;
[option: string]: CompilerOptionsValue | JsonSourceFile | undefined;
}
interface TypeAcquisition {
Expand Down
39 changes: 39 additions & 0 deletions tests/baselines/reference/treatWarningsAsErrorsFalse.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
tests/cases/compiler/treatWarningsAsErrorsFalse.ts(1,14): warning TS6133: 'p' is declared but its value is never read.
tests/cases/compiler/treatWarningsAsErrorsFalse.ts(2,5): warning TS7028: Unused label.
tests/cases/compiler/treatWarningsAsErrorsFalse.ts(5,11): warning TS6133: 'y' is declared but its value is never read.
tests/cases/compiler/treatWarningsAsErrorsFalse.ts(7,9): warning TS7029: Fallthrough case in switch.
tests/cases/compiler/treatWarningsAsErrorsFalse.ts(9,14): error TS2678: Type '"b"' is not comparable to type '"a"'.
tests/cases/compiler/treatWarningsAsErrorsFalse.ts(10,13): warning TS7030: Not all code paths return a value.
tests/cases/compiler/treatWarningsAsErrorsFalse.ts(11,13): warning TS7027: Unreachable code detected.


==== tests/cases/compiler/treatWarningsAsErrorsFalse.ts (7 errors) ====
function foo(p: any): string { // unused parameter
~
!!! warning TS6133: 'p' is declared but its value is never read.
foo: while (false) {} // unused label
~~~
!!! warning TS7028: Unused label.

const x: { kind: "a" } | { kind: "b" } = { kind: "a" };
const y = "any"; // unused variable
~
!!! warning TS6133: 'y' is declared but its value is never read.
switch (x.kind) {
case "a":
~~~~
!!! warning TS7029: Fallthrough case in switch.
void x; // implicit fallthrough
case "b":
~~~
!!! error TS2678: Type '"b"' is not comparable to type '"a"'.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't strictly related to the test, but when I went to remove it, I considered that it is useful in verifying that we're not reporting all errors as warnings.

return; // implicit return
~~~~~~~
!!! warning TS7030: Not all code paths return a value.
if (x === x) { // unreachable code
~~
!!! warning TS7027: Unreachable code detected.
void x;
}
}
}
32 changes: 32 additions & 0 deletions tests/baselines/reference/treatWarningsAsErrorsFalse.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//// [treatWarningsAsErrorsFalse.ts]
function foo(p: any): string { // unused parameter
foo: while (false) {} // unused label

const x: { kind: "a" } | { kind: "b" } = { kind: "a" };
const y = "any"; // unused variable
switch (x.kind) {
case "a":
void x; // implicit fallthrough
case "b":
return; // implicit return
if (x === x) { // unreachable code
void x;
}
}
}

//// [treatWarningsAsErrorsFalse.js]
function foo(p) {
foo: while (false) { } // unused label
var x = { kind: "a" };
var y = "any"; // unused variable
switch (x.kind) {
case "a":
void x; // implicit fallthrough
case "b":
return; // implicit return
if (x === x) {
void x;
}
}
}
36 changes: 36 additions & 0 deletions tests/baselines/reference/treatWarningsAsErrorsFalse.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
=== tests/cases/compiler/treatWarningsAsErrorsFalse.ts ===
function foo(p: any): string { // unused parameter
>foo : Symbol(foo, Decl(treatWarningsAsErrorsFalse.ts, 0, 0))
>p : Symbol(p, Decl(treatWarningsAsErrorsFalse.ts, 0, 13))

foo: while (false) {} // unused label

const x: { kind: "a" } | { kind: "b" } = { kind: "a" };
>x : Symbol(x, Decl(treatWarningsAsErrorsFalse.ts, 3, 9))
>kind : Symbol(kind, Decl(treatWarningsAsErrorsFalse.ts, 3, 14))
>kind : Symbol(kind, Decl(treatWarningsAsErrorsFalse.ts, 3, 30))
>kind : Symbol(kind, Decl(treatWarningsAsErrorsFalse.ts, 3, 46))

const y = "any"; // unused variable
>y : Symbol(y, Decl(treatWarningsAsErrorsFalse.ts, 4, 9))

switch (x.kind) {
>x.kind : Symbol(kind, Decl(treatWarningsAsErrorsFalse.ts, 3, 14))
>x : Symbol(x, Decl(treatWarningsAsErrorsFalse.ts, 3, 9))
>kind : Symbol(kind, Decl(treatWarningsAsErrorsFalse.ts, 3, 14))

case "a":
void x; // implicit fallthrough
>x : Symbol(x, Decl(treatWarningsAsErrorsFalse.ts, 3, 9))

case "b":
return; // implicit return
if (x === x) { // unreachable code
>x : Symbol(x, Decl(treatWarningsAsErrorsFalse.ts, 3, 9))
>x : Symbol(x, Decl(treatWarningsAsErrorsFalse.ts, 3, 9))

void x;
>x : Symbol(x, Decl(treatWarningsAsErrorsFalse.ts, 3, 9))
}
}
}
48 changes: 48 additions & 0 deletions tests/baselines/reference/treatWarningsAsErrorsFalse.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
=== tests/cases/compiler/treatWarningsAsErrorsFalse.ts ===
function foo(p: any): string { // unused parameter
>foo : (p: any) => string
>p : any

foo: while (false) {} // unused label
>foo : any
>false : false

const x: { kind: "a" } | { kind: "b" } = { kind: "a" };
>x : { kind: "a"; } | { kind: "b"; }
>kind : "a"
>kind : "b"
>{ kind: "a" } : { kind: "a"; }
>kind : string
>"a" : "a"

const y = "any"; // unused variable
>y : "any"
>"any" : "any"

switch (x.kind) {
>x.kind : "a"
>x : { kind: "a"; }
>kind : "a"

case "a":
>"a" : "a"

void x; // implicit fallthrough
>void x : undefined
>x : { kind: "a"; }

case "b":
>"b" : "b"

return; // implicit return
if (x === x) { // unreachable code
>x === x : boolean
>x : { kind: "a"; } | { kind: "b"; }
>x : { kind: "a"; } | { kind: "b"; }

void x;
>void x : undefined
>x : { kind: "a"; } | { kind: "b"; }
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
tests/cases/compiler/treatWarningsAsErrorsFalseNoEmitOnError.ts(1,14): warning TS6133: 'p' is declared but its value is never read.
tests/cases/compiler/treatWarningsAsErrorsFalseNoEmitOnError.ts(2,5): warning TS7028: Unused label.
tests/cases/compiler/treatWarningsAsErrorsFalseNoEmitOnError.ts(5,11): warning TS6133: 'y' is declared but its value is never read.
tests/cases/compiler/treatWarningsAsErrorsFalseNoEmitOnError.ts(7,9): warning TS7029: Fallthrough case in switch.
tests/cases/compiler/treatWarningsAsErrorsFalseNoEmitOnError.ts(10,13): warning TS7030: Not all code paths return a value.
tests/cases/compiler/treatWarningsAsErrorsFalseNoEmitOnError.ts(11,13): warning TS7027: Unreachable code detected.


==== tests/cases/compiler/treatWarningsAsErrorsFalseNoEmitOnError.ts (6 errors) ====
function foo(p: any): string { // unused parameter
~
!!! warning TS6133: 'p' is declared but its value is never read.
foo: while (false) {} // unused label
~~~
!!! warning TS7028: Unused label.

const x: { kind: "a" } | { kind: "b" } = { kind: "a" };
const y = "any"; // unused variable
~
!!! warning TS6133: 'y' is declared but its value is never read.
switch (x.kind) {
case "a":
~~~~
!!! warning TS7029: Fallthrough case in switch.
void x; // implicit fallthrough
case "a":
return; // implicit return
~~~~~~~
!!! warning TS7030: Not all code paths return a value.
if (x === x) { // unreachable code
~~
!!! warning TS7027: Unreachable code detected.
void x;
}
}
}
Loading