fix inconsistencies in import UMD code fixes adapting to module format (#19572)

* improve import code fixes for UMD modules
 - use default import under --allowSyntheticDefaultImports
 - import..require support
 - make make quick fix info match resulting import
 - make diagnostics

* Address PR feedback:
 - extract test for synethetic default imports into getAllowSyntheticDefaultImports in core.ts
 - use getAllowSyntheticDefaultImports in checker.ts and importFixes.ts
 - move compilerOptions to top level destructuring

* add tests

* remove `import =` quick fix and supporting code.

* update feature tests

* remove errant whitespace

Author: aluanhaddad

src/compiler/checker.ts

@@ -66,7 +66,7 @@ namespace ts {
         const languageVersion = getEmitScriptTarget(compilerOptions);
         const modulekind = getEmitModuleKind(compilerOptions);
         const noUnusedIdentifiers = !!compilerOptions.noUnusedLocals || !!compilerOptions.noUnusedParameters;
-        const allowSyntheticDefaultImports = typeof compilerOptions.allowSyntheticDefaultImports !== "undefined" ? compilerOptions.allowSyntheticDefaultImports : modulekind === ModuleKind.System;
+        const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);
         const strictNullChecks = getStrictOptionValue(compilerOptions, "strictNullChecks");
         const strictFunctionTypes = getStrictOptionValue(compilerOptions, "strictFunctionTypes");
         const noImplicitAny = getStrictOptionValue(compilerOptions, "noImplicitAny");

src/compiler/core.ts

@@ -1916,6 +1916,13 @@ namespace ts {
         return moduleResolution;
     }
 
+    export function getAllowSyntheticDefaultImports(compilerOptions: CompilerOptions) {
+        const moduleKind = getEmitModuleKind(compilerOptions);
+        return compilerOptions.allowSyntheticDefaultImports !== undefined
+            ? compilerOptions.allowSyntheticDefaultImports
+            : moduleKind === ModuleKind.System;
+    }
+
     export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "alwaysStrict";
 
     export function getStrictOptionValue(compilerOptions: CompilerOptions, flag: StrictOptionName): boolean {

src/compiler/diagnosticMessages.json

@@ -3801,5 +3801,13 @@
     "Install '{0}'": {
         "category": "Message",
         "code": 95014
+    },
+    "Import '{0}' = require(\"{1}\").": {
+        "category": "Message",
+        "code": 95015
+    },
+    "Import * as '{0}' from \"{1}\".": {
+        "category": "Message",
+        "code": 95016
     }
 }

src/services/codefixes/importFixes.ts

@@ -180,7 +180,7 @@ namespace ts.codefix {
     export const enum ImportKind {
         Named,
         Default,
-        Namespace,
+        Namespace
     }
 
     export function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixOptions): ImportCodeAction[] {
@@ -212,7 +212,7 @@ namespace ts.codefix {
 
     function getNamespaceImportName(declaration: AnyImportSyntax): Identifier {
         if (declaration.kind === SyntaxKind.ImportDeclaration) {
-            const namedBindings = declaration.importClause && declaration.importClause.namedBindings;
+            const namedBindings = declaration.importClause && isImportClause(declaration.importClause) && declaration.importClause.namedBindings;
             return namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport ? namedBindings.name : undefined;
         }
         else {
@@ -237,6 +237,8 @@ namespace ts.codefix {
                 return parent as ImportDeclaration;
             case SyntaxKind.ExternalModuleReference:
                 return (parent as ExternalModuleReference).parent;
+            case SyntaxKind.ImportEqualsDeclaration:
+                return parent as ImportEqualsDeclaration;
             default:
                 Debug.assert(parent.kind === SyntaxKind.ExportDeclaration);
                 // Ignore these, can't add imports to them.
@@ -249,11 +251,13 @@ namespace ts.codefix {
         const lastImportDeclaration = findLast(sourceFile.statements, isAnyImportSyntax);
 
         const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier);
+        const quotedModuleSpecifier = createStringLiteralWithQuoteStyle(sourceFile, moduleSpecifierWithoutQuotes);
         const importDecl = createImportDeclaration(
-            /*decorators*/ undefined,
-            /*modifiers*/ undefined,
+                /*decorators*/ undefined,
+                /*modifiers*/ undefined,
             createImportClauseOfKind(kind, symbolName),
-            createStringLiteralWithQuoteStyle(sourceFile, moduleSpecifierWithoutQuotes));
+            quotedModuleSpecifier);
+
         const changes = ChangeTracker.with(context, changeTracker => {
             if (lastImportDeclaration) {
                 changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: newLineCharacter });
@@ -263,11 +267,15 @@ namespace ts.codefix {
             }
         });
 
+        const actionFormat = kind === ImportKind.Namespace
+            ? Diagnostics.Import_Asterisk_as_0_from_1
+                : Diagnostics.Import_0_from_1;
+
         // if this file doesn't have any import statements, insert an import statement and then insert a new line
         // between the only import statement and user code. Otherwise just insert the statement because chances
         // are there are already a new line seperating code and import statements.
         return createCodeAction(
-            Diagnostics.Import_0_from_1,
+            actionFormat,
             [symbolName, moduleSpecifierWithoutQuotes],
             changes,
             "NewImport",
@@ -282,7 +290,7 @@ namespace ts.codefix {
         return literal;
     }
 
-    function createImportClauseOfKind(kind: ImportKind, symbolName: string) {
+    function createImportClauseOfKind(kind: ImportKind.Default | ImportKind.Named | ImportKind.Namespace, symbolName: string) {
         const id = createIdentifier(symbolName);
         switch (kind) {
             case ImportKind.Default:
@@ -534,7 +542,7 @@ namespace ts.codefix {
         declarations: ReadonlyArray<AnyImportSyntax>): ImportCodeAction {
         const fromExistingImport = firstDefined(declarations, declaration => {
             if (declaration.kind === SyntaxKind.ImportDeclaration && declaration.importClause) {
-                const changes = tryUpdateExistingImport(ctx, declaration.importClause);
+                const changes = tryUpdateExistingImport(ctx, isImportClause(declaration.importClause) && declaration.importClause || undefined);
                 if (changes) {
                     const moduleSpecifierWithoutQuotes = stripQuotes(declaration.moduleSpecifier.getText());
                     return createCodeAction(
@@ -564,9 +572,10 @@ namespace ts.codefix {
         return expression && isStringLiteral(expression) ? expression.text : undefined;
     }
 
-    function tryUpdateExistingImport(context: SymbolContext & { kind: ImportKind }, importClause: ImportClause): FileTextChanges[] | undefined {
+    function tryUpdateExistingImport(context: SymbolContext & { kind: ImportKind }, importClause: ImportClause | ImportEqualsDeclaration): FileTextChanges[] | undefined {
         const { symbolName, sourceFile, kind } = context;
-        const { name, namedBindings } = importClause;
+        const { name } = importClause;
+        const { namedBindings } = importClause.kind !== SyntaxKind.ImportEqualsDeclaration && importClause;
         switch (kind) {
             case ImportKind.Default:
                 return name ? undefined : ChangeTracker.with(context, t =>
@@ -627,7 +636,7 @@ namespace ts.codefix {
     }
 
     function getActionsForUMDImport(context: ImportCodeFixContext): ImportCodeAction[] {
-        const { checker, symbolToken } = context;
+        const { checker, symbolToken, compilerOptions } = context;
         const umdSymbol = checker.getSymbolAtLocation(symbolToken);
         let symbol: ts.Symbol;
         let symbolName: string;
@@ -644,6 +653,14 @@ namespace ts.codefix {
             Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here");
         }
 
+        const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);
+
+        // Import a synthetic `default` if enabled.
+        if (allowSyntheticDefaultImports) {
+            return getCodeActionForImport(symbol, { ...context, symbolName, kind: ImportKind.Default });
+        }
+
+        // Fall back to the `import * as ns` style import.
         return getCodeActionForImport(symbol, { ...context, symbolName, kind: ImportKind.Namespace });
     }
 

tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports0.ts

@@ -0,0 +1,18 @@
+/// <reference path="fourslash.ts" />
+// @AllowSyntheticDefaultImports: true
+
+// @Filename: a/f1.ts
+//// [|export var x = 0;
+//// bar/*0*/();|]
+
+// @Filename: a/foo.d.ts
+//// declare function bar(): number;
+//// export = bar;
+//// export as namespace bar;
+
+verify.importFixAtPosition([
+`import bar from "./foo";
+
+export var x = 0;
+bar();`
+  ]);

tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports1.ts

@@ -0,0 +1,18 @@
+/// <reference path="fourslash.ts" />
+// @Module: system
+
+// @Filename: a/f1.ts
+//// [|export var x = 0;
+//// bar/*0*/();|]
+
+// @Filename: a/foo.d.ts
+//// declare function bar(): number;
+//// export = bar;
+//// export as namespace bar;
+
+verify.importFixAtPosition([
+`import bar from "./foo";
+
+export var x = 0;
+bar();`
+]);

tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports2.ts

@@ -0,0 +1,18 @@
+/// <reference path="fourslash.ts" />
+// @AllowSyntheticDefaultImports: false
+
+// @Filename: a/f1.ts
+//// [|export var x = 0;
+//// bar/*0*/();|]
+
+// @Filename: a/foo.d.ts
+//// declare function bar(): number;
+//// export = bar;
+//// export as namespace bar;
+
+verify.importFixAtPosition([
+`import * as bar from "./foo";
+
+export var x = 0;
+bar();`
+]);