Avoid duplicate import completions when a namespace is exported by two different modules (#24473)

Author: Andy

src/harness/fourslash.ts

@@ -868,7 +868,7 @@ namespace FourSlash {
             for (const entry of actualCompletions.entries) {
                 if (actualByName.has(entry.name)) {
                     // TODO: GH#23587
-                    if (entry.name !== "undefined" && entry.name !== "require") this.raiseError(`Duplicate completions for ${entry.name}`);
+                    if (entry.name !== "undefined" && entry.name !== "require") this.raiseError(`Duplicate (${actualCompletions.entries.filter(a => a.name === entry.name).length}) completions for ${entry.name}`);
                 }
                 else {
                     actualByName.set(entry.name, entry);

src/services/completions.ts

@@ -1320,20 +1320,28 @@ namespace ts.Completions {
         function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string, target: ScriptTarget): void {
             const tokenTextLowerCase = tokenText.toLowerCase();
 
+            const seenResolvedModules = createMap<true>();
+
             codefix.forEachExternalModuleToImportFrom(typeChecker, sourceFile, program.getSourceFiles(), moduleSymbol => {
                 // Perf -- ignore other modules if this is a request for details
                 if (detailsEntryId && detailsEntryId.source && stripQuotes(moduleSymbol.name) !== detailsEntryId.source) {
                     return;
                 }
 
+                const resolvedModuleSymbol = typeChecker.resolveExternalModuleSymbol(moduleSymbol);
+                // resolvedModuleSymbol may be a namespace. A namespace may be `export =` by multiple module declarations, but only keep the first one.
+                if (!addToSeen(seenResolvedModules, getSymbolId(resolvedModuleSymbol))) {
+                    return;
+                }
+
                 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.
                     // This is just to avoid adding duplicate completion entries.
                     //
                     // If `symbol.parent !== ...`, this comes from an `export * from "foo"` re-export. Those don't create new symbols.
                     // If `some(...)`, this comes from an `export { foo } from "foo"` re-export, which creates a new symbol (thus isn't caught by the first check).
-                    if (typeChecker.getMergedSymbol(symbol.parent!) !== typeChecker.resolveExternalModuleSymbol(moduleSymbol)
+                    if (typeChecker.getMergedSymbol(symbol.parent!) !== resolvedModuleSymbol
                         || some(symbol.declarations, d => isExportSpecifier(d) && !!d.parent.parent.moduleSpecifier)) {
                         continue;
                     }

tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts

@@ -0,0 +1,25 @@
+/// <reference path='fourslash.ts'/>
+
+// @Filename: /node_modules/a/index.d.ts
+////declare namespace core {
+////    const foo: number;
+////}
+////declare module "a" {
+////    export = core;
+////}
+////declare module "a/alias" {
+////    export = core;
+////}
+
+// @Filename: /user.ts
+////import * as a from "a";
+/////**/foo;
+
+verify.completions({
+    marker: "",
+    // Tester will assert that it is only included once
+    includes: [{ name: "foo", hasAction: true }],
+    preferences: {
+        includeCompletionsForModuleExports: true,
+    }
+});