diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index fcddb03f63..0f4db171ab 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -310,21 +310,34 @@ namespace ts { const node = getParseTreeNode(nodeIn, isTypeNode); return node && getTypeArgumentConstraint(node); }, + getSuggestionDiagnostics: (file, ct) => { + let diagnostics: DiagnosticWithLocation[] | undefined; + try { + // Record the cancellation token so it can be checked later on during checkSourceElement. + // Do this in a finally block so we can ensure that it gets reset back to nothing after + // this call is done. + cancellationToken = ct; - getSuggestionDiagnostics: file => { - return (suggestionDiagnostics.get(file.fileName) || emptyArray).concat(getUnusedDiagnostics()); - function getUnusedDiagnostics(): ReadonlyArray { - if (file.isDeclarationFile) return emptyArray; - + // Ensure file is type checked checkSourceFile(file); - const diagnostics: DiagnosticWithLocation[] = []; Debug.assert(!!(getNodeLinks(file).flags & NodeCheckFlags.TypeChecked)); + + diagnostics = addRange(diagnostics, suggestionDiagnostics.get(file.fileName)); + if (!file.isDeclarationFile && (!unusedIsError(UnusedKind.Local) || !unusedIsError(UnusedKind.Parameter))) { + addUnusedDiagnostics(); + } + return diagnostics || emptyArray; + } + finally { + cancellationToken = undefined; + } + + function addUnusedDiagnostics() { checkUnusedIdentifiers(getPotentiallyUnusedIdentifiers(file), (kind, diag) => { if (!unusedIsError(kind)) { - diagnostics.push({ ...diag, category: DiagnosticCategory.Suggestion }); + (diagnostics || (diagnostics = [])).push({ ...diag, category: DiagnosticCategory.Suggestion }); } }); - return diagnostics; } }, diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 4dfb9039cc..715f5377b1 100755 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -683,6 +683,7 @@ namespace ts { getOptionsDiagnostics, getGlobalDiagnostics, getSemanticDiagnostics, + getSuggestionDiagnostics, getDeclarationDiagnostics, getTypeChecker, getClassifiableNames, @@ -1422,6 +1423,12 @@ namespace ts { }); } + function getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken: CancellationToken): ReadonlyArray { + return runWithCancellationToken(() => { + return getDiagnosticsProducingTypeChecker().getSuggestionDiagnostics(sourceFile, cancellationToken); + }); + } + /** * Skip errors if previous line start with '// @ts-ignore' comment, not counting non-empty non-comment lines */ diff --git a/src/compiler/types.ts b/src/compiler/types.ts index b13f4aea61..3a19762f0a 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2755,6 +2755,7 @@ namespace ts { getSemanticDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray; getDeclarationDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray; getConfigFileParsingDiagnostics(): ReadonlyArray; + /* @internal */ getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray; /** * Gets a type checker that can be used to semantically analyze source files in the program. @@ -3076,7 +3077,7 @@ namespace ts { * Does *not* get *all* suggestion diagnostics, just the ones that were convenient to report in the checker. * Others are added in computeSuggestionDiagnostics. */ - /* @internal */ getSuggestionDiagnostics(file: SourceFile): ReadonlyArray; + /* @internal */ getSuggestionDiagnostics(file: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray; /** * Depending on the operation performed, it may be appropriate to throw away the checker diff --git a/src/harness/unittests/cancellableLanguageServiceOperations.ts b/src/harness/unittests/cancellableLanguageServiceOperations.ts index 161b45841f..a198210f76 100644 --- a/src/harness/unittests/cancellableLanguageServiceOperations.ts +++ b/src/harness/unittests/cancellableLanguageServiceOperations.ts @@ -56,9 +56,19 @@ namespace ts { r => assert.exists(r.displayParts) ); }); + + it("can cancel suggestion diagnostics mid-request", () => { + 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 + service.getSuggestionDiagnostics("file.js"), + r => assert.notEqual(r.length, 0), + "file.js", + "function foo() { let a = 10; }", + { allowJs: true } + ); + }); }); - function verifyOperationCancelledAfter(content: string, cancelAfter: number, operation: (service: LanguageService) => T, validator: (arg: T) => void) { + function verifyOperationCancelledAfter(content: string, cancelAfter: number, operation: (service: LanguageService) => T, validator: (arg: T) => void, fileName?: string, fileContent?: string, options?: CompilerOptions) { let checks = 0; const token: HostCancellationToken = { isCancellationRequested() { @@ -70,9 +80,9 @@ namespace ts { return result; } }; - const adapter = new Harness.LanguageService.NativeLanguageServiceAdapter(token); + const adapter = new Harness.LanguageService.NativeLanguageServiceAdapter(token, options); const host = adapter.getHost(); - host.addScript("file.ts", content, /*isRootFile*/ true); + host.addScript(fileName || "file.ts", fileContent || content, /*isRootFile*/ true); const service = adapter.getLanguageService(); assertCancelled(() => operation(service)); validator(operation(service)); @@ -92,4 +102,4 @@ namespace ts { assert.exists(caught, "Expected operation to be cancelled, but was not"); assert.instanceOf(caught, OperationCanceledException); } -} \ No newline at end of file +} diff --git a/src/services/codeFixProvider.ts b/src/services/codeFixProvider.ts index 31b766d3bf..4dbdc23041 100644 --- a/src/services/codeFixProvider.ts +++ b/src/services/codeFixProvider.ts @@ -86,8 +86,8 @@ namespace ts { return createCombinedCodeActions(changes, commands.length === 0 ? undefined : commands); } - function eachDiagnostic({ program, sourceFile }: CodeFixAllContext, errorCodes: number[], cb: (diag: DiagnosticWithLocation) => void): void { - for (const diag of program.getSemanticDiagnostics(sourceFile).concat(computeSuggestionDiagnostics(sourceFile, program))) { + function eachDiagnostic({ program, sourceFile, cancellationToken }: CodeFixAllContext, errorCodes: number[], cb: (diag: DiagnosticWithLocation) => void): void { + for (const diag of program.getSemanticDiagnostics(sourceFile, cancellationToken).concat(computeSuggestionDiagnostics(sourceFile, program, cancellationToken))) { if (contains(errorCodes, diag.code)) { cb(diag as DiagnosticWithLocation); } diff --git a/src/services/services.ts b/src/services/services.ts index 441cb5a8eb..7d5e4ec0a8 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1415,7 +1415,7 @@ namespace ts { function getSuggestionDiagnostics(fileName: string): DiagnosticWithLocation[] { synchronizeHostData(); - return computeSuggestionDiagnostics(getValidSourceFile(fileName), program); + return computeSuggestionDiagnostics(getValidSourceFile(fileName), program, cancellationToken); } function getCompilerOptionsDiagnostics() { diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 19d919841b..1f4ced6119 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -1,8 +1,7 @@ /* @internal */ namespace ts { - export function computeSuggestionDiagnostics(sourceFile: SourceFile, program: Program): DiagnosticWithLocation[] { - program.getSemanticDiagnostics(sourceFile); - const checker = program.getDiagnosticsProducingTypeChecker(); + export function computeSuggestionDiagnostics(sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): DiagnosticWithLocation[] { + program.getSemanticDiagnostics(sourceFile, cancellationToken); const diags: DiagnosticWithLocation[] = []; if (sourceFile.commonJsModuleIndicator && @@ -13,49 +12,8 @@ namespace ts { const isJsFile = isSourceFileJavaScript(sourceFile); - function check(node: Node) { - if (isJsFile) { - switch (node.kind) { - case SyntaxKind.FunctionExpression: - const decl = getDeclarationOfJSInitializer(node); - if (decl) { - const symbol = decl.symbol; - if (symbol && (symbol.exports && symbol.exports.size || symbol.members && symbol.members.size)) { - diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration)); - break; - } - } - // falls through if no diagnostic was created - case SyntaxKind.FunctionDeclaration: - const symbol = node.symbol; - if (symbol.members && (symbol.members.size > 0)) { - diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration)); - } - break; - } - } - - if (!isJsFile && codefix.parameterShouldGetTypeFromJSDoc(node)) { - diags.push(createDiagnosticForNode(node.name || node, Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types)); - } - - node.forEachChild(check); - } check(sourceFile); - if (!isJsFile) { - for (const statement of sourceFile.statements) { - if (isVariableStatement(statement) && - statement.declarationList.flags & NodeFlags.Const && - statement.declarationList.declarations.length === 1) { - const init = statement.declarationList.declarations[0].initializer; - if (init && isRequireCall(init, /*checkArgumentIsStringLiteralLike*/ true)) { - diags.push(createDiagnosticForNode(init, Diagnostics.require_call_may_be_converted_to_an_import)); - } - } - } - } - if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) { for (const moduleSpecifier of sourceFile.imports) { const importNode = importFromModuleSpecifier(moduleSpecifier); @@ -70,7 +28,48 @@ namespace ts { } addRange(diags, sourceFile.bindSuggestionDiagnostics); - return diags.concat(checker.getSuggestionDiagnostics(sourceFile)).sort((d1, d2) => d1.start - d2.start); + addRange(diags, program.getSuggestionDiagnostics(sourceFile, cancellationToken)); + return diags.sort((d1, d2) => d1.start - d2.start); + + function check(node: Node) { + if (isJsFile) { + switch (node.kind) { + case SyntaxKind.FunctionExpression: + const decl = getDeclarationOfJSInitializer(node); + if (decl) { + const symbol = decl.symbol; + if (symbol && (symbol.exports && symbol.exports.size || symbol.members && symbol.members.size)) { + diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration)); + break; + } + } + // falls through if no diagnostic was created + case SyntaxKind.FunctionDeclaration: + const symbol = node.symbol; + if (symbol.members && (symbol.members.size > 0)) { + diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration)); + } + break; + } + } + else { + if (isVariableStatement(node) && + node.parent === sourceFile && + node.declarationList.flags & NodeFlags.Const && + node.declarationList.declarations.length === 1) { + const init = node.declarationList.declarations[0].initializer; + if (init && isRequireCall(init, /*checkArgumentIsStringLiteralLike*/ true)) { + diags.push(createDiagnosticForNode(init, Diagnostics.require_call_may_be_converted_to_an_import)); + } + } + + if (codefix.parameterShouldGetTypeFromJSDoc(node)) { + diags.push(createDiagnosticForNode(node.name || node, Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types)); + } + } + + node.forEachChild(check); + } } // convertToEs6Module only works on top-level, so don't trigger it if commonjs code only appears in nested scopes.