From 2ca96c1783e5bef10295b8d83e0ccf3e291c6c70 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 27 Jun 2019 11:42:13 -0700 Subject: [PATCH 1/8] Add failing tests --- src/harness/fourslash.ts | 5 ++++- ...FixNewImportExportEqualsESModuleInterop.ts | 19 +++++++++++++++++++ ...tNameCodeFixNewImportExportEqualsESNext.ts | 19 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 8f1a5ae15280d..2e1f1040afe30 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2322,7 +2322,10 @@ namespace FourSlash { public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) { this.goToMarker(markerName); - const details = this.getCompletionEntryDetails(options.name, options.source, options.preferences)!; + const details = this.getCompletionEntryDetails(options.name, options.source, options.preferences); + if (!details) { + return this.raiseError(`No completions were found for the given name, source, and preferences.`); + } const codeActions = details.codeActions!; if (codeActions.length !== 1) { this.raiseError(`Expected one code action, got ${codeActions.length}`); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts new file mode 100644 index 0000000000000..2c61d1fee398d --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts @@ -0,0 +1,19 @@ +/// + +// @EsModuleInterop: true + +// @Filename: /foo.d.ts +////declare module "foo" { +//// const _default: number; +//// export = _default; +////} + +// @Filename: /index.ts +////foo/**/ + +goTo.marker(''); +verify.getAndApplyCodeFix(2304, 0); +verify.currentFileContentIs( +`import foo from './a'; + +foo`); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts new file mode 100644 index 0000000000000..c8ccf5a51f552 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts @@ -0,0 +1,19 @@ +/// + +// @Module: esnext + +// @Filename: /foo.d.ts +////declare module "foo" { +//// const _default: number; +//// export = _default; +////} + +// @Filename: /index.ts +////foo/**/ + +goTo.marker(''); +verify.getAndApplyCodeFix(2304, 0); +verify.currentFileContentIs( +`import * as foo from './a'; + +foo`); From b31b4f86a46e24cb3d9689517c42526b0072c68c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 27 Jun 2019 13:24:08 -0700 Subject: [PATCH 2/8] Use default import or namespace import for import fixes when compiler options allow --- src/services/codefixes/importFixes.ts | 18 ++++++++++++++---- ...eFixNewImportExportEqualsESModuleInterop.ts | 2 +- ...rtNameCodeFixNewImportExportEqualsESNext.ts | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 5f1754c3036a9..55bc1646c6262 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -419,19 +419,29 @@ namespace ts.codefix { function getDefaultLikeExportInfo( moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions, - ): { readonly symbol: Symbol, readonly symbolForMeaning: Symbol, readonly name: string, readonly kind: ImportKind.Default | ImportKind.Equals } | undefined { - const exported = getDefaultLikeExportWorker(moduleSymbol, checker); + ): { readonly symbol: Symbol, readonly symbolForMeaning: Symbol, readonly name: string, readonly kind: ImportKind } | undefined { + const exported = getDefaultLikeExportWorker(moduleSymbol, checker, compilerOptions); if (!exported) return undefined; const { symbol, kind } = exported; const info = getDefaultExportInfoWorker(symbol, moduleSymbol, checker, compilerOptions); return info && { symbol, kind, ...info }; } - function getDefaultLikeExportWorker(moduleSymbol: Symbol, checker: TypeChecker): { readonly symbol: Symbol, readonly kind: ImportKind.Default | ImportKind.Equals } | undefined { + function getDefaultLikeExportWorker(moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbol: Symbol, readonly kind: ImportKind } | undefined { const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol); if (defaultExport) return { symbol: defaultExport, kind: ImportKind.Default }; const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol); - return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: ImportKind.Equals }; + return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getImportKindForExportEquals(compilerOptions) }; + } + + function getImportKindForExportEquals(compilerOptions: CompilerOptions): ImportKind { + if (getAllowSyntheticDefaultImports(compilerOptions)) { + return ImportKind.Default; + } + if (getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) { + return ImportKind.Namespace; + } + return ImportKind.Equals; } function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts index 2c61d1fee398d..88565fb26ef57 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts @@ -14,6 +14,6 @@ goTo.marker(''); verify.getAndApplyCodeFix(2304, 0); verify.currentFileContentIs( -`import foo from './a'; +`import foo from "foo"; foo`); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts index c8ccf5a51f552..8f9e857f6e790 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts @@ -14,6 +14,6 @@ goTo.marker(''); verify.getAndApplyCodeFix(2304, 0); verify.currentFileContentIs( -`import * as foo from './a'; +`import * as foo from "foo"; foo`); From a6bd95f6f08a7ebd58925d8aa2c174a2563f0b50 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 23 Jul 2019 09:13:44 -0700 Subject: [PATCH 3/8] =?UTF-8?q?Don=E2=80=99t=20do=20import=20*=20for=20exp?= =?UTF-8?q?ort=3D,=20ever?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/services/codefixes/importFixes.ts | 10 +++------- .../importNameCodeFixNewImportExportEqualsESNext.ts | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 55bc1646c6262..01857b78a8a2e 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -435,13 +435,9 @@ namespace ts.codefix { } function getImportKindForExportEquals(compilerOptions: CompilerOptions): ImportKind { - if (getAllowSyntheticDefaultImports(compilerOptions)) { - return ImportKind.Default; - } - if (getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) { - return ImportKind.Namespace; - } - return ImportKind.Equals; + return getAllowSyntheticDefaultImports(compilerOptions) + ? ImportKind.Default + : ImportKind.Equals; } function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts index 8f9e857f6e790..b492fba107219 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts @@ -14,6 +14,6 @@ goTo.marker(''); verify.getAndApplyCodeFix(2304, 0); verify.currentFileContentIs( -`import * as foo from "foo"; +`import foo = require("foo"); foo`); From 4566e74c527766d823fea5986f3e46d6cc0245b0 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 1 Aug 2019 09:54:59 -0700 Subject: [PATCH 4/8] Only do import default for export equals if nothing else will work --- src/services/codefixes/importFixes.ts | 2 +- .../importNameCodeFixNewImportExportEqualsESModuleInterop.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 01857b78a8a2e..af930d522b3f4 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -435,7 +435,7 @@ namespace ts.codefix { } function getImportKindForExportEquals(compilerOptions: CompilerOptions): ImportKind { - return getAllowSyntheticDefaultImports(compilerOptions) + return getAllowSyntheticDefaultImports(compilerOptions) && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015 ? ImportKind.Default : ImportKind.Equals; } diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts index 88565fb26ef57..41647e5bd18ee 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts @@ -1,6 +1,7 @@ /// // @EsModuleInterop: true +// @Module: es2015 // @Filename: /foo.d.ts ////declare module "foo" { From c9006dbe8a9999dfca8d4935fc3207b8fb1ece54 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 1 Aug 2019 10:35:38 -0700 Subject: [PATCH 5/8] Never do import/require in a JavaScript file --- src/services/codefixes/importFixes.ts | 26 +++++++++---------- ...eCodeFixNewImportExportEqualsJavaScript.ts | 20 ++++++++++++++ .../importNameCodeFixUMDGlobalJavaScript.ts | 21 +++++++++++++++ 3 files changed, 54 insertions(+), 13 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts create mode 100644 tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index af930d522b3f4..77ae2d687e7a5 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -163,7 +163,7 @@ namespace ts.codefix { position: number, preferences: UserPreferences, ): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } { - const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getCompilerOptions(), program.getTypeChecker(), program.getSourceFiles()); + const exportInfos = getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getCompilerOptions(), program.getTypeChecker(), program.getSourceFiles()); Debug.assert(exportInfos.some(info => info.moduleSymbol === moduleSymbol)); // We sort the best codefixes first, so taking `first` is best for completions. const moduleSpecifier = first(getNewImportInfos(program, sourceFile, position, exportInfos, host, preferences)).moduleSpecifier; @@ -175,7 +175,7 @@ namespace ts.codefix { return { description, changes, commands }; } - function getAllReExportingModules(exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker, allSourceFiles: ReadonlyArray): ReadonlyArray { + function getAllReExportingModules(importingFile: SourceFile, exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker, allSourceFiles: ReadonlyArray): ReadonlyArray { const result: SymbolExportInfo[] = []; forEachExternalModule(checker, allSourceFiles, (moduleSymbol, moduleFile) => { // Don't import from a re-export when looking "up" like to `./index` or `../index`. @@ -183,7 +183,7 @@ namespace ts.codefix { return; } - const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions); + const defaultInfo = getDefaultLikeExportInfo(importingFile, moduleSymbol, checker, compilerOptions); if (defaultInfo && defaultInfo.name === symbolName && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) { result.push({ moduleSymbol, importKind: defaultInfo.kind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(defaultInfo.symbol, checker) }); } @@ -329,7 +329,7 @@ namespace ts.codefix { if (!umdSymbol) return undefined; const symbol = checker.getAliasedSymbol(umdSymbol); const symbolName = umdSymbol.name; - const exportInfos: ReadonlyArray = [{ moduleSymbol: symbol, importKind: getUmdImportKind(program.getCompilerOptions()), exportedSymbolIsTypeOnly: false }]; + const exportInfos: ReadonlyArray = [{ moduleSymbol: symbol, importKind: getUmdImportKind(sourceFile, program.getCompilerOptions()), exportedSymbolIsTypeOnly: false }]; const fixes = getFixForImport(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, program, sourceFile, host, preferences); return { fixes, symbolName }; } @@ -345,7 +345,7 @@ namespace ts.codefix { : undefined; } - function getUmdImportKind(compilerOptions: CompilerOptions): ImportKind { + function getUmdImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind { // Import a synthetic `default` if enabled. if (getAllowSyntheticDefaultImports(compilerOptions)) { return ImportKind.Default; @@ -357,7 +357,7 @@ namespace ts.codefix { case ModuleKind.AMD: case ModuleKind.CommonJS: case ModuleKind.UMD: - return ImportKind.Equals; + return isInJSFile(importingFile) ? ImportKind.Default : ImportKind.Equals; case ModuleKind.System: case ModuleKind.ES2015: case ModuleKind.ESNext: @@ -403,7 +403,7 @@ namespace ts.codefix { forEachExternalModuleToImportFrom(checker, sourceFile, program.getSourceFiles(), moduleSymbol => { cancellationToken.throwIfCancellationRequested(); - const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, program.getCompilerOptions()); + const defaultInfo = getDefaultLikeExportInfo(sourceFile, moduleSymbol, checker, program.getCompilerOptions()); if (defaultInfo && defaultInfo.name === symbolName && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) { addSymbol(moduleSymbol, defaultInfo.symbol, defaultInfo.kind); } @@ -418,24 +418,24 @@ namespace ts.codefix { } function getDefaultLikeExportInfo( - moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions, + importingFile: SourceFile, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions, ): { readonly symbol: Symbol, readonly symbolForMeaning: Symbol, readonly name: string, readonly kind: ImportKind } | undefined { - const exported = getDefaultLikeExportWorker(moduleSymbol, checker, compilerOptions); + const exported = getDefaultLikeExportWorker(importingFile, moduleSymbol, checker, compilerOptions); if (!exported) return undefined; const { symbol, kind } = exported; const info = getDefaultExportInfoWorker(symbol, moduleSymbol, checker, compilerOptions); return info && { symbol, kind, ...info }; } - function getDefaultLikeExportWorker(moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbol: Symbol, readonly kind: ImportKind } | undefined { + function getDefaultLikeExportWorker(importingFile: SourceFile, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbol: Symbol, readonly kind: ImportKind } | undefined { const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol); if (defaultExport) return { symbol: defaultExport, kind: ImportKind.Default }; const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol); - return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getImportKindForExportEquals(compilerOptions) }; + return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getImportKindForExportEquals(importingFile, compilerOptions) }; } - function getImportKindForExportEquals(compilerOptions: CompilerOptions): ImportKind { - return getAllowSyntheticDefaultImports(compilerOptions) && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015 + function getImportKindForExportEquals(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind { + return (isInJSFile(importingFile) || getAllowSyntheticDefaultImports(compilerOptions) && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) ? ImportKind.Default : ImportKind.Equals; } diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts new file mode 100644 index 0000000000000..e5b16c920fd0e --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts @@ -0,0 +1,20 @@ +/// + +// @allowJs: true +// @checkJs: true + +// @Filename: /foo.d.ts +////declare module "foo" { +//// const _default: number; +//// export = _default; +////} + +// @Filename: /index.js +////foo/**/ + +goTo.marker(''); +verify.getAndApplyCodeFix(2304, 0); +verify.currentFileContentIs( +`import foo from "foo"; + +foo`); diff --git a/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts b/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts new file mode 100644 index 0000000000000..f00a17bbd3e3b --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts @@ -0,0 +1,21 @@ +/// + +// @AllowSyntheticDefaultImports: false +// @Module: commonjs +// @CheckJs: true +// @AllowJs: true + +// @Filename: a/f1.js +//// [|export function test() { }; +//// bar1/*0*/.bar;|] + +// @Filename: a/foo.d.ts +//// export declare function bar(): number; +//// export as namespace bar1; + +verify.importFixAtPosition([ +`import bar1 from "./foo"; + +export function test() { }; +bar1.bar;` +]); \ No newline at end of file From 385f2f57419142d064c1dca3e66097ad53c81ac6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 1 Aug 2019 15:29:33 -0700 Subject: [PATCH 6/8] Update tests for changes in master --- ...eCodeFixNewImportExportEqualsESModuleInterop.ts | 14 ++++++-------- ...importNameCodeFixNewImportExportEqualsESNext.ts | 14 ++++++-------- ...rtNameCodeFixNewImportExportEqualsJavaScript.ts | 14 ++++++-------- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts index 41647e5bd18ee..3803e05d0d161 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts @@ -5,16 +5,14 @@ // @Filename: /foo.d.ts ////declare module "foo" { -//// const _default: number; -//// export = _default; +//// const foo: number; +//// export = foo; ////} // @Filename: /index.ts -////foo/**/ +////[|foo|] -goTo.marker(''); -verify.getAndApplyCodeFix(2304, 0); -verify.currentFileContentIs( -`import foo from "foo"; +goTo.file('/index.ts'); +verify.importFixAtPosition([`import foo from "foo"; -foo`); +foo`]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts index b492fba107219..a87e50b00006b 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts @@ -4,16 +4,14 @@ // @Filename: /foo.d.ts ////declare module "foo" { -//// const _default: number; -//// export = _default; +//// const foo: number; +//// export = foo; ////} // @Filename: /index.ts -////foo/**/ +////[|foo|] -goTo.marker(''); -verify.getAndApplyCodeFix(2304, 0); -verify.currentFileContentIs( -`import foo = require("foo"); +goTo.file('/index.ts'); +verify.importFixAtPosition([`import foo = require("foo"); -foo`); +foo`]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts index e5b16c920fd0e..9cec36a99d2a9 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts @@ -5,16 +5,14 @@ // @Filename: /foo.d.ts ////declare module "foo" { -//// const _default: number; -//// export = _default; +//// const foo: number; +//// export = foo; ////} // @Filename: /index.js -////foo/**/ +////[|foo|] -goTo.marker(''); -verify.getAndApplyCodeFix(2304, 0); -verify.currentFileContentIs( -`import foo from "foo"; +goTo.file('/index.js'); +verify.importFixAtPosition([`import foo from "foo"; -foo`); +foo`]); From 7601ff81831942043628c01b2530dc89196ae341 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 1 Aug 2019 17:28:44 -0700 Subject: [PATCH 7/8] Add const/require fix for JS and select based on usage heuristic --- src/services/codefixes/importFixes.ts | 52 +++++++++++++++---- ...xNewImportExportEqualsCommonJSInteropOn.ts | 51 ++++++++++++++++++ ...xNewImportExportEqualsESNextInteropOff.ts} | 2 +- ...ixNewImportExportEqualsESNextInteropOn.ts} | 0 ...eCodeFixNewImportExportEqualsJavaScript.ts | 21 ++++++-- 5 files changed, 111 insertions(+), 15 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFixNewImportExportEqualsCommonJSInteropOn.ts rename tests/cases/fourslash/{importNameCodeFixNewImportExportEqualsESNext.ts => importNameCodeFixNewImportExportEqualsESNextInteropOff.ts} (95%) rename tests/cases/fourslash/{importNameCodeFixNewImportExportEqualsESModuleInterop.ts => importNameCodeFixNewImportExportEqualsESNextInteropOn.ts} (100%) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 77ae2d687e7a5..41f82621c6d1d 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -135,7 +135,8 @@ namespace ts.codefix { Named, Default, Namespace, - Equals + Equals, + ConstEquals } /** Information about how a symbol is exported from a module. (We don't need to store the exported symbol, just its module.) */ @@ -431,13 +432,28 @@ namespace ts.codefix { const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol); if (defaultExport) return { symbol: defaultExport, kind: ImportKind.Default }; const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol); - return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getImportKindForExportEquals(importingFile, compilerOptions) }; + return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getImportKindForExportEquals(importingFile, compilerOptions, checker) }; } - function getImportKindForExportEquals(importingFile: SourceFile, compilerOptions: CompilerOptions): ImportKind { - return (isInJSFile(importingFile) || getAllowSyntheticDefaultImports(compilerOptions) && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) - ? ImportKind.Default - : ImportKind.Equals; + function getImportKindForExportEquals(importingFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker): ImportKind { + if (getAllowSyntheticDefaultImports(compilerOptions) && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) { + return ImportKind.Default; + } + if (isInJSFile(importingFile)) { + return some(importingFile.statements, isImportDeclaration) ? ImportKind.Default : ImportKind.ConstEquals; + } + for (const statement of importingFile.statements) { + if (isImportEqualsDeclaration(statement)) { + return ImportKind.Equals; + } + if (isImportDeclaration(statement) && statement.importClause && statement.importClause.name) { + const moduleSymbol = checker.getImmediateAliasedSymbol(statement.importClause.symbol); + if (moduleSymbol && moduleSymbol.name !== InternalSymbolName.Default) { + return ImportKind.Default; + } + } + } + return ImportKind.Equals; } function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { @@ -549,7 +565,10 @@ namespace ts.codefix { interface ImportsCollection { readonly defaultImport: string | undefined; readonly namedImports: string[]; - readonly namespaceLikeImport: { readonly importKind: ImportKind.Equals | ImportKind.Namespace, readonly name: string } | undefined; + readonly namespaceLikeImport: { + readonly importKind: ImportKind.Equals | ImportKind.Namespace | ImportKind.ConstEquals; + readonly name: string; + } | undefined; } function addNewImports(changes: textChanges.ChangeTracker, sourceFile: SourceFile, moduleSpecifier: string, quotePreference: QuotePreference, { defaultImport, namedImports, namespaceLikeImport }: ImportsCollection): void { const quotedModuleSpecifier = makeStringLiteral(moduleSpecifier, quotePreference); @@ -560,12 +579,25 @@ namespace ts.codefix { namedImports.map(n => createImportSpecifier(/*propertyName*/ undefined, createIdentifier(n))), moduleSpecifier, quotePreference)); } if (namespaceLikeImport) { - insertImport(changes, sourceFile, namespaceLikeImport.importKind === ImportKind.Equals - ? createImportEqualsDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createIdentifier(namespaceLikeImport.name), createExternalModuleReference(quotedModuleSpecifier)) - : createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(namespaceLikeImport.name))), quotedModuleSpecifier)); + insertImport( + changes, + sourceFile, + namespaceLikeImport.importKind === ImportKind.Equals ? createImportEqualsDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createIdentifier(namespaceLikeImport.name), createExternalModuleReference(quotedModuleSpecifier)) : + namespaceLikeImport.importKind === ImportKind.ConstEquals ? createConstEqualsRequireDeclaration(namespaceLikeImport.name, quotedModuleSpecifier) : + createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(namespaceLikeImport.name))), quotedModuleSpecifier)); } } + function createConstEqualsRequireDeclaration(name: string, quotedModuleSpecifier: StringLiteral): VariableStatement { + return createVariableStatement(/*modifiers*/ undefined, createVariableDeclarationList([ + createVariableDeclaration( + createIdentifier(name), + /*type*/ undefined, + createCall(createIdentifier("require"), /*typeArguments*/ undefined, [quotedModuleSpecifier]) + ) + ], NodeFlags.Const)); + } + function symbolHasMeaning({ declarations }: Symbol, meaning: SemanticMeaning): boolean { return some(declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)); } diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsCommonJSInteropOn.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsCommonJSInteropOn.ts new file mode 100644 index 0000000000000..e8f223ca9f4d8 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsCommonJSInteropOn.ts @@ -0,0 +1,51 @@ +/// + +// @Module: commonjs +// @EsModuleInterop: true + +// @Filename: /foo.d.ts +////declare module "bar" { +//// const bar: number; +//// export = bar; +////} +////declare module "foo" { +//// const foo: number; +//// export = foo; +////} +////declare module "es" { +//// const es = 0; +//// export default es; +////} + +// @Filename: /a.ts +////import bar from "bar"; +//// +////foo + +// @Filename: /b.ts +////foo + +// @Filename: /c.ts +////import es from "es"; +//// +////foo + +// 1. Should match existing imports of 'export =' +goTo.file('/a.ts'); +verify.importFixAtPosition([`import bar from "bar"; +import foo from "foo"; + +foo`]); + +// 2. Should default to ImportEquals +goTo.file('/b.ts'); +verify.importFixAtPosition([`import foo = require("foo"); + +foo`]); + +// 3. Importing an 'export default' doesn’t count toward the usage heursitic +goTo.file('/c.ts'); +verify.importFixAtPosition([`import es from "es"; +import foo = require("foo"); + +foo`]); \ No newline at end of file diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOff.ts similarity index 95% rename from tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts rename to tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOff.ts index a87e50b00006b..329006cab93cd 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNext.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOff.ts @@ -9,7 +9,7 @@ ////} // @Filename: /index.ts -////[|foo|] +////foo goTo.file('/index.ts'); verify.importFixAtPosition([`import foo = require("foo"); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOn.ts similarity index 100% rename from tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESModuleInterop.ts rename to tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOn.ts diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts index 9cec36a99d2a9..d7a9e82c200a8 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts @@ -9,10 +9,23 @@ //// export = foo; ////} -// @Filename: /index.js -////[|foo|] +// @Filename: /a.js +////foo -goTo.file('/index.js'); -verify.importFixAtPosition([`import foo from "foo"; +// @Filename: /b.js +////import ""; +//// +////foo + +// 1. JavaScript should default to 'const ... = require...' without compiler flags set +goTo.file('/a.js'); +verify.importFixAtPosition([`const foo = require("foo"); + +foo`]); + +// 2. If there are any ImportDeclarations, assume a default import is fine +goTo.file('/b.js'); +verify.importFixAtPosition([`import ""; +import foo from "foo"; foo`]); From b7512bf84af7b2f3f3b0d2e3c7b4d6abbdea2976 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 2 Aug 2019 10:48:38 -0700 Subject: [PATCH 8/8] Fix JS UMD import --- src/services/codefixes/importFixes.ts | 11 +++++++---- .../fourslash/importNameCodeFixUMDGlobalJavaScript.ts | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 41f82621c6d1d..267415f6cc08f 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -358,7 +358,10 @@ namespace ts.codefix { case ModuleKind.AMD: case ModuleKind.CommonJS: case ModuleKind.UMD: - return isInJSFile(importingFile) ? ImportKind.Default : ImportKind.Equals; + if (isInJSFile(importingFile)) { + return isExternalModule(importingFile) ? ImportKind.Namespace : ImportKind.ConstEquals; + } + return ImportKind.Equals; case ModuleKind.System: case ModuleKind.ES2015: case ModuleKind.ESNext: @@ -432,15 +435,15 @@ namespace ts.codefix { const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol); if (defaultExport) return { symbol: defaultExport, kind: ImportKind.Default }; const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol); - return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getImportKindForExportEquals(importingFile, compilerOptions, checker) }; + return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getExportEqualsImportKind(importingFile, compilerOptions, checker) }; } - function getImportKindForExportEquals(importingFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker): ImportKind { + function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker): ImportKind { if (getAllowSyntheticDefaultImports(compilerOptions) && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) { return ImportKind.Default; } if (isInJSFile(importingFile)) { - return some(importingFile.statements, isImportDeclaration) ? ImportKind.Default : ImportKind.ConstEquals; + return isExternalModule(importingFile) ? ImportKind.Default : ImportKind.ConstEquals; } for (const statement of importingFile.statements) { if (isImportEqualsDeclaration(statement)) { diff --git a/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts b/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts index f00a17bbd3e3b..161496cc92aab 100644 --- a/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts +++ b/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts @@ -14,7 +14,7 @@ //// export as namespace bar1; verify.importFixAtPosition([ -`import bar1 from "./foo"; +`import * as bar1 from "./foo"; export function test() { }; bar1.bar;`