From 70cabdda419748c0ce10d0cd15e025cbb5bc0fae Mon Sep 17 00:00:00 2001 From: Aluan Haddad Date: Tue, 7 Nov 2017 01:55:37 -0500 Subject: [PATCH] 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 --- src/compiler/checker.ts | 2 +- src/compiler/core.ts | 7 ++++ src/compiler/diagnosticMessages.json | 8 ++++ src/services/codefixes/importFixes.ts | 39 +++++++++++++------ ...xNewImportAllowSyntheticDefaultImports0.ts | 18 +++++++++ ...xNewImportAllowSyntheticDefaultImports1.ts | 18 +++++++++ ...xNewImportAllowSyntheticDefaultImports2.ts | 18 +++++++++ 7 files changed, 98 insertions(+), 12 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports0.ts create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports1.ts create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports2.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 556046f3d76..7bce66892de 100644 --- a/src/compiler/checker.ts +++ b/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"); diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 5fb7019dfb2..4fb072b1112 100644 --- a/src/compiler/core.ts +++ b/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 { diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index d3669ce397d..60e3d6c4c4f 100644 --- a/src/compiler/diagnosticMessages.json +++ b/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 } } diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 3f0d27e5b07..73cedc883be 100644 --- a/src/services/codefixes/importFixes.ts +++ b/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): 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 }); } diff --git a/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports0.ts b/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports0.ts new file mode 100644 index 00000000000..3b62e0e9d5f --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports0.ts @@ -0,0 +1,18 @@ +/// +// @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();` + ]); \ No newline at end of file diff --git a/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports1.ts b/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports1.ts new file mode 100644 index 00000000000..c6c2a554874 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports1.ts @@ -0,0 +1,18 @@ +/// +// @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();` +]); \ No newline at end of file diff --git a/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports2.ts b/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports2.ts new file mode 100644 index 00000000000..3421d603297 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportAllowSyntheticDefaultImports2.ts @@ -0,0 +1,18 @@ +/// +// @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();` +]); \ No newline at end of file