Fix duplicate completions from two different copies of a node_modules package

src/compiler/moduleSpecifiers.ts

@@ -789,70 +789,6 @@ namespace ts.moduleSpecifiers {
         }
     }
 
-    interface NodeModulePathParts {
-        readonly topLevelNodeModulesIndex: number;
-        readonly topLevelPackageNameIndex: number;
-        readonly packageRootIndex: number;
-        readonly fileNameIndex: number;
-    }
-    function getNodeModulePathParts(fullPath: string): NodeModulePathParts | undefined {
-        // If fullPath can't be valid module file within node_modules, returns undefined.
-        // Example of expected pattern: /base/path/node_modules/[@scope/otherpackage/@otherscope/node_modules/]package/[subdirectory/]file.js
-        // Returns indices:                       ^            ^                                                      ^             ^
-
-        let topLevelNodeModulesIndex = 0;
-        let topLevelPackageNameIndex = 0;
-        let packageRootIndex = 0;
-        let fileNameIndex = 0;
-
-        const enum States {
-            BeforeNodeModules,
-            NodeModules,
-            Scope,
-            PackageContent
-        }
-
-        let partStart = 0;
-        let partEnd = 0;
-        let state = States.BeforeNodeModules;
-
-        while (partEnd >= 0) {
-            partStart = partEnd;
-            partEnd = fullPath.indexOf("/", partStart + 1);
-            switch (state) {
-                case States.BeforeNodeModules:
-                    if (fullPath.indexOf(nodeModulesPathPart, partStart) === partStart) {
-                        topLevelNodeModulesIndex = partStart;
-                        topLevelPackageNameIndex = partEnd;
-                        state = States.NodeModules;
-                    }
-                    break;
-                case States.NodeModules:
-                case States.Scope:
-                    if (state === States.NodeModules && fullPath.charAt(partStart + 1) === "@") {
-                        state = States.Scope;
-                    }
-                    else {
-                        packageRootIndex = partEnd;
-                        state = States.PackageContent;
-                    }
-                    break;
-                case States.PackageContent:
-                    if (fullPath.indexOf(nodeModulesPathPart, partStart) === partStart) {
-                        state = States.NodeModules;
-                    }
-                    else {
-                        state = States.PackageContent;
-                    }
-                    break;
-            }
-        }
-
-        fileNameIndex = partStart;
-
-        return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined;
-    }
-
     function getPathRelativeToRootDirs(path: string, rootDirs: readonly string[], getCanonicalFileName: GetCanonicalFileName): string | undefined {
         return firstDefined(rootDirs, rootDir => {
             const relativePath = getRelativePathIfInDirectory(path, rootDir, getCanonicalFileName);

src/compiler/utilities.ts

@@ -7501,4 +7501,68 @@ namespace ts {
     export function isThisTypeParameter(type: Type): boolean {
         return !!(type.flags & TypeFlags.TypeParameter && (type as TypeParameter).isThisType);
     }
+
+    export interface NodeModulePathParts {
+        readonly topLevelNodeModulesIndex: number;
+        readonly topLevelPackageNameIndex: number;
+        readonly packageRootIndex: number;
+        readonly fileNameIndex: number;
+    }
+    export function getNodeModulePathParts(fullPath: string): NodeModulePathParts | undefined {
+        // If fullPath can't be valid module file within node_modules, returns undefined.
+        // Example of expected pattern: /base/path/node_modules/[@scope/otherpackage/@otherscope/node_modules/]package/[subdirectory/]file.js
+        // Returns indices:                       ^            ^                                                      ^             ^
+
+        let topLevelNodeModulesIndex = 0;
+        let topLevelPackageNameIndex = 0;
+        let packageRootIndex = 0;
+        let fileNameIndex = 0;
+
+        const enum States {
+            BeforeNodeModules,
+            NodeModules,
+            Scope,
+            PackageContent
+        }
+
+        let partStart = 0;
+        let partEnd = 0;
+        let state = States.BeforeNodeModules;
+
+        while (partEnd >= 0) {
+            partStart = partEnd;
+            partEnd = fullPath.indexOf("/", partStart + 1);
+            switch (state) {
+                case States.BeforeNodeModules:
+                    if (fullPath.indexOf(nodeModulesPathPart, partStart) === partStart) {
+                        topLevelNodeModulesIndex = partStart;
+                        topLevelPackageNameIndex = partEnd;
+                        state = States.NodeModules;
+                    }
+                    break;
+                case States.NodeModules:
+                case States.Scope:
+                    if (state === States.NodeModules && fullPath.charAt(partStart + 1) === "@") {
+                        state = States.Scope;
+                    }
+                    else {
+                        packageRootIndex = partEnd;
+                        state = States.PackageContent;
+                    }
+                    break;
+                case States.PackageContent:
+                    if (fullPath.indexOf(nodeModulesPathPart, partStart) === partStart) {
+                        state = States.NodeModules;
+                    }
+                    else {
+                        state = States.PackageContent;
+                    }
+                    break;
+            }
+        }
+
+        fileNameIndex = partStart;
+
+        return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined;
+    }
 }

src/services/exportInfoMap.ts

@@ -32,6 +32,7 @@ namespace ts {
         symbolTableKey: __String;
         moduleName: string;
         moduleFile: SourceFile | undefined;
+        packageName: string | undefined;
 
         // SymbolExportInfo, but optional symbols
         readonly symbol: Symbol | undefined;
@@ -63,6 +64,17 @@ namespace ts {
         let exportInfoId = 1;
         const exportInfo = createMultiMap<string, CachedSymbolExportInfo>();
         const symbols = new Map<number, [symbol: Symbol, moduleSymbol: Symbol]>();
+        /**
+         * Key: node_modules package name (no @types).
+         * Value: path to deepest node_modules folder seen that is
+         * both visible to `usableByFileName` and contains the package.
+         *
+         * Later, we can see if a given SymbolExportInfo is shadowed by
+         * a another installation of the same package in a deeper
+         * node_modules folder by seeing if its path starts with the
+         * value stored here.
+         */
+        const packages = new Map<string, string>();
         let usableByFileName: Path | undefined;
         const cache: ExportInfoMap = {
             isUsableByFile: importingFile => importingFile === usableByFileName,
@@ -77,6 +89,29 @@ namespace ts {
                     cache.clear();
                     usableByFileName = importingFile;
                 }
+
+                let packageName;
+                if (moduleFile) {
+                    const nodeModulesPathParts = getNodeModulePathParts(moduleFile.fileName);
+                    if (nodeModulesPathParts) {
+                        const { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex } = nodeModulesPathParts;
+                        packageName = unmangleScopedPackageName(getPackageNameFromTypesPackageName(moduleFile.fileName.substring(topLevelPackageNameIndex + 1, packageRootIndex)));
+                        if (startsWith(importingFile, moduleFile.path.substring(0, topLevelNodeModulesIndex))) {
+                            const prevDeepestNodeModulesPath = packages.get(packageName);
+                            const nodeModulesPath = moduleFile.fileName.substring(0, topLevelPackageNameIndex);
+                            if (prevDeepestNodeModulesPath) {
+                                const prevDeepestNodeModulesIndex = prevDeepestNodeModulesPath.indexOf(nodeModulesPathPart);
+                                if (topLevelNodeModulesIndex > prevDeepestNodeModulesIndex) {
+                                    packages.set(packageName, nodeModulesPath);
+                                }
+                            }
+                            else {
+                                packages.set(packageName, nodeModulesPath);
+                            }
+                        }
+                    }
+                }
+
                 const isDefault = exportKind === ExportKind.Default;
                 const namedSymbol = isDefault && getLocalSymbolForExportDefault(symbol) || symbol;
                 // 1. A named export must be imported by its key in `moduleSymbol.exports` or `moduleSymbol.members`.
@@ -104,6 +139,7 @@ namespace ts {
                     moduleName,
                     moduleFile,
                     moduleFileName: moduleFile?.fileName,
+                    packageName,
                     exportKind,
                     targetFlags: target.flags,
                     isFromPackageJson,
@@ -121,17 +157,20 @@ namespace ts {
                 exportInfo.forEach((info, key) => {
                     const { symbolName, ambientModuleName } = parseKey(key);
                     const rehydrated = info.map(rehydrateCachedInfo);
-                    action(
-                        rehydrated,
-                        preferCapitalized => {
-                            const { symbol, exportKind } = rehydrated[0];
-                            const namedSymbol = exportKind === ExportKind.Default && getLocalSymbolForExportDefault(symbol) || symbol;
-                            return preferCapitalized
-                                ? getNameForExportedSymbol(namedSymbol, /*scriptTarget*/ undefined, /*preferCapitalized*/ true)
-                                : symbolName;
-                        },
-                        !!ambientModuleName,
-                        key);
+                    const filtered = rehydrated.filter((r, i) => isNotShadowedByDeeperNodeModulesPackage(r, info[i].packageName));
+                    if (filtered.length) {
+                        action(
+                            filtered,
+                            preferCapitalized => {
+                                const { symbol, exportKind } = rehydrated[0];
+                                const namedSymbol = exportKind === ExportKind.Default && getLocalSymbolForExportDefault(symbol) || symbol;
+                                return preferCapitalized
+                                    ? getNameForExportedSymbol(namedSymbol, /*scriptTarget*/ undefined, /*preferCapitalized*/ true)
+                                    : symbolName;
+                            },
+                            !!ambientModuleName,
+                            key);
+                    }
                 });
             },
             releaseSymbols: () => {
@@ -232,6 +271,12 @@ namespace ts {
             }
             return true;
         }
+
+        function isNotShadowedByDeeperNodeModulesPackage(info: SymbolExportInfo, packageName: string | undefined) {
+            if (!packageName || !info.moduleFileName) return true;
+            const packageDeepestNodeModulesPath = packages.get(packageName);
+            return !packageDeepestNodeModulesPath || startsWith(info.moduleFileName, packageDeepestNodeModulesPath);
+        }
     }
 
     export function isImportableFile(

tests/cases/fourslash/completionsImport_duplicatePackages_scoped.ts

@@ -0,0 +1,56 @@
+/// <reference path="fourslash.ts" />
+
+// @module: commonjs
+// @esModuleInterop: true
+
+// @Filename: /node_modules/@scope/react-dom/package.json
+//// { "name": "react-dom", "version": "1.0.0", "types": "./index.d.ts" }
+
+// @Filename: /node_modules/@scope/react-dom/index.d.ts
+//// import * as React from "react";
+//// export function render(): void;
+
+// @Filename: /node_modules/@scope/react/package.json
+//// { "name": "react", "version": "1.0.0", "types": "./index.d.ts" }
+
+// @Filename: /node_modules/@scope/react/index.d.ts
+//// import "./other";
+//// export declare function useState(): void;
+
+// @Filename: /node_modules/@scope/react/other.d.ts
+//// export declare function useRef(): void;
+
+// @Filename: /packages/a/node_modules/@scope/react/package.json
+//// { "name": "react", "version": "1.0.1", "types": "./index.d.ts" }
+
+// @Filename: /packages/a/node_modules/@scope/react/index.d.ts
+//// export declare function useState(): void;
+
+// @Filename: /packages/a/index.ts
+//// import "@scope/react-dom";
+//// import "@scope/react";
+
+// @Filename: /packages/a/foo.ts
+//// /**/
+
+goTo.marker("");
+verify.completions({
+  marker: "",
+  exact: completion.globalsPlus([{
+    name: "render",
+    source: "@scope/react-dom",
+    sourceDisplay: "@scope/react-dom",
+    hasAction: true,
+    sortText: completion.SortText.AutoImportSuggestions,
+  }, {
+    name: "useState",
+    source: "@scope/react",
+    sourceDisplay: "@scope/react",
+    hasAction: true,
+    sortText: completion.SortText.AutoImportSuggestions,
+  }]),
+  preferences: {
+    includeCompletionsForModuleExports: true,
+    allowIncompleteCompletions: true,
+  },
+});

tests/cases/fourslash/completionsImport_duplicatePackages_scopedTypes.ts

@@ -0,0 +1,56 @@
+/// <reference path="fourslash.ts" />
+
+// @module: commonjs
+// @esModuleInterop: true
+
+// @Filename: /node_modules/@types/scope__react-dom/package.json
+//// { "name": "react-dom", "version": "1.0.0", "types": "./index.d.ts" }
+
+// @Filename: /node_modules/@types/scope__react-dom/index.d.ts
+//// import * as React from "react";
+//// export function render(): void;
+
+// @Filename: /node_modules/@types/scope__react/package.json
+//// { "name": "react", "version": "1.0.0", "types": "./index.d.ts" }
+
+// @Filename: /node_modules/@types/scope__react/index.d.ts
+//// import "./other";
+//// export declare function useState(): void;
+
+// @Filename: /node_modules/@types/scope__react/other.d.ts
+//// export declare function useRef(): void;
+
+// @Filename: /packages/a/node_modules/@types/scope__react/package.json
+//// { "name": "react", "version": "1.0.1", "types": "./index.d.ts" }
+
+// @Filename: /packages/a/node_modules/@types/scope__react/index.d.ts
+//// export declare function useState(): void;
+
+// @Filename: /packages/a/index.ts
+//// import "@scope/react-dom";
+//// import "@scope/react";
+
+// @Filename: /packages/a/foo.ts
+//// /**/
+
+goTo.marker("");
+verify.completions({
+  marker: "",
+  exact: completion.globalsPlus([{
+    name: "render",
+    source: "@scope/react-dom",
+    sourceDisplay: "@scope/react-dom",
+    hasAction: true,
+    sortText: completion.SortText.AutoImportSuggestions,
+  }, {
+    name: "useState",
+    source: "@scope/react",
+    sourceDisplay: "@scope/react",
+    hasAction: true,
+    sortText: completion.SortText.AutoImportSuggestions,
+  }]),
+  preferences: {
+    includeCompletionsForModuleExports: true,
+    allowIncompleteCompletions: true,
+  },
+});