From 2eba84cd29c846536284fc26941e5d52fdab2681 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 16 Jul 2018 18:37:48 -0700 Subject: [PATCH] Support import fix/completions for `export =` --- src/compiler/checker.ts | 2 - src/compiler/types.ts | 4 ++ src/harness/fourslash.ts | 6 ++- src/services/codefixes/importFixes.ts | 47 ++++++++++------ src/services/completions.ts | 8 +++ .../completionsImport_default_anonymous.ts | 2 +- .../completionsImport_exportEquals.ts | 53 +++++++++++++++++++ ...mport_exportEqualsNamespace_noDuplicate.ts | 2 +- .../cases/fourslash/completionsImport_tsx.ts | 2 +- .../completionsUniqueSymbol_import.ts | 2 +- .../cases/fourslash/importNameCodeFix_all.ts | 2 +- .../importNameCodeFix_exportEquals.ts | 25 +++++++++ 12 files changed, 130 insertions(+), 25 deletions(-) create mode 100644 tests/cases/fourslash/completionsImport_exportEquals.ts create mode 100644 tests/cases/fourslash/importNameCodeFix_exportEquals.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e59641bed412e..d0c7934ed9b2e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2253,8 +2253,6 @@ namespace ts { r && r.packageId && r.packageId.name === getTypesPackageName(packageName))); } - // An external module with an 'export =' declaration resolves to the target of the 'export =' declaration, - // and an external module with no 'export =' declaration resolves to the module itself. function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol; function resolveExternalModuleSymbol(moduleSymbol: Symbol | undefined, dontResolveAlias?: boolean): Symbol | undefined; function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index eb8a0c061d5c4..9b5035ed54e44 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3070,6 +3070,10 @@ namespace ts { */ /* @internal */ getAccessibleSymbolChain(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, useOnlyExternalAliasing: boolean): Symbol[] | undefined; /* @internal */ getTypePredicateOfSignature(signature: Signature): TypePredicate; + /** + * An external module with an 'export =' declaration resolves to the target of the 'export =' declaration, + * and an external module with no 'export =' declaration resolves to the module itself. + */ /* @internal */ resolveExternalModuleSymbol(symbol: Symbol): Symbol; /** @param node A location where we might consider accessing `this`. Not necessarily a ThisExpression. */ /* @internal */ tryGetThisTypeAt(node: Node): Type | undefined; diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index b7da07f603db7..f3f3cfd6f8f71 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -911,8 +911,8 @@ namespace FourSlash { } private verifyCompletionEntry(actual: ts.CompletionEntry, expected: FourSlashInterface.ExpectedCompletionEntry) { - const { insertText, replacementSpan, hasAction, isRecommended, kind, text, documentation, sourceDisplay } = typeof expected === "string" - ? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, kind: undefined, text: undefined, documentation: undefined, sourceDisplay: undefined } + const { insertText, replacementSpan, hasAction, isRecommended, kind, text, documentation, source, sourceDisplay } = typeof expected === "string" + ? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, kind: undefined, text: undefined, documentation: undefined, source: undefined, sourceDisplay: undefined } : expected; if (actual.insertText !== insertText) { @@ -930,6 +930,7 @@ namespace FourSlash { assert.equal(actual.hasAction, hasAction); assert.equal(actual.isRecommended, isRecommended); + assert.equal(actual.source, source); if (text) { const actualDetails = this.getCompletionEntryDetails(actual.name, actual.source)!; @@ -4752,6 +4753,7 @@ namespace FourSlashInterface { export type ExpectedCompletionEntry = string | { readonly name: string, + readonly source?: string, // when did this go away??? readonly insertText?: string, readonly replacementSpan?: FourSlash.Range, readonly hasAction?: boolean, // If not specified, will assert that this is false. diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index a745ef02fd422..4b187e9c717a4 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -149,14 +149,14 @@ namespace ts.codefix { symbolToken: Node | undefined, preferences: UserPreferences, ): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } { - const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, checker, allSourceFiles); + const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getCompilerOptions(), checker, allSourceFiles); 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, exportInfos, host, preferences)).moduleSpecifier; const fix = first(getFixForImport(exportInfos, symbolName, symbolToken, program, sourceFile, host, preferences)); return { moduleSpecifier, codeAction: codeActionForFix({ host, formatContext }, sourceFile, symbolName, fix, getQuotePreference(sourceFile, preferences)) }; } - function getAllReExportingModules(exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, checker: TypeChecker, allSourceFiles: ReadonlyArray): ReadonlyArray { + function getAllReExportingModules(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`. @@ -164,10 +164,14 @@ namespace ts.codefix { return; } + const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions); + if (defaultInfo && defaultInfo.name === symbolName && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) { + result.push({ moduleSymbol, importKind: defaultInfo.kind }); + } + for (const exported of checker.getExportsOfModule(moduleSymbol)) { - if ((exported.escapedName === InternalSymbolName.Default || exported.name === symbolName) && skipAlias(exported, checker) === exportedSymbol) { - const isDefaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol) === exported; - result.push({ moduleSymbol, importKind: isDefaultExport ? ImportKind.Default : ImportKind.Named }); + if (exported.name === symbolName && skipAlias(exported, checker) === exportedSymbol) { + result.push({ moduleSymbol, importKind: ImportKind.Named }); } } }); @@ -373,13 +377,9 @@ namespace ts.codefix { forEachExternalModuleToImportFrom(checker, sourceFile, program.getSourceFiles(), moduleSymbol => { cancellationToken.throwIfCancellationRequested(); - // check the default export - const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol); - if (defaultExport) { - const info = getDefaultExportInfo(defaultExport, moduleSymbol, program); - if (info && info.name === symbolName && symbolHasMeaning(info.symbolForMeaning, currentTokenMeaning)) { - addSymbol(moduleSymbol, defaultExport, ImportKind.Default); - } + const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, program.getCompilerOptions()); + if (defaultInfo && defaultInfo.name === symbolName && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) { + addSymbol(moduleSymbol, defaultInfo.symbol, defaultInfo.kind); } // check exports with the same name @@ -391,9 +391,24 @@ namespace ts.codefix { return originalSymbolToExportInfos; } - function getDefaultExportInfo(defaultExport: Symbol, moduleSymbol: Symbol, program: Program): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { - const checker = program.getTypeChecker(); + 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); + if (!exported) return undefined; + const { symbol, kind } = exported; + const info = getDefaultExportInfoWorker(symbol, moduleSymbol, checker, compilerOptions); + return info && { symbol, symbolForMeaning: info.symbolForMeaning, name: info.name, kind }; + } + + function getDefaultLikeExportWorker(moduleSymbol: Symbol, checker: TypeChecker): { readonly symbol: Symbol, readonly kind: ImportKind.Default | ImportKind.Equals } | 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 }; + } + function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { const localSymbol = getLocalSymbolForExportDefault(defaultExport); if (localSymbol) return { symbolForMeaning: localSymbol, name: localSymbol.name }; @@ -402,10 +417,10 @@ namespace ts.codefix { if (defaultExport.flags & SymbolFlags.Alias) { const aliased = checker.getAliasedSymbol(defaultExport); - return getDefaultExportInfo(aliased, Debug.assertDefined(aliased.parent), program); + return getDefaultExportInfoWorker(aliased, Debug.assertDefined(aliased.parent), checker, compilerOptions); } else { - const moduleName = moduleSymbolToValidIdentifier(moduleSymbol, program.getCompilerOptions().target!); + const moduleName = moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target!); // TODO: GH#18217 return moduleName === undefined ? undefined : { symbolForMeaning: defaultExport, name: moduleName }; } } diff --git a/src/services/completions.ts b/src/services/completions.ts index 1353ba8be218e..ce62ce4eb0bae 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1369,6 +1369,14 @@ namespace ts.Completions { return; } + if (resolvedModuleSymbol !== moduleSymbol && + // Don't add another completion for `export =` of a symbol that's already global. + // So in `declare namespace foo {} declare module "foo" { export = foo; }`, there will just be the global completion for `foo`. + resolvedModuleSymbol.declarations.some(d => !!d.getSourceFile().externalModuleIndicator)) { + symbols.push(resolvedModuleSymbol); + symbolToOriginInfoMap[getSymbolId(resolvedModuleSymbol)] = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport: false }; + } + for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) { // Don't add a completion for a re-export, only for the original. // The actual import fix might end up coming from a re-export -- we don't compute that until getting completion details. diff --git a/tests/cases/fourslash/completionsImport_default_anonymous.ts b/tests/cases/fourslash/completionsImport_default_anonymous.ts index 17a26b4b5aac4..67575c5f4bd95 100644 --- a/tests/cases/fourslash/completionsImport_default_anonymous.ts +++ b/tests/cases/fourslash/completionsImport_default_anonymous.ts @@ -11,7 +11,7 @@ ////fooB/*1*/ goTo.marker("0"); -const preferences = { includeCompletionsForModuleExports: true }; +const preferences: FourSlashInterface.UserPreferences = { includeCompletionsForModuleExports: true }; verify.completions( { marker: "0", excludes: { name: "default", source: "/src/foo-bar" }, preferences }, { marker: "1", includes: { name: "fooBar", source: "/src/foo-bar", sourceDisplay: "./foo-bar", text: "(property) default: 0", kind: "property", hasAction: true }, preferences } diff --git a/tests/cases/fourslash/completionsImport_exportEquals.ts b/tests/cases/fourslash/completionsImport_exportEquals.ts new file mode 100644 index 0000000000000..37e78e09f386f --- /dev/null +++ b/tests/cases/fourslash/completionsImport_exportEquals.ts @@ -0,0 +1,53 @@ +/// + +// @module: commonjs + +// @Filename: /a.d.ts +////declare function a(): void; +////declare namespace a { +//// export interface b {} +////} +////export = a; + +// @Filename: /b.ts +////a/*0*/; +////let x: b/*1*/; + +const preferences: FourSlashInterface.UserPreferences = { includeCompletionsForModuleExports: true }; +verify.completions( + { + marker: "0", + includes: { name: "a", source: "/a", hasAction: true, }, + preferences, + }, + { + marker: "1", + includes: { name: "b", source: "/a", hasAction: true }, + preferences, + } +); + +// Import { b } first, or it will just add a qualified name from 'a' (which isn't what we're trying to test) +verify.applyCodeActionFromCompletion("1", { + name: "b", + source: "/a", + description: `Import 'b' from module "./a"`, + newFileContent: +`import { b } from "./a"; + +a; +let x: b;`, +}); + +verify.applyCodeActionFromCompletion("0", { + name: "a", + source: "/a", + description: `Import 'a' from module "./a"`, + newFileContent: +`import { b } from "./a"; +import a = require("./a"); + +a; +let x: b;`, +}); + diff --git a/tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts b/tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts index ede97c2d810f3..8a687f5a19194 100644 --- a/tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts +++ b/tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts @@ -18,7 +18,7 @@ verify.completions({ marker: "", // Tester will assert that it is only included once - includes: [{ name: "foo", hasAction: true }], + includes: [{ name: "foo", source: "a", hasAction: true }], preferences: { includeCompletionsForModuleExports: true, } diff --git a/tests/cases/fourslash/completionsImport_tsx.ts b/tests/cases/fourslash/completionsImport_tsx.ts index f2f8752082b2b..9fb4d23922369 100644 --- a/tests/cases/fourslash/completionsImport_tsx.ts +++ b/tests/cases/fourslash/completionsImport_tsx.ts @@ -12,7 +12,7 @@ verify.completions({ marker: "", - includes: { name: "Foo", source: "/a.tsx", hasAction: true }, + includes: { name: "Foo", source: "/a", hasAction: true }, excludes: "Bar", preferences: { includeCompletionsForModuleExports: true, diff --git a/tests/cases/fourslash/completionsUniqueSymbol_import.ts b/tests/cases/fourslash/completionsUniqueSymbol_import.ts index df3034e56fe9e..7f98f8c1dd732 100644 --- a/tests/cases/fourslash/completionsUniqueSymbol_import.ts +++ b/tests/cases/fourslash/completionsUniqueSymbol_import.ts @@ -24,7 +24,7 @@ verify.completions({ marker: "", exact: [ "n", - { name: "publicSym", insertText: "[publicSym]", replacementSpan: test.ranges()[0], hasAction: true }, + { name: "publicSym", source: "/a", insertText: "[publicSym]", replacementSpan: test.ranges()[0], hasAction: true }, ], preferences: { includeInsertTextCompletions: true, diff --git a/tests/cases/fourslash/importNameCodeFix_all.ts b/tests/cases/fourslash/importNameCodeFix_all.ts index 4aa22413d5a1f..d5bb525900a6b 100644 --- a/tests/cases/fourslash/importNameCodeFix_all.ts +++ b/tests/cases/fourslash/importNameCodeFix_all.ts @@ -37,11 +37,11 @@ verify.codeFixAll({ fixId: "fixMissingImport", fixAllDescription: "Add all missing imports", newFileContent: -// TODO: GH#25135 (should import 'e') `import bd, * as b from "./b"; import cd, { c0 } from "./c"; import dd, { d0, d1 } from "./d"; import ad, { a0 } from "./a"; +import e = require("./e"); ad; ad; a0; a0; bd; bd; b.b0; b.b0; diff --git a/tests/cases/fourslash/importNameCodeFix_exportEquals.ts b/tests/cases/fourslash/importNameCodeFix_exportEquals.ts new file mode 100644 index 0000000000000..4ab87df7e7618 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_exportEquals.ts @@ -0,0 +1,25 @@ +/// + +// @Filename: /a.d.ts +////declare function a(): void; +////declare namespace a { +//// export interface b {} +////} +////export = a; + +// @Filename: /b.ts +////a; +////let x: b; + +goTo.file("/b.ts"); +verify.codeFixAll({ + fixId: "fixMissingImport", + fixAllDescription: "Add all missing imports", + newFileContent: +`import { b } from "./a"; + +import a = require("./a"); + +a; +let x: b;`, +});