Skip to content

Commit 9918231

Browse files
JoshuaKGoldbergJoshuaKGoldberg
committed
Added @ts-expect-error to @ts-ignore directives
Similar to `// @ts-ignore`, but will itself cause a new error diagnostic if it does not cause an existing diagnostic to be ignored. Technical summary: 1. The scanner will now keep track of `CommentDirective`s it comes across: both `@ts-expect-error` and `@ts-ignore` 2. During type checking, the program will turn those directives into a map keying them by line number 3. For each diagnostic, if it's preceded by a directive, that directive is marked as "used" 4. All `@ts-expect-error` directives not marked as used generate a new diagnostic error
1 parent bb306a7 commit 9918231

17 files changed

+411
-41
lines changed

Diff for: ‎src/compiler/diagnosticMessages.json

+4
Original file line numberDiff line numberDiff line change
@@ -2153,6 +2153,10 @@
21532153
"category": "Error",
21542154
"code": 2577
21552155
},
2156+
"Unused '@ts-expect-error' directive.": {
2157+
"category": "Error",
2158+
"code": 2578
2159+
},
21562160
"Cannot find name '{0}'. Do you need to install type definitions for node? Try `npm i @types/node`.": {
21572161
"category": "Error",
21582162
"code": 2580

Diff for: ‎src/compiler/parser.ts

+2
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,7 @@ namespace ts {
842842

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

@@ -875,6 +876,7 @@ namespace ts {
875876

876877
setExternalModuleIndicator(sourceFile);
877878

879+
sourceFile.commentDirectives = scanner.getCommentDirectives();
878880
sourceFile.nodeCount = nodeCount;
879881
sourceFile.identifierCount = identifierCount;
880882
sourceFile.identifiers = identifiers;

Diff for: ‎src/compiler/program.ts

+57-41
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
namespace ts {
2-
const ignoreDiagnosticCommentRegEx = /(^\s*$)|(^\s*\/\/\/?\s*(@ts-ignore)?)/;
3-
42
export function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName = "tsconfig.json"): string | undefined {
53
return forEachAncestorDirectory(searchPath, ancestor => {
64
const fileName = combinePaths(ancestor, configName);
@@ -1650,17 +1648,16 @@ namespace ts {
16501648
const fileProcessingDiagnosticsInFile = fileProcessingDiagnostics.getDiagnostics(sourceFile.fileName);
16511649
const programDiagnosticsInFile = programDiagnostics.getDiagnostics(sourceFile.fileName);
16521650

1653-
let diagnostics: Diagnostic[] | undefined;
1654-
for (const diags of [fileProcessingDiagnosticsInFile, programDiagnosticsInFile]) {
1655-
if (diags) {
1656-
for (const diag of diags) {
1657-
if (shouldReportDiagnostic(diag)) {
1658-
diagnostics = append(diagnostics, diag);
1659-
}
1660-
}
1661-
}
1651+
return getMergedProgramDiagnostics(sourceFile, fileProcessingDiagnosticsInFile, programDiagnosticsInFile);
1652+
}
1653+
1654+
function getMergedProgramDiagnostics(sourceFile: SourceFile, ...allDiagnostics: (readonly Diagnostic[] | undefined)[]) {
1655+
const flatDiagnostics = flatten(allDiagnostics);
1656+
if (!sourceFile.commentDirectives?.length) {
1657+
return flatDiagnostics;
16621658
}
1663-
return diagnostics || emptyArray;
1659+
1660+
return getDiagnosticsPastDirectives(sourceFile, sourceFile.commentDirectives, flatDiagnostics).diagnostics;
16641661
}
16651662

16661663
function getDeclarationDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): readonly DiagnosticWithLocation[] {
@@ -1738,49 +1735,68 @@ namespace ts {
17381735
const bindDiagnostics: readonly Diagnostic[] = includeBindAndCheckDiagnostics ? sourceFile.bindDiagnostics : emptyArray;
17391736
const checkDiagnostics = includeBindAndCheckDiagnostics ? typeChecker.getDiagnostics(sourceFile, cancellationToken) : emptyArray;
17401737

1741-
let diagnostics: Diagnostic[] | undefined;
1742-
for (const diags of [bindDiagnostics, checkDiagnostics, isCheckJs ? sourceFile.jsDocDiagnostics : undefined]) {
1743-
if (diags) {
1744-
for (const diag of diags) {
1745-
if (shouldReportDiagnostic(diag)) {
1746-
diagnostics = append(diagnostics, diag);
1747-
}
1748-
}
1749-
}
1750-
}
1751-
return diagnostics || emptyArray;
1738+
return getMergedBindAndCheckDiagnostics(sourceFile, bindDiagnostics, checkDiagnostics, isCheckJs ? sourceFile.jsDocDiagnostics : undefined);
17521739
});
17531740
}
17541741

1742+
function getMergedBindAndCheckDiagnostics(sourceFile: SourceFile, ...allDiagnostics: (readonly Diagnostic[] | undefined)[]) {
1743+
const flatDiagnostics = flatten(allDiagnostics);
1744+
if (!sourceFile.commentDirectives?.length) {
1745+
return flatDiagnostics;
1746+
}
1747+
1748+
const { diagnostics, directives } = getDiagnosticsPastDirectives(sourceFile, sourceFile.commentDirectives, flatDiagnostics);
1749+
1750+
for (const errorExpectation of directives.getUnusedExpectations()) {
1751+
diagnostics.push(createDiagnosticForRange(sourceFile, errorExpectation.range, Diagnostics.Unused_ts_expect_error_directive));
1752+
}
1753+
1754+
return diagnostics;
1755+
}
1756+
1757+
function getDiagnosticsPastDirectives(sourceFile: SourceFile, commentDirectives: CommentDirective[], flatDiagnostics: Diagnostic[]) {
1758+
// Diagnostics are only reported if there is no comment directive preceding them
1759+
// This will modify the directives map by marking "used" ones with a corresponding diagnostic
1760+
const directives = createCommentDirectivesMap(sourceFile, commentDirectives);
1761+
const diagnostics = flatDiagnostics.filter(diagnostic => markPrecedingCommentDirectiveLine(diagnostic, directives) === -1);
1762+
1763+
return { diagnostics, directives };
1764+
}
1765+
17551766
function getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken: CancellationToken): readonly DiagnosticWithLocation[] {
17561767
return runWithCancellationToken(() => {
17571768
return getDiagnosticsProducingTypeChecker().getSuggestionDiagnostics(sourceFile, cancellationToken);
17581769
});
17591770
}
17601771

17611772
/**
1762-
* Skip errors if previous line start with '// @ts-ignore' comment, not counting non-empty non-comment lines
1773+
* @returns The line index marked as preceding the diagnostic, or -1 if none was.
17631774
*/
1764-
function shouldReportDiagnostic(diagnostic: Diagnostic) {
1775+
function markPrecedingCommentDirectiveLine(diagnostic: Diagnostic, directives: CommentDirectivesMap) {
17651776
const { file, start } = diagnostic;
1766-
if (file) {
1767-
const lineStarts = getLineStarts(file);
1768-
let { line } = computeLineAndCharacterOfPosition(lineStarts, start!); // TODO: GH#18217
1769-
while (line > 0) {
1770-
const previousLineText = file.text.slice(lineStarts[line - 1], lineStarts[line]);
1771-
const result = ignoreDiagnosticCommentRegEx.exec(previousLineText);
1772-
if (!result) {
1773-
// non-empty line
1774-
return true;
1775-
}
1776-
if (result[3]) {
1777-
// @ts-ignore
1778-
return false;
1779-
}
1780-
line--;
1777+
if (!file) {
1778+
return -1;
1779+
}
1780+
1781+
// Start out with the line just before the text
1782+
const lineStarts = getLineStarts(file);
1783+
let line = computeLineAndCharacterOfPosition(lineStarts, start!).line - 1; // TODO: GH#18217
1784+
while (line >= 0) {
1785+
// As soon as that line is known to have a comment directive, use that
1786+
if (directives.markUsed(line)) {
1787+
return line;
17811788
}
1789+
1790+
// Stop searching if the line is not empty and not a comment
1791+
const lineText = file.text.slice(lineStarts[line - 1], lineStarts[line]).trim();
1792+
if (lineText !== "" && !/^(\s*)\/\/(.*)$/.test(lineText)) {
1793+
return -1;
1794+
}
1795+
1796+
line--;
17821797
}
1783-
return true;
1798+
1799+
return -1;
17841800
}
17851801

17861802
function getJSSyntacticDiagnosticsForFile(sourceFile: SourceFile): DiagnosticWithLocation[] {

Diff for: ‎src/compiler/scanner.ts

+46
Large diffs are not rendered by default.

Diff for: ‎src/compiler/types.ts

+20
Original file line numberDiff line numberDiff line change
@@ -2963,6 +2963,8 @@ namespace ts {
29632963
// This field should never be used directly to obtain line map, use getLineMap function instead.
29642964
/* @internal */ lineMap: readonly number[];
29652965
/* @internal */ classifiableNames?: ReadonlyUnderscoreEscapedMap<true>;
2966+
// Comments comtaining @ts-* directives, in order.
2967+
/* @internal */ commentDirectives?: CommentDirective[];
29662968
// Stores a mapping 'external module reference text' -> 'resolved file name' | undefined
29672969
// It is used to resolve module names in the checker.
29682970
// Content of this field should never be used directly - use getResolvedModuleFileName/setResolvedModuleFileName functions instead
@@ -2982,6 +2984,18 @@ namespace ts {
29822984
/*@internal*/ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
29832985
}
29842986

2987+
/* @internal */
2988+
export interface CommentDirective {
2989+
range: TextRange;
2990+
type: CommentDirectiveType,
2991+
}
2992+
2993+
/* @internal */
2994+
export const enum CommentDirectiveType {
2995+
ExpectError,
2996+
Ignore,
2997+
}
2998+
29852999
/*@internal*/
29863000
export type ExportedModulesFromDeclarationEmit = readonly Symbol[];
29873001

@@ -6566,6 +6580,12 @@ namespace ts {
65666580
forEach(action: <TKey extends keyof PragmaPseudoMap>(value: PragmaPseudoMap[TKey] | PragmaPseudoMap[TKey][], key: TKey) => void): void;
65676581
}
65686582

6583+
/* @internal */
6584+
export interface CommentDirectivesMap {
6585+
getUnusedExpectations(): CommentDirective[];
6586+
markUsed(matchedLine: number): boolean;
6587+
}
6588+
65696589
export interface UserPreferences {
65706590
readonly disableSuggestions?: boolean;
65716591
readonly quotePreference?: "auto" | "double" | "single";

Diff for: ‎src/compiler/utilities.ts

+39
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,34 @@ namespace ts {
466466
text.charCodeAt(start + 2) === CharacterCodes.exclamation;
467467
}
468468

469+
export function createCommentDirectivesMap(sourceFile: SourceFile, commentDirectives: CommentDirective[]): CommentDirectivesMap {
470+
const directivesByLine = createMapFromEntries(
471+
commentDirectives.map(commentDirective => ([
472+
`${getLineAndCharacterOfPosition(sourceFile, commentDirective.range.pos).line}`,
473+
commentDirective,
474+
]))
475+
);
476+
477+
const usedLines = createMap<boolean>();
478+
479+
return { getUnusedExpectations, markUsed };
480+
481+
function getUnusedExpectations() {
482+
return arrayFrom(directivesByLine.entries())
483+
.filter(([line, directive]) => directive.type === CommentDirectiveType.ExpectError && !usedLines.get(line))
484+
.map(([_, directive]) => directive);
485+
}
486+
487+
function markUsed(line: number) {
488+
if (!directivesByLine.has(`${line}`)) {
489+
return false;
490+
}
491+
492+
usedLines.set(`${line}`, true);
493+
return true;
494+
}
495+
}
496+
469497
export function getTokenPosOfNode(node: Node, sourceFile?: SourceFileLike, includeJsDoc?: boolean): number {
470498
// With nodes that have no width (i.e. 'Missing' nodes), we actually *don't*
471499
// want to skip trivia because this will launch us forward to the next token.
@@ -901,6 +929,17 @@ namespace ts {
901929
};
902930
}
903931

932+
export function createDiagnosticForRange(sourceFile: SourceFile, range: TextRange, message: DiagnosticMessage): DiagnosticWithLocation {
933+
return {
934+
file: sourceFile,
935+
start: range.pos,
936+
length: range.end - range.pos,
937+
code: message.code,
938+
category: message.category,
939+
messageText: message.message,
940+
};
941+
}
942+
904943
export function getSpanOfTokenAtPosition(sourceFile: SourceFile, pos: number): TextSpan {
905944
const scanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/ true, sourceFile.languageVariant, sourceFile.text, /*onError:*/ undefined, pos);
906945
scanner.scan();

Diff for: ‎src/services/services.ts

+1
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,7 @@ namespace ts {
614614
private namedDeclarations: Map<Declaration[]> | undefined;
615615
public ambientModuleNames!: string[];
616616
public checkJsDirective: CheckJsDirective | undefined;
617+
public errorExpectations: TextRange[] | undefined;
617618
public possiblyContainDynamicImport?: boolean;
618619
public pragmas!: PragmaMap;
619620
public localJsxFactory: EntityName | undefined;

Diff for: ‎tests/baselines/reference/ts-expect-error.errors.txt

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
tests/cases/conformance/directives/ts-expect-error.ts(4,1): error TS2578: Unused '@ts-expect-error' directive.
2+
tests/cases/conformance/directives/ts-expect-error.ts(10,1): error TS2578: Unused '@ts-expect-error' directive.
3+
tests/cases/conformance/directives/ts-expect-error.ts(13,5): error TS2322: Type '"nope"' is not assignable to type 'number'.
4+
5+
6+
==== tests/cases/conformance/directives/ts-expect-error.ts (3 errors) ====
7+
// @ts-expect-error additional commenting
8+
var invalidCommentedFancy: number = 'nope';
9+
10+
// @ts-expect-error additional commenting
11+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
12+
!!! error TS2578: Unused '@ts-expect-error' directive.
13+
var validCommentedFancy: string = 'nope';
14+
15+
// @ts-expect-error
16+
var invalidCommentedPlain: number = 'nope';
17+
18+
// @ts-expect-error
19+
~~~~~~~~~~~~~~~~~~~
20+
!!! error TS2578: Unused '@ts-expect-error' directive.
21+
var validCommentedPlain: string = 'nope';
22+
23+
var invalidPlain: number = 'nope';
24+
~~~~~~~~~~~~
25+
!!! error TS2322: Type '"nope"' is not assignable to type 'number'.
26+
27+
var validPlain: string = 'nope';
28+

Diff for: ‎tests/baselines/reference/ts-expect-error.js

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//// [ts-expect-error.ts]
2+
// @ts-expect-error additional commenting
3+
var invalidCommentedFancy: number = 'nope';
4+
5+
// @ts-expect-error additional commenting
6+
var validCommentedFancy: string = 'nope';
7+
8+
// @ts-expect-error
9+
var invalidCommentedPlain: number = 'nope';
10+
11+
// @ts-expect-error
12+
var validCommentedPlain: string = 'nope';
13+
14+
var invalidPlain: number = 'nope';
15+
16+
var validPlain: string = 'nope';
17+
18+
19+
//// [ts-expect-error.js]
20+
// @ts-expect-error additional commenting
21+
var invalidCommentedFancy = 'nope';
22+
// @ts-expect-error additional commenting
23+
var validCommentedFancy = 'nope';
24+
// @ts-expect-error
25+
var invalidCommentedPlain = 'nope';
26+
// @ts-expect-error
27+
var validCommentedPlain = 'nope';
28+
var invalidPlain = 'nope';
29+
var validPlain = 'nope';

Diff for: ‎tests/baselines/reference/ts-expect-error.symbols

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
=== tests/cases/conformance/directives/ts-expect-error.ts ===
2+
// @ts-expect-error additional commenting
3+
var invalidCommentedFancy: number = 'nope';
4+
>invalidCommentedFancy : Symbol(invalidCommentedFancy, Decl(ts-expect-error.ts, 1, 3))
5+
6+
// @ts-expect-error additional commenting
7+
var validCommentedFancy: string = 'nope';
8+
>validCommentedFancy : Symbol(validCommentedFancy, Decl(ts-expect-error.ts, 4, 3))
9+
10+
// @ts-expect-error
11+
var invalidCommentedPlain: number = 'nope';
12+
>invalidCommentedPlain : Symbol(invalidCommentedPlain, Decl(ts-expect-error.ts, 7, 3))
13+
14+
// @ts-expect-error
15+
var validCommentedPlain: string = 'nope';
16+
>validCommentedPlain : Symbol(validCommentedPlain, Decl(ts-expect-error.ts, 10, 3))
17+
18+
var invalidPlain: number = 'nope';
19+
>invalidPlain : Symbol(invalidPlain, Decl(ts-expect-error.ts, 12, 3))
20+
21+
var validPlain: string = 'nope';
22+
>validPlain : Symbol(validPlain, Decl(ts-expect-error.ts, 14, 3))
23+

Diff for: ‎tests/baselines/reference/ts-expect-error.types

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
=== tests/cases/conformance/directives/ts-expect-error.ts ===
2+
// @ts-expect-error additional commenting
3+
var invalidCommentedFancy: number = 'nope';
4+
>invalidCommentedFancy : number
5+
>'nope' : "nope"
6+
7+
// @ts-expect-error additional commenting
8+
var validCommentedFancy: string = 'nope';
9+
>validCommentedFancy : string
10+
>'nope' : "nope"
11+
12+
// @ts-expect-error
13+
var invalidCommentedPlain: number = 'nope';
14+
>invalidCommentedPlain : number
15+
>'nope' : "nope"
16+
17+
// @ts-expect-error
18+
var validCommentedPlain: string = 'nope';
19+
>validCommentedPlain : string
20+
>'nope' : "nope"
21+
22+
var invalidPlain: number = 'nope';
23+
>invalidPlain : number
24+
>'nope' : "nope"
25+
26+
var validPlain: string = 'nope';
27+
>validPlain : string
28+
>'nope' : "nope"
29+

Diff for: ‎tests/baselines/reference/ts-ignore.errors.txt

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
tests/cases/conformance/directives/ts-ignore.ts(13,5): error TS2322: Type '"nope"' is not assignable to type 'number'.
2+
3+
4+
==== tests/cases/conformance/directives/ts-ignore.ts (1 errors) ====
5+
// @ts-ignore with additional commenting
6+
var invalidCommentedFancy: number = 'nope';
7+
8+
// @ts-ignore with additional commenting
9+
var validCommentedFancy: string = 'nope';
10+
11+
// @ts-ignore
12+
var invalidCommentedPlain: number = 'nope';
13+
14+
// @ts-ignore
15+
var validCommentedPlain: string = 'nope';
16+
17+
var invalidPlain: number = 'nope';
18+
~~~~~~~~~~~~
19+
!!! error TS2322: Type '"nope"' is not assignable to type 'number'.
20+
21+
var validPlain: string = 'nope';
22+

0 commit comments

Comments
 (0)