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

Allow skipping diagnostics in .js file using comments and quick fixes to add them #14568

Merged
merged 9 commits into from
Mar 14, 2017
2 changes: 1 addition & 1 deletion Jakefile.js
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,7 @@ task("generate-code-coverage", ["tests", builtLocalDirectory], function () {
// Browser tests
var nodeServerOutFile = "tests/webTestServer.js";
var nodeServerInFile = "tests/webTestServer.ts";
compileFile(nodeServerOutFile, [nodeServerInFile], [builtLocalDirectory, tscFile], [], /*useBuiltCompiler:*/ true, { noOutFile: true });
compileFile(nodeServerOutFile, [nodeServerInFile], [builtLocalDirectory, tscFile], [], /*useBuiltCompiler:*/ true, { noOutFile: true, lib: "es6" });

desc("Runs browserify on run.js to produce a file suitable for running tests in the browser");
task("browserify", ["tests", builtLocalDirectory, nodeServerOutFile], function() {
Expand Down
10 changes: 10 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3355,6 +3355,16 @@
"category": "Message",
"code": 90017
},
"Disable checking for this file.": {
"category": "Message",
"code": 90018
},
"Suppress this error message.": {
"category": "Message",
"code": 90019
},


"Octal literal types must use ES2015 syntax. Use the syntax '{0}'.": {
"category": "Error",
"code": 8017
Expand Down
29 changes: 28 additions & 1 deletion src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace ts {
const emptyArray: any[] = [];
const suppressDiagnosticCommentRegEx = /(^\s*$)|(^\s*\/\/\/?\s*(@ts-suppress)?)/;

export function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName = "tsconfig.json"): string {
while (true) {
Expand Down Expand Up @@ -923,10 +924,36 @@ namespace ts {
const fileProcessingDiagnosticsInFile = fileProcessingDiagnostics.getDiagnostics(sourceFile.fileName);
const programDiagnosticsInFile = programDiagnostics.getDiagnostics(sourceFile.fileName);

return bindDiagnostics.concat(checkDiagnostics, fileProcessingDiagnosticsInFile, programDiagnosticsInFile);
const diagnostics = bindDiagnostics.concat(checkDiagnostics, fileProcessingDiagnosticsInFile, programDiagnosticsInFile);
return isSourceFileJavaScript(sourceFile)
? filter(diagnostics, shouldReportDiagnostic)
: diagnostics;
});
}

/**
* Skip errors if previous line start with '// @ts-suppress' comment, not counting non-empty non-comment lines
*/
function shouldReportDiagnostic(diagnostic: Diagnostic) {
const { file, start } = diagnostic;
const lineStarts = getLineStarts(file);
let { line } = computeLineAndCharacterOfPosition(lineStarts, start);
while (line > 0) {
const previousLineText = file.text.slice(lineStarts[line - 1], lineStarts[line]);
const result = suppressDiagnosticCommentRegEx.exec(previousLineText);
if (!result) {
// non-empty line
return true;
}
if (result[3]) {
// @ts-suppress
return false;
}
line--;
}
return true;
}

function getJavaScriptSyntacticDiagnosticsForFile(sourceFile: SourceFile): Diagnostic[] {
return runWithCancellationToken(() => {
const diagnostics: Diagnostic[] = [];
Expand Down
74 changes: 74 additions & 0 deletions src/services/codefixes/disableJsDiagnostics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/* @internal */
namespace ts.codefix {
registerCodeFix({
errorCodes: getApplicableDiagnosticCodes(),
getCodeActions: getDisableJsDiagnosticsCodeActions
});

function getApplicableDiagnosticCodes(): number[] {
const allDiagnostcs = <MapLike<DiagnosticMessage>>Diagnostics;
return Object.keys(allDiagnostcs)
.filter(d => allDiagnostcs[d] && allDiagnostcs[d].category === DiagnosticCategory.Error)
.map(d => allDiagnostcs[d].code);
}

function shouldCheckJsFile(sourceFile: SourceFile, compilerOptions: CompilerOptions) {
return sourceFile.checkJsDirective ? sourceFile.checkJsDirective.enabled : compilerOptions.checkJs;
}

function getSuppressCommentLocationForLocation(sourceFile: SourceFile, position: number, newLineCharacter: string) {
let { line } = getLineAndCharacterOfPosition(sourceFile, position);
const lineStartPosition = getStartPositionOfLine(line, sourceFile);
const startPosition = getFirstNonSpaceCharacterPosition(sourceFile.text, lineStartPosition);

// First try to see if we can put the '// @ts-suppress' on the previous line.
// We need to make sure that we are not in the middle of a string literal or a comment.
// We also want to check if the previous line holds a comment for a node on the next line
// if so, we do not want to separate the node from its comment if we can.
if (!isInComment(sourceFile, startPosition) && !isInString(sourceFile, startPosition) && !isInTemplateString(sourceFile, startPosition)) {
const token = getTouchingToken(sourceFile, startPosition);
const tokenLeadingCommnets = getLeadingCommentRangesOfNode(token, sourceFile);
if (!tokenLeadingCommnets || !tokenLeadingCommnets.length || tokenLeadingCommnets[0].pos >= startPosition) {
return {
span: { start: startPosition, length: 0 },
newText: `// @ts-suppress${newLineCharacter}`
};
}
}

// If all fails, add an extra new line immediatlly before the error span.
return {
span: { start: position, length: 0 },
newText: `${position === startPosition ? "" : newLineCharacter}// @ts-suppress${newLineCharacter}`
};
}

function getDisableJsDiagnosticsCodeActions(context: CodeFixContext): CodeAction[] | undefined {
const { sourceFile, program, newLineCharacter, span } = context;

if (!isInJavaScriptFile(sourceFile) || !shouldCheckJsFile(sourceFile, program.getCompilerOptions())) {
return undefined;
}

return [{
description: getLocaleSpecificMessage(Diagnostics.Suppress_this_error_message),
changes: [{
fileName: sourceFile.fileName,
textChanges: [getSuppressCommentLocationForLocation(sourceFile, span.start, newLineCharacter)]
}]
},
{
description: getLocaleSpecificMessage(Diagnostics.Disable_checking_for_this_file),
changes: [{
fileName: sourceFile.fileName,
textChanges: [{
span: {
start: sourceFile.checkJsDirective ? sourceFile.checkJsDirective.pos : 0,
length: sourceFile.checkJsDirective ? sourceFile.checkJsDirective.end - sourceFile.checkJsDirective.pos : 0
},
newText: `// @ts-nocheck${newLineCharacter}`
}]
}]
}];
}
}
1 change: 1 addition & 0 deletions src/services/codefixes/fixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@
/// <reference path="fixForgottenThisPropertyAccess.ts" />
/// <reference path='unusedIdentifierFixes.ts' />
/// <reference path='importFixes.ts' />
/// <reference path='disableJsDiagnostics.ts' />
/// <reference path='helpers.ts' />
12 changes: 3 additions & 9 deletions src/services/codefixes/unusedIdentifierFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ namespace ts.codefix {
else {
// import |d,| * as ns from './file'
const start = importClause.name.getStart();
let end = findFirstNonSpaceCharPosStarting(importClause.name.end);
const text = sourceFile.text;
let end = getFirstNonSpaceCharacterPosition(text, importClause.name.end);
if (sourceFile.text.charCodeAt(end) === CharacterCodes.comma) {
end = findFirstNonSpaceCharPosStarting(end + 1);
end = getFirstNonSpaceCharacterPosition(text, end + 1);
}

return createCodeFix("", start, end - start);
Expand Down Expand Up @@ -166,13 +167,6 @@ namespace ts.codefix {
return createCodeFix("", start, end - start);
}

function findFirstNonSpaceCharPosStarting(start: number) {
while (isWhiteSpace(sourceFile.text.charCodeAt(start))) {
start += 1;
}
return start;
}

function createCodeFix(newText: string, start: number, length: number): CodeAction[] {
return [{
description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Remove_declaration_for_Colon_0), { 0: token.getText() }),
Expand Down
3 changes: 2 additions & 1 deletion src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
"codefixes/fixes.ts",
"codefixes/helpers.ts",
"codefixes/importFixes.ts",
"codefixes/unusedIdentifierFixes.ts"
"codefixes/unusedIdentifierFixes.ts",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to add an entry in src/harness/tsconfig.json as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added.

"codefixes/disableJsDiagnostics.ts"
]
}
7 changes: 7 additions & 0 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1388,4 +1388,11 @@ namespace ts {
// First token is the open curly, this is where we want to put the 'super' call.
return constructor.body.getFirstToken(sourceFile).getEnd();
}

export function getFirstNonSpaceCharacterPosition(text: string, position: number) {
while (isWhiteSpace(text.charCodeAt(position))) {
position += 1;
}
return position;
}
}
38 changes: 38 additions & 0 deletions tests/baselines/reference/checkJsFiles_skipDiagnostics.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
=== tests/cases/compiler/a.js ===

var x = 0;
>x : Symbol(x, Decl(a.js, 1, 3))


/// @ts-suppress
x();
>x : Symbol(x, Decl(a.js, 1, 3))

/// @ts-suppress
x();
>x : Symbol(x, Decl(a.js, 1, 3))

/// @ts-suppress
x(
>x : Symbol(x, Decl(a.js, 1, 3))

2,
3);



// @ts-suppress
// come comment
// some other comment

// @anohter

x();
>x : Symbol(x, Decl(a.js, 1, 3))



// @ts-suppress: no call signature

Choose a reason for hiding this comment

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

This is just an arbitrary message rather than something indicating a specific diagnostic to suppress, right?

Could be useful to have a way to give a specific one to hide (say, something matching @ts-suppress\(T(\d+)\)), in a similar way how linter comments have a way to give a specific rule to disable; but that could be a future enhancement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is what i had in mind with test. but the error code seemed fairly useless. like you need to go somewhere to know what TS 20124 means. Linter rules usually have descriptive names.

I think this is something we can target for in the future.

x();
>x : Symbol(x, Decl(a.js, 1, 3))

47 changes: 47 additions & 0 deletions tests/baselines/reference/checkJsFiles_skipDiagnostics.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
=== tests/cases/compiler/a.js ===

var x = 0;
>x : number
>0 : 0


/// @ts-suppress
x();
>x() : any
>x : number

/// @ts-suppress
x();
>x() : any
>x : number

/// @ts-suppress
x(
>x( 2, 3) : any
>x : number

2,
>2 : 2

3);
>3 : 3



// @ts-suppress
// come comment
// some other comment

// @anohter

x();
>x() : any
>x : number



// @ts-suppress: no call signature
x();
>x() : any
>x : number

33 changes: 33 additions & 0 deletions tests/cases/compiler/checkJsFiles_skipDiagnostics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// @allowJs: true
// @checkJs: true
// @noEmit: true

// @fileName: a.js
var x = 0;


/// @ts-suppress
x();

Choose a reason for hiding this comment

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

Is it supposed to work in .ts files too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope. js only and only for semantic errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually wanted this functionality in .ts a few days ago -- embedding a chunk of stable JS code in TS code.

Sometimes you need to embed code rather than have it as an external module, for encapsulation. Such as, I was writing a plugin that needed to use compression, and I've embedded LZW algo in my ts plugin.

Of course LZW algo in JS needed a bunch of : any kicks to pass TSC checks. Would be great to be able to exclude chunks of code in TS from checks in such cases.


/// @ts-suppress
x();

/// @ts-suppress
x(
2,
3);



// @ts-suppress
// come comment
// some other comment

// @anohter

x();



// @ts-suppress: no call signature
x();
11 changes: 11 additions & 0 deletions tests/cases/fourslash/codeFixDisableJsDiagnosticsInFile.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

// @allowjs: true
// @noEmit: true

// @Filename: a.js
////[|// @ts-check|]
////var x = "";
////x = 1;

verify.rangeAfterCodeFix("// @ts-nocheck", /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 1);
15 changes: 15 additions & 0 deletions tests/cases/fourslash/codeFixDisableJsDiagnosticsInFile2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/// <reference path='fourslash.ts' />

// @allowjs: true
// @noEmit: true
// @checkJs: true

// @Filename: a.js
////[|var x = "";
////x = 1;|]

// Disable checking for the whole file
verify.rangeAfterCodeFix(`// @ts-nocheck
var x = "";
x = 1;`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 1);

14 changes: 14 additions & 0 deletions tests/cases/fourslash/codeFixDisableJsDiagnosticsInFile3.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path='fourslash.ts' />

// @allowjs: true
// @noEmit: true
// @checkJs: true

// @Filename: a.js
////[|var x = "";
////x = 1;|]

// Disable checking for next line
verify.rangeAfterCodeFix(`var x = "";
// @ts-suppress
x = 1;`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);
18 changes: 18 additions & 0 deletions tests/cases/fourslash/codeFixDisableJsDiagnosticsInFile4.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

// @allowjs: true
// @noEmit: true
// @checkJs: true

// @Filename: a.js
////var x = "";
////
////[|"test \
////"; x = 1;|]

// Disable checking for next line
verify.rangeAfterCodeFix(`"test \\
";
// @ts-suppress
x = 1;`, /*includeWhiteSpace*/ false, /*errorCode*/ undefined, /*index*/ 0);

Loading