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

Added @ts-expect-error to @ts-ignore directives #36014

Merged
merged 4 commits into from
Mar 5, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2245,6 +2245,10 @@
"category": "Error",
"code": 2577
},
"Unused '@ts-expect-error' directive.": {
"category": "Error",
"code": 2578
},
"Cannot find name '{0}'. Do you need to install type definitions for node? Try `npm i @types/node`.": {
"category": "Error",
"code": 2580
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,7 @@ namespace ts {

function clearState() {
// Clear out the text the scanner is pointing at, so it doesn't keep anything alive unnecessarily.
scanner.clearCommentDirectives();
scanner.setText("");
scanner.setOnError(undefined);

Expand Down Expand Up @@ -948,6 +949,7 @@ namespace ts {

setExternalModuleIndicator(sourceFile);

sourceFile.commentDirectives = scanner.getCommentDirectives();
sourceFile.nodeCount = nodeCount;
sourceFile.identifierCount = identifierCount;
sourceFile.identifiers = identifiers;
Expand Down
98 changes: 57 additions & 41 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
namespace ts {
const ignoreDiagnosticCommentRegEx = /(^\s*$)|(^\s*\/\/\/?\s*(@ts-ignore)?)/;

export function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName = "tsconfig.json"): string | undefined {
return forEachAncestorDirectory(searchPath, ancestor => {
const fileName = combinePaths(ancestor, configName);
Expand Down Expand Up @@ -1661,17 +1659,16 @@ namespace ts {
const fileProcessingDiagnosticsInFile = fileProcessingDiagnostics.getDiagnostics(sourceFile.fileName);
const programDiagnosticsInFile = programDiagnostics.getDiagnostics(sourceFile.fileName);

let diagnostics: Diagnostic[] | undefined;
for (const diags of [fileProcessingDiagnosticsInFile, programDiagnosticsInFile]) {
if (diags) {
for (const diag of diags) {
if (shouldReportDiagnostic(diag)) {
diagnostics = append(diagnostics, diag);
}
}
}
return getMergedProgramDiagnostics(sourceFile, fileProcessingDiagnosticsInFile, programDiagnosticsInFile);
}

function getMergedProgramDiagnostics(sourceFile: SourceFile, ...allDiagnostics: (readonly Diagnostic[] | undefined)[]) {
const flatDiagnostics = flatten(allDiagnostics);
if (!sourceFile.commentDirectives?.length) {
return flatDiagnostics;
}
return diagnostics || emptyArray;

return getDiagnosticsPastDirectives(sourceFile, sourceFile.commentDirectives, flatDiagnostics).diagnostics;
}

function getDeclarationDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): readonly DiagnosticWithLocation[] {
Expand Down Expand Up @@ -1749,49 +1746,68 @@ namespace ts {
const bindDiagnostics: readonly Diagnostic[] = includeBindAndCheckDiagnostics ? sourceFile.bindDiagnostics : emptyArray;
const checkDiagnostics = includeBindAndCheckDiagnostics ? typeChecker.getDiagnostics(sourceFile, cancellationToken) : emptyArray;

let diagnostics: Diagnostic[] | undefined;
for (const diags of [bindDiagnostics, checkDiagnostics, isCheckJs ? sourceFile.jsDocDiagnostics : undefined]) {
if (diags) {
for (const diag of diags) {
if (shouldReportDiagnostic(diag)) {
diagnostics = append(diagnostics, diag);
}
}
}
}
return diagnostics || emptyArray;
return getMergedBindAndCheckDiagnostics(sourceFile, bindDiagnostics, checkDiagnostics, isCheckJs ? sourceFile.jsDocDiagnostics : undefined);
});
}

function getMergedBindAndCheckDiagnostics(sourceFile: SourceFile, ...allDiagnostics: (readonly Diagnostic[] | undefined)[]) {
const flatDiagnostics = flatten(allDiagnostics);
if (!sourceFile.commentDirectives?.length) {
return flatDiagnostics;
}

const { diagnostics, directives } = getDiagnosticsPastDirectives(sourceFile, sourceFile.commentDirectives, flatDiagnostics);

for (const errorExpectation of directives.getUnusedExpectations()) {
diagnostics.push(createDiagnosticForRange(sourceFile, errorExpectation.range, Diagnostics.Unused_ts_expect_error_directive));
}

return diagnostics;
}

function getDiagnosticsPastDirectives(sourceFile: SourceFile, commentDirectives: CommentDirective[], flatDiagnostics: Diagnostic[]) {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
// Diagnostics are only reported if there is no comment directive preceding them
// This will modify the directives map by marking "used" ones with a corresponding diagnostic
const directives = createCommentDirectivesMap(sourceFile, commentDirectives);
const diagnostics = flatDiagnostics.filter(diagnostic => markPrecedingCommentDirectiveLine(diagnostic, directives) === -1);

return { diagnostics, directives };
}

function getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken: CancellationToken): readonly DiagnosticWithLocation[] {
return runWithCancellationToken(() => {
return getDiagnosticsProducingTypeChecker().getSuggestionDiagnostics(sourceFile, cancellationToken);
});
}

/**
* Skip errors if previous line start with '// @ts-ignore' comment, not counting non-empty non-comment lines
* @returns The line index marked as preceding the diagnostic, or -1 if none was.
*/
function shouldReportDiagnostic(diagnostic: Diagnostic) {
function markPrecedingCommentDirectiveLine(diagnostic: Diagnostic, directives: CommentDirectivesMap) {
const { file, start } = diagnostic;
if (file) {
const lineStarts = getLineStarts(file);
let { line } = computeLineAndCharacterOfPosition(lineStarts, start!); // TODO: GH#18217
while (line > 0) {
const previousLineText = file.text.slice(lineStarts[line - 1], lineStarts[line]);
const result = ignoreDiagnosticCommentRegEx.exec(previousLineText);
if (!result) {
// non-empty line
return true;
}
if (result[3]) {
// @ts-ignore
return false;
}
line--;
if (!file) {
return -1;
}

// Start out with the line just before the text
const lineStarts = getLineStarts(file);
let line = computeLineAndCharacterOfPosition(lineStarts, start!).line - 1; // TODO: GH#18217
while (line >= 0) {
// As soon as that line is known to have a comment directive, use that
if (directives.markUsed(line)) {
return line;
}

// Stop searching if the line is not empty and not a comment
const lineText = file.text.slice(lineStarts[line - 1], lineStarts[line]).trim();
if (lineText !== "" && !/^(\s*)\/\/(.*)$/.test(lineText)) {
return -1;
}

line--;
}
return true;

return -1;
}

function getJSSyntacticDiagnosticsForFile(sourceFile: SourceFile): DiagnosticWithLocation[] {
Expand Down
46 changes: 46 additions & 0 deletions src/compiler/scanner.ts

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2990,6 +2990,8 @@ namespace ts {
// This field should never be used directly to obtain line map, use getLineMap function instead.
/* @internal */ lineMap: readonly number[];
/* @internal */ classifiableNames?: ReadonlyUnderscoreEscapedMap<true>;
// Comments containing @ts-* directives, in order.
/* @internal */ commentDirectives?: CommentDirective[];
// Stores a mapping 'external module reference text' -> 'resolved file name' | undefined
// It is used to resolve module names in the checker.
// Content of this field should never be used directly - use getResolvedModuleFileName/setResolvedModuleFileName functions instead
Expand All @@ -3009,6 +3011,18 @@ namespace ts {
/*@internal*/ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
}

/* @internal */
export interface CommentDirective {
range: TextRange;
type: CommentDirectiveType,
}

/* @internal */
export const enum CommentDirectiveType {
ExpectError,
Ignore,
}

/*@internal*/
export type ExportedModulesFromDeclarationEmit = readonly Symbol[];

Expand Down Expand Up @@ -6666,6 +6680,12 @@ namespace ts {
forEach(action: <TKey extends keyof PragmaPseudoMap>(value: PragmaPseudoMap[TKey] | PragmaPseudoMap[TKey][], key: TKey) => void): void;
}

/* @internal */
export interface CommentDirectivesMap {
getUnusedExpectations(): CommentDirective[];
markUsed(matchedLine: number): boolean;
}

export interface UserPreferences {
readonly disableSuggestions?: boolean;
readonly quotePreference?: "auto" | "double" | "single";
Expand Down
39 changes: 39 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,34 @@ namespace ts {
text.charCodeAt(start + 2) === CharacterCodes.exclamation;
}

export function createCommentDirectivesMap(sourceFile: SourceFile, commentDirectives: CommentDirective[]): CommentDirectivesMap {
const directivesByLine = createMapFromEntries(
commentDirectives.map(commentDirective => ([
`${getLineAndCharacterOfPosition(sourceFile, commentDirective.range.pos).line}`,
commentDirective,
]))
);

const usedLines = createMap<boolean>();

return { getUnusedExpectations, markUsed };

function getUnusedExpectations() {
return arrayFrom(directivesByLine.entries())
.filter(([line, directive]) => directive.type === CommentDirectiveType.ExpectError && !usedLines.get(line))
.map(([_, directive]) => directive);
}

function markUsed(line: number) {
if (!directivesByLine.has(`${line}`)) {
return false;
}

usedLines.set(`${line}`, true);
return true;
}
}

export function getTokenPosOfNode(node: Node, sourceFile?: SourceFileLike, includeJsDoc?: boolean): number {
// With nodes that have no width (i.e. 'Missing' nodes), we actually *don't*
// want to skip trivia because this will launch us forward to the next token.
Expand Down Expand Up @@ -914,6 +942,17 @@ namespace ts {
};
}

export function createDiagnosticForRange(sourceFile: SourceFile, range: TextRange, message: DiagnosticMessage): DiagnosticWithLocation {
return {
file: sourceFile,
start: range.pos,
length: range.end - range.pos,
code: message.code,
category: message.category,
messageText: message.message,
};
}

export function getSpanOfTokenAtPosition(sourceFile: SourceFile, pos: number): TextSpan {
const scanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/ true, sourceFile.languageVariant, sourceFile.text, /*onError:*/ undefined, pos);
scanner.scan();
Expand Down
1 change: 1 addition & 0 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,7 @@ namespace ts {
private namedDeclarations: Map<Declaration[]> | undefined;
public ambientModuleNames!: string[];
public checkJsDirective: CheckJsDirective | undefined;
public errorExpectations: TextRange[] | undefined;
public possiblyContainDynamicImport?: boolean;
public pragmas!: PragmaMap;
public localJsxFactory: EntityName | undefined;
Expand Down
28 changes: 28 additions & 0 deletions tests/baselines/reference/ts-expect-error.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
tests/cases/conformance/directives/ts-expect-error.ts(4,1): error TS2578: Unused '@ts-expect-error' directive.
tests/cases/conformance/directives/ts-expect-error.ts(10,1): error TS2578: Unused '@ts-expect-error' directive.
tests/cases/conformance/directives/ts-expect-error.ts(13,5): error TS2322: Type '"nope"' is not assignable to type 'number'.


==== tests/cases/conformance/directives/ts-expect-error.ts (3 errors) ====
// @ts-expect-error additional commenting
var invalidCommentedFancy: number = 'nope';

// @ts-expect-error additional commenting
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2578: Unused '@ts-expect-error' directive.
var validCommentedFancy: string = 'nope';

// @ts-expect-error
var invalidCommentedPlain: number = 'nope';

// @ts-expect-error
~~~~~~~~~~~~~~~~~~~
!!! error TS2578: Unused '@ts-expect-error' directive.
var validCommentedPlain: string = 'nope';

var invalidPlain: number = 'nope';
~~~~~~~~~~~~
!!! error TS2322: Type '"nope"' is not assignable to type 'number'.

var validPlain: string = 'nope';

29 changes: 29 additions & 0 deletions tests/baselines/reference/ts-expect-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [ts-expect-error.ts]
// @ts-expect-error additional commenting
var invalidCommentedFancy: number = 'nope';

// @ts-expect-error additional commenting
var validCommentedFancy: string = 'nope';

// @ts-expect-error
var invalidCommentedPlain: number = 'nope';

// @ts-expect-error
var validCommentedPlain: string = 'nope';

var invalidPlain: number = 'nope';

var validPlain: string = 'nope';


//// [ts-expect-error.js]
// @ts-expect-error additional commenting
var invalidCommentedFancy = 'nope';
// @ts-expect-error additional commenting
var validCommentedFancy = 'nope';
// @ts-expect-error
var invalidCommentedPlain = 'nope';
// @ts-expect-error
var validCommentedPlain = 'nope';
var invalidPlain = 'nope';
var validPlain = 'nope';
23 changes: 23 additions & 0 deletions tests/baselines/reference/ts-expect-error.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
=== tests/cases/conformance/directives/ts-expect-error.ts ===
// @ts-expect-error additional commenting
var invalidCommentedFancy: number = 'nope';
>invalidCommentedFancy : Symbol(invalidCommentedFancy, Decl(ts-expect-error.ts, 1, 3))

// @ts-expect-error additional commenting
var validCommentedFancy: string = 'nope';
>validCommentedFancy : Symbol(validCommentedFancy, Decl(ts-expect-error.ts, 4, 3))

// @ts-expect-error
var invalidCommentedPlain: number = 'nope';
>invalidCommentedPlain : Symbol(invalidCommentedPlain, Decl(ts-expect-error.ts, 7, 3))

// @ts-expect-error
var validCommentedPlain: string = 'nope';
>validCommentedPlain : Symbol(validCommentedPlain, Decl(ts-expect-error.ts, 10, 3))

var invalidPlain: number = 'nope';
>invalidPlain : Symbol(invalidPlain, Decl(ts-expect-error.ts, 12, 3))

var validPlain: string = 'nope';
>validPlain : Symbol(validPlain, Decl(ts-expect-error.ts, 14, 3))

29 changes: 29 additions & 0 deletions tests/baselines/reference/ts-expect-error.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
=== tests/cases/conformance/directives/ts-expect-error.ts ===
// @ts-expect-error additional commenting
var invalidCommentedFancy: number = 'nope';
>invalidCommentedFancy : number
>'nope' : "nope"

// @ts-expect-error additional commenting
var validCommentedFancy: string = 'nope';
>validCommentedFancy : string
>'nope' : "nope"

// @ts-expect-error
var invalidCommentedPlain: number = 'nope';
>invalidCommentedPlain : number
>'nope' : "nope"

// @ts-expect-error
var validCommentedPlain: string = 'nope';
>validCommentedPlain : string
>'nope' : "nope"

var invalidPlain: number = 'nope';
>invalidPlain : number
>'nope' : "nope"

var validPlain: string = 'nope';
>validPlain : string
>'nope' : "nope"

22 changes: 22 additions & 0 deletions tests/baselines/reference/ts-ignore.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
tests/cases/conformance/directives/ts-ignore.ts(13,5): error TS2322: Type '"nope"' is not assignable to type 'number'.


==== tests/cases/conformance/directives/ts-ignore.ts (1 errors) ====
// @ts-ignore with additional commenting
var invalidCommentedFancy: number = 'nope';

// @ts-ignore with additional commenting
var validCommentedFancy: string = 'nope';

// @ts-ignore
var invalidCommentedPlain: number = 'nope';

// @ts-ignore
var validCommentedPlain: string = 'nope';

var invalidPlain: number = 'nope';
~~~~~~~~~~~~
!!! error TS2322: Type '"nope"' is not assignable to type 'number'.

var validPlain: string = 'nope';

Loading