Add quickfix and refactoring to install @types packages (microsoft#19130)

* Add quickfix and refactoring to install @types packages

* Move `validatePackageName` to `jsTyping.ts`

* Remove combinePaths overloads

* Respond to code review

* Update api baselines

* Use native PromiseConstructor

* Return false instead of undefined

* Remove getProjectRootPath

* Update api

Author: Andy

scripts/buildProtocol.ts

@@ -51,22 +51,25 @@ class DeclarationsWalker {
             return this.processType((<any>type).typeArguments[0]);
         }
         else {
-            for (const decl of s.getDeclarations()) {
-                const sourceFile = decl.getSourceFile();
-                if (sourceFile === this.protocolFile || path.basename(sourceFile.fileName) === "lib.d.ts") {
-                    return;
-                }
-                if (decl.kind === ts.SyntaxKind.EnumDeclaration && !isStringEnum(decl as ts.EnumDeclaration)) {
-                    this.removedTypes.push(type);
-                    return;
-                }
-                else {
-                    // splice declaration in final d.ts file
-                    let text = decl.getFullText();
-                    this.text += `${text}\n`;
-                    // recursively pull all dependencies into result dts file
+            const declarations = s.getDeclarations();
+            if (declarations) {
+                for (const decl of declarations) {
+                    const sourceFile = decl.getSourceFile();
+                    if (sourceFile === this.protocolFile || path.basename(sourceFile.fileName) === "lib.d.ts") {
+                        return;
+                    }
+                    if (decl.kind === ts.SyntaxKind.EnumDeclaration && !isStringEnum(decl as ts.EnumDeclaration)) {
+                        this.removedTypes.push(type);
+                        return;
+                    }
+                    else {
+                        // splice declaration in final d.ts file
+                        let text = decl.getFullText();
+                        this.text += `${text}\n`;
+                        // recursively pull all dependencies into result dts file
 
-                    this.visitTypeNodes(decl);
+                        this.visitTypeNodes(decl);
+                    }
                 }
             }
         }

src/compiler/core.ts

@@ -1861,7 +1861,7 @@ namespace ts {
         return i < 0 ? path : path.substring(i + 1);
     }
 
-    export function combinePaths(path1: string, path2: string) {
+    export function combinePaths(path1: string, path2: string): string {
         if (!(path1 && path1.length)) return path2;
         if (!(path2 && path2.length)) return path1;
         if (getRootLength(path2) !== 0) return path2;

src/compiler/moduleNameResolver.ts

@@ -1005,7 +1005,8 @@ namespace ts {
         return withPackageId(packageId, pathAndExtension);
     }
 
-    function getPackageName(moduleName: string): { packageName: string, rest: string } {
+    /* @internal */
+    export function getPackageName(moduleName: string): { packageName: string, rest: string } {
         let idx = moduleName.indexOf(directorySeparator);
         if (moduleName[0] === "@") {
             idx = moduleName.indexOf(directorySeparator, idx + 1);
@@ -1063,18 +1064,27 @@ namespace ts {
     const mangledScopedPackageSeparator = "__";
 
     /** For a scoped package, we must look in `@types/foo__bar` instead of `@types/@foo/bar`. */
-    function mangleScopedPackage(moduleName: string, state: ModuleResolutionState): string {
-        if (startsWith(moduleName, "@")) {
-            const replaceSlash = moduleName.replace(ts.directorySeparator, mangledScopedPackageSeparator);
-            if (replaceSlash !== moduleName) {
-                const mangled = replaceSlash.slice(1); // Take off the "@"
-                if (state.traceEnabled) {
-                    trace(state.host, Diagnostics.Scoped_package_detected_looking_in_0, mangled);
-                }
-                return mangled;
+    function mangleScopedPackage(packageName: string, state: ModuleResolutionState): string {
+        const mangled = getMangledNameForScopedPackage(packageName);
+        if (state.traceEnabled && mangled !== packageName) {
+            trace(state.host, Diagnostics.Scoped_package_detected_looking_in_0, mangled);
+        }
+        return mangled;
+    }
+
+    /* @internal */
+    export function getTypesPackageName(packageName: string): string {
+        return `@types/${getMangledNameForScopedPackage(packageName)}`;
+    }
+
+    function getMangledNameForScopedPackage(packageName: string): string {
+        if (startsWith(packageName, "@")) {
+            const replaceSlash = packageName.replace(ts.directorySeparator, mangledScopedPackageSeparator);
+            if (replaceSlash !== packageName) {
+                return replaceSlash.slice(1); // Take off the "@"
             }
         }
-        return moduleName;
+        return packageName;
     }
 
     /* @internal */

src/harness/fourslash.ts

@@ -953,6 +953,10 @@ namespace FourSlash {
             return this.getChecker().getSymbolsInScope(node, ts.SymbolFlags.Value | ts.SymbolFlags.Type | ts.SymbolFlags.Namespace);
         }
 
+        public setTypesRegistry(map: ts.MapLike<void>): void {
+            this.languageServiceAdapterHost.typesRegistry = ts.createMapFromTemplate(map);
+        }
+
         public verifyTypeOfSymbolAtLocation(range: Range, symbol: ts.Symbol, expected: string): void {
             const node = this.goToAndGetNode(range);
             const checker = this.getChecker();
@@ -2777,16 +2781,26 @@ Actual: ${stringify(fullActual)}`);
             }
         }
 
-        public verifyCodeFixAvailable(negative: boolean) {
-            const codeFix = this.getCodeFixActions(this.activeFile.fileName);
+        public verifyCodeFixAvailable(negative: boolean, info: FourSlashInterface.VerifyCodeFixAvailableOptions[] | undefined) {
+            const codeFixes = this.getCodeFixActions(this.activeFile.fileName);
 
-            if (negative && codeFix.length) {
-                this.raiseError(`verifyCodeFixAvailable failed - expected no fixes but found one.`);
+            if (negative) {
+                if (codeFixes.length) {
+                    this.raiseError(`verifyCodeFixAvailable failed - expected no fixes but found one.`);
+                }
+                return;
             }
 
-            if (!(negative || codeFix.length)) {
+            if (!codeFixes.length) {
                 this.raiseError(`verifyCodeFixAvailable failed - expected code fixes but none found.`);
             }
+            if (info) {
+                assert.equal(info.length, codeFixes.length);
+                ts.zipWith(codeFixes, info, (fix, info) => {
+                    assert.equal(fix.description, info.description);
+                    this.assertObjectsEqual(fix.commands, info.commands);
+                });
+            }
         }
 
         public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) {
@@ -2830,6 +2844,14 @@ Actual: ${stringify(fullActual)}`);
             }
         }
 
+        public verifyRefactor({ name, actionName, refactors }: FourSlashInterface.VerifyRefactorOptions) {
+            const selection = this.getSelection();
+
+            const actualRefactors = (this.languageService.getApplicableRefactors(this.activeFile.fileName, selection) || ts.emptyArray)
+                .filter(r => r.name === name && r.actions.some(a => a.name === actionName));
+            this.assertObjectsEqual(actualRefactors, refactors);
+        }
+
         public verifyApplicableRefactorAvailableForRange(negative: boolean) {
             const ranges = this.getRanges();
             if (!(ranges && ranges.length === 1)) {
@@ -3614,6 +3636,10 @@ namespace FourSlashInterface {
         public symbolsInScope(range: FourSlash.Range): ts.Symbol[] {
             return this.state.symbolsInScope(range);
         }
+
+        public setTypesRegistry(map: ts.MapLike<void>): void {
+            this.state.setTypesRegistry(map);
+        }
     }
 
     export class GoTo {
@@ -3789,8 +3815,8 @@ namespace FourSlashInterface {
             this.state.verifyCodeFix(options);
         }
 
-        public codeFixAvailable() {
-            this.state.verifyCodeFixAvailable(this.negative);
+        public codeFixAvailable(options?: VerifyCodeFixAvailableOptions[]) {
+            this.state.verifyCodeFixAvailable(this.negative, options);
         }
 
         public applicableRefactorAvailableAtMarker(markerName: string) {
@@ -3801,6 +3827,10 @@ namespace FourSlashInterface {
             this.state.verifyApplicableRefactorAvailableForRange(this.negative);
         }
 
+        public refactor(options: VerifyRefactorOptions) {
+            this.state.verifyRefactor(options);
+        }
+
         public refactorAvailable(name: string, actionName?: string) {
             this.state.verifyRefactorAvailable(this.negative, name, actionName);
         }
@@ -4449,6 +4479,17 @@ namespace FourSlashInterface {
         index?: number;
     }
 
+    export interface VerifyCodeFixAvailableOptions {
+        description: string;
+        commands?: ts.CodeActionCommand[];
+    }
+
+    export interface VerifyRefactorOptions {
+        name: string;
+        actionName: string;
+        refactors: ts.ApplicableRefactorInfo[];
+    }
+
     export interface VerifyCompletionActionOptions extends NewContentOptions {
         name: string;
         description: string;

src/harness/harnessLanguageService.ts

@@ -123,6 +123,7 @@ namespace Harness.LanguageService {
     }
 
     export class LanguageServiceAdapterHost {
+        public typesRegistry: ts.Map<void> | undefined;
         protected virtualFileSystem: Utils.VirtualFileSystem = new Utils.VirtualFileSystem(virtualFileSystemRoot, /*useCaseSensitiveFilenames*/false);
 
         constructor(protected cancellationToken = DefaultHostCancellationToken.Instance,
@@ -182,6 +183,11 @@ namespace Harness.LanguageService {
 
     /// Native adapter
     class NativeLanguageServiceHost extends LanguageServiceAdapterHost implements ts.LanguageServiceHost {
+        isKnownTypesPackageName(name: string): boolean {
+            return this.typesRegistry && this.typesRegistry.has(name);
+        }
+        installPackage = ts.notImplemented;
+
         getCompilationSettings() { return this.settings; }
         getCancellationToken() { return this.cancellationToken; }
         getDirectories(path: string): string[] {
@@ -493,6 +499,7 @@ namespace Harness.LanguageService {
         getCodeFixesAtPosition(): ts.CodeAction[] {
             throw new Error("Not supported on the shim.");
         }
+        applyCodeActionCommand = ts.notImplemented;
         getCodeFixDiagnostics(): ts.Diagnostic[] {
             throw new Error("Not supported on the shim.");
         }

src/harness/unittests/compileOnSave.ts

@@ -12,7 +12,7 @@ namespace ts.projectSystem {
 
     describe("CompileOnSave affected list", () => {
         function sendAffectedFileRequestAndCheckResult(session: server.Session, request: server.protocol.Request, expectedFileList: { projectFileName: string, files: FileOrFolder[] }[]) {
-            const response: server.protocol.CompileOnSaveAffectedFileListSingleProject[] = session.executeCommand(request).response;
+            const response = session.executeCommand(request).response as server.protocol.CompileOnSaveAffectedFileListSingleProject[];
             const actualResult = response.sort((list1, list2) => compareStrings(list1.projectFileName, list2.projectFileName));
             expectedFileList = expectedFileList.sort((list1, list2) => compareStrings(list1.projectFileName, list2.projectFileName));
 

src/harness/unittests/extractTestHelpers.ts

@@ -97,6 +97,14 @@ namespace ts {
         return rulesProvider;
     }
 
+    const notImplementedHost: LanguageServiceHost = {
+        getCompilationSettings: notImplemented,
+        getScriptFileNames: notImplemented,
+        getScriptVersion: notImplemented,
+        getScriptSnapshot: notImplemented,
+        getDefaultLibFileName: notImplemented,
+    };
+
     export function testExtractSymbol(caption: string, text: string, baselineFolder: string, description: DiagnosticMessage) {
         const t = extractTest(text);
         const selectionRange = t.ranges.get("selection");
@@ -125,6 +133,7 @@ namespace ts {
                 file: sourceFile,
                 startPosition: selectionRange.start,
                 endPosition: selectionRange.end,
+                host: notImplementedHost,
                 rulesProvider: getRuleProvider()
             };
             const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
@@ -188,6 +197,7 @@ namespace ts {
                 file: sourceFile,
                 startPosition: selectionRange.start,
                 endPosition: selectionRange.end,
+                host: notImplementedHost,
                 rulesProvider: getRuleProvider()
             };
             const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));

src/harness/unittests/projectErrors.ts

@@ -57,23 +57,23 @@ namespace ts.projectSystem {
                 });
 
                 checkNumberOfProjects(projectService, { externalProjects: 1 });
-                const diags = session.executeCommand(compilerOptionsRequest).response;
+                const diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
                 // only file1 exists - expect error
                 checkDiagnosticsWithLinePos(diags, ["File '/a/b/applib.ts' not found."]);
             }
             host.reloadFS([file2, libFile]);
             {
                 // only file2 exists - expect error
                 checkNumberOfProjects(projectService, { externalProjects: 1 });
-                const diags = session.executeCommand(compilerOptionsRequest).response;
+                const diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
                 checkDiagnosticsWithLinePos(diags, ["File '/a/b/app.ts' not found."]);
             }
 
             host.reloadFS([file1, file2, libFile]);
             {
                 // both files exist - expect no errors
                 checkNumberOfProjects(projectService, { externalProjects: 1 });
-                const diags = session.executeCommand(compilerOptionsRequest).response;
+                const diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
                 checkDiagnosticsWithLinePos(diags, []);
             }
         });
@@ -103,13 +103,13 @@ namespace ts.projectSystem {
                 seq: 2,
                 arguments: { projectFileName: project.getProjectName() }
             };
-            let diags = session.executeCommand(compilerOptionsRequest).response;
+            let diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
             checkDiagnosticsWithLinePos(diags, ["File '/a/b/applib.ts' not found."]);
 
             host.reloadFS([file1, file2, config, libFile]);
 
             checkNumberOfProjects(projectService, { configuredProjects: 1 });
-            diags = session.executeCommand(compilerOptionsRequest).response;
+            diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
             checkDiagnosticsWithLinePos(diags, []);
         });
 

src/harness/unittests/session.ts

@@ -315,7 +315,7 @@ namespace ts.server {
                     item: false
                 };
                 const command = "newhandle";
-                const result = {
+                const result: ts.server.HandlerResponse = {
                     response: respBody,
                     responseRequired: true
                 };
@@ -332,7 +332,7 @@ namespace ts.server {
                 const respBody = {
                     item: false
                 };
-                const resp = {
+                const resp: ts.server.HandlerResponse = {
                     response: respBody,
                     responseRequired: true
                 };
@@ -372,7 +372,7 @@ namespace ts.server {
                 };
                 const command = "test";
 
-                session.output(body, command);
+                session.output(body, command, /*reqSeq*/ 0);
 
                 expect(lastSent).to.deep.equal({
                     seq: 0,
@@ -475,7 +475,7 @@ namespace ts.server {
             };
             const command = "test";
 
-            session.output(body, command);
+            session.output(body, command, /*reqSeq*/ 0);
 
             expect(session.lastSent).to.deep.equal({
                 seq: 0,
@@ -542,7 +542,7 @@ namespace ts.server {
             handleRequest(msg: protocol.Request) {
                 let response: protocol.Response;
                 try {
-                    ({ response } = this.executeCommand(msg));
+                    response = this.executeCommand(msg).response as protocol.Response;
                 }
                 catch (e) {
                     this.output(undefined, msg.command, msg.seq, e.toString());