Include type reference directives in symlink cache, wait until program is present to create it (#44259)

* Fix discovery of more pnpm symlinks

* Add some tests

* Never show pnpm paths in auto imports, even if there’s no other path

* Import statement completions can return none

* Fix tests

* Add failing test showing poor symlink cache reuse

* Fix test, fails for right reasons now

* Preserve cache built up during program creation, then fill in with program resolutions

* Remove obsolete comment

* Remove obsolete type assertion

* Revert fully filtering out ignored paths

src/compiler/checker.ts

@@ -4485,7 +4485,6 @@ namespace ts {
                     // If no full tracker is provided, fake up a dummy one with a basic limited-functionality moduleResolverHost
                     tracker: tracker && tracker.trackSymbol ? tracker : { trackSymbol: () => false, moduleResolverHost: flags! & NodeBuilderFlags.DoNotIncludeSymbolChain ? {
                         getCommonSourceDirectory: !!(host as Program).getCommonSourceDirectory ? () => (host as Program).getCommonSourceDirectory() : () => "",
-                        getSourceFiles: () => host.getSourceFiles(),
                         getCurrentDirectory: () => host.getCurrentDirectory(),
                         getSymlinkCache: maybeBind(host, host.getSymlinkCache),
                         useCaseSensitiveFileNames: maybeBind(host, host.useCaseSensitiveFileNames),

src/compiler/moduleSpecifiers.ts

@@ -294,11 +294,8 @@ namespace ts.moduleSpecifiers {
             const result = forEach(targets, p => !(shouldFilterIgnoredPaths && containsIgnoredPath(p)) && cb(p, referenceRedirect === p));
             if (result) return result;
         }
-        const links = host.getSymlinkCache
-            ? host.getSymlinkCache()
-            : discoverProbableSymlinks(host.getSourceFiles(), getCanonicalFileName, cwd);
 
-        const symlinkedDirectories = links.getSymlinkedDirectoriesByRealpath();
+        const symlinkedDirectories = host.getSymlinkCache?.().getSymlinkedDirectoriesByRealpath();
         const fullImportedFileName = getNormalizedAbsolutePath(importedFileName, cwd);
         const result = symlinkedDirectories && forEachAncestorDirectory(getDirectoryPath(fullImportedFileName), realPathDirectory => {
             const symlinkDirectories = symlinkedDirectories.get(ensureTrailingDirectorySeparator(toPath(realPathDirectory, cwd, getCanonicalFileName)));

src/compiler/program.ts

@@ -3668,10 +3668,13 @@ namespace ts {
             if (host.getSymlinkCache) {
                 return host.getSymlinkCache();
             }
-            return symlinks || (symlinks = discoverProbableSymlinks(
-                files,
-                getCanonicalFileName,
-                host.getCurrentDirectory()));
+            if (!symlinks) {
+                symlinks = createSymlinkCache(currentDirectory, getCanonicalFileName);
+            }
+            if (files && resolvedTypeReferenceDirectives && !symlinks.hasProcessedResolutions()) {
+                symlinks.setSymlinksFromResolutions(files, resolvedTypeReferenceDirectives);
+            }
+            return symlinks;
         }
     }
 

src/compiler/types.ts

@@ -8141,7 +8141,6 @@ namespace ts {
         getGlobalTypingsCacheLocation?(): string | undefined;
         getNearestAncestorDirectoryWithPackageJson?(fileName: string, rootDir?: string): string | undefined;
 
-        getSourceFiles(): readonly SourceFile[];
         readonly redirectTargetsMap: RedirectTargetsMap;
         getProjectReferenceRedirect(fileName: string): string | undefined;
         isSourceOfProjectReferenceRedirect(fileName: string): boolean;

src/compiler/utilities.ts

@@ -6167,12 +6167,25 @@ namespace ts {
         setSymlinkedFile(symlinkPath: Path, real: string): void;
         /*@internal*/
         setSymlinkedDirectoryFromSymlinkedFile(symlink: string, real: string): void;
+        /**
+         * @internal
+         * Uses resolvedTypeReferenceDirectives from program instead of from files, since files
+         * don't include automatic type reference directives. Must be called only when
+         * `hasProcessedResolutions` returns false (once per cache instance).
+         */
+        setSymlinksFromResolutions(files: readonly SourceFile[], typeReferenceDirectives: ReadonlyESMap<string, ResolvedTypeReferenceDirective | undefined> | undefined): void;
+        /**
+         * @internal
+         * Whether `setSymlinksFromResolutions` has already been called.
+         */
+        hasProcessedResolutions(): boolean;
     }
 
     export function createSymlinkCache(cwd: string, getCanonicalFileName: GetCanonicalFileName): SymlinkCache {
         let symlinkedDirectories: ESMap<Path, SymlinkedDirectory | false> | undefined;
         let symlinkedDirectoriesByRealpath: MultiMap<Path, string> | undefined;
         let symlinkedFiles: ESMap<Path, string> | undefined;
+        let hasProcessedResolutions = false;
         return {
             getSymlinkedFiles: () => symlinkedFiles,
             getSymlinkedDirectories: () => symlinkedDirectories,
@@ -6201,28 +6214,28 @@ namespace ts {
                     });
                 }
             },
+            setSymlinksFromResolutions(files, typeReferenceDirectives) {
+                Debug.assert(!hasProcessedResolutions);
+                hasProcessedResolutions = true;
+                for (const file of files) {
+                    file.resolvedModules?.forEach(resolution => processResolution(this, resolution));
+                }
+                typeReferenceDirectives?.forEach(resolution => processResolution(this, resolution));
+            },
+            hasProcessedResolutions: () => hasProcessedResolutions,
         };
-    }
 
-    export function discoverProbableSymlinks(files: readonly SourceFile[], getCanonicalFileName: GetCanonicalFileName, cwd: string): SymlinkCache {
-        const cache = createSymlinkCache(cwd, getCanonicalFileName);
-        const symlinks = flatMap(files, sf => {
-            const pairs = sf.resolvedModules && arrayFrom(mapDefinedIterator(sf.resolvedModules.values(), res =>
-                res?.originalPath ? [res.resolvedFileName, res.originalPath] as const : undefined));
-            return concatenate(pairs, sf.resolvedTypeReferenceDirectiveNames && arrayFrom(mapDefinedIterator(sf.resolvedTypeReferenceDirectiveNames.values(), res =>
-                res?.originalPath && res.resolvedFileName ? [res.resolvedFileName, res.originalPath] as const : undefined)));
-        });
-
-        for (const [resolvedPath, originalPath] of symlinks) {
-            cache.setSymlinkedFile(toPath(originalPath, cwd, getCanonicalFileName), resolvedPath);
-            const [commonResolved, commonOriginal] = guessDirectorySymlink(resolvedPath, originalPath, cwd, getCanonicalFileName) || emptyArray;
+        function processResolution(cache: SymlinkCache, resolution: ResolvedModuleFull | ResolvedTypeReferenceDirective | undefined) {
+            if (!resolution || !resolution.originalPath || !resolution.resolvedFileName) return;
+            const { resolvedFileName, originalPath } = resolution;
+            cache.setSymlinkedFile(toPath(originalPath, cwd, getCanonicalFileName), resolvedFileName);
+            const [commonResolved, commonOriginal] = guessDirectorySymlink(resolvedFileName, originalPath, cwd, getCanonicalFileName) || emptyArray;
             if (commonResolved && commonOriginal) {
                 cache.setSymlinkedDirectory(
                     commonOriginal,
                     { real: commonResolved, realPath: toPath(commonResolved, cwd, getCanonicalFileName) });
             }
         }
-        return cache;
     }
 
     function guessDirectorySymlink(a: string, b: string, cwd: string, getCanonicalFileName: GetCanonicalFileName): [string, string] | undefined {

src/harness/client.ts

@@ -196,18 +196,16 @@ namespace ts.server {
             // Not passing along 'preferences' because server should already have those from the 'configure' command
             const args: protocol.CompletionsRequestArgs = this.createFileLocationRequestArgs(fileName, position);
 
-            const request = this.processRequest<protocol.CompletionsRequest>(CommandNames.Completions, args);
-            const response = this.processResponse<protocol.CompletionsResponse>(request);
+            const request = this.processRequest<protocol.CompletionsRequest>(CommandNames.CompletionInfo, args);
+            const response = this.processResponse<protocol.CompletionInfoResponse>(request);
 
             return {
-                isGlobalCompletion: false,
-                isMemberCompletion: false,
-                isNewIdentifierLocation: false,
-                entries: response.body!.map<CompletionEntry>(entry => { // TODO: GH#18217
+                isGlobalCompletion: response.body!.isGlobalCompletion,
+                isMemberCompletion: response.body!.isMemberCompletion,
+                isNewIdentifierLocation: response.body!.isNewIdentifierLocation,
+                entries: response.body!.entries.map<CompletionEntry>(entry => { // TODO: GH#18217
                     if (entry.replacementSpan !== undefined) {
-                        const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source, data, isRecommended } = entry;
-                        // TODO: GH#241
-                        const res: CompletionEntry = { name, kind, kindModifiers, sortText, replacementSpan: this.decodeSpan(replacementSpan, fileName), hasAction, source, data: data as any, isRecommended };
+                        const res: CompletionEntry = { ...entry, data: entry.data as any, replacementSpan: this.decodeSpan(entry.replacementSpan, fileName) };
                         return res;
                     }
 

src/server/project.ts

@@ -353,10 +353,15 @@ namespace ts.server {
 
         /*@internal*/
         getSymlinkCache(): SymlinkCache {
-            return this.symlinks || (this.symlinks = discoverProbableSymlinks(
-                this.program?.getSourceFiles() || emptyArray,
-                this.getCanonicalFileName,
-                this.getCurrentDirectory()));
+            if (!this.symlinks) {
+                this.symlinks = createSymlinkCache(this.getCurrentDirectory(), this.getCanonicalFileName);
+            }
+            if (this.program && !this.symlinks.hasProcessedResolutions()) {
+                this.symlinks.setSymlinksFromResolutions(
+                    this.program.getSourceFiles(),
+                    this.program.getResolvedTypeReferenceDirectives());
+            }
+            return this.symlinks;
         }
 
         // Method of LanguageServiceHost

src/services/codefixes/importFixes.ts

@@ -66,7 +66,9 @@ namespace ts.codefix {
             const preferTypeOnlyImport = !!usageIsTypeOnly && compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error;
             const useRequire = shouldUseRequire(sourceFile, program);
             const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, /*position*/ undefined, preferTypeOnlyImport, useRequire, host, preferences);
-            addImport({ fixes: [fix], symbolName });
+            if (fix) {
+                addImport({ fixes: [fix], symbolName });
+            }
         }
 
         function addImport(info: FixesInfo) {
@@ -203,7 +205,7 @@ namespace ts.codefix {
             : getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, host, program, /*useAutoImportProvider*/ true);
         const useRequire = shouldUseRequire(sourceFile, program);
         const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && !isSourceFileJS(sourceFile) && isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
-        const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, preferTypeOnlyImport, useRequire, host, preferences);
+        const fix = Debug.checkDefined(getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, preferTypeOnlyImport, useRequire, host, preferences));
         return { moduleSpecifier: fix.moduleSpecifier, codeAction: codeFixActionToCodeAction(codeActionForFix({ host, formatContext, preferences }, sourceFile, symbolName, fix, getQuotePreference(sourceFile, preferences))) };
     }
 
@@ -274,7 +276,7 @@ namespace ts.codefix {
         program: Program,
         host: LanguageServiceHost,
         preferences: UserPreferences
-    ): { exportInfo?: SymbolExportInfo, moduleSpecifier: string } {
+    ): { exportInfo?: SymbolExportInfo, moduleSpecifier: string } | undefined {
         return getBestFix(getNewImportFixes(program, importingFile, /*position*/ undefined, /*preferTypeOnlyImport*/ false, /*useRequire*/ false, exportInfo, host, preferences), importingFile, host);
     }
 
@@ -529,7 +531,8 @@ namespace ts.codefix {
         return sort(fixes, (a, b) => compareValues(a.kind, b.kind) || compareModuleSpecifiers(a, b, allowsImportingSpecifier));
     }
 
-    function getBestFix<T extends ImportFix>(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost): T {
+    function getBestFix<T extends ImportFix>(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost): T | undefined {
+        if (!some(fixes)) return;
         // These will always be placed first if available, and are better than other kinds
         if (fixes[0].kind === ImportFixKind.UseNamespace || fixes[0].kind === ImportFixKind.AddToExisting) {
             return fixes[0];

src/services/completions.ts

@@ -1594,6 +1594,7 @@ namespace ts.Completions {
 
         function tryGetImportCompletionSymbols(): GlobalsSearch {
             if (!importCompletionNode) return GlobalsSearch.Continue;
+            isNewIdentifierLocation = true;
             collectAutoImports(/*resolveModuleSpecifiers*/ true);
             return GlobalsSearch.Success;
         }
@@ -1762,7 +1763,7 @@ namespace ts.Completions {
                     // If we don't need to resolve module specifiers, we can use any re-export that is importable at all
                     // (We need to ensure that at least one is importable to show a completion.)
                     const { moduleSpecifier, exportInfo } = resolveModuleSpecifiers
-                        ? codefix.getModuleSpecifierForBestExportInfo(info, sourceFile, program, host, preferences)
+                        ? codefix.getModuleSpecifierForBestExportInfo(info, sourceFile, program, host, preferences) || {}
                         : { moduleSpecifier: undefined, exportInfo: find(info, isImportableExportInfo) };
                     if (!exportInfo) return;
                     const moduleFile = tryCast(exportInfo.moduleSymbol.valueDeclaration, isSourceFile);

src/services/utilities.ts

@@ -1845,7 +1845,6 @@ namespace ts {
             getSymlinkCache: maybeBind(host, host.getSymlinkCache) || program.getSymlinkCache,
             getModuleSpecifierCache: maybeBind(host, host.getModuleSpecifierCache),
             getGlobalTypingsCacheLocation: maybeBind(host, host.getGlobalTypingsCacheLocation),
-            getSourceFiles: () => program.getSourceFiles(),
             redirectTargetsMap: program.redirectTargetsMap,
             getProjectReferenceRedirect: fileName => program.getProjectReferenceRedirect(fileName),
             isSourceOfProjectReferenceRedirect: fileName => program.isSourceOfProjectReferenceRedirect(fileName),

src/testRunner/tsconfig.json

@@ -213,6 +213,7 @@
         "unittests/tsserver/session.ts",
         "unittests/tsserver/skipLibCheck.ts",
         "unittests/tsserver/smartSelection.ts",
+        "unittests/tsserver/symlinkCache.ts",
         "unittests/tsserver/symLinks.ts",
         "unittests/tsserver/syntacticServer.ts",
         "unittests/tsserver/syntaxOperations.ts",

src/testRunner/unittests/tsserver/symlinkCache.ts

@@ -0,0 +1,80 @@
+namespace ts.projectSystem {
+    const appTsconfigJson: File = {
+        path: "/packages/app/tsconfig.json",
+        content: `
+        {
+            "compilerOptions": {
+                "module": "commonjs",
+                "outDir": "dist",
+                "rootDir": "src",
+                "baseUrl": "."
+            }
+            "references": [{ "path": "../dep" }]
+        }`
+    };
+
+    const appSrcIndexTs: File = {
+        path: "/packages/app/src/index.ts",
+        content: `import "dep/does/not/exist";`
+    };
+
+    const depPackageJson: File = {
+        path: "/packages/dep/package.json",
+        content: `{ "name": "dep", "main": "dist/index.js", "types": "dist/index.d.ts" }`
+    };
+
+    const depTsconfigJson: File = {
+        path: "/packages/dep/tsconfig.json",
+        content: `
+        {
+            "compilerOptions": { "outDir": "dist", "rootDir": "src", "module": "commonjs" }
+        }`
+    };
+
+    const depSrcIndexTs: File = {
+        path: "/packages/dep/src/index.ts",
+        content: `
+        import "./sub/folder";`
+    };
+
+    const depSrcSubFolderIndexTs: File = {
+        path: "/packages/dep/src/sub/folder/index.ts",
+        content: `export const dep = 0;`
+    };
+
+    const link: SymLink = {
+        path: "/packages/app/node_modules/dep",
+        symLink: "../../dep",
+    };
+
+    describe("unittests:: tsserver:: symlinkCache", () => {
+        it("contains symlinks discovered by project references resolution after program creation", () => {
+            const { session, projectService } = setup();
+            openFilesForSession([appSrcIndexTs], session);
+            const project = projectService.configuredProjects.get(appTsconfigJson.path)!;
+            assert.deepEqual(
+                project.getSymlinkCache()?.getSymlinkedDirectories()?.get(link.path + "/" as Path),
+                { real: "/packages/dep/", realPath: "/packages/dep/" as Path }
+            );
+        });
+    });
+
+    function setup() {
+        const host = createServerHost([
+            appTsconfigJson,
+            appSrcIndexTs,
+            depPackageJson,
+            depTsconfigJson,
+            depSrcIndexTs,
+            depSrcSubFolderIndexTs,
+            link,
+        ]);
+        const session = createSession(host);
+        const projectService = session.getProjectService();
+        return {
+            host,
+            projectService,
+            session,
+        };
+    }
+}

tests/cases/fourslash/fourslash.ts

@@ -625,6 +625,8 @@ declare namespace FourSlashInterface {
         readonly includeCompletionsForModuleExports?: boolean;
         readonly includeCompletionsForImportStatements?: boolean;
         readonly includeCompletionsWithSnippetText?: boolean;
+        readonly includeCompletionsWithInsertText?: boolean;
+        /** @deprecated use `includeCompletionsWithInsertText` */
         readonly includeInsertTextCompletions?: boolean;
         readonly includeAutomaticOptionalChainCompletions?: boolean;
         readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";

tests/cases/fourslash/importStatementCompletions1.ts

@@ -23,6 +23,7 @@
 
 [0, 1, 2, 3, 4, 5].forEach(marker => {
   verify.completions({
+    isNewIdentifierLocation: true,
     marker: "" + marker,
     exact: [{
       name: "foo",
@@ -65,6 +66,7 @@
 
 [6, 7, 8, 9, 10, 11, 12].forEach(marker => {
   verify.completions({
+    isNewIdentifierLocation: true,
     marker: "" + marker,
     exact: [],
     preferences: {

tests/cases/fourslash/importStatementCompletions_esModuleInterop1.ts

@@ -10,6 +10,7 @@
 //// [|import f/**/|]
 
 verify.completions({
+  isNewIdentifierLocation: true,
   marker: "",
   exact: [{
     name: "foo",

tests/cases/fourslash/importStatementCompletions_esModuleInterop2.ts

@@ -10,6 +10,7 @@
 //// [|import f/**/|]
 
 verify.completions({
+  isNewIdentifierLocation: true,
   marker: "",
   exact: [{
     name: "foo",

tests/cases/fourslash/importStatementCompletions_noPatternAmbient.ts

@@ -10,6 +10,7 @@
 //// import style/**/
 
 verify.completions({
+  isNewIdentifierLocation: true,
   marker: "",
   exact: [],
   preferences: {

tests/cases/fourslash/importStatementCompletions_noSnippet.ts

@@ -7,6 +7,7 @@
 //// [|import f/**/|]
 
 verify.completions({
+  isNewIdentifierLocation: true,
   marker: "",
   exact: [{
     name: "foo",

tests/cases/fourslash/importStatementCompletions_quotes.ts

@@ -8,6 +8,7 @@
 //// [|import f/**/|]
 
 verify.completions({
+  isNewIdentifierLocation: true,
   marker: "",
   exact: [{
     name: "foo",

tests/cases/fourslash/importStatementCompletions_semicolons.ts

@@ -8,6 +8,7 @@
 //// [|import f/**/|]
 
 verify.completions({
+  isNewIdentifierLocation: true,
   marker: "",
   exact: [{
     name: "foo",

tests/cases/fourslash/server/importNameCodeFix_pnpm1.ts

@@ -0,0 +1,15 @@
+/// <reference path="../fourslash.ts" />
+
+// @Filename: /project/tsconfig.json
+//// { "compilerOptions": { "module": "commonjs" } }
+
+// @Filename: /project/node_modules/.pnpm/@types+react@17.0.7/node_modules/@types/react/index.d.ts
+//// export declare function Component(): void;
+
+// @Filename: /project/index.ts
+//// Component/**/
+
+// @link: /project/node_modules/.pnpm/@types+react@17.0.7/node_modules/@types/react -> /project/node_modules/@types/react
+
+goTo.marker("");
+verify.importFixAtPosition([`import { Component } from "react";\r\n\r\nComponent`]);

tests/cases/fourslash/server/importStatementCompletions_pnpm1.ts

@@ -0,0 +1,31 @@
+/// <reference path="../fourslash.ts" />
+
+// @Filename: /project/tsconfig.json
+//// { "compilerOptions": { "module": "commonjs" } }
+
+// @Filename: /project/node_modules/.pnpm/@types+react@17.0.7/node_modules/@types/react/index.d.ts
+//// export declare function Component(): void;
+
+// @Filename: /project/index.ts
+//// [|import Com/**/|]
+
+// @link: /project/node_modules/.pnpm/@types+react@17.0.7/node_modules/@types/react -> /project/node_modules/@types/react
+
+goTo.marker("");
+verify.completions({
+  isNewIdentifierLocation: true,
+  marker: "",
+  exact: [{
+    name: "Component",
+    source: "react",
+    insertText: `import { Component$1 } from "react";`,
+    isSnippet: true,
+    replacementSpan: test.ranges()[0],
+    sourceDisplay: "react",
+  }],
+  preferences: {
+    includeCompletionsForImportStatements: true,
+    includeCompletionsWithInsertText: true,
+    includeCompletionsWithSnippetText: true,
+  }
+});