Fix re-exporting

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);
                     }
                 }

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<true>();
 
             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;
+            }
+        }
+    }
 }

tests/cases/fourslash/completionsImport_filteredByPackageJson_reexport.ts

@@ -0,0 +1,58 @@
+/// <reference path="fourslash.ts" />
+
+//@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
+  }
+});

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,
     ],