Fix import statement completions for export= in JS files (microsoft#45128)

Author: andrewbranch

src/services/codefixes/importFixes.ts

@@ -560,17 +560,21 @@ namespace ts.codefix {
             : undefined;
     }
 
-    export function getImportKind(importingFile: SourceFile, exportKind: ExportKind, compilerOptions: CompilerOptions): ImportKind {
+    /**
+     * @param forceImportKeyword Indicates that the user has already typed `import`, so the result must start with `import`.
+     * (In other words, do not allow `const x = require("...")` for JS files.)
+     */
+    export function getImportKind(importingFile: SourceFile, exportKind: ExportKind, compilerOptions: CompilerOptions, forceImportKeyword?: boolean): ImportKind {
         switch (exportKind) {
             case ExportKind.Named: return ImportKind.Named;
             case ExportKind.Default: return ImportKind.Default;
-            case ExportKind.ExportEquals: return getExportEqualsImportKind(importingFile, compilerOptions);
-            case ExportKind.UMD: return getUmdImportKind(importingFile, compilerOptions);
+            case ExportKind.ExportEquals: return getExportEqualsImportKind(importingFile, compilerOptions, !!forceImportKeyword);
+            case ExportKind.UMD: return getUmdImportKind(importingFile, compilerOptions, !!forceImportKeyword);
             default: return Debug.assertNever(exportKind);
         }
     }
 
-    function getUmdImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind {
+    function getUmdImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, forceImportKeyword: boolean): ImportKind {
         // Import a synthetic `default` if enabled.
         if (getAllowSyntheticDefaultImports(compilerOptions)) {
             return ImportKind.Default;
@@ -583,7 +587,7 @@ namespace ts.codefix {
             case ModuleKind.CommonJS:
             case ModuleKind.UMD:
                 if (isInJSFile(importingFile)) {
-                    return isExternalModule(importingFile) ? ImportKind.Namespace : ImportKind.CommonJS;
+                    return isExternalModule(importingFile) || forceImportKeyword ? ImportKind.Namespace : ImportKind.CommonJS;
                 }
                 return ImportKind.CommonJS;
             case ModuleKind.System:
@@ -671,18 +675,20 @@ namespace ts.codefix {
         return originalSymbolToExportInfos;
     }
 
-    function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind {
+    function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, forceImportKeyword: boolean): ImportKind {
         const allowSyntheticDefaults = getAllowSyntheticDefaultImports(compilerOptions);
         const isJS = isInJSFile(importingFile);
         // 1. 'import =' will not work in es2015+ TS files, so the decision is between a default
         //    and a namespace import, based on allowSyntheticDefaultImports/esModuleInterop.
         if (!isJS && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) {
             return allowSyntheticDefaults ? ImportKind.Default : ImportKind.Namespace;
         }
-        // 2. 'import =' will not work in JavaScript, so the decision is between a default
-        //    and const/require.
+        // 2. 'import =' will not work in JavaScript, so the decision is between a default import,
+        //    a namespace import, and const/require.
         if (isJS) {
-            return isExternalModule(importingFile) ? ImportKind.Default : ImportKind.CommonJS;
+            return isExternalModule(importingFile) || forceImportKeyword
+                ? allowSyntheticDefaults ? ImportKind.Default : ImportKind.Namespace
+                : ImportKind.CommonJS;
         }
         // 3. At this point the most correct choice is probably 'import =', but people
         //    really hate that, so look to see if the importing file has any precedent

src/services/completions.ts

@@ -713,12 +713,12 @@ namespace ts.Completions {
             origin.exportName === InternalSymbolName.ExportEquals ? ExportKind.ExportEquals :
             ExportKind.Named;
         const tabStop = preferences.includeCompletionsWithSnippetText ? "$1" : "";
-        const importKind = codefix.getImportKind(sourceFile, exportKind, options);
+        const importKind = codefix.getImportKind(sourceFile, exportKind, options, /*forceImportKeyword*/ true);
         const suffix = useSemicolons ? ";" : "";
         switch (importKind) {
             case ImportKind.CommonJS: return { replacementSpan, insertText: `import ${name}${tabStop} = require(${quotedModuleSpecifier})${suffix}` };
             case ImportKind.Default: return { replacementSpan, insertText: `import ${name}${tabStop} from ${quotedModuleSpecifier}${suffix}` };
-            case ImportKind.Namespace: return { replacementSpan, insertText: `import * as ${name}${tabStop} from ${quotedModuleSpecifier}${suffix}` };
+            case ImportKind.Namespace: return { replacementSpan, insertText: `import * as ${name} from ${quotedModuleSpecifier}${suffix}` };
             case ImportKind.Named: return { replacementSpan, insertText: `import { ${name}${tabStop} } from ${quotedModuleSpecifier}${suffix}` };
         }
     }

tests/cases/fourslash/importStatementCompletions_js.ts

@@ -0,0 +1,35 @@
+/// <reference path="fourslash.ts" />
+
+// These options resemble the defaults for inferred projects with JS
+
+// @allowJs: true
+// @target: es2020
+// @checkJs: true
+// @module: commonjs
+// @noEmit: true
+// @allowSyntheticDefaultImports: true
+
+// @Filename: /node_modules/react/index.d.ts
+//// declare namespace React {}
+//// export = React;
+
+// @Filename: /test.js
+//// [|import R/**/|]
+
+verify.completions({
+  marker: "",
+  isNewIdentifierLocation: true,
+  exact: [{
+    name: "React",
+    source: "react",
+    insertText: `import React$1 from "react";`,
+    isSnippet: true,
+    replacementSpan: test.ranges()[0],
+    sourceDisplay: "react",
+  }],
+  preferences: {
+    includeCompletionsForImportStatements: true,
+    includeInsertTextCompletions: true,
+    includeCompletionsWithSnippetText: true,
+  }
+});

tests/cases/fourslash/importStatementCompletions_js2.ts

@@ -0,0 +1,37 @@
+/// <reference path="fourslash.ts" />
+
+// These options resemble the defaults for inferred projects with JS,
+// except notably lacking --allowSyntheticDefaultImports. Probably nobody
+// ever wants a configuration like this, but we maintain that this is
+// correct and consistent behavior for these settings.
+
+// @allowJs: true
+// @target: es2020
+// @checkJs: true
+// @module: commonjs
+// @noEmit: true
+
+// @Filename: /node_modules/react/index.d.ts
+//// declare namespace React {}
+//// export = React;
+
+// @Filename: /test.js
+//// [|import R/**/|]
+
+verify.completions({
+  marker: "",
+  isNewIdentifierLocation: true,
+  exact: [{
+    name: "React",
+    source: "react",
+    insertText: `import * as React from "react";`,
+    isSnippet: true,
+    replacementSpan: test.ranges()[0],
+    sourceDisplay: "react",
+  }],
+  preferences: {
+    includeCompletionsForImportStatements: true,
+    includeInsertTextCompletions: true,
+    includeCompletionsWithSnippetText: true,
+  }
+});