Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
1 change: 0 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4482,7 +4482,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),
Expand Down
5 changes: 1 addition & 4 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While refactoring for better preservation of a single SymlinkCache instance, I decided to get rid of this fallback. ModuleSpecifierResolutionHost is internal and all our implementations of it implement getSymlinkCache. It always delegates to Program.getSymlinkCache which delegates to Project.getSymlinkCache, but Program itself has a fallback. This should never be called directly by public API consumers, and even those who are using internal things should be taken care of via ts.createModuleSpecifierResolutionHost, so I think this is safe to remove.


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)));
Expand Down
11 changes: 7 additions & 4 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3663,10 +3663,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;
}
}

Expand Down
1 change: 0 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8137,7 +8137,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;
Expand Down
41 changes: 27 additions & 14 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6190,12 +6190,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,
Expand Down Expand Up @@ -6224,28 +6237,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 {
Expand Down
16 changes: 7 additions & 9 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
13 changes: 9 additions & 4 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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))) };
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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];
Expand Down
3 changes: 2 additions & 1 deletion src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1594,6 +1594,7 @@ namespace ts.Completions {

function tryGetImportCompletionSymbols(): GlobalsSearch {
if (!importCompletionNode) return GlobalsSearch.Continue;
isNewIdentifierLocation = true;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discovered this was missing only because I added fourslash server tests... unrelated to main change

collectAutoImports(/*resolveModuleSpecifiers*/ true);
return GlobalsSearch.Success;
}
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,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",
Expand Down
80 changes: 80 additions & 0 deletions src/testRunner/unittests/tsserver/symlinkCache.ts
Original file line number Diff line number Diff line change
@@ -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 }
Comment on lines +55 to +57
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This proves that directoryExists calls during program creation stayed in the cache because the /app/packages/tsconfig.json project has no source files with successful resolutions to the symlinked project, but it did discover the symlink as it tried to resolve import "dep/does/not/exist".

);
});
});

function setup() {
const host = createServerHost([
appTsconfigJson,
appSrcIndexTs,
depPackageJson,
depTsconfigJson,
depSrcIndexTs,
depSrcSubFolderIndexTs,
link,
]);
const session = createSession(host);
const projectService = session.getProjectService();
return {
host,
projectService,
session,
};
}
}
2 changes: 2 additions & 0 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 2 additions & 0 deletions tests/cases/fourslash/importStatementCompletions1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

[0, 1, 2, 3, 4, 5].forEach(marker => {
verify.completions({
isNewIdentifierLocation: true,
marker: "" + marker,
exact: [{
name: "foo",
Expand Down Expand Up @@ -65,6 +66,7 @@

[6, 7, 8, 9, 10, 11, 12].forEach(marker => {
verify.completions({
isNewIdentifierLocation: true,
marker: "" + marker,
exact: [],
preferences: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//// [|import f/**/|]

verify.completions({
isNewIdentifierLocation: true,
marker: "",
exact: [{
name: "foo",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//// [|import f/**/|]

verify.completions({
isNewIdentifierLocation: true,
marker: "",
exact: [{
name: "foo",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//// import style/**/

verify.completions({
isNewIdentifierLocation: true,
marker: "",
exact: [],
preferences: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//// [|import f/**/|]

verify.completions({
isNewIdentifierLocation: true,
marker: "",
exact: [{
name: "foo",
Expand Down
1 change: 1 addition & 0 deletions tests/cases/fourslash/importStatementCompletions_quotes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//// [|import f/**/|]

verify.completions({
isNewIdentifierLocation: true,
marker: "",
exact: [{
name: "foo",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//// [|import f/**/|]

verify.completions({
isNewIdentifierLocation: true,
marker: "",
exact: [{
name: "foo",
Expand Down
Loading