From ff590b622ed01bb93a771e5452204b7da6bc4835 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Tue, 29 Oct 2019 08:40:49 -0700 Subject: [PATCH 1/3] Fix a crash when transforming functions in modules. (#34513) When transforming a module declaration and block, parse tree nodes contained in the module block have their parent pointers reset due to `shouldEmitModuleDeclaration` calling into `isInstantiatedModule`, which needs to set parent pointers to operate. That causes a crash when later transforming any nodes within the module, as retrieving their source file in `getSourceFileOfNode` (via `getOrCreateEmitNode`) fails, due to their new synthesized parent nodes not being in a source file. This change avoids the issue by using the parse tree node in `ts.ts` to decide whether a module declaration should be emitted (i.e. whether the module contains values). This means transformers cannot add values to modules that previously did not contain any. Fixes #34644. --- src/compiler/transformers/ts.ts | 7 ++++- src/testRunner/unittests/transform.ts | 29 +++++++++++++++++++ ...msCorrectly.transformUpdateModuleMember.js | 5 ++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/transformApi/transformsCorrectly.transformUpdateModuleMember.js diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index bdd0f5efcc7..d413a835d0d 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -2452,7 +2452,12 @@ namespace ts { * * @param node The module declaration node. */ - function shouldEmitModuleDeclaration(node: ModuleDeclaration) { + function shouldEmitModuleDeclaration(nodeIn: ModuleDeclaration) { + const node = getParseTreeNode(nodeIn, isModuleDeclaration); + if (!node) { + // If we can't find a parse tree node, assume the node is instantiated. + return true; + } return isInstantiatedModule(node, !!compilerOptions.preserveConstEnums || !!compilerOptions.isolatedModules); } diff --git a/src/testRunner/unittests/transform.ts b/src/testRunner/unittests/transform.ts index ffd5ba7609f..bcb0c93d1a8 100644 --- a/src/testRunner/unittests/transform.ts +++ b/src/testRunner/unittests/transform.ts @@ -447,6 +447,35 @@ namespace Foo { }).outputText; }); + testBaseline("transformUpdateModuleMember", () => { + return transpileModule(` +module MyModule { + const myVariable = 1; + function foo(param: string) {} +} +`, { + transformers: { + before: [renameVariable], + }, + compilerOptions: { + target: ScriptTarget.ES2015, + newLine: NewLineKind.CarriageReturnLineFeed, + } + }).outputText; + + function renameVariable(context: TransformationContext) { + return (sourceFile: SourceFile): SourceFile => { + return visitNode(sourceFile, rootTransform, isSourceFile); + }; + function rootTransform(node: T): Node { + if (isVariableDeclaration(node)) { + return updateVariableDeclaration(node, createIdentifier("newName"), /* type */ undefined, node.initializer); + } + return visitEachChild(node, rootTransform, context); + } + } + }); + // https://github.com/Microsoft/TypeScript/issues/24709 testBaseline("issue24709", () => { const fs = vfs.createFromFileSystem(Harness.IO, /*caseSensitive*/ true); diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.transformUpdateModuleMember.js b/tests/baselines/reference/transformApi/transformsCorrectly.transformUpdateModuleMember.js new file mode 100644 index 00000000000..4dad11f737c --- /dev/null +++ b/tests/baselines/reference/transformApi/transformsCorrectly.transformUpdateModuleMember.js @@ -0,0 +1,5 @@ +var MyModule; +(function (MyModule) { + const newName = 1; + function foo(param) { } +})(MyModule || (MyModule = {})); From 554bd24734f95a4d82f43c01df1fefbfe1021a88 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 29 Oct 2019 08:56:11 -0700 Subject: [PATCH 2/3] Fix checker handling for empty type argument lists (#34790) --- src/compiler/checker.ts | 6 +++--- .../reference/emptyTypeArgumentList.errors.txt | 10 +++++++--- tests/baselines/reference/emptyTypeArgumentList.js | 9 ++++++++- .../reference/emptyTypeArgumentList.symbols | 7 +++++++ .../baselines/reference/emptyTypeArgumentList.types | 10 +++++++++- .../emptyTypeArgumentListWithNew.errors.txt | 10 +++++++--- .../reference/emptyTypeArgumentListWithNew.js | 13 ++++++++++++- .../reference/emptyTypeArgumentListWithNew.symbols | 7 +++++++ .../reference/emptyTypeArgumentListWithNew.types | 10 +++++++++- .../reference/tsxTypeArgumentResolution.errors.txt | 8 +------- tests/cases/compiler/emptyTypeArgumentList.ts | 6 +++++- .../cases/compiler/emptyTypeArgumentListWithNew.ts | 6 +++++- 12 files changed, 80 insertions(+), 22 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index c8df4873377..145e3351754 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -23435,7 +23435,7 @@ namespace ts { // the declared number of type parameters, the call has an incorrect arity. const numTypeParameters = length(signature.typeParameters); const minTypeArgumentCount = getMinTypeArgumentCount(signature.typeParameters); - return !typeArguments || + return !some(typeArguments) || (typeArguments.length >= minTypeArgumentCount && typeArguments.length <= numTypeParameters); } @@ -24195,7 +24195,7 @@ namespace ts { if (isSingleNonGenericCandidate) { const candidate = candidates[0]; - if (typeArguments || !hasCorrectArity(node, args, candidate, signatureHelpTrailingComma)) { + if (some(typeArguments) || !hasCorrectArity(node, args, candidate, signatureHelpTrailingComma)) { return undefined; } if (getSignatureApplicabilityError(node, args, candidate, relation, CheckMode.Normal, /*reportErrors*/ false, /*containingMessageChain*/ undefined)) { @@ -24216,7 +24216,7 @@ namespace ts { if (candidate.typeParameters) { let typeArgumentTypes: Type[] | undefined; - if (typeArguments) { + if (some(typeArguments)) { typeArgumentTypes = checkTypeArguments(candidate, typeArguments, /*reportErrors*/ false); if (!typeArgumentTypes) { candidateForTypeArgumentError = candidate; diff --git a/tests/baselines/reference/emptyTypeArgumentList.errors.txt b/tests/baselines/reference/emptyTypeArgumentList.errors.txt index 395c990a3d9..4f970efb5f2 100644 --- a/tests/baselines/reference/emptyTypeArgumentList.errors.txt +++ b/tests/baselines/reference/emptyTypeArgumentList.errors.txt @@ -1,5 +1,5 @@ tests/cases/compiler/emptyTypeArgumentList.ts(2,4): error TS1099: Type argument list cannot be empty. -tests/cases/compiler/emptyTypeArgumentList.ts(2,5): error TS2558: Expected 1 type arguments, but got 0. +tests/cases/compiler/emptyTypeArgumentList.ts(6,9): error TS1099: Type argument list cannot be empty. ==== tests/cases/compiler/emptyTypeArgumentList.ts (2 errors) ==== @@ -7,5 +7,9 @@ tests/cases/compiler/emptyTypeArgumentList.ts(2,5): error TS2558: Expected 1 typ foo<>(); ~~ !!! error TS1099: Type argument list cannot be empty. - -!!! error TS2558: Expected 1 type arguments, but got 0. \ No newline at end of file + + // https://github.com/microsoft/TypeScript/issues/33041 + function noParams() {} + noParams<>(); + ~~ +!!! error TS1099: Type argument list cannot be empty. \ No newline at end of file diff --git a/tests/baselines/reference/emptyTypeArgumentList.js b/tests/baselines/reference/emptyTypeArgumentList.js index 30093113be4..8662c255dc2 100644 --- a/tests/baselines/reference/emptyTypeArgumentList.js +++ b/tests/baselines/reference/emptyTypeArgumentList.js @@ -1,7 +1,14 @@ //// [emptyTypeArgumentList.ts] function foo() { } -foo<>(); +foo<>(); + +// https://github.com/microsoft/TypeScript/issues/33041 +function noParams() {} +noParams<>(); //// [emptyTypeArgumentList.js] function foo() { } foo(); +// https://github.com/microsoft/TypeScript/issues/33041 +function noParams() { } +noParams(); diff --git a/tests/baselines/reference/emptyTypeArgumentList.symbols b/tests/baselines/reference/emptyTypeArgumentList.symbols index efbe5705f4f..78573ea57c5 100644 --- a/tests/baselines/reference/emptyTypeArgumentList.symbols +++ b/tests/baselines/reference/emptyTypeArgumentList.symbols @@ -6,3 +6,10 @@ function foo() { } foo<>(); >foo : Symbol(foo, Decl(emptyTypeArgumentList.ts, 0, 0)) +// https://github.com/microsoft/TypeScript/issues/33041 +function noParams() {} +>noParams : Symbol(noParams, Decl(emptyTypeArgumentList.ts, 1, 8)) + +noParams<>(); +>noParams : Symbol(noParams, Decl(emptyTypeArgumentList.ts, 1, 8)) + diff --git a/tests/baselines/reference/emptyTypeArgumentList.types b/tests/baselines/reference/emptyTypeArgumentList.types index 94eb2d878c8..32f6f64d1dc 100644 --- a/tests/baselines/reference/emptyTypeArgumentList.types +++ b/tests/baselines/reference/emptyTypeArgumentList.types @@ -3,6 +3,14 @@ function foo() { } >foo : () => void foo<>(); ->foo<>() : any +>foo<>() : void >foo : () => void +// https://github.com/microsoft/TypeScript/issues/33041 +function noParams() {} +>noParams : () => void + +noParams<>(); +>noParams<>() : void +>noParams : () => void + diff --git a/tests/baselines/reference/emptyTypeArgumentListWithNew.errors.txt b/tests/baselines/reference/emptyTypeArgumentListWithNew.errors.txt index 92eb250291a..f0e756d8d93 100644 --- a/tests/baselines/reference/emptyTypeArgumentListWithNew.errors.txt +++ b/tests/baselines/reference/emptyTypeArgumentListWithNew.errors.txt @@ -1,5 +1,5 @@ tests/cases/compiler/emptyTypeArgumentListWithNew.ts(2,8): error TS1099: Type argument list cannot be empty. -tests/cases/compiler/emptyTypeArgumentListWithNew.ts(2,9): error TS2558: Expected 1 type arguments, but got 0. +tests/cases/compiler/emptyTypeArgumentListWithNew.ts(6,13): error TS1099: Type argument list cannot be empty. ==== tests/cases/compiler/emptyTypeArgumentListWithNew.ts (2 errors) ==== @@ -7,5 +7,9 @@ tests/cases/compiler/emptyTypeArgumentListWithNew.ts(2,9): error TS2558: Expecte new foo<>(); ~~ !!! error TS1099: Type argument list cannot be empty. - -!!! error TS2558: Expected 1 type arguments, but got 0. \ No newline at end of file + + // https://github.com/microsoft/TypeScript/issues/33041 + class noParams {} + new noParams<>(); + ~~ +!!! error TS1099: Type argument list cannot be empty. \ No newline at end of file diff --git a/tests/baselines/reference/emptyTypeArgumentListWithNew.js b/tests/baselines/reference/emptyTypeArgumentListWithNew.js index c30fa31609a..bda839b5361 100644 --- a/tests/baselines/reference/emptyTypeArgumentListWithNew.js +++ b/tests/baselines/reference/emptyTypeArgumentListWithNew.js @@ -1,6 +1,10 @@ //// [emptyTypeArgumentListWithNew.ts] class foo { } -new foo<>(); +new foo<>(); + +// https://github.com/microsoft/TypeScript/issues/33041 +class noParams {} +new noParams<>(); //// [emptyTypeArgumentListWithNew.js] var foo = /** @class */ (function () { @@ -9,3 +13,10 @@ var foo = /** @class */ (function () { return foo; }()); new foo(); +// https://github.com/microsoft/TypeScript/issues/33041 +var noParams = /** @class */ (function () { + function noParams() { + } + return noParams; +}()); +new noParams(); diff --git a/tests/baselines/reference/emptyTypeArgumentListWithNew.symbols b/tests/baselines/reference/emptyTypeArgumentListWithNew.symbols index 34ce38aa478..9b2404960af 100644 --- a/tests/baselines/reference/emptyTypeArgumentListWithNew.symbols +++ b/tests/baselines/reference/emptyTypeArgumentListWithNew.symbols @@ -6,3 +6,10 @@ class foo { } new foo<>(); >foo : Symbol(foo, Decl(emptyTypeArgumentListWithNew.ts, 0, 0)) +// https://github.com/microsoft/TypeScript/issues/33041 +class noParams {} +>noParams : Symbol(noParams, Decl(emptyTypeArgumentListWithNew.ts, 1, 12)) + +new noParams<>(); +>noParams : Symbol(noParams, Decl(emptyTypeArgumentListWithNew.ts, 1, 12)) + diff --git a/tests/baselines/reference/emptyTypeArgumentListWithNew.types b/tests/baselines/reference/emptyTypeArgumentListWithNew.types index d89c3dd041f..57f1947c574 100644 --- a/tests/baselines/reference/emptyTypeArgumentListWithNew.types +++ b/tests/baselines/reference/emptyTypeArgumentListWithNew.types @@ -3,6 +3,14 @@ class foo { } >foo : foo new foo<>(); ->new foo<>() : any +>new foo<>() : foo >foo : typeof foo +// https://github.com/microsoft/TypeScript/issues/33041 +class noParams {} +>noParams : noParams + +new noParams<>(); +>new noParams<>() : noParams +>noParams : typeof noParams + diff --git a/tests/baselines/reference/tsxTypeArgumentResolution.errors.txt b/tests/baselines/reference/tsxTypeArgumentResolution.errors.txt index 7b429f073dd..cf6dedd16dd 100644 --- a/tests/baselines/reference/tsxTypeArgumentResolution.errors.txt +++ b/tests/baselines/reference/tsxTypeArgumentResolution.errors.txt @@ -3,9 +3,7 @@ tests/cases/conformance/jsx/file.tsx(18,26): error TS2322: Type 'number' is not tests/cases/conformance/jsx/file.tsx(20,13): error TS2558: Expected 1 type arguments, but got 2. tests/cases/conformance/jsx/file.tsx(22,13): error TS2558: Expected 1 type arguments, but got 2. tests/cases/conformance/jsx/file.tsx(24,12): error TS1099: Type argument list cannot be empty. -tests/cases/conformance/jsx/file.tsx(24,13): error TS2558: Expected 1 type arguments, but got 0. tests/cases/conformance/jsx/file.tsx(26,12): error TS1099: Type argument list cannot be empty. -tests/cases/conformance/jsx/file.tsx(26,13): error TS2558: Expected 1 type arguments, but got 0. tests/cases/conformance/jsx/file.tsx(39,14): error TS2344: Type 'Prop' does not satisfy the constraint '{ a: string; }'. Types of property 'a' are incompatible. Type 'number' is not assignable to type 'string'. @@ -16,7 +14,7 @@ tests/cases/conformance/jsx/file.tsx(51,47): error TS2322: Type 'string' is not tests/cases/conformance/jsx/file.tsx(53,47): error TS2322: Type 'string' is not assignable to type 'number'. -==== tests/cases/conformance/jsx/file.tsx (14 errors) ==== +==== tests/cases/conformance/jsx/file.tsx (12 errors) ==== import React = require('react'); interface Prop { @@ -53,14 +51,10 @@ tests/cases/conformance/jsx/file.tsx(53,47): error TS2322: Type 'string' is not x = a={10} b="hi" />; // error ~~ !!! error TS1099: Type argument list cannot be empty. - -!!! error TS2558: Expected 1 type arguments, but got 0. x = a={10} b="hi">; // error ~~ !!! error TS1099: Type argument list cannot be empty. - -!!! error TS2558: Expected 1 type arguments, but got 0. x= /> // OK diff --git a/tests/cases/compiler/emptyTypeArgumentList.ts b/tests/cases/compiler/emptyTypeArgumentList.ts index f06bbc3afd2..8089b3ed2b1 100644 --- a/tests/cases/compiler/emptyTypeArgumentList.ts +++ b/tests/cases/compiler/emptyTypeArgumentList.ts @@ -1,2 +1,6 @@ function foo() { } -foo<>(); \ No newline at end of file +foo<>(); + +// https://github.com/microsoft/TypeScript/issues/33041 +function noParams() {} +noParams<>(); \ No newline at end of file diff --git a/tests/cases/compiler/emptyTypeArgumentListWithNew.ts b/tests/cases/compiler/emptyTypeArgumentListWithNew.ts index 3a4926f5a8a..f7350ac8017 100644 --- a/tests/cases/compiler/emptyTypeArgumentListWithNew.ts +++ b/tests/cases/compiler/emptyTypeArgumentListWithNew.ts @@ -1,2 +1,6 @@ class foo { } -new foo<>(); \ No newline at end of file +new foo<>(); + +// https://github.com/microsoft/TypeScript/issues/33041 +class noParams {} +new noParams<>(); \ No newline at end of file From dbef230eb84d9bd6de539400de9ee4b9295a09e4 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 29 Oct 2019 10:49:14 -0700 Subject: [PATCH 3/3] This handles when packages are symbol links in mono repo like scenarios to use source files instead of output d.ts from project reference (#34743) * Fix incorrect outDir usage instead of out * Handle symlinks of packages in mono repo like packages Fixes #34723 * Added clarified comment --- src/compiler/program.ts | 14 +- src/server/project.ts | 128 +++++++++++++++--- .../unittests/tsserver/projectReferences.ts | 94 ++++++++++++- .../reference/api/tsserverlibrary.d.ts | 8 ++ 4 files changed, 226 insertions(+), 18 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 59b57bfa99a..c9592d55cf9 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -2271,7 +2271,19 @@ namespace ts { // Get source file from normalized fileName function findSourceFile(fileName: string, path: Path, isDefaultLib: boolean, ignoreNoDefaultLib: boolean, refFile: RefFile | undefined, packageId: PackageId | undefined): SourceFile | undefined { if (useSourceOfProjectReferenceRedirect) { - const source = getSourceOfProjectReferenceRedirect(fileName); + let source = getSourceOfProjectReferenceRedirect(fileName); + // If preserveSymlinks is true, module resolution wont jump the symlink + // but the resolved real path may be the .d.ts from project reference + // Note:: Currently we try the real path only if the + // file is from node_modules to avoid having to run real path on all file paths + if (!source && + host.realpath && + options.preserveSymlinks && + isDeclarationFileName(fileName) && + stringContains(fileName, nodeModulesPathPart)) { + const realPath = host.realpath(fileName); + if (realPath !== fileName) source = getSourceOfProjectReferenceRedirect(realPath); + } if (source) { const file = isString(source) ? findSourceFile(source, toPath(source), isDefaultLib, ignoreNoDefaultLib, refFile, packageId) : diff --git a/src/server/project.ts b/src/server/project.ts index 28f9166064d..5121556f53b 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -258,7 +258,8 @@ namespace ts.server { private compilerOptions: CompilerOptions, public compileOnSaveEnabled: boolean, directoryStructureHost: DirectoryStructureHost, - currentDirectory: string | undefined) { + currentDirectory: string | undefined, + customRealpath?: (s: string) => string) { this.directoryStructureHost = directoryStructureHost; this.currentDirectory = this.projectService.getNormalizedAbsolutePath(currentDirectory || ""); this.getCanonicalFileName = this.projectService.toCanonicalFileName; @@ -286,7 +287,7 @@ namespace ts.server { } if (host.realpath) { - this.realpath = path => host.realpath!(path); + this.realpath = customRealpath || (path => host.realpath!(path)); } // Use the current directory as resolution root only if the project created using current directory string @@ -1660,6 +1661,12 @@ namespace ts.server { } } + /*@internal*/ + interface SymlinkedDirectory { + real: string; + realPath: Path; + } + /** * If a file is opened, the server will look for a tsconfig (or jsconfig) * and if successfull create a ConfiguredProject for it. @@ -1673,6 +1680,8 @@ namespace ts.server { readonly canonicalConfigFilePath: NormalizedPath; private projectReferenceCallbacks: ResolvedProjectReferenceCallbacks | undefined; private mapOfDeclarationDirectories: Map | undefined; + private symlinkedDirectories: Map | undefined; + private symlinkedFiles: Map | undefined; /* @internal */ pendingReload: ConfigFileProgramReloadLevel | undefined; @@ -1714,7 +1723,9 @@ namespace ts.server { /*compilerOptions*/ {}, /*compileOnSaveEnabled*/ false, cachedDirectoryStructureHost, - getDirectoryPath(configFileName)); + getDirectoryPath(configFileName), + projectService.host.realpath && (s => this.getRealpath(s)) + ); this.canonicalConfigFilePath = asNormalizedPath(projectService.toCanonicalFileName(configFileName)); } @@ -1727,18 +1738,34 @@ namespace ts.server { useSourceOfProjectReferenceRedirect = () => !!this.languageServiceEnabled && !this.getCompilerOptions().disableSourceOfProjectReferenceRedirect; + private fileExistsIfProjectReferenceDts(file: string) { + const source = this.projectReferenceCallbacks!.getSourceOfProjectReferenceRedirect(file); + return source !== undefined ? + isString(source) ? super.fileExists(source) : true : + undefined; + } + /** * This implementation of fileExists checks if the file being requested is * .d.ts file for the referenced Project. * If it is it returns true irrespective of whether that file exists on host */ fileExists(file: string): boolean { + if (super.fileExists(file)) return true; + if (!this.useSourceOfProjectReferenceRedirect() || !this.projectReferenceCallbacks) return false; + if (!isDeclarationFileName(file)) return false; + // Project references go to source file instead of .d.ts file - if (this.useSourceOfProjectReferenceRedirect() && this.projectReferenceCallbacks) { - const source = this.projectReferenceCallbacks.getSourceOfProjectReferenceRedirect(file); - if (source) return isString(source) ? super.fileExists(source) : true; - } - return super.fileExists(file); + return this.fileOrDirectoryExistsUsingSource(file, /*isFile*/ true); + } + + private directoryExistsIfProjectReferenceDeclDir(dir: string) { + const dirPath = this.toPath(dir); + const dirPathWithTrailingDirectorySeparator = `${dirPath}${directorySeparator}`; + return forEachKey( + this.mapOfDeclarationDirectories!, + declDirPath => dirPath === declDirPath || startsWith(declDirPath, dirPathWithTrailingDirectorySeparator) + ); } /** @@ -1747,14 +1774,17 @@ namespace ts.server { * If it is it returns true irrespective of whether that directory exists on host */ directoryExists(path: string): boolean { - if (super.directoryExists(path)) return true; + if (super.directoryExists(path)) { + this.handleDirectoryCouldBeSymlink(path); + return true; + } if (!this.useSourceOfProjectReferenceRedirect() || !this.projectReferenceCallbacks) return false; if (!this.mapOfDeclarationDirectories) { this.mapOfDeclarationDirectories = createMap(); this.projectReferenceCallbacks.forEachResolvedProjectReference(ref => { if (!ref) return; - const out = ref.commandLine.options.outFile || ref.commandLine.options.outDir; + const out = ref.commandLine.options.outFile || ref.commandLine.options.out; if (out) { this.mapOfDeclarationDirectories!.set(getDirectoryPath(this.toPath(out)), true); } @@ -1767,12 +1797,74 @@ namespace ts.server { } }); } - const dirPath = this.toPath(path); - const dirPathWithTrailingDirectorySeparator = `${dirPath}${directorySeparator}`; - return !!forEachKey( - this.mapOfDeclarationDirectories, - declDirPath => dirPath === declDirPath || startsWith(declDirPath, dirPathWithTrailingDirectorySeparator) - ); + + return this.fileOrDirectoryExistsUsingSource(path, /*isFile*/ false); + } + + private realpathIfSymlinkedProjectReferenceDts(s: string): string | undefined { + return this.symlinkedFiles && this.symlinkedFiles.get(this.toPath(s)); + } + + private getRealpath(s: string): string { + return this.realpathIfSymlinkedProjectReferenceDts(s) || + this.projectService.host.realpath!(s); + } + + private handleDirectoryCouldBeSymlink(directory: string) { + if (!this.useSourceOfProjectReferenceRedirect() || !this.projectReferenceCallbacks) return; + + // Because we already watch node_modules, handle symlinks in there + if (!this.realpath || !stringContains(directory, nodeModulesPathPart)) return; + if (!this.symlinkedDirectories) this.symlinkedDirectories = createMap(); + const directoryPath = ensureTrailingDirectorySeparator(this.toPath(directory)); + if (this.symlinkedDirectories.has(directoryPath)) return; + + const real = this.projectService.host.realpath!(directory); + let realPath: Path; + if (real === directory || + (realPath = ensureTrailingDirectorySeparator(this.toPath(real))) === directoryPath) { + // not symlinked + this.symlinkedDirectories.set(directoryPath, false); + return; + } + + this.symlinkedDirectories.set(directoryPath, { + real: ensureTrailingDirectorySeparator(real), + realPath + }); + } + + private fileOrDirectoryExistsUsingSource(fileOrDirectory: string, isFile: boolean): boolean { + const fileOrDirectoryExistsUsingSource = isFile ? + (file: string) => this.fileExistsIfProjectReferenceDts(file) : + (dir: string) => this.directoryExistsIfProjectReferenceDeclDir(dir); + // Check current directory or file + const result = fileOrDirectoryExistsUsingSource(fileOrDirectory); + if (result !== undefined) return result; + + if (!this.symlinkedDirectories) return false; + const fileOrDirectoryPath = this.toPath(fileOrDirectory); + if (!stringContains(fileOrDirectoryPath, nodeModulesPathPart)) return false; + if (isFile && this.symlinkedFiles && this.symlinkedFiles.has(fileOrDirectoryPath)) return true; + + // If it contains node_modules check if its one of the symlinked path we know of + return firstDefinedIterator( + this.symlinkedDirectories.entries(), + ([directoryPath, symlinkedDirectory]) => { + if (!symlinkedDirectory || !startsWith(fileOrDirectoryPath, directoryPath)) return undefined; + const result = fileOrDirectoryExistsUsingSource(fileOrDirectoryPath.replace(directoryPath, symlinkedDirectory.realPath)); + if (isFile && result) { + if (!this.symlinkedFiles) this.symlinkedFiles = createMap(); + // Store the real path for the file' + const absolutePath = getNormalizedAbsolutePath(fileOrDirectory, this.currentDirectory); + this.symlinkedFiles.set( + fileOrDirectoryPath, + `${symlinkedDirectory.real}${absolutePath.replace(new RegExp(directoryPath, "i"), "")}` + ); + } + return result; + } + ) || false; } /** @@ -1785,6 +1877,8 @@ namespace ts.server { this.pendingReload = ConfigFileProgramReloadLevel.None; this.projectReferenceCallbacks = undefined; this.mapOfDeclarationDirectories = undefined; + this.symlinkedDirectories = undefined; + this.symlinkedFiles = undefined; let result: boolean; switch (reloadLevel) { case ConfigFileProgramReloadLevel.Partial: @@ -1917,6 +2011,8 @@ namespace ts.server { this.configFileSpecs = undefined; this.projectReferenceCallbacks = undefined; this.mapOfDeclarationDirectories = undefined; + this.symlinkedDirectories = undefined; + this.symlinkedFiles = undefined; super.close(); } diff --git a/src/testRunner/unittests/tsserver/projectReferences.ts b/src/testRunner/unittests/tsserver/projectReferences.ts index bbc368b6d8a..0966ad911e3 100644 --- a/src/testRunner/unittests/tsserver/projectReferences.ts +++ b/src/testRunner/unittests/tsserver/projectReferences.ts @@ -1,6 +1,6 @@ namespace ts.projectSystem { describe("unittests:: tsserver:: with project references and tsbuild", () => { - function createHost(files: readonly File[], rootNames: readonly string[]) { + function createHost(files: readonly TestFSWithWatch.FileOrFolderOrSymLink[], rootNames: readonly string[]) { const host = createServerHost(files); // ts build should succeed @@ -1373,5 +1373,97 @@ function foo() { assert.isTrue(projectA.dirty); projectA.updateGraph(); }); + + describe("when references are monorepo like with symlinks", () => { + function verifySession(alreadyBuilt: boolean, extraOptions: CompilerOptions) { + const bPackageJson: File = { + path: `${projectRoot}/packages/B/package.json`, + content: JSON.stringify({ + main: "lib/index.js", + types: "lib/index.d.ts" + }) + }; + const aConfig = config("A", extraOptions, ["../B"]); + const bConfig = config("B", extraOptions); + const aIndex = index("A", `import { foo } from 'b'; +import { bar } from 'b/lib/bar'; +foo(); +bar();`); + const bIndex = index("B", `export function foo() { }`); + const bBar: File = { + path: `${projectRoot}/packages/B/src/bar.ts`, + content: `export function bar() { }` + }; + const bSymlink: SymLink = { + path: `${projectRoot}/node_modules/b`, + symLink: `${projectRoot}/packages/B` + }; + + const files = [libFile, bPackageJson, aConfig, bConfig, aIndex, bIndex, bBar, bSymlink]; + const host = alreadyBuilt ? + createHost(files, [aConfig.path]) : + createServerHost(files); + + // Create symlink in node module + const session = createSession(host, { canUseEvents: true }); + openFilesForSession([aIndex], session); + const service = session.getProjectService(); + const project = service.configuredProjects.get(aConfig.path.toLowerCase())!; + assert.deepEqual(project.getAllProjectErrors(), []); + checkProjectActualFiles( + project, + [aConfig.path, aIndex.path, bIndex.path, bBar.path, libFile.path] + ); + verifyGetErrRequest({ + host, + session, + expected: [ + { file: aIndex, syntax: [], semantic: [], suggestion: [] } + ] + }); + } + + function verifySymlinkScenario(alreadyBuilt: boolean) { + it("with preserveSymlinks turned off", () => { + verifySession(alreadyBuilt, {}); + }); + + it("with preserveSymlinks turned on", () => { + verifySession(alreadyBuilt, { preserveSymlinks: true }); + }); + } + + describe("when solution is not built", () => { + verifySymlinkScenario(/*alreadyBuilt*/ false); + }); + + describe("when solution is already built", () => { + verifySymlinkScenario(/*alreadyBuilt*/ true); + }); + + function config(packageName: string, extraOptions: CompilerOptions, references?: string[]): File { + return { + path: `${projectRoot}/packages/${packageName}/tsconfig.json`, + content: JSON.stringify({ + compilerOptions: { + baseUrl: ".", + outDir: "lib", + rootDir: "src", + composite: true, + ...extraOptions + }, + include: ["src"], + ...(references ? { references: references.map(path => ({ path })) } : {}) + }) + }; + } + + function index(packageName: string, content: string): File { + return { + path: `${projectRoot}/packages/${packageName}/src/index.ts`, + content + }; + } + }); }); } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index c06d040800d..e775580c1fb 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8684,23 +8684,31 @@ declare namespace ts.server { readonly canonicalConfigFilePath: NormalizedPath; private projectReferenceCallbacks; private mapOfDeclarationDirectories; + private symlinkedDirectories; + private symlinkedFiles; /** Ref count to the project when opened from external project */ private externalProjectRefCount; private projectErrors; private projectReferences; protected isInitialLoadPending: () => boolean; + private fileExistsIfProjectReferenceDts; /** * This implementation of fileExists checks if the file being requested is * .d.ts file for the referenced Project. * If it is it returns true irrespective of whether that file exists on host */ fileExists(file: string): boolean; + private directoryExistsIfProjectReferenceDeclDir; /** * This implementation of directoryExists checks if the directory being requested is * directory of .d.ts file for the referenced Project. * If it is it returns true irrespective of whether that directory exists on host */ directoryExists(path: string): boolean; + private realpathIfSymlinkedProjectReferenceDts; + private getRealpath; + private handleDirectoryCouldBeSymlink; + private fileOrDirectoryExistsUsingSource; /** * If the project has reload from disk pending, it reloads (and then updates graph as part of that) instead of just updating the graph * @returns: true if set of files in the project stays the same and false - otherwise.