Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
29 changes: 21 additions & 8 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<DiagnosticWithLocation> {
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;
}
},

Expand Down
7 changes: 7 additions & 0 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,7 @@ namespace ts {
getOptionsDiagnostics,
getGlobalDiagnostics,
getSemanticDiagnostics,
getSuggestionDiagnostics,
getDeclarationDiagnostics,
getTypeChecker,
getClassifiableNames,
Expand Down Expand Up @@ -1422,6 +1423,12 @@ namespace ts {
});
}

function getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken: CancellationToken): ReadonlyArray<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
*/
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2755,6 +2755,7 @@ namespace ts {
getSemanticDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<Diagnostic>;
getDeclarationDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<DiagnosticWithLocation>;
getConfigFileParsingDiagnostics(): ReadonlyArray<Diagnostic>;
/* @internal */ getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<DiagnosticWithLocation>;

/**
* Gets a type checker that can be used to semantically analyze source files in the program.
Expand Down Expand Up @@ -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<DiagnosticWithLocation>;
/* @internal */ getSuggestionDiagnostics(file: SourceFile, cancellationToken?: CancellationToken): ReadonlyArray<DiagnosticWithLocation>;

/**
* Depending on the operation performed, it may be appropriate to throw away the checker
Expand Down
18 changes: 14 additions & 4 deletions src/harness/unittests/cancellableLanguageServiceOperations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(content: string, cancelAfter: number, operation: (service: LanguageService) => T, validator: (arg: T) => void) {
function verifyOperationCancelledAfter<T>(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() {
Expand All @@ -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));
Expand All @@ -92,4 +102,4 @@ namespace ts {
assert.exists(caught, "Expected operation to be cancelled, but was not");
assert.instanceOf(caught, OperationCanceledException);
}
}
}
4 changes: 2 additions & 2 deletions src/services/codeFixProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
67 changes: 33 additions & 34 deletions src/services/suggestionDiagnostics.ts
Original file line number Diff line number Diff line change
@@ -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 &&
Expand All @@ -13,6 +12,25 @@ namespace ts {

const isJsFile = isSourceFileJavaScript(sourceFile);

check(sourceFile);

if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) {
for (const moduleSpecifier of sourceFile.imports) {
const importNode = importFromModuleSpecifier(moduleSpecifier);
const name = importNameForConvertToDefaultImport(importNode);
if (!name) continue;
const module = getResolvedModule(sourceFile, moduleSpecifier.text);
const resolvedFile = module && program.getSourceFile(module.resolvedFileName);
if (resolvedFile && resolvedFile.externalModuleIndicator && isExportAssignment(resolvedFile.externalModuleIndicator) && resolvedFile.externalModuleIndicator.isExportEquals) {
diags.push(createDiagnosticForNode(name, Diagnostics.Import_may_be_converted_to_a_default_import));
}
}
}

addRange(diags, sourceFile.bindSuggestionDiagnostics);
addRange(diags, program.getSuggestionDiagnostics(sourceFile, cancellationToken));
return diags.sort((d1, d2) => d1.start - d2.start);

function check(node: Node) {
if (isJsFile) {
switch (node.kind) {
Expand All @@ -25,52 +43,33 @@ namespace ts {
break;
}
}
// falls through if no diagnostic was created
// 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;
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 (getAllowSyntheticDefaultImports(program.getCompilerOptions())) {
for (const moduleSpecifier of sourceFile.imports) {
const importNode = importFromModuleSpecifier(moduleSpecifier);
const name = importNameForConvertToDefaultImport(importNode);
if (!name) continue;
const module = getResolvedModule(sourceFile, moduleSpecifier.text);
const resolvedFile = module && program.getSourceFile(module.resolvedFileName);
if (resolvedFile && resolvedFile.externalModuleIndicator && isExportAssignment(resolvedFile.externalModuleIndicator) && resolvedFile.externalModuleIndicator.isExportEquals) {
diags.push(createDiagnosticForNode(name, Diagnostics.Import_may_be_converted_to_a_default_import));
if (codefix.parameterShouldGetTypeFromJSDoc(node)) {
diags.push(createDiagnosticForNode(node.name || node, Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types));
}
}
}

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

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