Skip to content

Commit 997991f

Browse files
authored
Merge pull request #24529 from Microsoft/suggestionDiagnosticsToken
Suggestion diagnostics to wire cancellationToken
2 parents fd24225 + eab21e5 commit 997991f

File tree

7 files changed

+80
-50
lines changed

7 files changed

+80
-50
lines changed

src/compiler/checker.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -310,21 +310,34 @@ namespace ts {
310310
const node = getParseTreeNode(nodeIn, isTypeNode);
311311
return node && getTypeArgumentConstraint(node);
312312
},
313+
getSuggestionDiagnostics: (file, ct) => {
314+
let diagnostics: DiagnosticWithLocation[] | undefined;
315+
try {
316+
// Record the cancellation token so it can be checked later on during checkSourceElement.
317+
// Do this in a finally block so we can ensure that it gets reset back to nothing after
318+
// this call is done.
319+
cancellationToken = ct;
313320

314-
getSuggestionDiagnostics: file => {
315-
return (suggestionDiagnostics.get(file.fileName) || emptyArray).concat(getUnusedDiagnostics());
316-
function getUnusedDiagnostics(): ReadonlyArray<DiagnosticWithLocation> {
317-
if (file.isDeclarationFile) return emptyArray;
318-
321+
// Ensure file is type checked
319322
checkSourceFile(file);
320-
const diagnostics: DiagnosticWithLocation[] = [];
321323
Debug.assert(!!(getNodeLinks(file).flags & NodeCheckFlags.TypeChecked));
324+
325+
diagnostics = addRange(diagnostics, suggestionDiagnostics.get(file.fileName));
326+
if (!file.isDeclarationFile && (!unusedIsError(UnusedKind.Local) || !unusedIsError(UnusedKind.Parameter))) {
327+
addUnusedDiagnostics();
328+
}
329+
return diagnostics || emptyArray;
330+
}
331+
finally {
332+
cancellationToken = undefined;
333+
}
334+
335+
function addUnusedDiagnostics() {
322336
checkUnusedIdentifiers(getPotentiallyUnusedIdentifiers(file), (kind, diag) => {
323337
if (!unusedIsError(kind)) {
324-
diagnostics.push({ ...diag, category: DiagnosticCategory.Suggestion });
338+
(diagnostics || (diagnostics = [])).push({ ...diag, category: DiagnosticCategory.Suggestion });
325339
}
326340
});
327-
return diagnostics;
328341
}
329342
},
330343

src/compiler/program.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,7 @@ namespace ts {
683683
getOptionsDiagnostics,
684684
getGlobalDiagnostics,
685685
getSemanticDiagnostics,
686+
getSuggestionDiagnostics,
686687
getDeclarationDiagnostics,
687688
getTypeChecker,
688689
getClassifiableNames,
@@ -1422,6 +1423,12 @@ namespace ts {
14221423
});
14231424
}
14241425

1426+
function getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken: CancellationToken): ReadonlyArray<DiagnosticWithLocation> {
1427+
return runWithCancellationToken(() => {
1428+
return getDiagnosticsProducingTypeChecker().getSuggestionDiagnostics(sourceFile, cancellationToken);
1429+
});
1430+
}
1431+
14251432
/**
14261433
* Skip errors if previous line start with '// @ts-ignore' comment, not counting non-empty non-comment lines
14271434
*/

