From 6d582c90e8eb321d585d60b026b5658995a01348 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 12 Jun 2019 17:35:06 -0700 Subject: [PATCH] Fix re-exporting --- src/harness/fourslash.ts | 2 +- src/services/completions.ts | 65 ++++++++++++++----- ...nsImport_filteredByPackageJson_reexport.ts | 58 +++++++++++++++++ .../fourslash/completionsImport_ofAlias.ts | 17 +++-- 4 files changed, 117 insertions(+), 25 deletions(-) create mode 100644 tests/cases/fourslash/completionsImport_filteredByPackageJson_reexport.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index f12506be2fb..5bdbf9cae54 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -798,7 +798,7 @@ namespace FourSlash { const name = typeof include === "string" ? include : include.name; const found = nameToEntries.get(name); if (!found) throw this.raiseError(`Includes: completion '${name}' not found.`); - assert(found.length === 1); // Must use 'exact' for multiple completions with same name + assert(found.length === 1, `Must use 'exact' for multiple completions with same name: '${name}'`); this.verifyCompletionEntry(ts.first(found), include); } } diff --git a/src/services/completions.ts b/src/services/completions.ts index 328ba2a119c..37078bfe2cb 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1272,7 +1272,7 @@ namespace ts.Completions { function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string, target: ScriptTarget, host: LanguageServiceHost): void { const tokenTextLowerCase = tokenText.toLowerCase(); - + const potentialDuplicateSymbols = createMap<{ alias: Symbol, moduleSymbol: Symbol, insertAt: number }>(); const seenResolvedModules = createMap(); codefix.forEachExternalModuleToImportFrom(typeChecker, host, sourceFile, program.getSourceFiles(), moduleSymbol => { @@ -1296,33 +1296,55 @@ namespace ts.Completions { symbolToOriginInfoMap[getSymbolId(resolvedModuleSymbol)] = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport: false }; } - for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) { + for (const 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. // This is just to avoid adding duplicate completion entries. // // If `symbol.parent !== ...`, this is an `export * from "foo"` re-export. Those don't create new symbols. - if (typeChecker.getMergedSymbol(symbol.parent!) !== resolvedModuleSymbol - || some(symbol.declarations, d => - // If `!!d.name.originalKeywordKind`, this is `export { _break as break };` -- skip this and prefer the keyword completion. - // If `!!d.parent.parent.moduleSpecifier`, this is `export { foo } from "foo"` re-export, which creates a new symbol (thus isn't caught by the first check). - isExportSpecifier(d) && (d.propertyName ? isIdentifierANonContextualKeyword(d.name) : !!d.parent.parent.moduleSpecifier))) { + if (typeChecker.getMergedSymbol(symbol.parent!) !== resolvedModuleSymbol) { continue; } - - const isDefaultExport = symbol.escapedName === InternalSymbolName.Default; - if (isDefaultExport) { - symbol = getLocalSymbolForExportDefault(symbol) || symbol; + // If `!!d.name.originalKeywordKind`, this is `export { _break as break };` -- skip this and prefer the keyword completion. + if (some(symbol.declarations, d => isExportSpecifier(d) && !!d.propertyName && isIdentifierANonContextualKeyword(d.name))) { + continue; + } + // If `!!d.parent.parent.moduleSpecifier`, this is `export { foo } from "foo"` re-export, which creates a new symbol (thus isn't caught by the first check). + if (some(symbol.declarations, d => isExportSpecifier(d) && !d.propertyName && !!d.parent.parent.moduleSpecifier)) { + // If we haven’t yet seen the original symbol, note the alias so we can add it at the end if the original doesn’t show up eventually + const exportSymbol = findAlias(typeChecker, symbol, alias => some(alias.declarations, d => !!d.localSymbol)); + const localSymbol = exportSymbol && find(exportSymbol.declarations, d => !!d.localSymbol)!.localSymbol; + if (localSymbol && !symbolToOriginInfoMap[getSymbolId(localSymbol)]) { + potentialDuplicateSymbols.set(getSymbolId(localSymbol).toString(), { alias: symbol, moduleSymbol, insertAt: symbols.length }); + } + continue; + } + else { + // This is not a re-export, so see if we have any aliases pending and remove them + potentialDuplicateSymbols.delete(getSymbolId(symbol).toString()); } - const origin: SymbolOriginInfoExport = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport }; - if (detailsEntryId || stringContainsCharactersInOrder(getSymbolName(symbol, origin, target).toLowerCase(), tokenTextLowerCase)) { - symbols.push(symbol); - symbolToSortTextMap[getSymbolId(symbol)] = SortText.AutoImportSuggestions; - symbolToOriginInfoMap[getSymbolId(symbol)] = origin; - } + pushSymbol(symbol, moduleSymbol); } }); + + // By this point, any potential duplicates that were actually duplicates have been + // removed, so the rest need to be added. + potentialDuplicateSymbols.forEach(({ alias, moduleSymbol, insertAt }) => pushSymbol(alias, moduleSymbol, insertAt)); + + function pushSymbol(symbol: Symbol, moduleSymbol: Symbol, insertAt = symbols.length) { + const isDefaultExport = symbol.escapedName === InternalSymbolName.Default; + if (isDefaultExport) { + symbol = getLocalSymbolForExportDefault(symbol) || symbol; + } + const origin: SymbolOriginInfoExport = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport }; + if (detailsEntryId || stringContainsCharactersInOrder(getSymbolName(symbol, origin, target).toLowerCase(), tokenTextLowerCase)) { + symbols.splice(insertAt, 0, symbol); + symbolToSortTextMap[getSymbolId(symbol)] = SortText.AutoImportSuggestions; + symbolToOriginInfoMap[getSymbolId(symbol)] = origin; + } + return symbol; + } } /** @@ -2250,4 +2272,13 @@ namespace ts.Completions { function binaryExpressionMayBeOpenTag({ left }: BinaryExpression): boolean { return nodeIsMissing(left); } + + function findAlias(typeChecker: TypeChecker, symbol: Symbol, predicate: (symbol: Symbol) => boolean): Symbol | undefined { + let currentAlias: Symbol | undefined = symbol; + while (currentAlias.flags & SymbolFlags.Alias && (currentAlias = typeChecker.getImmediateAliasedSymbol(currentAlias))) { + if (predicate(currentAlias)) { + return currentAlias; + } + } + } } diff --git a/tests/cases/fourslash/completionsImport_filteredByPackageJson_reexport.ts b/tests/cases/fourslash/completionsImport_filteredByPackageJson_reexport.ts new file mode 100644 index 00000000000..8e17a3c3a44 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_filteredByPackageJson_reexport.ts @@ -0,0 +1,58 @@ +/// + +//@noEmit: true + +//@Filename: /package.json +////{ +//// "dependencies": { +//// "@emotion/core": "*" +//// } +////} + +//@Filename: /node_modules/@emotion/css/index.d.ts +////export declare const css: any; +////const css2: any; +////export { css2 }; + +//@Filename: /node_modules/@emotion/css/package.json +////{ +//// "name": "@emotion/css", +//// "types": "./index.d.ts" +////} + +//@Filename: /node_modules/@emotion/core/index.d.ts +////import { css2 } from "@emotion/css"; +////export { css } from "@emotion/css"; +////export { css2 }; + +//@Filename: /node_modules/@emotion/core/package.json +////{ +//// "name": "@emotion/core", +//// "types": "./index.d.ts" +////} + +//@Filename: /src/index.ts +////cs/**/ + +verify.completions({ + marker: test.marker(""), + includes: [ + completion.undefinedVarEntry, + { + name: "css", + source: "/node_modules/@emotion/core/index", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions + }, + { + name: "css2", + source: "/node_modules/@emotion/core/index", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions + }, + ...completion.statementKeywordsWithTypes + ], + preferences: { + includeCompletionsForModuleExports: true + } +}); diff --git a/tests/cases/fourslash/completionsImport_ofAlias.ts b/tests/cases/fourslash/completionsImport_ofAlias.ts index 9a9cb4a2b18..1319391eb0b 100644 --- a/tests/cases/fourslash/completionsImport_ofAlias.ts +++ b/tests/cases/fourslash/completionsImport_ofAlias.ts @@ -16,6 +16,9 @@ // @Filename: /a_reexport_2.ts ////export * from "./a"; +// @Filename: /a_reexport_3.ts +////export { foo } from "./a_reexport"; + // @Filename: /b.ts ////fo/**/ @@ -24,13 +27,13 @@ verify.completions({ includes: [ completion.undefinedVarEntry, { - name: "foo", - source: "/a", - sourceDisplay: "./a", - text: "(alias) const foo: 0\nexport foo", - kind: "alias", - hasAction: true, - sortText: completion.SortText.AutoImportSuggestions + name: "foo", + source: "/a", + sourceDisplay: "./a", + text: "(alias) const foo: 0\nexport foo", + kind: "alias", + hasAction: true, + sortText: completion.SortText.AutoImportSuggestions }, ...completion.statementKeywordsWithTypes, ],