Skip to content

Commit b0f039c

Browse files
sheetalkamatsheetalkamat
committed
Make suggestion diagnostics to wire cancellationToken
This especially needed if its a js file without the ts-check, the file wont be typechecked in getSemanticDiagnostics Fixes part of microsoft#19458
1 parent 576a733 commit b0f039c

7 files changed

Lines changed: 63 additions & 33 deletions

File tree

‎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: 16 additions & 17 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 &&
@@ -23,6 +22,18 @@ namespace ts {
2322
}
2423
}
2524
break;
25+
case SyntaxKind.VariableStatement:
26+
if (!isJsFile && node.parent === sourceFile) {
27+
const statement = node as VariableStatement;
28+
if (statement.declarationList.flags & NodeFlags.Const &&
29+
statement.declarationList.declarations.length === 1) {
30+
const init = statement.declarationList.declarations[0].initializer;
31+
if (init && isRequireCall(init, /*checkArgumentIsStringLiteralLike*/ true)) {
32+
diags.push(createDiagnosticForNode(init, Diagnostics.require_call_may_be_converted_to_an_import));
33+
}
34+
}
35+
}
36+
break;
2637
}
2738

2839
if (!isJsFile && codefix.parameterShouldGetTypeFromJSDoc(node)) {
@@ -33,19 +44,6 @@ namespace ts {
3344
}
3445
check(sourceFile);
3546

36-
if (!isJsFile) {
37-
for (const statement of sourceFile.statements) {
38-
if (isVariableStatement(statement) &&
39-
statement.declarationList.flags & NodeFlags.Const &&
40-
statement.declarationList.declarations.length === 1) {
41-
const init = statement.declarationList.declarations[0].initializer;
42-
if (init && isRequireCall(init, /*checkArgumentIsStringLiteralLike*/ true)) {
43-
diags.push(createDiagnosticForNode(init, Diagnostics.require_call_may_be_converted_to_an_import));
44-
}
45-
}
46-
}
47-
}
48-
4947
if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) {
5048
for (const moduleSpecifier of sourceFile.imports) {
5149
const importNode = importFromModuleSpecifier(moduleSpecifier);
@@ -60,7 +58,8 @@ namespace ts {
6058
}
6159

6260
addRange(diags, sourceFile.bindSuggestionDiagnostics);
63-
return diags.concat(checker.getSuggestionDiagnostics(sourceFile)).sort((d1, d2) => d1.start - d2.start);
61+
addRange(diags, program.getSuggestionDiagnostics(sourceFile, cancellationToken));
62+
return diags.sort((d1, d2) => d1.start - d2.start);
6463
}
6564

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

0 commit comments

Comments
 (0)