Make suggestion diagnostics to wire cancellationToken

sheetalkamat · sheetalkamat · commit b0f039c9e231 · 2018-05-31T10:47:17.000-07:00
This especially needed if its a js file without the ts-check, the file wont be typechecked in getSemanticDiagnostics Fixes part of #19458
diff --git 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<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;
                 }
             },
 
diff --git 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<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
          */
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -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.
@@ -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
diff --git 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<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() {
@@ -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);
     }
-}
+}
diff --git 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
@@ -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
@@ -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 &&
@@ -23,6 +22,18 @@ namespace ts {
                         }
                     }
                     break;
+                case SyntaxKind.VariableStatement:
+                    if (!isJsFile && node.parent === sourceFile) {
+                        const statement = node as VariableStatement;
+                        if (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));
+                            }
+                        }
+                    }
+                    break;
             }
 
             if (!isJsFile && codefix.parameterShouldGetTypeFromJSDoc(node)) {
@@ -33,19 +44,6 @@ namespace ts {
         }
         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);
@@ -60,7 +58,8 @@ 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);
     }
 
     // convertToEs6Module only works on top-level, so don't trigger it if commonjs code only appears in nested scopes.

Original file line number	Diff line number	Diff line change
`@@ -86,8 +86,8 @@ namespace ts {`
`86`	`86`	`return createCombinedCodeActions(changes, commands.length === 0 ? undefined : commands);`
`87`	`87`	`}`
`88`	`88`
`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))) {`
`91`	`91`	`if (contains(errorCodes, diag.code)) {`
`92`	`92`	`cb(diag as DiagnosticWithLocation);`
`93`	`93`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1415,7 +1415,7 @@ namespace ts {`
`1415`	`1415`
`1416`	`1416`	`function getSuggestionDiagnostics(fileName: string): DiagnosticWithLocation[] {`
`1417`	`1417`	`synchronizeHostData();`
`1418`		`- return computeSuggestionDiagnostics(getValidSourceFile(fileName), program);`
	`1418`	`+ return computeSuggestionDiagnostics(getValidSourceFile(fileName), program, cancellationToken);`
`1419`	`1419`	`}`
`1420`	`1420`
`1421`	`1421`	`function getCompilerOptionsDiagnostics() {`