src/compiler/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2755,6 +2755,7 @@ namespace ts {
27552755
getSemanticDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<Diagnostic>;
27562756
getDeclarationDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<DiagnosticWithLocation>;
27572757
getConfigFileParsingDiagnostics(): ReadonlyArray<Diagnostic>;
2758+
/* @internal */ getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<DiagnosticWithLocation>;
27582759

27592760
/**
27602761
* Gets a type checker that can be used to semantically analyze source files in the program.
@@ -3076,7 +3077,7 @@ namespace ts {
30763077
* Does *not* get *all* suggestion diagnostics, just the ones that were convenient to report in the checker.
30773078
* Others are added in computeSuggestionDiagnostics.
30783079
*/
3079-
/* @internal */ getSuggestionDiagnostics(file: SourceFile): ReadonlyArray<DiagnosticWithLocation>;
3080+
/* @internal */ getSuggestionDiagnostics(file: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<DiagnosticWithLocation>;
30803081

30813082
/**
30823083
* Depending on the operation performed, it may be appropriate to throw away the checker

src/harness/unittests/cancellableLanguageServiceOperations.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,19 @@ namespace ts {
5656
r => assert.exists(r.displayParts)
5757
);
5858
});
59+
60+
it("can cancel suggestion diagnostics mid-request", () => {
61+
verifyOperationCancelledAfter(file, 1, service => // The LS doesn't do any top-level checks on the token for suggestion diagnostics, so the first check is within the checker
62+
service.getSuggestionDiagnostics("file.js"),
63+
r => assert.notEqual(r.length, 0),
64+
"file.js",
65+
"function foo() { let a = 10; }",
66+
{ allowJs: true }
67+
);
68+
});
5969
});
6070

61-
function verifyOperationCancelledAfter<T>(content: string, cancelAfter: number, operation: (service: LanguageService) => T, validator: (arg: T) => void) {
71+
function verifyOperationCancelledAfter<T>(content: string, cancelAfter: number, operation: (service: LanguageService) => T, validator: (arg: T) => void, fileName?: string, fileContent?: string, options?: CompilerOptions) {
6272
let checks = 0;
6373
const token: HostCancellationToken = {
6474
isCancellationRequested() {
@@ -70,9 +80,9 @@ namespace ts {
7080
return result;
7181
}
7282
};
73-
const adapter = new Harness.LanguageService.NativeLanguageServiceAdapter(token);
83+
const adapter = new Harness.LanguageService.NativeLanguageServiceAdapter(token, options);
7484
const host = adapter.getHost();
75-
host.addScript("file.ts", content, /*isRootFile*/ true);
85+
host.addScript(fileName || "file.ts", fileContent || content, /*isRootFile*/ true);
7686
const service = adapter.getLanguageService();
7787
assertCancelled(() => operation(service));
7888
validator(operation(service));
@@ -92,4 +102,4 @@ namespace ts {
92102
assert.exists(caught, "Expected operation to be cancelled, but was not");
93103
assert.instanceOf(caught, OperationCanceledException);
94104
}
95-
}
105+
}

src/services/codeFixProvider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ namespace ts {
8686
return createCombinedCodeActions(changes, commands.length === 0 ? undefined : commands);
8787
}
8888

