Skip to content

Commit 703c1bc

Browse files
andrewbranchandrewbranch
authored
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
1 parent bf4642f commit 703c1bc

22 files changed

+197
-43
lines changed

‎src/compiler/checker.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4485,7 +4485,6 @@ namespace ts {
44854485
// If no full tracker is provided, fake up a dummy one with a basic limited-functionality moduleResolverHost
44864486
tracker: tracker && tracker.trackSymbol ? tracker : { trackSymbol: () => false, moduleResolverHost: flags! & NodeBuilderFlags.DoNotIncludeSymbolChain ? {
44874487
getCommonSourceDirectory: !!(host as Program).getCommonSourceDirectory ? () => (host as Program).getCommonSourceDirectory() : () => "",
4488-
getSourceFiles: () => host.getSourceFiles(),
44894488
getCurrentDirectory: () => host.getCurrentDirectory(),
44904489
getSymlinkCache: maybeBind(host, host.getSymlinkCache),
44914490
useCaseSensitiveFileNames: maybeBind(host, host.useCaseSensitiveFileNames),

‎src/compiler/moduleSpecifiers.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,11 +294,8 @@ namespace ts.moduleSpecifiers {
294294
const result = forEach(targets, p => !(shouldFilterIgnoredPaths && containsIgnoredPath(p)) && cb(p, referenceRedirect === p));
295295
if (result) return result;
296296
}
297-
const links = host.getSymlinkCache
298-
? host.getSymlinkCache()
299-
: discoverProbableSymlinks(host.getSourceFiles(), getCanonicalFileName, cwd);
300297

301-
const symlinkedDirectories = links.getSymlinkedDirectoriesByRealpath();
298+
const symlinkedDirectories = host.getSymlinkCache?.().getSymlinkedDirectoriesByRealpath();
302299
const fullImportedFileName = getNormalizedAbsolutePath(importedFileName, cwd);
303300
const result = symlinkedDirectories && forEachAncestorDirectory(getDirectoryPath(fullImportedFileName), realPathDirectory => {
304301
const symlinkDirectories = symlinkedDirectories.get(ensureTrailingDirectorySeparator(toPath(realPathDirectory, cwd, getCanonicalFileName)));

‎src/compiler/program.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3668,10 +3668,13 @@ namespace ts {
36683668
if (host.getSymlinkCache) {
36693669
return host.getSymlinkCache();
36703670
}
3671-
return symlinks || (symlinks = discoverProbableSymlinks(
3672-
files,
3673-
getCanonicalFileName,
3674-
host.getCurrentDirectory()));
3671+
if (!symlinks) {
3672+
symlinks = createSymlinkCache(currentDirectory, getCanonicalFileName);
3673+
}
3674+
if (files && resolvedTypeReferenceDirectives && !symlinks.hasProcessedResolutions()) {
3675+
symlinks.setSymlinksFromResolutions(files, resolvedTypeReferenceDirectives);
3676+
}
3677+
return symlinks;
36753678
}
36763679
}
36773680

‎src/compiler/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8141,7 +8141,6 @@ namespace ts {
81418141
getGlobalTypingsCacheLocation?(): string | undefined;
81428142
getNearestAncestorDirectoryWithPackageJson?(fileName: string, rootDir?: string): string | undefined;
81438143

8144-
getSourceFiles(): readonly SourceFile[];
81458144
readonly redirectTargetsMap: RedirectTargetsMap;
81468145
getProjectReferenceRedirect(fileName: string): string | undefined;
81478146
isSourceOfProjectReferenceRedirect(fileName: string): boolean;

‎src/compiler/utilities.ts

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6167,12 +6167,25 @@ namespace ts {
61676167
setSymlinkedFile(symlinkPath: Path, real: string): void;
61686168
/*@internal*/
61696169
setSymlinkedDirectoryFromSymlinkedFile(symlink: string, real: string): void;
6170+
/**
6171+
* @internal
6172+
* Uses resolvedTypeReferenceDirectives from program instead of from files, since files
6173+
* don't include automatic type reference directives. Must be called only when
6174+
* `hasProcessedResolutions` returns false (once per cache instance).
6175+
*/
6176+
setSymlinksFromResolutions(files: readonly SourceFile[], typeReferenceDirectives: ReadonlyESMap<string, ResolvedTypeReferenceDirective | undefined> | undefined): void;
6177+
/**
6178+
* @internal
6179+
* Whether `setSymlinksFromResolutions` has already been called.
6180+
*/
6181+
hasProcessedResolutions(): boolean;
61706182
}
61716183

61726184
export function createSymlinkCache(cwd: string, getCanonicalFileName: GetCanonicalFileName): SymlinkCache {
61736185
let symlinkedDirectories: ESMap<Path, SymlinkedDirectory | false> | undefined;
61746186
let symlinkedDirectoriesByRealpath: MultiMap<Path, string> | undefined;
61756187
let symlinkedFiles: ESMap<Path, string> | undefined;
6188+
let hasProcessedResolutions = false;
61766189
return {
61776190
getSymlinkedFiles: () => symlinkedFiles,
61786191
getSymlinkedDirectories: () => symlinkedDirectories,
@@ -6201,28 +6214,28 @@ namespace ts {
62016214
});
62026215
}
62036216
},
6217+
setSymlinksFromResolutions(files, typeReferenceDirectives) {
6218+
Debug.assert(!hasProcessedResolutions);
6219+
hasProcessedResolutions = true;
6220+
for (const file of files) {
6221+
file.resolvedModules?.forEach(resolution => processResolution(this, resolution));
6222+
}
6223+
typeReferenceDirectives?.forEach(resolution => processResolution(this, resolution));
6224+
},
6225+
hasProcessedResolutions: () => hasProcessedResolutions,
62046226
};
6205-
}
6206-
6207-
export function discoverProbableSymlinks(files: readonly SourceFile[], getCanonicalFileName: GetCanonicalFileName, cwd: string): SymlinkCache {
6208-
const cache = createSymlinkCache(cwd, getCanonicalFileName);
6209-
const symlinks = flatMap(files, sf => {
6210-
const pairs = sf.resolvedModules && arrayFrom(mapDefinedIterator(sf.resolvedModules.values(), res =>
6211-
res?.originalPath ? [res.resolvedFileName, res.originalPath] as const : undefined));
6212-
return concatenate(pairs, sf.resolvedTypeReferenceDirectiveNames && arrayFrom(mapDefinedIterator(sf.resolvedTypeReferenceDirectiveNames.values(), res =>
6213-
res?.originalPath && res.resolvedFileName ? [res.resolvedFileName, res.originalPath] as const : undefined)));
6214-
});
62156227

6216-
for (const [resolvedPath, originalPath] of symlinks) {
6217-
cache.setSymlinkedFile(toPath(originalPath, cwd, getCanonicalFileName), resolvedPath);
6218-
const [commonResolved, commonOriginal] = guessDirectorySymlink(resolvedPath, originalPath, cwd, getCanonicalFileName) || emptyArray;
6228+
function processResolution(cache: SymlinkCache, resolution: ResolvedModuleFull | ResolvedTypeReferenceDirective | undefined) {
6229+
if (!resolution || !resolution.originalPath || !resolution.resolvedFileName) return;
6230+
const { resolvedFileName, originalPath } = resolution;
6231+
cache.setSymlinkedFile(toPath(originalPath, cwd, getCanonicalFileName), resolvedFileName);
6232+
const [commonResolved, commonOriginal] = guessDirectorySymlink(resolvedFileName, originalPath, cwd, getCanonicalFileName) || emptyArray;
62196233
if (commonResolved && commonOriginal) {
62206234
cache.setSymlinkedDirectory(
62216235
commonOriginal,
62226236
{ real: commonResolved, realPath: toPath(commonResolved, cwd, getCanonicalFileName) });
62236237
}
62246238
}
6225-
return cache;
62266239
}
62276240

62286241
function guessDirectorySymlink(a: string, b: string, cwd: string, getCanonicalFileName: GetCanonicalFileName): [string, string] | undefined {

‎src/harness/client.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -196,18 +196,16 @@ namespace ts.server {
196196
// Not passing along 'preferences' because server should already have those from the 'configure' command
197197
const args: protocol.CompletionsRequestArgs = this.createFileLocationRequestArgs(fileName, position);
198198

199-
const request = this.processRequest<protocol.CompletionsRequest>(CommandNames.Completions, args);
200-
const response = this.processResponse<protocol.CompletionsResponse>(request);
199+
const request = this.processRequest<protocol.CompletionsRequest>(CommandNames.CompletionInfo, args);
200+
const response = this.processResponse<protocol.CompletionInfoResponse>(request);
201201

202202
return {
203-
isGlobalCompletion: false,
204-
isMemberCompletion: false,
205-
isNewIdentifierLocation: false,
206-
entries: response.body!.map<CompletionEntry>(entry => { // TODO: GH#18217
203+
isGlobalCompletion: response.body!.isGlobalCompletion,
204+
isMemberCompletion: response.body!.isMemberCompletion,
205+
isNewIdentifierLocation: response.body!.isNewIdentifierLocation,
206+
entries: response.body!.entries.map<CompletionEntry>(entry => { // TODO: GH#18217
207207
if (entry.replacementSpan !== undefined) {
208-
const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source, data, isRecommended } = entry;
209-
// TODO: GH#241
210-
const res: CompletionEntry = { name, kind, kindModifiers, sortText, replacementSpan: this.decodeSpan(replacementSpan, fileName), hasAction, source, data: data as any, isRecommended };
208+
const res: CompletionEntry = { ...entry, data: entry.data as any, replacementSpan: this.decodeSpan(entry.replacementSpan, fileName) };
211209
return res;
212210
}
213211

‎src/server/project.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -353,10 +353,15 @@ namespace ts.server {
353353

354354
/*@internal*/
355355
getSymlinkCache(): SymlinkCache {
356-
return this.symlinks || (this.symlinks = discoverProbableSymlinks(
357-
this.program?.getSourceFiles() || emptyArray,
358-
this.getCanonicalFileName,
359-
this.getCurrentDirectory()));
356+
if (!this.symlinks) {
357+
this.symlinks = createSymlinkCache(this.getCurrentDirectory(), this.getCanonicalFileName);
358+
}
359+
if (this.program && !this.symlinks.hasProcessedResolutions()) {
360+
this.symlinks.setSymlinksFromResolutions(
361+
this.program.getSourceFiles(),
362+
this.program.getResolvedTypeReferenceDirectives());
363+
}
364+
return this.symlinks;
360365
}
361366

362367
// Method of LanguageServiceHost

‎src/services/codefixes/importFixes.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,9 @@ namespace ts.codefix {
6666
const preferTypeOnlyImport = !!usageIsTypeOnly && compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error;
6767
const useRequire = shouldUseRequire(sourceFile, program);
6868
const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, /*position*/ undefined, preferTypeOnlyImport, useRequire, host, preferences);
69-
addImport({ fixes: [fix], symbolName });
69+
if (fix) {
70+
addImport({ fixes: [fix], symbolName });
71+
}
7072
}
7173

7274
function addImport(info: FixesInfo) {
@@ -203,7 +205,7 @@ namespace ts.codefix {
203205
: getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, host, program, /*useAutoImportProvider*/ true);
204206
const useRequire = shouldUseRequire(sourceFile, program);
205207
const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && !isSourceFileJS(sourceFile) && isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
206-
const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, preferTypeOnlyImport, useRequire, host, preferences);
208+
const fix = Debug.checkDefined(getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, preferTypeOnlyImport, useRequire, host, preferences));
207209
return { moduleSpecifier: fix.moduleSpecifier, codeAction: codeFixActionToCodeAction(codeActionForFix({ host, formatContext, preferences }, sourceFile, symbolName, fix, getQuotePreference(sourceFile, preferences))) };
208210
}
209211

@@ -274,7 +276,7 @@ namespace ts.codefix {
274276
program: Program,
275277
host: LanguageServiceHost,
276278
preferences: UserPreferences
277-
): { exportInfo?: SymbolExportInfo, moduleSpecifier: string } {
279+
): { exportInfo?: SymbolExportInfo, moduleSpecifier: string } | undefined {
278280
return getBestFix(getNewImportFixes(program, importingFile, /*position*/ undefined, /*preferTypeOnlyImport*/ false, /*useRequire*/ false, exportInfo, host, preferences), importingFile, host);
279281
}
280282

@@ -529,7 +531,8 @@ namespace ts.codefix {
529531
return sort(fixes, (a, b) => compareValues(a.kind, b.kind) || compareModuleSpecifiers(a, b, allowsImportingSpecifier));
530532
}
531533

532-
function getBestFix<T extends ImportFix>(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost): T {
534+
function getBestFix<T extends ImportFix>(fixes: readonly T[], sourceFile: SourceFile, host: LanguageServiceHost): T | undefined {
535+
if (!some(fixes)) return;
533536
// These will always be placed first if available, and are better than other kinds
534537
if (fixes[0].kind === ImportFixKind.UseNamespace || fixes[0].kind === ImportFixKind.AddToExisting) {
535538
return fixes[0];

‎src/services/completions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1594,6 +1594,7 @@ namespace ts.Completions {
15941594

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

‎src/services/utilities.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1845,7 +1845,6 @@ namespace ts {
18451845
getSymlinkCache: maybeBind(host, host.getSymlinkCache) || program.getSymlinkCache,
18461846
getModuleSpecifierCache: maybeBind(host, host.getModuleSpecifierCache),
18471847
getGlobalTypingsCacheLocation: maybeBind(host, host.getGlobalTypingsCacheLocation),
1848-
getSourceFiles: () => program.getSourceFiles(),
18491848
redirectTargetsMap: program.redirectTargetsMap,
18501849
getProjectReferenceRedirect: fileName => program.getProjectReferenceRedirect(fileName),
18511850
isSourceOfProjectReferenceRedirect: fileName => program.isSourceOfProjectReferenceRedirect(fileName),

‎src/testRunner/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@
213213
"unittests/tsserver/session.ts",
214214
"unittests/tsserver/skipLibCheck.ts",
215215
"unittests/tsserver/smartSelection.ts",
216+
"unittests/tsserver/symlinkCache.ts",
216217
"unittests/tsserver/symLinks.ts",
217218
"unittests/tsserver/syntacticServer.ts",
218219
"unittests/tsserver/syntaxOperations.ts",

‎src/testRunner/unittests/tsserver/symlinkCache.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
namespace ts.projectSystem {
2+
const appTsconfigJson: File = {
3+
path: "/packages/app/tsconfig.json",
4+
content: `
5+
{
6+
"compilerOptions": {
7+
"module": "commonjs",
8+
"outDir": "dist",
9+
"rootDir": "src",
10+
"baseUrl": "."
11+
}
12+
"references": [{ "path": "../dep" }]
13+
}`
14+
};
15+
16+
const appSrcIndexTs: File = {
17+
path: "/packages/app/src/index.ts",
18+
content: `import "dep/does/not/exist";`
19+
};
20+
21+
const depPackageJson: File = {
22+
path: "/packages/dep/package.json",
23+
content: `{ "name": "dep", "main": "dist/index.js", "types": "dist/index.d.ts" }`
24+
};
25+
26+
const depTsconfigJson: File = {
27+
path: "/packages/dep/tsconfig.json",
28+
content: `
29+
{
30+
"compilerOptions": { "outDir": "dist", "rootDir": "src", "module": "commonjs" }
31+
}`
32+
};
33+
34+
const depSrcIndexTs: File = {
35+
path: "/packages/dep/src/index.ts",
36+
content: `
37+
import "./sub/folder";`
38+
};
39+
40+
const depSrcSubFolderIndexTs: File = {
41+
path: "/packages/dep/src/sub/folder/index.ts",
42+
content: `export const dep = 0;`
43+
};
44+
45+
const link: SymLink = {
46+
path: "/packages/app/node_modules/dep",
47+
symLink: "../../dep",
48+
};
49+
50+
describe("unittests:: tsserver:: symlinkCache", () => {
51+
it("contains symlinks discovered by project references resolution after program creation", () => {
52+
const { session, projectService } = setup();
53+
openFilesForSession([appSrcIndexTs], session);
54+
const project = projectService.configuredProjects.get(appTsconfigJson.path)!;
55+
assert.deepEqual(
56+
project.getSymlinkCache()?.getSymlinkedDirectories()?.get(link.path + "/" as Path),
57+
{ real: "/packages/dep/", realPath: "/packages/dep/" as Path }
58+
);
59+
});
60+
});
61+
62+
function setup() {
63+
const host = createServerHost([
64+
appTsconfigJson,
65+
appSrcIndexTs,
66+
depPackageJson,
67+
depTsconfigJson,
68+
depSrcIndexTs,
69+
depSrcSubFolderIndexTs,
70+
link,
71+
]);
72+
const session = createSession(host);
73+
const projectService = session.getProjectService();
74+
return {
75+
host,
76+
projectService,
77+
session,
78+
};
79+
}
80+
}

‎tests/cases/fourslash/fourslash.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,8 @@ declare namespace FourSlashInterface {
625625
readonly includeCompletionsForModuleExports?: boolean;
626626
readonly includeCompletionsForImportStatements?: boolean;
627627
readonly includeCompletionsWithSnippetText?: boolean;
628+
readonly includeCompletionsWithInsertText?: boolean;
629+
/** @deprecated use `includeCompletionsWithInsertText` */
628630
readonly includeInsertTextCompletions?: boolean;
629631
readonly includeAutomaticOptionalChainCompletions?: boolean;
630632
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";

‎tests/cases/fourslash/importStatementCompletions1.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
[0, 1, 2, 3, 4, 5].forEach(marker => {
2525
verify.completions({
26+
isNewIdentifierLocation: true,
2627
marker: "" + marker,
2728
exact: [{
2829
name: "foo",
@@ -65,6 +66,7 @@
6566

6667
[6, 7, 8, 9, 10, 11, 12].forEach(marker => {
6768
verify.completions({
69+
isNewIdentifierLocation: true,
6870
marker: "" + marker,
6971
exact: [],
7072
preferences: {

‎tests/cases/fourslash/importStatementCompletions_esModuleInterop1.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//// [|import f/**/|]
1111

1212
verify.completions({
13+
isNewIdentifierLocation: true,
1314
marker: "",
1415
exact: [{
1516
name: "foo",

‎tests/cases/fourslash/importStatementCompletions_esModuleInterop2.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//// [|import f/**/|]
1111

1212
verify.completions({
13+
isNewIdentifierLocation: true,
1314
marker: "",
1415
exact: [{
1516
name: "foo",

‎tests/cases/fourslash/importStatementCompletions_noPatternAmbient.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//// import style/**/
1111

1212
verify.completions({
13+
isNewIdentifierLocation: true,
1314
marker: "",
1415
exact: [],
1516
preferences: {

‎tests/cases/fourslash/importStatementCompletions_noSnippet.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//// [|import f/**/|]
88

99
verify.completions({
10+
isNewIdentifierLocation: true,
1011
marker: "",
1112
exact: [{
1213
name: "foo",

‎tests/cases/fourslash/importStatementCompletions_quotes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
//// [|import f/**/|]
99

1010
verify.completions({
11+
isNewIdentifierLocation: true,
1112
marker: "",
1213
exact: [{
1314
name: "foo",

‎tests/cases/fourslash/importStatementCompletions_semicolons.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
//// [|import f/**/|]
99

1010
verify.completions({
11+
isNewIdentifierLocation: true,
1112
marker: "",
1213
exact: [{
1314
name: "foo",

0 commit comments

Comments
 (0)