Use import().T for import completions/fixes of pure types in JS files (microsoft#25852)

Andy · web-flow · commit 0227997fa574 · 2018-07-26T16:16:57.000-07:00
* Use `import().T` for import completions/fixes of pure types in JS files

* Don't call tryUseExistingNamespaceImport if position undefined
diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
diff --git a/src/services/completions.ts b/src/services/completions.ts
@@ -592,7 +592,7 @@ namespace ts.Completions {
             }
             case "symbol": {
                 const { symbol, location, symbolToOriginInfoMap, previousToken } = symbolCompletion;
-                const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, previousToken, formatContext, program.getSourceFiles(), preferences);
+                const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, position, previousToken, formatContext, preferences);
                 return createCompletionDetailsForSymbol(symbol, typeChecker, sourceFile, location!, cancellationToken, codeActions, sourceDisplay); // TODO: GH#18217
             }
             case "literal": {
@@ -652,9 +652,9 @@ namespace ts.Completions {
         host: LanguageServiceHost,
         compilerOptions: CompilerOptions,
         sourceFile: SourceFile,
+        position: number,
         previousToken: Node | undefined,
         formatContext: formatting.FormatContext,
-        allSourceFiles: ReadonlyArray<SourceFile>,
         preferences: UserPreferences,
     ): CodeActionsAndSourceDisplay {
         const symbolOriginInfo = symbolToOriginInfoMap[getSymbolId(symbol)];
@@ -671,10 +671,8 @@ namespace ts.Completions {
             getSymbolName(symbol, symbolOriginInfo, compilerOptions.target!),
             host,
             program,
-            checker,
-            allSourceFiles,
             formatContext,
-            previousToken,
+            previousToken && isIdentifier(previousToken) ? previousToken.getStart(sourceFile) : position,
             preferences);
         return { sourceDisplay: [textPart(moduleSpecifier)], codeActions: [codeAction] };
     }
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
@@ -333,7 +333,7 @@ namespace ts.textChanges {
             this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text });
         }
 
-        private insertText(sourceFile: SourceFile, pos: number, text: string): void {
+        public insertText(sourceFile: SourceFile, pos: number, text: string): void {
             this.replaceRangeWithText(sourceFile, createTextRange(pos), text);
         }
 
diff --git a/src/services/utilities.ts b/src/services/utilities.ts
@@ -1291,6 +1291,14 @@ namespace ts {
         }
     }
 
+    export function getQuoteFromPreference(qp: QuotePreference): string {
+        switch (qp) {
+            case QuotePreference.Single: return "'";
+            case QuotePreference.Double: return '"';
+            default: return Debug.assertNever(qp);
+        }
+    }
+
     export function symbolNameNoDefault(symbol: Symbol): string | undefined {
         const escaped = symbolEscapedNameNoDefault(symbol);
         return escaped === undefined ? undefined : unescapeLeadingUnderscores(escaped);
diff --git a/tests/cases/fourslash/completionsImport_importType.ts b/tests/cases/fourslash/completionsImport_importType.ts
@@ -0,0 +1,60 @@
+/// <reference path="fourslash.ts" />
+
+// @allowJs: true
+
+// @Filename: /a.js
+////export const x = 0;
+////export class C {}
+/////** @typedef {number} T */
+
+// @Filename: /b.js
+/////** @type {/*0*/} */
+/////** @type {/*1*/} */
+
+verify.completions({
+    marker: ["0", "1"],
+    includes: [
+        {
+            name: "C",
+            source: "/a",
+            sourceDisplay: "./a",
+            text: "class C",
+            hasAction: true,
+        },
+        {
+            name: "T",
+            source: "/a",
+            sourceDisplay: "./a",
+            text: "type T = number",
+            hasAction: true,
+        },
+    ],
+    excludes: "x",
+    preferences: {
+        includeCompletionsForModuleExports: true,
+    },
+});
+
+// Something with a value-side will get a normal import.
+verify.applyCodeActionFromCompletion("0", {
+    name: "C",
+    source: "/a",
+    description: `Import 'C' from module "./a"`,
+    newFileContent:
+`import { C } from "./a";
+
+/** @type {} */
+/** @type {} */`,
+});
+
+// A pure type will get `import().T`
+verify.applyCodeActionFromCompletion("1", {
+    name: "T",
+    source: "/a",
+    description: `Change 'T' to 'import("./a").T'`,
+    newFileContent:
+`import { C } from "./a";
+
+/** @type {} */
+/** @type {import("./a").} */`,
+});
diff --git a/tests/cases/fourslash/importNameCodeFix_all_js.ts b/tests/cases/fourslash/importNameCodeFix_all_js.ts
@@ -0,0 +1,25 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+
+// @Filename: /a.js
+////export class C {}
+/////** @typedef {number} T */
+
+// @Filename: /b.js
+////C;
+/////** @type {T} */
+////const x = 0;
+
+goTo.file("/b.js");
+verify.codeFixAll({
+    fixId: "fixMissingImport",
+    fixAllDescription: "Add all missing imports",
+    newFileContent:
+`import { C } from "./a";
+
+C;
+/** @type {import("./a").T} */
+const x = 0;`,
+});
diff --git a/tests/cases/fourslash/importNameCodeFix_importType.ts b/tests/cases/fourslash/importNameCodeFix_importType.ts
@@ -0,0 +1,17 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+// @checkJs: true
+
+// @Filename: /a.js
+////export {};
+/////** @typedef {number} T */
+
+// @Filename: /b.js
+/////** @type {T} */
+////const x = 0;
+
+goTo.file("/b.js");
+verify.importFixAtPosition([
+`/** @type {import("./a").T} */
+const x = 0;`]);

Original file line number	Diff line number	Diff line change
`@@ -333,7 +333,7 @@ namespace ts.textChanges {`
`333`	`333`	`this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text });`
`334`	`334`	`}`
`335`	`335`
`336`		`- private insertText(sourceFile: SourceFile, pos: number, text: string): void {`
	`336`	`+ public insertText(sourceFile: SourceFile, pos: number, text: string): void {`
`337`	`337`	`this.replaceRangeWithText(sourceFile, createTextRange(pos), text);`
`338`	`338`	`}`
`339`	`339`
Original file line number	Diff line number	Diff line change
`@@ -1291,6 +1291,14 @@ namespace ts {`
`1291`	`1291`	`}`
`1292`	`1292`	`}`
`1293`	`1293`
	`1294`	`+ export function getQuoteFromPreference(qp: QuotePreference): string {`
	`1295`	`+ switch (qp) {`
	`1296`	`+ case QuotePreference.Single: return "'";`
	`1297`	`+ case QuotePreference.Double: return '"';`
	`1298`	`+ default: return Debug.assertNever(qp);`
	`1299`	`+ }`
	`1300`	`+ }`
	`1301`	`+`
`1294`	`1302`	`export function symbolNameNoDefault(symbol: Symbol): string \| undefined {`
`1295`	`1303`	`const escaped = symbolEscapedNameNoDefault(symbol);`
`1296`	`1304`	`return escaped === undefined ? undefined : unescapeLeadingUnderscores(escaped);`