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/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 5f1754c3036a9..267415f6cc08f 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.) */ @@ -163,7 +164,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 +176,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 +184,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 +330,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 +346,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 +358,10 @@ namespace ts.codefix { case ModuleKind.AMD: case ModuleKind.CommonJS: case ModuleKind.UMD: - return ImportKind.Equals; + if (isInJSFile(importingFile)) { + return isExternalModule(importingFile) ? ImportKind.Namespace : ImportKind.ConstEquals; + } + return ImportKind.Equals; case ModuleKind.System: case ModuleKind.ES2015: case ModuleKind.ESNext: @@ -403,7 +407,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,20 +422,41 @@ 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); + importingFile: SourceFile, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions, + ): { readonly symbol: Symbol, readonly symbolForMeaning: Symbol, readonly name: string, readonly kind: ImportKind } | undefined { + 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): { readonly symbol: Symbol, readonly kind: ImportKind.Default | ImportKind.Equals } | 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: ImportKind.Equals }; + return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: getExportEqualsImportKind(importingFile, compilerOptions, checker) }; + } + + function getExportEqualsImportKind(importingFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker): ImportKind { + if (getAllowSyntheticDefaultImports(compilerOptions) && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015) { + return ImportKind.Default; + } + if (isInJSFile(importingFile)) { + return isExternalModule(importingFile) ? 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 { @@ -543,7 +568,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); @@ -554,12 +582,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/importNameCodeFixNewImportExportEqualsESNextInteropOff.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOff.ts new file mode 100644 index 0000000000000..329006cab93cd --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOff.ts @@ -0,0 +1,17 @@ +/// + +// @Module: esnext + +// @Filename: /foo.d.ts +////declare module "foo" { +//// const foo: number; +//// export = foo; +////} + +// @Filename: /index.ts +////foo + +goTo.file('/index.ts'); +verify.importFixAtPosition([`import foo = require("foo"); + +foo`]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOn.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOn.ts new file mode 100644 index 0000000000000..3803e05d0d161 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsESNextInteropOn.ts @@ -0,0 +1,18 @@ +/// + +// @EsModuleInterop: true +// @Module: es2015 + +// @Filename: /foo.d.ts +////declare module "foo" { +//// const foo: number; +//// export = foo; +////} + +// @Filename: /index.ts +////[|foo|] + +goTo.file('/index.ts'); +verify.importFixAtPosition([`import foo from "foo"; + +foo`]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts new file mode 100644 index 0000000000000..d7a9e82c200a8 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixNewImportExportEqualsJavaScript.ts @@ -0,0 +1,31 @@ +/// + +// @allowJs: true +// @checkJs: true + +// @Filename: /foo.d.ts +////declare module "foo" { +//// const foo: number; +//// export = foo; +////} + +// @Filename: /a.js +////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`]); diff --git a/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts b/tests/cases/fourslash/importNameCodeFixUMDGlobalJavaScript.ts new file mode 100644 index 0000000000000..161496cc92aab --- /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 * as bar1 from "./foo"; + +export function test() { }; +bar1.bar;` +]); \ No newline at end of file