Fix path completions for typesVersions (#49154)

* Fix path completions for typesVersions

* Add more tests

* Fix case when * is a fragment of a path component

* Once a path pattern matches, only return completions from that pattern and higher priority ones

* Fix iteration order issue

* Aesthetics

src/compiler/core.ts

@@ -2299,7 +2299,7 @@ namespace ts {
         return startsWith(getCanonicalFileName(str), getCanonicalFileName(prefix)) ? str.substring(prefix.length) : undefined;
     }
 
-    function isPatternMatch({ prefix, suffix }: Pattern, candidate: string) {
+    export function isPatternMatch({ prefix, suffix }: Pattern, candidate: string) {
         return candidate.length >= prefix.length + suffix.length &&
             startsWith(candidate, prefix) &&
             endsWith(candidate, suffix);

src/services/stringCompletions.ts

@@ -448,10 +448,28 @@ namespace ts.Completions.StringCompletions {
 
         fragment = ensureTrailingDirectorySeparator(fragment);
 
-        // const absolutePath = normalizeAndPreserveTrailingSlash(isRootedDiskPath(fragment) ? fragment : combinePaths(scriptPath, fragment)); // TODO(rbuckton): should use resolvePaths
         const absolutePath = resolvePath(scriptPath, fragment);
         const baseDirectory = hasTrailingDirectorySeparator(absolutePath) ? absolutePath : getDirectoryPath(absolutePath);
 
+        // check for a version redirect
+        const packageJsonPath = findPackageJson(baseDirectory, host);
+        if (packageJsonPath) {
+            const packageJson = readJson(packageJsonPath, host as { readFile: (filename: string) => string | undefined });
+            const typesVersions = (packageJson as any).typesVersions;
+            if (typeof typesVersions === "object") {
+                const versionPaths = getPackageJsonTypesVersionsPaths(typesVersions)?.paths;
+                if (versionPaths) {
+                    const packageDirectory = getDirectoryPath(packageJsonPath);
+                    const pathInPackage = absolutePath.slice(ensureTrailingDirectorySeparator(packageDirectory).length);
+                    if (addCompletionEntriesFromPaths(result, pathInPackage, packageDirectory, extensions, versionPaths, host)) {
+                        // A true result means one of the `versionPaths` was matched, which will block relative resolution
+                        // to files and folders from here. All reachable paths given the pattern match are already added.
+                        return result;
+                    }
+                }
+            }
+        }
+
         const ignoreCase = !(host.useCaseSensitiveFileNames && host.useCaseSensitiveFileNames());
         if (!tryDirectoryExists(host, baseDirectory)) return result;
 
@@ -505,37 +523,51 @@ namespace ts.Completions.StringCompletions {
             }
         }
 
-        // check for a version redirect
-        const packageJsonPath = findPackageJson(baseDirectory, host);
-        if (packageJsonPath) {
-            const packageJson = readJson(packageJsonPath, host as { readFile: (filename: string) => string | undefined });
-            const typesVersions = (packageJson as any).typesVersions;
-            if (typeof typesVersions === "object") {
-                const versionResult = getPackageJsonTypesVersionsPaths(typesVersions);
-                const versionPaths = versionResult && versionResult.paths;
-                const rest = absolutePath.slice(ensureTrailingDirectorySeparator(baseDirectory).length);
-                if (versionPaths) {
-                    addCompletionEntriesFromPaths(result, rest, baseDirectory, extensions, versionPaths, host);
-                }
-            }
-        }
-
         return result;
     }
 
+    /** @returns whether `fragment` was a match for any `paths` (which should indicate whether any other path completions should be offered) */
     function addCompletionEntriesFromPaths(result: NameAndKind[], fragment: string, baseDirectory: string, fileExtensions: readonly string[], paths: MapLike<string[]>, host: LanguageServiceHost) {
+        let pathResults: { results: NameAndKind[], matchedPattern: boolean }[] = [];
+        let matchedPathPrefixLength = -1;
         for (const path in paths) {
             if (!hasProperty(paths, path)) continue;
             const patterns = paths[path];
             if (patterns) {
-                for (const { name, kind, extension } of getCompletionsForPathMapping(path, patterns, fragment, baseDirectory, fileExtensions, host)) {
-                    // Path mappings may provide a duplicate way to get to something we've already added, so don't add again.
-                    if (!result.some(entry => entry.name === name)) {
-                        result.push(nameAndKind(name, kind, extension));
-                    }
+                const pathPattern = tryParsePattern(path);
+                if (!pathPattern) continue;
+                const isMatch = typeof pathPattern === "object" && isPatternMatch(pathPattern, fragment);
+                const isLongestMatch = isMatch && (matchedPathPrefixLength === undefined || pathPattern.prefix.length > matchedPathPrefixLength);
+                if (isLongestMatch) {
+                    // If this is a higher priority match than anything we've seen so far, previous results from matches are invalid, e.g.
+                    // for `import {} from "some-package/|"` with a typesVersions:
+                    // {
+                    //   "bar/*": ["bar/*"], // <-- 1. We add 'bar', but 'bar/*' doesn't match yet.
+                    //   "*": ["dist/*"],    // <-- 2. We match here and add files from dist. 'bar' is still ok because it didn't come from a match.
+                    //   "foo/*": ["foo/*"]  // <-- 3. We matched '*' earlier and added results from dist, but if 'foo/*' also matched,
+                    // }                               results in dist would not be visible. 'bar' still stands because it didn't come from a match.
+                    //                                 This is especially important if `dist/foo` is a folder, because if we fail to clear results
+                    //                                 added by the '*' match, after typing `"some-package/foo/|"` we would get file results from both
+                    //                                 ./dist/foo and ./foo, when only the latter will actually be resolvable.
+                    //                                 See pathCompletionsTypesVersionsWildcard6.ts.
+                    matchedPathPrefixLength = pathPattern.prefix.length;
+                    pathResults = pathResults.filter(r => !r.matchedPattern);
+                }
+                if (typeof pathPattern === "string" || matchedPathPrefixLength === undefined || pathPattern.prefix.length >= matchedPathPrefixLength) {
+                    pathResults.push({
+                        matchedPattern: isMatch,
+                        results: getCompletionsForPathMapping(path, patterns, fragment, baseDirectory, fileExtensions, host)
+                            .map(({ name, kind, extension }) => nameAndKind(name, kind, extension)),
+                    });
                 }
             }
         }
+
+        const equatePaths = host.useCaseSensitiveFileNames?.() ? equateStringsCaseSensitive : equateStringsCaseInsensitive;
+        const equateResults: EqualityComparer<NameAndKind> = (a, b) => equatePaths(a.name, b.name);
+        pathResults.forEach(pathResult => pathResult.results.forEach(pathResult => pushIfUnique(result, pathResult, equateResults)));
+
+        return matchedPathPrefixLength > -1;
     }
 
     /**
@@ -659,11 +691,15 @@ namespace ts.Completions.StringCompletions {
 
         const pathPrefix = path.slice(0, path.length - 1);
         const remainingFragment = tryRemovePrefix(fragment, pathPrefix);
-        return remainingFragment === undefined ? justPathMappingName(pathPrefix) : flatMap(patterns, pattern =>
-            getModulesForPathsPattern(remainingFragment, baseUrl, pattern, fileExtensions, host));
+        if (remainingFragment === undefined) {
+            const starIsFullPathComponent = path[path.length - 2] === "/";
+            return starIsFullPathComponent ? justPathMappingName(pathPrefix) : flatMap(patterns, pattern =>
+                getModulesForPathsPattern("", baseUrl, pattern, fileExtensions, host)?.map(({ name, ...rest }) => ({ name: pathPrefix + name, ...rest })));
+        }
+        return flatMap(patterns, pattern => getModulesForPathsPattern(remainingFragment, baseUrl, pattern, fileExtensions, host));
 
         function justPathMappingName(name: string): readonly NameAndKind[] {
-            return startsWith(name, fragment) ? [directoryResult(name)] : emptyArray;
+            return startsWith(name, fragment) ? [directoryResult(removeTrailingDirectorySeparator(name))] : emptyArray;
         }
     }
 

tests/cases/fourslash/completionForStringLiteralNonrelativeImport13.ts

@@ -31,6 +31,6 @@
 
 verify.completions({
     marker: test.markerNames(),
-    exact: ["aaa", "index", "ts3.1", "zzz"],
+    exact: ["index", "zzz"],
     isNewIdentifierLocation: true,
 });

tests/cases/fourslash/completionsPaths_pathMapping_topLevel.ts

@@ -15,6 +15,6 @@
 
 verify.completions({
     marker: "",
-    exact: ["src", "foo/"].map(name => ({ name, kind: "directory" })),
+    exact: ["src", "foo"].map(name => ({ name, kind: "directory" })),
     isNewIdentifierLocation: true,
 });

tests/cases/fourslash/pathCompletionsTypesVersionsWildcard1.ts

@@ -0,0 +1,42 @@
+/// <reference path="fourslash.ts" />
+
+// @module: commonjs
+
+// @Filename: /node_modules/foo/package.json
+//// {
+////   "types": "index.d.ts",
+////   "typesVersions": {
+////     "*": {
+////       "*": ["dist/*"]
+////     }
+////   }
+//// }
+
+// @Filename: /node_modules/foo/nope.d.ts
+//// export const nope = 0;
+
+// @Filename: /node_modules/foo/dist/index.d.ts
+//// export const index = 0;
+
+// @Filename: /node_modules/foo/dist/blah.d.ts
+//// export const blah = 0;
+
+// @Filename: /node_modules/foo/dist/subfolder/one.d.ts
+//// export const one = 0;
+
+// @Filename: /a.ts
+//// import { } from "foo//**/";
+
+verify.completions({
+  marker: "",
+  isNewIdentifierLocation: true,
+  exact: ["blah", "index", "subfolder"],
+});
+
+edit.insert("subfolder/");
+
+verify.completions({
+  marker: "",
+  isNewIdentifierLocation: true,
+  exact: ["one"],
+});

tests/cases/fourslash/pathCompletionsTypesVersionsWildcard2.ts

@@ -0,0 +1,34 @@
+/// <reference path="fourslash.ts" />
+
+// @module: commonjs
+
+// @Filename: /node_modules/foo/package.json
+//// {
+////   "types": "index.d.ts",
+////   "typesVersions": {
+////     "<=3.4.1": {
+////       "*": ["ts-old/*"]
+////     }
+////   }
+//// }
+
+// @Filename: /node_modules/foo/nope.d.ts
+//// export const nope = 0;
+
+// @Filename: /node_modules/foo/ts-old/index.d.ts
+//// export const index = 0;
+
+// @Filename: /node_modules/foo/ts-old/blah.d.ts
+//// export const blah = 0;
+
+// @Filename: /node_modules/foo/ts-old/subfolder/one.d.ts
+//// export const one = 0;
+
+// @Filename: /a.ts
+//// import { } from "foo//**/";
+
+verify.completions({
+  marker: "",
+  isNewIdentifierLocation: true,
+  exact: ["nope", "ts-old"],
+});

tests/cases/fourslash/pathCompletionsTypesVersionsWildcard3.ts

@@ -0,0 +1,48 @@
+/// <reference path="fourslash.ts" />
+
+// @module: commonjs
+
+// @Filename: /node_modules/foo/package.json
+//// {
+////   "types": "index.d.ts",
+////   "typesVersions": {
+////     ">=4.3.5": {
+////       "browser/*": ["dist/*"]
+////     }
+////   }
+//// }
+
+// @Filename: /node_modules/foo/nope.d.ts
+//// export const nope = 0;
+
+// @Filename: /node_modules/foo/dist/index.d.ts
+//// export const index = 0;
+
+// @Filename: /node_modules/foo/dist/blah.d.ts
+//// export const blah = 0;
+
+// @Filename: /node_modules/foo/dist/subfolder/one.d.ts
+//// export const one = 0;
+
+// @Filename: /a.ts
+//// import { } from "foo//**/";
+
+verify.completions({
+  marker: "",
+  isNewIdentifierLocation: true,
+  exact: ["browser", "nope", "dist"],
+});
+
+edit.insert("browser/");
+
+verify.completions({
+  isNewIdentifierLocation: true,
+  exact: ["blah", "index", "subfolder"],
+});
+
+edit.insert("subfolder/");
+
+verify.completions({
+  isNewIdentifierLocation: true,
+  exact: ["one"],
+});

tests/cases/fourslash/pathCompletionsTypesVersionsWildcard4.ts

@@ -0,0 +1,41 @@
+/// <reference path="fourslash.ts" />
+
+// @module: commonjs
+
+// @Filename: /node_modules/foo/package.json
+//// {
+////   "types": "index.d.ts",
+////   "typesVersions": {
+////     ">=4.3.5": {
+////       "component-*": ["cjs/components/*"]
+////     }
+////   }
+//// }
+
+// @Filename: /node_modules/foo/nope.d.ts
+//// export const nope = 0;
+
+// @Filename: /node_modules/foo/cjs/components/index.d.ts
+//// export const index = 0;
+
+// @Filename: /node_modules/foo/cjs/components/blah.d.ts
+//// export const blah = 0;
+
+// @Filename: /node_modules/foo/cjs/components/subfolder/one.d.ts
+//// export const one = 0;
+
+// @Filename: /a.ts
+//// import { } from "foo//**/";
+
+verify.completions({
+  marker: "",
+  isNewIdentifierLocation: true,
+  exact: ["component-blah", "component-index", "component-subfolder", "nope", "cjs"],
+});
+
+edit.insert("component-subfolder/");
+
+verify.completions({
+  isNewIdentifierLocation: true,
+  exact: ["one"],
+});

tests/cases/fourslash/pathCompletionsTypesVersionsWildcard5.ts

@@ -0,0 +1,54 @@
+/// <reference path="fourslash.ts" />
+
+// @module: commonjs
+
+// @Filename: /node_modules/foo/package.json
+//// {
+////   "types": "index.d.ts",
+////   "typesVersions": {
+////     "*": {
+////       "*": ["dist/*"],
+////       "foo/*": ["dist/*"],
+////       "bar/*": ["dist/*"],
+////       "exact-match": ["dist/index.d.ts"]
+////     }
+////   }
+//// }
+
+// @Filename: /node_modules/foo/nope.d.ts
+//// export const nope = 0;
+
+// @Filename: /node_modules/foo/dist/index.d.ts
+//// export const index = 0;
+
+// @Filename: /node_modules/foo/dist/blah.d.ts
+//// export const blah = 0;
+
+// @Filename: /node_modules/foo/dist/foo/onlyInFooFolder.d.ts
+//// export const foo = 0;
+
+// @Filename: /node_modules/foo/dist/subfolder/one.d.ts
+//// export const one = 0;
+
+// @Filename: /a.ts
+//// import { } from "foo//**/";
+
+verify.completions({
+  marker: "",
+  isNewIdentifierLocation: true,
+  exact: ["blah", "index", "foo", "subfolder", "bar", "exact-match"],
+});
+
+edit.insert("foo/");
+
+verify.completions({
+  isNewIdentifierLocation: true,
+  exact: ["blah", "index", "foo", "subfolder"],
+});
+
+edit.insert("foo/");
+
+verify.completions({
+  isNewIdentifierLocation: true,
+  exact: ["onlyInFooFolder"],
+});