89-
function eachDiagnostic({ program, sourceFile }: CodeFixAllContext, errorCodes: number[], cb: (diag: DiagnosticWithLocation) => void): void {
90-
for (const diag of program.getSemanticDiagnostics(sourceFile).concat(computeSuggestionDiagnostics(sourceFile, program))) {
89+
function eachDiagnostic({ program, sourceFile, cancellationToken }: CodeFixAllContext, errorCodes: number[], cb: (diag: DiagnosticWithLocation) => void): void {
90+
for (const diag of program.getSemanticDiagnostics(sourceFile, cancellationToken).concat(computeSuggestionDiagnostics(sourceFile, program, cancellationToken))) {
9191
if (contains(errorCodes, diag.code)) {
9292
cb(diag as DiagnosticWithLocation);
9393
}

src/services/services.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1415,7 +1415,7 @@ namespace ts {
14151415

14161416
function getSuggestionDiagnostics(fileName: string): DiagnosticWithLocation[] {
14171417
synchronizeHostData();
1418-
return computeSuggestionDiagnostics(getValidSourceFile(fileName), program);
1418+
return computeSuggestionDiagnostics(getValidSourceFile(fileName), program, cancellationToken);
14191419
}
14201420

14211421
function getCompilerOptionsDiagnostics() {

src/services/suggestionDiagnostics.ts

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
/* @internal */
22
namespace ts {
3-
export function computeSuggestionDiagnostics(sourceFile: SourceFile, program: Program): DiagnosticWithLocation[] {
4-
program.getSemanticDiagnostics(sourceFile);
5-
const checker = program.getDiagnosticsProducingTypeChecker();
3+
export function computeSuggestionDiagnostics(sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): DiagnosticWithLocation[] {
4+
program.getSemanticDiagnostics(sourceFile, cancellationToken);
65
const diags: DiagnosticWithLocation[] = [];
76

87
if (sourceFile.commonJsModuleIndicator &&
@@ -13,6 +12,25 @@ namespace ts {
1312

1413
const isJsFile = isSourceFileJavaScript(sourceFile);
1514

15+
check(sourceFile);
16+
17+
if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) {
18+
for (const moduleSpecifier of sourceFile.imports) {
19+
const importNode = importFromModuleSpecifier(moduleSpecifier);
20+
const name = importNameForConvertToDefaultImport(importNode);
21+
if (!name) continue;
22+
const module = getResolvedModule(sourceFile, moduleSpecifier.text);
23+
const resolvedFile = module && program.getSourceFile(module.resolvedFileName);
24+
if (resolvedFile && resolvedFile.externalModuleIndicator && isExportAssignment(resolvedFile.externalModuleIndicator) && resolvedFile.externalModuleIndicator.isExportEquals) {
25+
diags.push(createDiagnosticForNode(name, Diagnostics.Import_may_be_converted_to_a_default_import));
26+
}
27+
}
28+
}
29+
30+
addRange(diags, sourceFile.bindSuggestionDiagnostics);
31+
addRange(diags, program.getSuggestionDiagnostics(sourceFile, cancellationToken));
32+
return diags.sort((d1, d2) => d1.start - d2.start);
33+
1634
function check(node: Node) {
1735
if (isJsFile) {
1836
switch (node.kind) {
@@ -25,52 +43,33 @@ namespace ts {
2543
break;
2644
}
2745
}
28-
// falls through if no diagnostic was created
46+
// falls through if no diagnostic was created
2947
case SyntaxKind.FunctionDeclaration:
3048
const symbol = node.symbol;
3149
if (symbol.members && (symbol.members.size > 0)) {
3250
diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration));
3351
}
3452
break;
35-
}
36-
}
37-
38-
if (!isJsFile && codefix.parameterShouldGetTypeFromJSDoc(node)) {
39-
diags.push(createDiagnosticForNode(node.name || node, Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types));
53+
}
4054
}
41-
42-
node.forEachChild(check);
43-
}
44-
check(sourceFile);
45-
46-
if (!isJsFile) {
47-
for (const statement of sourceFile.statements) {
48-
if (isVariableStatement(statement) &&
49-
statement.declarationList.flags & NodeFlags.Const &&
50-
statement.declarationList.declarations.length === 1) {
51-
const init = statement.declarationList.declarations[0].initializer;
55+
else {
56+
if (isVariableStatement(node) &&
57+
node.parent === sourceFile &&
58+
node.declarationList.flags & NodeFlags.Const &&
59+
node.declarationList.declarations.length === 1) {
60+
const init = node.declarationList.declarations[0].initializer;
5261
if (init && isRequireCall(init, /*checkArgumentIsStringLiteralLike*/ true)) {
5362
diags.push(createDiagnosticForNode(init, Diagnostics.require_call_may_be_converted_to_an_import));
5463
}
5564
}
56-
}
57-
}
5865

59-
if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) {
60-
for (const moduleSpecifier of sourceFile.imports) {
61-
const importNode = importFromModuleSpecifier(moduleSpecifier);
62-
const name = importNameForConvertToDefaultImport(importNode);
63-
if (!name) continue;
64-
const module = getResolvedModule(sourceFile, moduleSpecifier.text);
65-
const resolvedFile = module && program.getSourceFile(module.resolvedFileName);
66-
if (resolvedFile && resolvedFile.externalModuleIndicator && isExportAssignment(resolvedFile.externalModuleIndicator) && resolvedFile.externalModuleIndicator.isExportEquals) {
67-
diags.push(createDiagnosticForNode(name, Diagnostics.Import_may_be_converted_to_a_default_import));
66+
if (codefix.parameterShouldGetTypeFromJSDoc(node)) {
67+
diags.push(createDiagnosticForNode(node.name || node, Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types));
6868
}
6969
}
70-
}
7170

72-
addRange(diags, sourceFile.bindSuggestionDiagnostics);
73-
return diags.concat(checker.getSuggestionDiagnostics(sourceFile)).sort((d1, d2) => d1.start - d2.start);
71+
node.forEachChild(check);
72+
}
7473
}
7574

7675
// convertToEs6Module only works on top-level, so don't trigger it if commonjs code only appears in nested scopes.

0 commit comments

Comments
 (0)