From c65e43e85e3a4e5b0e4384189d9a3837038e3fce Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 15 Nov 2018 10:17:00 -0800 Subject: [PATCH 01/23] Do not close over program in getSourceFileLike --- src/services/sourcemaps.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 92ebcc8c261..9de8557cd19 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -39,9 +39,10 @@ namespace ts { // obviously invalid map return file.sourceMapper = identitySourceMapConsumer; } - const program = getProgram(); + return file.sourceMapper = createDocumentPositionMapper({ getSourceFileLike: s => { + const program = getProgram(); // Lookup file in program, if provided const file = program && program.getSourceFileByPath(s); // file returned here could be .d.ts when asked for .ts file if projectReferences and module resolution created this source file From 86857d5b09d6723b6f9043d96abfe6f553f6e7cf Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 15 Nov 2018 10:40:59 -0800 Subject: [PATCH 02/23] Use program directly to get the sourceFile of source position --- src/services/sourcemaps.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 9de8557cd19..9b8f97a6640 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -93,8 +93,9 @@ namespace ts { function tryGetSourcePosition(info: DocumentPosition): DocumentPosition | undefined { if (!isDeclarationFileName(info.fileName)) return undefined; - const file = getFile(info.fileName); + const file = getSourceFile(info.fileName); if (!file) return undefined; + const newLoc = getSourceMapper(info.fileName, file).getSourcePosition(info); return newLoc === info ? undefined : tryGetSourcePosition(newLoc) || newLoc; } @@ -103,27 +104,31 @@ namespace ts { const program = getProgram(); const options = program.getCompilerOptions(); const outPath = options.outFile || options.out; + const declarationPath = outPath ? removeFileExtension(outPath) + Extension.Dts : getDeclarationEmitOutputFilePathWorker(info.fileName, program.getCompilerOptions(), currentDirectory, program.getCommonSourceDirectory(), getCanonicalFileName); if (declarationPath === undefined) return undefined; - const declarationFile = getFile(declarationPath); + + const declarationFile = getGeneratedFile(declarationPath); if (!declarationFile) return undefined; + const newLoc = getSourceMapper(declarationPath, declarationFile).getGeneratedPosition(info); return newLoc === info ? undefined : newLoc; } - function getFile(fileName: string): SourceFileLike | undefined { - const path = toPath(fileName); - const file = getProgram().getSourceFileByPath(path); - if (file && file.resolvedPath === path) { - return file; - } - return sourcemappedFileCache.get(path); + function getSourceFile(fileName: string) { + const program = getProgram(); + return program && program.getSourceFileByPath(toPath(fileName)); + } + + function getGeneratedFile(fileName: string) { + return sourcemappedFileCache.get(toPath(fileName)); // TODO: should ask host instead? } function toLineColumnOffset(fileName: string, position: number): LineAndCharacter { - const file = getFile(fileName)!; // TODO: GH#18217 + const sourceFile = getSourceFile(fileName); + const file = sourceFile && sourceFile.resolvedPath === toPath(fileName) ? sourceFile : getGeneratedFile(fileName)!; // TODO: GH#18217 return file.getLineAndCharacterOfPosition(position); } From 0a8c47bd45fb4265c9d783a5492bdf86d91e5ddd Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 15 Nov 2018 11:29:04 -0800 Subject: [PATCH 03/23] Refactoring to createSourceMapperHost --- src/compiler/types.ts | 2 +- src/services/services.ts | 29 ++++++---- src/services/sourcemaps.ts | 107 +++++++++++++++++-------------------- src/services/utilities.ts | 4 +- 4 files changed, 72 insertions(+), 70 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 495465f7032..f52d82a8a2f 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5530,7 +5530,7 @@ namespace ts { export interface DocumentPositionMapperHost { getSourceFileLike(path: Path): SourceFileLike | undefined; getCanonicalFileName(path: string): string; - log?(text: string): void; + log(text: string): void; } /** diff --git a/src/services/services.ts b/src/services/services.ts index 20400a3ec47..67f9e537574 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1139,7 +1139,14 @@ namespace ts { const useCaseSensitiveFileNames = hostUsesCaseSensitiveFileNames(host); const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); - const sourceMapper = getSourceMapper(useCaseSensitiveFileNames, currentDirectory, log, host, () => program); + const sourceMapper = getSourceMapper({ + useCaseSensitiveFileNames: () => useCaseSensitiveFileNames, + getCurrentDirectory: () => currentDirectory, + getProgram, + fileExists: host.fileExists ? f => host.fileExists!(f) : returnFalse, + readFile: host.readFile ? (f, encoding) => host.readFile!(f, encoding) : () => undefined, + log + }); function getValidSourceFile(fileName: string): SourceFile { const sourceFile = program.getSourceFile(fileName); @@ -1203,15 +1210,7 @@ namespace ts { writeFile: noop, getCurrentDirectory: () => currentDirectory, fileExists, - readFile(fileName) { - // stub missing host functionality - const path = toPath(fileName, currentDirectory, getCanonicalFileName); - const entry = hostCache && hostCache.getEntryByPath(path); - if (entry) { - return isString(entry) ? undefined : getSnapshotText(entry.scriptSnapshot); - } - return host.readFile && host.readFile(fileName); - }, + readFile, realpath: host.realpath && (path => host.realpath!(path)), directoryExists: directoryName => { return directoryProbablyExists(directoryName, host); @@ -1272,6 +1271,16 @@ namespace ts { (!!host.fileExists && host.fileExists(fileName)); } + function readFile(fileName: string) { + // stub missing host functionality + const path = toPath(fileName, currentDirectory, getCanonicalFileName); + const entry = hostCache && hostCache.getEntryByPath(path); + if (entry) { + return isString(entry) ? undefined : getSnapshotText(entry.scriptSnapshot); + } + return host.readFile && host.readFile(fileName); + } + // Release any files we have acquired in the old program but are // not part of the new program. function onReleaseOldSourceFile(oldSourceFile: SourceFile, oldOptions: CompilerOptions) { diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 9b8f97a6640..c2b9dbfb46a 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -9,15 +9,21 @@ namespace ts { clearCache(): void; } + export interface SourceMapperHost { + useCaseSensitiveFileNames(): boolean; + getCurrentDirectory(): string; + getProgram(): Program | undefined; + fileExists(path: string): boolean; + readFile(path: string, encoding?: string): string | undefined; + log(s: string): void; + } + export function getSourceMapper( - useCaseSensitiveFileNames: boolean, - currentDirectory: string, - log: (message: string) => void, - host: LanguageServiceHost, - getProgram: () => Program, + host: SourceMapperHost ): SourceMapper { - const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); - let sourcemappedFileCache: SourceFileLikeCache; + const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames()); + const currentDirectory = host.getCurrentDirectory(); + const generatedFileCache = createMap(); return { tryGetSourcePosition, tryGetGeneratedPosition, toLineColumnOffset, clearCache }; function toPath(fileName: string) { @@ -25,7 +31,7 @@ namespace ts { } function scanForSourcemapURL(fileName: string) { - const mappedFile = sourcemappedFileCache.get(toPath(fileName)); + const mappedFile = generatedFileCache.get(toPath(fileName)); if (!mappedFile) { return; } @@ -41,26 +47,13 @@ namespace ts { } return file.sourceMapper = createDocumentPositionMapper({ - getSourceFileLike: s => { - const program = getProgram(); - // Lookup file in program, if provided - const file = program && program.getSourceFileByPath(s); - // file returned here could be .d.ts when asked for .ts file if projectReferences and module resolution created this source file - if (file === undefined || file.resolvedPath !== s) { - // Otherwise check the cache (which may hit disk) - return sourcemappedFileCache.get(s); - } - return file; - }, + getSourceFileLike, getCanonicalFileName, - log, + log: s => host.log(s), }, map, mapFileName); } function getSourceMapper(fileName: string, file: SourceFileLike): DocumentPositionMapper { - if (!host.readFile || !host.fileExists) { - return file.sourceMapper = identitySourceMapConsumer; - } if (file.sourceMapper) { return file.sourceMapper; } @@ -101,7 +94,9 @@ namespace ts { } function tryGetGeneratedPosition(info: DocumentPosition): DocumentPosition | undefined { - const program = getProgram(); + const program = host.getProgram(); + if (!program) return undefined; + const options = program.getCompilerOptions(); const outPath = options.outFile || options.out; @@ -118,49 +113,47 @@ namespace ts { } function getSourceFile(fileName: string) { - const program = getProgram(); + const program = host.getProgram(); return program && program.getSourceFileByPath(toPath(fileName)); } - function getGeneratedFile(fileName: string) { - return sourcemappedFileCache.get(toPath(fileName)); // TODO: should ask host instead? + function getGeneratedFile(fileName: string): SourceFileLike | undefined { + const path = toPath(fileName); + const fileFromCache = generatedFileCache.get(path); + if (fileFromCache !== undefined) return fileFromCache ? fileFromCache : undefined; + + // TODO: should ask host instead? + if (!host.fileExists(path)) { + generatedFileCache.set(path, false); + return undefined; + } + + // And failing that, check the disk + const text = host.readFile(path); + const file: SourceFileLike | false = text ? { + text, + lineMap: undefined, + getLineAndCharacterOfPosition(pos: number) { + return computeLineAndCharacterOfPosition(getLineStarts(this as SourceFileLike), pos); + } + } : false; + generatedFileCache.set(path, file); + return file ? file : undefined; + } + + function getSourceFileLike(fileName: string) { + const sourceFile = getSourceFile(fileName); + // file returned here could be .d.ts when asked for .ts file if projectReferences and module resolution created this source file + return sourceFile && sourceFile.resolvedPath === toPath(fileName) ? sourceFile : getGeneratedFile(fileName); } function toLineColumnOffset(fileName: string, position: number): LineAndCharacter { - const sourceFile = getSourceFile(fileName); - const file = sourceFile && sourceFile.resolvedPath === toPath(fileName) ? sourceFile : getGeneratedFile(fileName)!; // TODO: GH#18217 + const file = getSourceFileLike(fileName)!; // TODO: GH#18217 return file.getLineAndCharacterOfPosition(position); } function clearCache(): void { - sourcemappedFileCache = createSourceFileLikeCache(host); + generatedFileCache.clear(); } } - - interface SourceFileLikeCache { - get(path: Path): SourceFileLike | undefined; - } - - function createSourceFileLikeCache(host: { readFile?: (path: string) => string | undefined, fileExists?: (path: string) => boolean }): SourceFileLikeCache { - const cached = createMap(); - return { - get(path: Path) { - if (cached.has(path)) { - return cached.get(path); - } - if (!host.fileExists || !host.readFile || !host.fileExists(path)) return; - // And failing that, check the disk - const text = host.readFile(path)!; // TODO: GH#18217 - const file = { - text, - lineMap: undefined, - getLineAndCharacterOfPosition(pos: number) { - return computeLineAndCharacterOfPosition(getLineStarts(this), pos); - } - } as SourceFileLike; - cached.set(path, file); - return file; - } - }; - } } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 22e6b31b55c..2f78ef2324b 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1282,11 +1282,11 @@ namespace ts { return !!compilerOptions.module || compilerOptions.target! >= ScriptTarget.ES2015 || !!compilerOptions.noEmit; } - export function hostUsesCaseSensitiveFileNames(host: LanguageServiceHost): boolean { + export function hostUsesCaseSensitiveFileNames(host: { useCaseSensitiveFileNames?(): boolean; }): boolean { return host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : false; } - export function hostGetCanonicalFileName(host: LanguageServiceHost): GetCanonicalFileName { + export function hostGetCanonicalFileName(host: { useCaseSensitiveFileNames?(): boolean; }): GetCanonicalFileName { return createGetCanonicalFileName(hostUsesCaseSensitiveFileNames(host)); } From 1db8bb062c1fda1f901352471d90816656abc9ee Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 15 Nov 2018 11:49:54 -0800 Subject: [PATCH 04/23] Use file names instead of paths for reading files --- src/compiler/sourcemap.ts | 9 +++------ src/compiler/types.ts | 2 +- src/services/sourcemaps.ts | 27 ++++++++++++++------------- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/compiler/sourcemap.ts b/src/compiler/sourcemap.ts index 9b74fb22d58..b3b4e1fbb5e 100644 --- a/src/compiler/sourcemap.ts +++ b/src/compiler/sourcemap.ts @@ -592,11 +592,9 @@ namespace ts { const mapDirectory = getDirectoryPath(mapPath); const sourceRoot = map.sourceRoot ? getNormalizedAbsolutePath(map.sourceRoot, mapDirectory) : mapDirectory; const generatedAbsoluteFilePath = getNormalizedAbsolutePath(map.file, mapDirectory); - const generatedCanonicalFilePath = host.getCanonicalFileName(generatedAbsoluteFilePath) as Path; - const generatedFile = host.getSourceFileLike(generatedCanonicalFilePath); + const generatedFile = host.getSourceFileLike(generatedAbsoluteFilePath); const sourceFileAbsolutePaths = map.sources.map(source => getNormalizedAbsolutePath(source, sourceRoot)); - const sourceFileCanonicalPaths = sourceFileAbsolutePaths.map(source => host.getCanonicalFileName(source) as Path); - const sourceToSourceIndexMap = createMapFromEntries(sourceFileCanonicalPaths.map((source, i) => [source, i] as [string, number])); + const sourceToSourceIndexMap = createMapFromEntries(sourceFileAbsolutePaths.map((source, i) => [host.getCanonicalFileName(source), i] as [string, number])); let decodedMappings: ReadonlyArray | undefined; let generatedMappings: SortedReadonlyArray | undefined; let sourceMappings: ReadonlyArray> | undefined; @@ -613,8 +611,7 @@ namespace ts { let source: string | undefined; let sourcePosition: number | undefined; if (isSourceMapping(mapping)) { - const sourceFilePath = sourceFileCanonicalPaths[mapping.sourceIndex]; - const sourceFile = host.getSourceFileLike(sourceFilePath); + const sourceFile = host.getSourceFileLike(sourceFileAbsolutePaths[mapping.sourceIndex]); source = map.sources[mapping.sourceIndex]; sourcePosition = sourceFile !== undefined ? getPositionOfLineAndCharacterWithEdits(sourceFile, mapping.sourceLine, mapping.sourceCharacter) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index f52d82a8a2f..4da96730a93 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5528,7 +5528,7 @@ namespace ts { /* @internal */ export interface DocumentPositionMapperHost { - getSourceFileLike(path: Path): SourceFileLike | undefined; + getSourceFileLike(fileName: string): SourceFileLike | undefined; getCanonicalFileName(path: string): string; log(text: string): void; } diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index c2b9dbfb46a..47f39d85136 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -23,7 +23,7 @@ namespace ts { ): SourceMapper { const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames()); const currentDirectory = host.getCurrentDirectory(); - const generatedFileCache = createMap(); + const sourceFileLike = createMap(); return { tryGetSourcePosition, tryGetGeneratedPosition, toLineColumnOffset, clearCache }; function toPath(fileName: string) { @@ -31,7 +31,7 @@ namespace ts { } function scanForSourcemapURL(fileName: string) { - const mappedFile = generatedFileCache.get(toPath(fileName)); + const mappedFile = sourceFileLike.get(toPath(fileName)); if (!mappedFile) { return; } @@ -39,7 +39,7 @@ namespace ts { return tryGetSourceMappingURL(mappedFile.text, getLineStarts(mappedFile)); } - function convertDocumentToSourceMapper(file: { sourceMapper?: DocumentPositionMapper }, contents: string, mapFileName: string) { + function convertDocumentToSourceMapper(file: SourceFileLike, contents: string, mapFileName: string) { const map = tryParseRawSourceMap(contents); if (!map || !map.sources || !map.file || !map.mappings) { // obviously invalid map @@ -75,9 +75,9 @@ namespace ts { } possibleMapLocations.push(fileName + ".map"); for (const location of possibleMapLocations) { - const mapPath = ts.toPath(location, getDirectoryPath(fileName), getCanonicalFileName); - if (host.fileExists(mapPath)) { - return convertDocumentToSourceMapper(file, host.readFile(mapPath)!, mapPath); // TODO: GH#18217 + const mapFileName = getNormalizedAbsolutePath(location, getDirectoryPath(fileName)); + if (host.fileExists(mapFileName)) { + return convertDocumentToSourceMapper(file, host.readFile(mapFileName)!, mapFileName); // TODO: GH#18217 } } return file.sourceMapper = identitySourceMapConsumer; @@ -105,7 +105,7 @@ namespace ts { getDeclarationEmitOutputFilePathWorker(info.fileName, program.getCompilerOptions(), currentDirectory, program.getCommonSourceDirectory(), getCanonicalFileName); if (declarationPath === undefined) return undefined; - const declarationFile = getGeneratedFile(declarationPath); + const declarationFile = getSourceFileLikeFromCache(declarationPath); if (!declarationFile) return undefined; const newLoc = getSourceMapper(declarationPath, declarationFile).getGeneratedPosition(info); @@ -117,14 +117,14 @@ namespace ts { return program && program.getSourceFileByPath(toPath(fileName)); } - function getGeneratedFile(fileName: string): SourceFileLike | undefined { + function getSourceFileLikeFromCache(fileName: string): SourceFileLike | undefined { const path = toPath(fileName); - const fileFromCache = generatedFileCache.get(path); + const fileFromCache = sourceFileLike.get(path); if (fileFromCache !== undefined) return fileFromCache ? fileFromCache : undefined; // TODO: should ask host instead? if (!host.fileExists(path)) { - generatedFileCache.set(path, false); + sourceFileLike.set(path, false); return undefined; } @@ -137,14 +137,15 @@ namespace ts { return computeLineAndCharacterOfPosition(getLineStarts(this as SourceFileLike), pos); } } : false; - generatedFileCache.set(path, file); + sourceFileLike.set(path, file); return file ? file : undefined; } + // This can be called from source mapper in either source program or program that includes generated file function getSourceFileLike(fileName: string) { const sourceFile = getSourceFile(fileName); // file returned here could be .d.ts when asked for .ts file if projectReferences and module resolution created this source file - return sourceFile && sourceFile.resolvedPath === toPath(fileName) ? sourceFile : getGeneratedFile(fileName); + return sourceFile && sourceFile.resolvedPath === toPath(fileName) ? sourceFile : getSourceFileLikeFromCache(fileName); } function toLineColumnOffset(fileName: string, position: number): LineAndCharacter { @@ -153,7 +154,7 @@ namespace ts { } function clearCache(): void { - generatedFileCache.clear(); + sourceFileLike.clear(); } } } From 0dad79e8b3c077ef8b1870091ac61025b57d51ae Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 15 Nov 2018 12:02:03 -0800 Subject: [PATCH 05/23] Handle source and generated files more gracefully --- src/services/sourcemaps.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 47f39d85136..7b51c9ecff9 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -18,9 +18,7 @@ namespace ts { log(s: string): void; } - export function getSourceMapper( - host: SourceMapperHost - ): SourceMapper { + export function getSourceMapper(host: SourceMapperHost): SourceMapper { const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames()); const currentDirectory = host.getCurrentDirectory(); const sourceFileLike = createMap(); @@ -94,9 +92,12 @@ namespace ts { } function tryGetGeneratedPosition(info: DocumentPosition): DocumentPosition | undefined { - const program = host.getProgram(); - if (!program) return undefined; + if (isDeclarationFileName(info.fileName)) return undefined; + const sourceFile = getSourceFile(info.fileName); + if (!sourceFile) return undefined; + + const program = host.getProgram()!; const options = program.getCompilerOptions(); const outPath = options.outFile || options.out; @@ -114,7 +115,12 @@ namespace ts { function getSourceFile(fileName: string) { const program = host.getProgram(); - return program && program.getSourceFileByPath(toPath(fileName)); + if (!program) return undefined; + + const path = toPath(fileName); + // file returned here could be .d.ts when asked for .ts file if projectReferences and module resolution created this source file + const file = program.getSourceFileByPath(path); + return file && file.resolvedPath === path ? file : undefined; } function getSourceFileLikeFromCache(fileName: string): SourceFileLike | undefined { @@ -143,9 +149,7 @@ namespace ts { // This can be called from source mapper in either source program or program that includes generated file function getSourceFileLike(fileName: string) { - const sourceFile = getSourceFile(fileName); - // file returned here could be .d.ts when asked for .ts file if projectReferences and module resolution created this source file - return sourceFile && sourceFile.resolvedPath === toPath(fileName) ? sourceFile : getSourceFileLikeFromCache(fileName); + return getSourceFile(fileName) || getSourceFileLikeFromCache(fileName); } function toLineColumnOffset(fileName: string, position: number): LineAndCharacter { From 12428d45c02c46b38d76009774700be840ad63dc Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 16 Nov 2018 10:15:51 -0800 Subject: [PATCH 06/23] Add method on host to get DocumentPositionMapper so it can be cached. --- src/compiler/scanner.ts | 8 +- src/compiler/sourcemap.ts | 20 ++- src/compiler/types.ts | 2 + src/harness/sourceMapRecorder.ts | 2 +- src/server/project.ts | 60 +++++++ src/server/scriptInfo.ts | 31 +++- src/server/scriptVersionCache.ts | 14 +- src/server/session.ts | 2 +- src/services/services.ts | 8 +- src/services/sourcemaps.ts | 151 ++++++++++-------- src/services/types.ts | 6 +- src/testRunner/unittests/textStorage.ts | 2 +- .../reference/api/tsserverlibrary.d.ts | 2 +- 13 files changed, 219 insertions(+), 89 deletions(-) diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index a9438110593..e9ccc23d6eb 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -338,12 +338,16 @@ namespace ts { } export function getPositionOfLineAndCharacter(sourceFile: SourceFileLike, line: number, character: number): number { - return computePositionOfLineAndCharacter(getLineStarts(sourceFile), line, character, sourceFile.text); + return sourceFile.getPositionOfLineAndCharacter ? + sourceFile.getPositionOfLineAndCharacter(line, character) : + computePositionOfLineAndCharacter(getLineStarts(sourceFile), line, character, sourceFile.text); } /* @internal */ export function getPositionOfLineAndCharacterWithEdits(sourceFile: SourceFileLike, line: number, character: number): number { - return computePositionOfLineAndCharacter(getLineStarts(sourceFile), line, character, sourceFile.text, /*allowEdits*/ true); + return sourceFile.getPositionOfLineAndCharacter ? + sourceFile.getPositionOfLineAndCharacter(line, character) : + computePositionOfLineAndCharacter(getLineStarts(sourceFile), line, character, sourceFile.text, /*allowEdits*/ true); } /* @internal */ diff --git a/src/compiler/sourcemap.ts b/src/compiler/sourcemap.ts index b3b4e1fbb5e..190cac3b761 100644 --- a/src/compiler/sourcemap.ts +++ b/src/compiler/sourcemap.ts @@ -266,14 +266,24 @@ namespace ts { const sourceMapCommentRegExp = /^\/\/[@#] source[M]appingURL=(.+)\s*$/; const whitespaceOrMapCommentRegExp = /^\s*(\/\/[@#] .*)?$/; + export interface LineInfo { + getLineCount(): number; + getLineText(line: number): string; + } + + export function getLineInfo(text: string, lineStarts: ReadonlyArray): LineInfo { + return { + getLineCount: () => lineStarts.length, + getLineText: line => text.substring(lineStarts[line], lineStarts[line + 1]) + }; + } + /** * Tries to find the sourceMappingURL comment at the end of a file. - * @param text The source text of the file. - * @param lineStarts The line starts of the file. */ - export function tryGetSourceMappingURL(text: string, lineStarts: ReadonlyArray = computeLineStarts(text)) { - for (let index = lineStarts.length - 1; index >= 0; index--) { - const line = text.substring(lineStarts[index], lineStarts[index + 1]); + export function tryGetSourceMappingURL(lineInfo: LineInfo) { + for (let index = lineInfo.getLineCount() - 1; index >= 0; index--) { + const line = lineInfo.getLineText(index); const comment = sourceMapCommentRegExp.exec(line); if (comment) { return comment[1]; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 4da96730a93..e654f4faab7 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2614,6 +2614,8 @@ namespace ts { export interface SourceFileLike { readonly text: string; lineMap?: ReadonlyArray; + /* @internal */ + getPositionOfLineAndCharacter?(line: number, character: number): number; } diff --git a/src/harness/sourceMapRecorder.ts b/src/harness/sourceMapRecorder.ts index 92c3e2d4a94..d16aca21d66 100644 --- a/src/harness/sourceMapRecorder.ts +++ b/src/harness/sourceMapRecorder.ts @@ -69,7 +69,7 @@ namespace Harness.SourceMapRecorder { SourceMapDecoder.initializeSourceMapDecoding(sourceMapData); sourceMapRecorder.WriteLine("==================================================================="); sourceMapRecorder.WriteLine("JsFile: " + sourceMapData.sourceMap.file); - sourceMapRecorder.WriteLine("mapUrl: " + ts.tryGetSourceMappingURL(jsFile.text, jsLineMap)); + sourceMapRecorder.WriteLine("mapUrl: " + ts.tryGetSourceMappingURL(ts.getLineInfo(jsFile.text, jsLineMap))); sourceMapRecorder.WriteLine("sourceRoot: " + sourceMapData.sourceMap.sourceRoot); sourceMapRecorder.WriteLine("sources: " + sourceMapData.sourceMap.sources); if (sourceMapData.sourceMap.sourcesContent) { diff --git a/src/server/project.ts b/src/server/project.ts index fa39aa4abe9..566a2b9abe9 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -503,6 +503,66 @@ namespace ts.server { return this.getLanguageService().getSourceMapper(); } + /*@internal*/ + getDocumentPositionMapper(fileName: string): DocumentPositionMapper | undefined { + const declarationInfo = this.projectService.getOrCreateScriptInfoNotOpenedByClient(fileName, this.currentDirectory, this.directoryStructureHost); + if (!declarationInfo) return undefined; + + declarationInfo.getSnapshot(); // Ensure synchronized + const existingMapper = declarationInfo.textStorage.mapper; + if (existingMapper !== undefined) { + return existingMapper ? existingMapper : undefined; + } + + // Create the mapper + declarationInfo.mapInfo = undefined; + + const mapper = getDocumentPositionMapper({ + getCanonicalFileName: this.projectService.toCanonicalFileName, + log: s => this.log(s), + readMapFile: f => this.readMapFile(f, declarationInfo), + getSourceFileLike: f => this.getSourceFileLike(f) + }, declarationInfo.fileName, declarationInfo.textStorage.getLineInfo()); + declarationInfo.textStorage.mapper = mapper || false; + return mapper; + } + + private readMapFile(fileName: string, declarationInfo: ScriptInfo) { + const mapInfo = this.projectService.getOrCreateScriptInfoNotOpenedByClient(fileName, this.currentDirectory, this.directoryStructureHost); + if (!mapInfo) return undefined; + declarationInfo.mapInfo = mapInfo; + const snap = mapInfo.getSnapshot(); + return snap.getText(0, snap.getLength()); + } + + /*@internal*/ + getSourceFileLike(fileName: string) { + const path = this.toPath(fileName); + const sourceFile = this.getSourceFile(path); + if (sourceFile && sourceFile.resolvedPath === path) return sourceFile; + + // Need to look for other files. + const info = this.projectService.getOrCreateScriptInfoNotOpenedByClient(fileName, this.currentDirectory, this.directoryStructureHost); + if (!info) return undefined; + + // Key doesnt matter since its only for text and lines + if (info.cacheSourceFile) return info.cacheSourceFile.sourceFile; + if (info.textStorage.sourceFileLike) return info.textStorage.sourceFileLike; + + info.textStorage.sourceFileLike = { + get text() { + Debug.fail("shouldnt need text"); + return ""; + }, + getLineAndCharacterOfPosition: pos => { + const lineOffset = info.positionToLineOffset(pos); + return { line: lineOffset.line - 1, character: lineOffset.offset - 1 }; + }, + getPositionOfLineAndCharacter: (line, character) => info.lineOffsetToPosition(line + 1, character + 1) + }; + return info.textStorage.sourceFileLike; + } + private shouldEmitFile(scriptInfo: ScriptInfo) { return scriptInfo && !scriptInfo.isDynamicOrHasMixedContent(); } diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index c090b4b14d0..3502fff1556 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -46,6 +46,9 @@ namespace ts.server { */ private pendingReloadFromDisk = false; + mapper: DocumentPositionMapper | false | undefined = false; + sourceFileLike: SourceFileLike | undefined; + constructor(private readonly host: ServerHost, private readonly fileName: NormalizedPath, initialVersion: ScriptInfoVersion | undefined, private readonly info: ScriptInfo) { this.version = initialVersion || { svc: 0, text: 0 }; } @@ -70,6 +73,8 @@ namespace ts.server { this.text = newText; this.lineMap = undefined; this.fileSize = undefined; + this.mapper = undefined; + this.sourceFileLike = undefined; this.version.text++; } @@ -79,6 +84,8 @@ namespace ts.server { this.text = undefined; this.lineMap = undefined; this.fileSize = undefined; + this.mapper = undefined; + this.sourceFileLike = undefined; } /** @@ -156,8 +163,8 @@ namespace ts.server { : ScriptSnapshot.fromString(this.getOrLoadText()); } - public getLineInfo(line: number): AbsolutePositionAndLineText { - return this.switchToScriptVersionCache().getLineInfo(line); + public getAbsolutePositionAndLineText(line: number): AbsolutePositionAndLineText { + return this.switchToScriptVersionCache().getAbsolutePositionAndLineText(line); } /** * @param line 0 based index @@ -246,6 +253,17 @@ namespace ts.server { Debug.assert(!this.svc, "ScriptVersionCache should not be set"); return this.lineMap || (this.lineMap = computeLineStarts(this.getOrLoadText())); } + + getLineInfo(): LineInfo { + if (this.svc) { + return { + getLineCount: () => this.svc!.getLineCount(), + getLineText: line => this.svc!.getAbsolutePositionAndLineText(line + 1).lineText! + }; + } + const lineMap = this.getLineMap(); + return getLineInfo(this.text!, lineMap); + } } /*@internal*/ @@ -269,7 +287,7 @@ namespace ts.server { /* @internal */ fileWatcher: FileWatcher | undefined; - private textStorage: TextStorage; + /* @internal */ textStorage: TextStorage; /*@internal*/ readonly isDynamic: boolean; @@ -284,6 +302,9 @@ namespace ts.server { /*@internal*/ mTime?: number; + /*@internal*/ + mapInfo?: ScriptInfo; + constructor( private readonly host: ServerHost, readonly fileName: NormalizedPath, @@ -521,8 +542,8 @@ namespace ts.server { } /*@internal*/ - getLineInfo(line: number): AbsolutePositionAndLineText { - return this.textStorage.getLineInfo(line); + getAbsolutePositionAndLineText(line: number): AbsolutePositionAndLineText { + return this.textStorage.getAbsolutePositionAndLineText(line); } editContent(start: number, end: number, newText: string): void { diff --git a/src/server/scriptVersionCache.ts b/src/server/scriptVersionCache.ts index a350ed99f71..afd8f67ecbe 100644 --- a/src/server/scriptVersionCache.ts +++ b/src/server/scriptVersionCache.ts @@ -308,8 +308,8 @@ namespace ts.server { return this._getSnapshot().version; } - getLineInfo(line: number): AbsolutePositionAndLineText { - return this._getSnapshot().index.lineNumberToInfo(line); + getAbsolutePositionAndLineText(oneBasedLine: number): AbsolutePositionAndLineText { + return this._getSnapshot().index.lineNumberToInfo(oneBasedLine); } lineOffsetToPosition(line: number, column: number): number { @@ -348,6 +348,10 @@ namespace ts.server { } } + getLineCount() { + return this._getSnapshot().index.getLineCount(); + } + static fromString(script: string) { const svc = new ScriptVersionCache(); const snap = new LineIndexSnapshot(0, svc, new LineIndex()); @@ -400,8 +404,12 @@ namespace ts.server { return this.root.charOffsetToLineInfo(1, position); } + getLineCount() { + return this.root.lineCount(); + } + lineNumberToInfo(oneBasedLine: number): AbsolutePositionAndLineText { - const lineCount = this.root.lineCount(); + const lineCount = this.getLineCount(); if (oneBasedLine <= lineCount) { const { position, leaf } = this.root.lineNumberToInfo(oneBasedLine, 0); return { absolutePosition: position, lineText: leaf && leaf.text }; diff --git a/src/server/session.ts b/src/server/session.ts index 0b9ad913f8a..d534f445ee3 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1474,7 +1474,7 @@ namespace ts.server { // only to the previous line. If all this is true, then // add edits necessary to properly indent the current line. if ((args.key === "\n") && ((!edits) || (edits.length === 0) || allEditsBeforePos(edits, position))) { - const { lineText, absolutePosition } = scriptInfo.getLineInfo(args.line); + const { lineText, absolutePosition } = scriptInfo.getAbsolutePositionAndLineText(args.line); if (lineText && lineText.search("\\S") < 0) { const preferredIndent = languageService.getIndentationAtPosition(file, position, formatOptions); let hasIndent = 0; diff --git a/src/services/services.ts b/src/services/services.ts index 67f9e537574..a0c41ffe94d 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -603,7 +603,7 @@ namespace ts { } public getPositionOfLineAndCharacter(line: number, character: number): number { - return getPositionOfLineAndCharacter(this, line, character); + return computePositionOfLineAndCharacter(getLineStarts(this), line, character, this.text); } public getLineEndOfPosition(pos: number): number { @@ -1143,8 +1143,10 @@ namespace ts { useCaseSensitiveFileNames: () => useCaseSensitiveFileNames, getCurrentDirectory: () => currentDirectory, getProgram, - fileExists: host.fileExists ? f => host.fileExists!(f) : returnFalse, - readFile: host.readFile ? (f, encoding) => host.readFile!(f, encoding) : () => undefined, + fileExists: host.fileExists && (f => host.fileExists!(f)), + readFile: host.readFile && ((f, encoding) => host.readFile!(f, encoding)), + getDocumentPositionMapper: host.getDocumentPositionMapper && (f => host.getDocumentPositionMapper!(f)), + getSourceFileLike: host.getSourceFileLike && (f => host.getSourceFileLike!(f)), log }); diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 7b51c9ecff9..c679688e6c0 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -13,8 +13,10 @@ namespace ts { useCaseSensitiveFileNames(): boolean; getCurrentDirectory(): string; getProgram(): Program | undefined; - fileExists(path: string): boolean; - readFile(path: string, encoding?: string): string | undefined; + fileExists?(path: string): boolean; + readFile?(path: string, encoding?: string): string | undefined; + getSourceFileLike?(fileName: string): SourceFileLike | undefined; + getDocumentPositionMapper?(fileName: string): DocumentPositionMapper | undefined; log(s: string): void; } @@ -22,63 +24,33 @@ namespace ts { const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames()); const currentDirectory = host.getCurrentDirectory(); const sourceFileLike = createMap(); + const documentPositionMappers = createMap(); return { tryGetSourcePosition, tryGetGeneratedPosition, toLineColumnOffset, clearCache }; function toPath(fileName: string) { return ts.toPath(fileName, currentDirectory, getCanonicalFileName); } - function scanForSourcemapURL(fileName: string) { - const mappedFile = sourceFileLike.get(toPath(fileName)); - if (!mappedFile) { - return; - } + function getDocumentPositionMapper(fileName: string) { + const path = toPath(fileName); + const value = documentPositionMappers.get(path); + if (value) return value; - return tryGetSourceMappingURL(mappedFile.text, getLineStarts(mappedFile)); - } - - function convertDocumentToSourceMapper(file: SourceFileLike, contents: string, mapFileName: string) { - const map = tryParseRawSourceMap(contents); - if (!map || !map.sources || !map.file || !map.mappings) { - // obviously invalid map - return file.sourceMapper = identitySourceMapConsumer; + let mapper: DocumentPositionMapper | undefined; + if (host.getDocumentPositionMapper) { + mapper = host.getDocumentPositionMapper(fileName); } - - return file.sourceMapper = createDocumentPositionMapper({ - getSourceFileLike, - getCanonicalFileName, - log: s => host.log(s), - }, map, mapFileName); - } - - function getSourceMapper(fileName: string, file: SourceFileLike): DocumentPositionMapper { - if (file.sourceMapper) { - return file.sourceMapper; + else if (host.readFile) { + const file = getSourceFileLike(fileName); + mapper = file && ts.getDocumentPositionMapper({ + getSourceFileLike, + getCanonicalFileName, + log: s => host.log(s), + readMapFile: f => !host.fileExists || host.fileExists(f) ? host.readFile!(f) : undefined + }, fileName, getLineInfo(file.text, getLineStarts(file))); } - let mapFileName = scanForSourcemapURL(fileName); - if (mapFileName) { - const match = base64UrlRegExp.exec(mapFileName); - if (match) { - if (match[1]) { - const base64Object = match[1]; - return convertDocumentToSourceMapper(file, base64decode(sys, base64Object), fileName); - } - // Not a data URL we can parse, skip it - mapFileName = undefined; - } - } - const possibleMapLocations: string[] = []; - if (mapFileName) { - possibleMapLocations.push(mapFileName); - } - possibleMapLocations.push(fileName + ".map"); - for (const location of possibleMapLocations) { - const mapFileName = getNormalizedAbsolutePath(location, getDirectoryPath(fileName)); - if (host.fileExists(mapFileName)) { - return convertDocumentToSourceMapper(file, host.readFile(mapFileName)!, mapFileName); // TODO: GH#18217 - } - } - return file.sourceMapper = identitySourceMapConsumer; + documentPositionMappers.set(path, mapper || identitySourceMapConsumer); + return mapper || identitySourceMapConsumer; } function tryGetSourcePosition(info: DocumentPosition): DocumentPosition | undefined { @@ -87,8 +59,8 @@ namespace ts { const file = getSourceFile(info.fileName); if (!file) return undefined; - const newLoc = getSourceMapper(info.fileName, file).getSourcePosition(info); - return newLoc === info ? undefined : tryGetSourcePosition(newLoc) || newLoc; + const newLoc = getDocumentPositionMapper(info.fileName).getSourcePosition(info); + return !newLoc || newLoc === info ? undefined : tryGetSourcePosition(newLoc) || newLoc; } function tryGetGeneratedPosition(info: DocumentPosition): DocumentPosition | undefined { @@ -106,10 +78,7 @@ namespace ts { getDeclarationEmitOutputFilePathWorker(info.fileName, program.getCompilerOptions(), currentDirectory, program.getCommonSourceDirectory(), getCanonicalFileName); if (declarationPath === undefined) return undefined; - const declarationFile = getSourceFileLikeFromCache(declarationPath); - if (!declarationFile) return undefined; - - const newLoc = getSourceMapper(declarationPath, declarationFile).getGeneratedPosition(info); + const newLoc = getDocumentPositionMapper(declarationPath).getGeneratedPosition(info); return newLoc === info ? undefined : newLoc; } @@ -123,42 +92,92 @@ namespace ts { return file && file.resolvedPath === path ? file : undefined; } - function getSourceFileLikeFromCache(fileName: string): SourceFileLike | undefined { + function getOrCreateSourceFileLike(fileName: string): SourceFileLike | undefined { const path = toPath(fileName); const fileFromCache = sourceFileLike.get(path); if (fileFromCache !== undefined) return fileFromCache ? fileFromCache : undefined; // TODO: should ask host instead? - if (!host.fileExists(path)) { + if (!host.readFile || host.fileExists && !host.fileExists(path)) { sourceFileLike.set(path, false); return undefined; } // And failing that, check the disk const text = host.readFile(path); - const file: SourceFileLike | false = text ? { - text, - lineMap: undefined, - getLineAndCharacterOfPosition(pos: number) { - return computeLineAndCharacterOfPosition(getLineStarts(this as SourceFileLike), pos); - } - } : false; + const file = text ? createSourceFileLike(text) : false; sourceFileLike.set(path, file); return file ? file : undefined; } // This can be called from source mapper in either source program or program that includes generated file function getSourceFileLike(fileName: string) { - return getSourceFile(fileName) || getSourceFileLikeFromCache(fileName); + return !host.getSourceFileLike ? + getSourceFile(fileName) || getOrCreateSourceFileLike(fileName) : + host.getSourceFileLike(fileName); } function toLineColumnOffset(fileName: string, position: number): LineAndCharacter { + // TODO:: shkamat const file = getSourceFileLike(fileName)!; // TODO: GH#18217 return file.getLineAndCharacterOfPosition(position); } function clearCache(): void { sourceFileLike.clear(); + documentPositionMappers.clear(); } } + + export interface GetDocumentPositionMapperHost extends DocumentPositionMapperHost { + readMapFile(fileName: string): string | undefined; + } + + export function getDocumentPositionMapper(host: GetDocumentPositionMapperHost, generatedFileName: string, generatedFileLineInfo: LineInfo) { + let mapFileName = tryGetSourceMappingURL(generatedFileLineInfo); + if (mapFileName) { + const match = base64UrlRegExp.exec(mapFileName); + if (match) { + if (match[1]) { + const base64Object = match[1]; + return convertDocumentToSourceMapper(host, base64decode(sys, base64Object), generatedFileName); + } + // Not a data URL we can parse, skip it + mapFileName = undefined; + } + } + const possibleMapLocations: string[] = []; + if (mapFileName) { + possibleMapLocations.push(mapFileName); + } + possibleMapLocations.push(generatedFileName + ".map"); + for (const location of possibleMapLocations) { + const mapFileName = getNormalizedAbsolutePath(location, getDirectoryPath(generatedFileName)); + const mapFileContents = host.readMapFile(mapFileName); + if (mapFileContents) { + return convertDocumentToSourceMapper(host, mapFileContents, mapFileName); + } + } + return undefined; + } + + function convertDocumentToSourceMapper(host: DocumentPositionMapperHost, contents: string, mapFileName: string) { + const map = tryParseRawSourceMap(contents); + if (!map || !map.sources || !map.file || !map.mappings) { + // obviously invalid map + return undefined; + } + + return createDocumentPositionMapper(host, map, mapFileName); + } + + function createSourceFileLike(text: string, lineMap?: SourceFileLike["lineMap"]): SourceFileLike { + return { + text, + lineMap, + getLineAndCharacterOfPosition(pos: number) { + return computeLineAndCharacterOfPosition(getLineStarts(this), pos); + } + }; + } } diff --git a/src/services/types.ts b/src/services/types.ts index 69f07b705a7..d3a8f8cdd52 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -91,7 +91,6 @@ namespace ts { export interface SourceFileLike { getLineAndCharacterOfPosition(pos: number): LineAndCharacter; - /*@internal*/ sourceMapper?: DocumentPositionMapper; } export interface SourceMapSource { @@ -233,6 +232,11 @@ namespace ts { installPackage?(options: InstallPackageOptions): Promise; /* @internal */ inspectValue?(options: InspectValueOptions): Promise; writeFile?(fileName: string, content: string): void; + + /* @internal */ + getDocumentPositionMapper?(fileName: string): DocumentPositionMapper | undefined; + /* @internal */ + getSourceFileLike?(fileName: string): SourceFileLike | undefined; } /* @internal */ diff --git a/src/testRunner/unittests/textStorage.ts b/src/testRunner/unittests/textStorage.ts index 5dce083120b..f5fbb512e52 100644 --- a/src/testRunner/unittests/textStorage.ts +++ b/src/testRunner/unittests/textStorage.ts @@ -60,7 +60,7 @@ namespace ts.textStorage { ts1.useText(); assert.isFalse(ts1.hasScriptVersionCache_TestOnly(), "should not have script version cache - 2"); - ts1.getLineInfo(0); + ts1.getAbsolutePositionAndLineText(0); assert.isTrue(ts1.hasScriptVersionCache_TestOnly(), "have script version cache - 2"); }); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 2b78178587e..b788d46ac9f 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8046,7 +8046,6 @@ declare namespace ts.server { readonly containingProjects: Project[]; private formatSettings; private preferences; - private textStorage; constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path, initialVersion?: ScriptInfoVersion); isScriptOpen(): boolean; open(newText: string): void; @@ -8211,6 +8210,7 @@ declare namespace ts.server { getGlobalProjectErrors(): ReadonlyArray; getAllProjectErrors(): ReadonlyArray; getLanguageService(ensureSynchronized?: boolean): LanguageService; + private readMapFile; private shouldEmitFile; getCompileOnSaveAffectedFileList(scriptInfo: ScriptInfo): string[]; /** From afdf1e90ecd9c438cc860860fcb676af8c78d8d5 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 16 Nov 2018 11:24:01 -0800 Subject: [PATCH 07/23] Dont depend on project in document position mapper so that we can unload or remove projects independently --- src/server/editorServices.ts | 64 +++++++++++++++++++ src/server/project.ts | 54 +--------------- src/services/sourcemaps.ts | 25 ++++---- .../reference/api/tsserverlibrary.d.ts | 1 - 4 files changed, 78 insertions(+), 66 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 90b516dadfc..dd05b562cff 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2195,6 +2195,70 @@ namespace ts.server { return this.filenameToScriptInfo.get(fileName); } + /*@internal*/ + getDocumentPositionMapper(fileName: string, project: Project): DocumentPositionMapper | undefined { + const declarationInfo = this.getOrCreateScriptInfoNotOpenedByClient(fileName, project.currentDirectory, project.directoryStructureHost); + if (!declarationInfo) return undefined; + + declarationInfo.getSnapshot(); // Ensure synchronized + const existingMapper = declarationInfo.textStorage.mapper; + if (existingMapper !== undefined) { + return existingMapper ? existingMapper : undefined; + } + + // Create the mapper + declarationInfo.mapInfo = undefined; + + let readMapFile: ((fileName: string) => string | undefined) | undefined = fileName => { + const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(fileName, project.currentDirectory, project.directoryStructureHost); + if (!mapInfo) return undefined; + declarationInfo.mapInfo = mapInfo; + const snap = mapInfo.getSnapshot(); + return snap.getText(0, snap.getLength()); + }; + const projectName = project.projectName; + const mapper = getDocumentPositionMapper( + { getCanonicalFileName: this.toCanonicalFileName, log: s => this.logger.info(s), getSourceFileLike: f => this.getSourceFileLike(f, projectName) }, + declarationInfo.fileName, + declarationInfo.textStorage.getLineInfo(), + readMapFile + ); + readMapFile = undefined; // Remove ref to project + declarationInfo.textStorage.mapper = mapper || false; + return mapper; + } + + /*@internal*/ + getSourceFileLike(fileName: string, projectName: string | Project) { + const project = (projectName as Project).projectName ? projectName as Project : this.findProject(projectName as string); + if (project) { + const path = project.toPath(fileName); + const sourceFile = project.getSourceFile(path); + if (sourceFile && sourceFile.resolvedPath === path) return sourceFile; + } + + // Need to look for other files. + const info = this.getOrCreateScriptInfoNotOpenedByClient(fileName, (project || this).currentDirectory, project ? project.directoryStructureHost : this.host); + if (!info) return undefined; + + // Key doesnt matter since its only for text and lines + if (info.cacheSourceFile) return info.cacheSourceFile.sourceFile; + if (info.textStorage.sourceFileLike) return info.textStorage.sourceFileLike; + + info.textStorage.sourceFileLike = { + get text() { + Debug.fail("shouldnt need text"); + return ""; + }, + getLineAndCharacterOfPosition: pos => { + const lineOffset = info.positionToLineOffset(pos); + return { line: lineOffset.line - 1, character: lineOffset.offset - 1 }; + }, + getPositionOfLineAndCharacter: (line, character) => info.lineOffsetToPosition(line + 1, character + 1) + }; + return info.textStorage.sourceFileLike; + } + setHostConfiguration(args: protocol.ConfigureRequestArguments) { if (args.file) { const info = this.getScriptInfoForNormalizedPath(toNormalizedPath(args.file)); diff --git a/src/server/project.ts b/src/server/project.ts index 566a2b9abe9..6ea9589cb4b 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -505,62 +505,12 @@ namespace ts.server { /*@internal*/ getDocumentPositionMapper(fileName: string): DocumentPositionMapper | undefined { - const declarationInfo = this.projectService.getOrCreateScriptInfoNotOpenedByClient(fileName, this.currentDirectory, this.directoryStructureHost); - if (!declarationInfo) return undefined; - - declarationInfo.getSnapshot(); // Ensure synchronized - const existingMapper = declarationInfo.textStorage.mapper; - if (existingMapper !== undefined) { - return existingMapper ? existingMapper : undefined; - } - - // Create the mapper - declarationInfo.mapInfo = undefined; - - const mapper = getDocumentPositionMapper({ - getCanonicalFileName: this.projectService.toCanonicalFileName, - log: s => this.log(s), - readMapFile: f => this.readMapFile(f, declarationInfo), - getSourceFileLike: f => this.getSourceFileLike(f) - }, declarationInfo.fileName, declarationInfo.textStorage.getLineInfo()); - declarationInfo.textStorage.mapper = mapper || false; - return mapper; - } - - private readMapFile(fileName: string, declarationInfo: ScriptInfo) { - const mapInfo = this.projectService.getOrCreateScriptInfoNotOpenedByClient(fileName, this.currentDirectory, this.directoryStructureHost); - if (!mapInfo) return undefined; - declarationInfo.mapInfo = mapInfo; - const snap = mapInfo.getSnapshot(); - return snap.getText(0, snap.getLength()); + return this.projectService.getDocumentPositionMapper(fileName, this); } /*@internal*/ getSourceFileLike(fileName: string) { - const path = this.toPath(fileName); - const sourceFile = this.getSourceFile(path); - if (sourceFile && sourceFile.resolvedPath === path) return sourceFile; - - // Need to look for other files. - const info = this.projectService.getOrCreateScriptInfoNotOpenedByClient(fileName, this.currentDirectory, this.directoryStructureHost); - if (!info) return undefined; - - // Key doesnt matter since its only for text and lines - if (info.cacheSourceFile) return info.cacheSourceFile.sourceFile; - if (info.textStorage.sourceFileLike) return info.textStorage.sourceFileLike; - - info.textStorage.sourceFileLike = { - get text() { - Debug.fail("shouldnt need text"); - return ""; - }, - getLineAndCharacterOfPosition: pos => { - const lineOffset = info.positionToLineOffset(pos); - return { line: lineOffset.line - 1, character: lineOffset.offset - 1 }; - }, - getPositionOfLineAndCharacter: (line, character) => info.lineOffsetToPosition(line + 1, character + 1) - }; - return info.textStorage.sourceFileLike; + return this.projectService.getSourceFileLike(fileName, this); } private shouldEmitFile(scriptInfo: ScriptInfo) { diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index c679688e6c0..00d2696abf5 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -42,12 +42,12 @@ namespace ts { } else if (host.readFile) { const file = getSourceFileLike(fileName); - mapper = file && ts.getDocumentPositionMapper({ - getSourceFileLike, - getCanonicalFileName, - log: s => host.log(s), - readMapFile: f => !host.fileExists || host.fileExists(f) ? host.readFile!(f) : undefined - }, fileName, getLineInfo(file.text, getLineStarts(file))); + mapper = file && ts.getDocumentPositionMapper( + { getSourceFileLike, getCanonicalFileName, log: s => host.log(s) }, + fileName, + getLineInfo(file.text, getLineStarts(file)), + f => !host.fileExists || host.fileExists(f) ? host.readFile!(f) : undefined + ); } documentPositionMappers.set(path, mapper || identitySourceMapConsumer); return mapper || identitySourceMapConsumer; @@ -118,7 +118,6 @@ namespace ts { } function toLineColumnOffset(fileName: string, position: number): LineAndCharacter { - // TODO:: shkamat const file = getSourceFileLike(fileName)!; // TODO: GH#18217 return file.getLineAndCharacterOfPosition(position); } @@ -129,11 +128,11 @@ namespace ts { } } - export interface GetDocumentPositionMapperHost extends DocumentPositionMapperHost { - readMapFile(fileName: string): string | undefined; - } - - export function getDocumentPositionMapper(host: GetDocumentPositionMapperHost, generatedFileName: string, generatedFileLineInfo: LineInfo) { + export function getDocumentPositionMapper( + host: DocumentPositionMapperHost, + generatedFileName: string, + generatedFileLineInfo: LineInfo, + readMapFile: (fileName: string) => string | undefined) { let mapFileName = tryGetSourceMappingURL(generatedFileLineInfo); if (mapFileName) { const match = base64UrlRegExp.exec(mapFileName); @@ -153,7 +152,7 @@ namespace ts { possibleMapLocations.push(generatedFileName + ".map"); for (const location of possibleMapLocations) { const mapFileName = getNormalizedAbsolutePath(location, getDirectoryPath(generatedFileName)); - const mapFileContents = host.readMapFile(mapFileName); + const mapFileContents = readMapFile(mapFileName); if (mapFileContents) { return convertDocumentToSourceMapper(host, mapFileContents, mapFileName); } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index b788d46ac9f..1a104716c28 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8210,7 +8210,6 @@ declare namespace ts.server { getGlobalProjectErrors(): ReadonlyArray; getAllProjectErrors(): ReadonlyArray; getLanguageService(ensureSynchronized?: boolean): LanguageService; - private readMapFile; private shouldEmitFile; getCompileOnSaveAffectedFileList(scriptInfo: ScriptInfo): string[]; /** From 5c920f336812ef910b75a9ad3a325d713a7b9ef9 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 16 Nov 2018 12:31:23 -0800 Subject: [PATCH 08/23] Dont unnecessarily make textStorage internal --- src/server/editorServices.ts | 34 ++++++++++--------- src/server/scriptInfo.ts | 22 +++++++----- src/testRunner/unittests/textStorage.ts | 14 ++++---- .../reference/api/tsserverlibrary.d.ts | 1 + 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index dd05b562cff..49f94f3fc95 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2201,7 +2201,7 @@ namespace ts.server { if (!declarationInfo) return undefined; declarationInfo.getSnapshot(); // Ensure synchronized - const existingMapper = declarationInfo.textStorage.mapper; + const existingMapper = declarationInfo.mapper; if (existingMapper !== undefined) { return existingMapper ? existingMapper : undefined; } @@ -2220,11 +2220,11 @@ namespace ts.server { const mapper = getDocumentPositionMapper( { getCanonicalFileName: this.toCanonicalFileName, log: s => this.logger.info(s), getSourceFileLike: f => this.getSourceFileLike(f, projectName) }, declarationInfo.fileName, - declarationInfo.textStorage.getLineInfo(), + declarationInfo.getLineInfo(), readMapFile ); readMapFile = undefined; // Remove ref to project - declarationInfo.textStorage.mapper = mapper || false; + declarationInfo.mapper = mapper || false; return mapper; } @@ -2243,20 +2243,22 @@ namespace ts.server { // Key doesnt matter since its only for text and lines if (info.cacheSourceFile) return info.cacheSourceFile.sourceFile; - if (info.textStorage.sourceFileLike) return info.textStorage.sourceFileLike; - info.textStorage.sourceFileLike = { - get text() { - Debug.fail("shouldnt need text"); - return ""; - }, - getLineAndCharacterOfPosition: pos => { - const lineOffset = info.positionToLineOffset(pos); - return { line: lineOffset.line - 1, character: lineOffset.offset - 1 }; - }, - getPositionOfLineAndCharacter: (line, character) => info.lineOffsetToPosition(line + 1, character + 1) - }; - return info.textStorage.sourceFileLike; + // Create sourceFileLike + if (!info.sourceFileLike) { + info.sourceFileLike = { + get text() { + Debug.fail("shouldnt need text"); + return ""; + }, + getLineAndCharacterOfPosition: pos => { + const lineOffset = info.positionToLineOffset(pos); + return { line: lineOffset.line - 1, character: lineOffset.offset - 1 }; + }, + getPositionOfLineAndCharacter: (line, character) => info.lineOffsetToPosition(line + 1, character + 1) + }; + } + return info.sourceFileLike; } setHostConfiguration(args: protocol.ConfigureRequestArguments) { diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 3502fff1556..71b8fe2f2b5 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -46,9 +46,6 @@ namespace ts.server { */ private pendingReloadFromDisk = false; - mapper: DocumentPositionMapper | false | undefined = false; - sourceFileLike: SourceFileLike | undefined; - constructor(private readonly host: ServerHost, private readonly fileName: NormalizedPath, initialVersion: ScriptInfoVersion | undefined, private readonly info: ScriptInfo) { this.version = initialVersion || { svc: 0, text: 0 }; } @@ -73,8 +70,8 @@ namespace ts.server { this.text = newText; this.lineMap = undefined; this.fileSize = undefined; - this.mapper = undefined; - this.sourceFileLike = undefined; + this.info.mapper = undefined; + this.info.sourceFileLike = undefined; this.version.text++; } @@ -84,8 +81,8 @@ namespace ts.server { this.text = undefined; this.lineMap = undefined; this.fileSize = undefined; - this.mapper = undefined; - this.sourceFileLike = undefined; + this.info.mapper = undefined; + this.info.sourceFileLike = undefined; } /** @@ -287,7 +284,7 @@ namespace ts.server { /* @internal */ fileWatcher: FileWatcher | undefined; - /* @internal */ textStorage: TextStorage; + private textStorage: TextStorage; /*@internal*/ readonly isDynamic: boolean; @@ -304,6 +301,10 @@ namespace ts.server { /*@internal*/ mapInfo?: ScriptInfo; + /*@internal*/ + mapper: DocumentPositionMapper | false | undefined = false; + /*@internal*/ + sourceFileLike: SourceFileLike | undefined; constructor( private readonly host: ServerHost, @@ -583,5 +584,10 @@ namespace ts.server { public isJavaScript() { return this.scriptKind === ScriptKind.JS || this.scriptKind === ScriptKind.JSX; } + + /*@internal*/ + getLineInfo(): LineInfo { + return this.textStorage.getLineInfo(); + } } } diff --git a/src/testRunner/unittests/textStorage.ts b/src/testRunner/unittests/textStorage.ts index f5fbb512e52..724dc8e4ed3 100644 --- a/src/testRunner/unittests/textStorage.ts +++ b/src/testRunner/unittests/textStorage.ts @@ -14,8 +14,8 @@ namespace ts.textStorage { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!); - const ts2 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); + const ts2 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); ts1.useScriptVersionCache_TestOnly(); ts2.useText(); @@ -49,7 +49,7 @@ namespace ts.textStorage { it("should switch to script version cache if necessary", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); ts1.getSnapshot(); assert.isFalse(ts1.hasScriptVersionCache_TestOnly(), "should not have script version cache - 1"); @@ -67,7 +67,7 @@ namespace ts.textStorage { it("should be able to return the file size immediately after construction", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); assert.strictEqual(f.content.length, ts1.getTelemetryFileSize()); }); @@ -75,7 +75,7 @@ namespace ts.textStorage { it("should be able to return the file size when backed by text", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); ts1.useText(f.content); assert.isFalse(ts1.hasScriptVersionCache_TestOnly()); @@ -86,7 +86,7 @@ namespace ts.textStorage { it("should be able to return the file size when backed by a script version cache", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, /*info*/undefined!); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); ts1.useScriptVersionCache_TestOnly(); assert.isTrue(ts1.hasScriptVersionCache_TestOnly()); @@ -126,7 +126,7 @@ namespace ts.textStorage { const host = projectSystem.createServerHost([changingFile]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(changingFile.path), /*initialVersion*/ undefined, /*info*/undefined!); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(changingFile.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); assert.isTrue(ts1.reloadFromDisk()); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 1a104716c28..2b78178587e 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8046,6 +8046,7 @@ declare namespace ts.server { readonly containingProjects: Project[]; private formatSettings; private preferences; + private textStorage; constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path, initialVersion?: ScriptInfoVersion); isScriptOpen(): boolean; open(newText: string): void; From 0aa4da43adc0f73012a59942d03f2df04954d6be Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 16 Nov 2018 11:52:29 -0800 Subject: [PATCH 09/23] Life time of declaration, sources and map infos Map Info and sources is not ideal and need to revisited since we need to update mapper and projects correctly // TODO: lifetime of source project and declaration map --- src/server/editorServices.ts | 44 ++++++--- src/server/scriptInfo.ts | 2 + .../unittests/tsserverProjectSystem.ts | 95 +++++++++++++++---- .../reference/api/tsserverlibrary.d.ts | 4 - 4 files changed, 112 insertions(+), 33 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 49f94f3fc95..d98aa948113 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -437,7 +437,8 @@ namespace ts.server { /** * Container of all known scripts */ - private readonly filenameToScriptInfo = createMap(); + /*@internal*/ + readonly filenameToScriptInfo = createMap(); private readonly scriptInfoInNodeModulesWatchers = createMap (); /** * Contains all the deleted script info's version information so that @@ -2209,6 +2210,7 @@ namespace ts.server { // Create the mapper declarationInfo.mapInfo = undefined; + // TODO: shkamat Lifetime of declarationInfo and mapInfo let readMapFile: ((fileName: string) => string | undefined) | undefined = fileName => { const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(fileName, project.currentDirectory, project.directoryStructureHost); if (!mapInfo) return undefined; @@ -2218,7 +2220,7 @@ namespace ts.server { }; const projectName = project.projectName; const mapper = getDocumentPositionMapper( - { getCanonicalFileName: this.toCanonicalFileName, log: s => this.logger.info(s), getSourceFileLike: f => this.getSourceFileLike(f, projectName) }, + { getCanonicalFileName: this.toCanonicalFileName, log: s => this.logger.info(s), getSourceFileLike: f => this.getSourceFileLike(f, projectName, declarationInfo) }, declarationInfo.fileName, declarationInfo.getLineInfo(), readMapFile @@ -2229,8 +2231,8 @@ namespace ts.server { } /*@internal*/ - getSourceFileLike(fileName: string, projectName: string | Project) { - const project = (projectName as Project).projectName ? projectName as Project : this.findProject(projectName as string); + getSourceFileLike(fileName: string, projectNameOrProject: string | Project, declarationInfo?: ScriptInfo) { + const project = (projectNameOrProject as Project).projectName ? projectNameOrProject as Project : this.findProject(projectNameOrProject as string); if (project) { const path = project.toPath(fileName); const sourceFile = project.getSourceFile(path); @@ -2241,6 +2243,11 @@ namespace ts.server { const info = this.getOrCreateScriptInfoNotOpenedByClient(fileName, (project || this).currentDirectory, project ? project.directoryStructureHost : this.host); if (!info) return undefined; + // Attach as source + if (declarationInfo && declarationInfo.mapInfo && info !== declarationInfo) { + (declarationInfo.mapInfo.sourceInfos || (declarationInfo.mapInfo.sourceInfos = createMap())).set(info.path, true); + } + // Key doesnt matter since its only for text and lines if (info.cacheSourceFile) return info.cacheSourceFile.sourceFile; @@ -2556,13 +2563,7 @@ namespace ts.server { // when some file/s were closed which resulted in project removal. // It was then postponed to cleanup these script infos so that they can be reused if // the file from that old project is reopened because of opening file from here. - this.filenameToScriptInfo.forEach(info => { - if (!info.isScriptOpen() && info.isOrphan()) { - // if there are not projects that include this script info - delete it - this.stopWatchingScriptInfo(info); - this.deleteScriptInfo(info); - } - }); + this.removeOrphanScriptInfos(); this.printProjects(); @@ -2605,6 +2606,27 @@ namespace ts.server { } } + private removeOrphanScriptInfos() { + const toRemoveScriptInfos = cloneMap(this.filenameToScriptInfo); + this.filenameToScriptInfo.forEach(info => { + if (info.isScriptOpen() || !info.isOrphan()) { + toRemoveScriptInfos.delete(info.path); + if (info.mapInfo) { + toRemoveScriptInfos.delete(info.mapInfo.path); + if (info.mapInfo.sourceInfos) { + info.mapInfo.sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); + } + } + } + }); + + toRemoveScriptInfos.forEach(info => { + // if there are not projects that include this script info - delete it + this.stopWatchingScriptInfo(info); + this.deleteScriptInfo(info); + }); + } + private telemetryOnOpenFile(scriptInfo: ScriptInfo): void { if (this.syntaxOnly || !this.eventHandler || !scriptInfo.isJavaScript() || !addToSeen(this.allJsFilesForOpenFileTelemetry, scriptInfo.path)) { return; diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 71b8fe2f2b5..3d8c737064f 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -302,6 +302,8 @@ namespace ts.server { /*@internal*/ mapInfo?: ScriptInfo; /*@internal*/ + sourceInfos?: Map; + /*@internal*/ mapper: DocumentPositionMapper | false | undefined = false; /*@internal*/ sourceFileLike: SourceFileLike | undefined; diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 6681b4bfa54..217149efd88 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -475,6 +475,10 @@ namespace ts.projectSystem { checkArray("Open files", arrayFrom(projectService.openFiles.keys(), path => projectService.getScriptInfoForPath(path as Path)!.fileName), expectedFiles.map(file => file.path)); } + function checkScriptInfos(projectService: server.ProjectService, expectedFiles: ReadonlyArray) { + checkArray("ScriptInfos files", arrayFrom(projectService.filenameToScriptInfo.values(), info => info.fileName), expectedFiles); + } + function protocolLocationFromSubstring(str: string, substring: string): protocol.Location { const start = str.indexOf(substring); Debug.assert(start !== -1); @@ -10667,7 +10671,7 @@ declare class TestLib { }); }); - it("can go to definition correctly", () => { + describe("with main and depedency project", () => { const projectLocation = "/user/username/projects/myproject"; const dependecyLocation = `${projectLocation}/dependency`; const mainLocation = `${projectLocation}/main`; @@ -10704,24 +10708,79 @@ fn5();` }) }; - const files = [dependencyTs, dependencyConfig, mainTs, mainConfig, libFile]; - const host = createHost(files, [mainConfig.path]); - const session = createSession(host); - const service = session.getProjectService(); - openFilesForSession([mainTs], session); - checkNumberOfProjects(service, { configuredProjects: 1 }); - checkProjectActualFiles(service.configuredProjects.get(mainConfig.path)!, [mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`]); - for (let i = 0; i < 5; i++) { - const startSpan = { line: i + 5, offset: 1 }; - const response = session.executeCommandSeq({ - command: protocol.CommandTypes.DefinitionAndBoundSpan, - arguments: { file: mainTs.path, ...startSpan } - }).response as protocol.DefinitionInfoAndBoundSpan; - assert.deepEqual(response, { - definitions: [{ file: dependencyTs.path, start: { line: i + 1, offset: 17 }, end: { line: i + 1, offset: 20 } }], - textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } - }); + const randomFile: File = { + path: `${projectLocation}/random/random.ts`, + content: "let a = 10;" + }; + const randomConfig: File = { + path: `${projectLocation}/random/tsconfig.json`, + content: "{}" + }; + + const files = [dependencyTs, dependencyConfig, mainTs, mainConfig, libFile, randomFile, randomConfig]; + + function verifyInfos(service: server.ProjectService, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { + checkScriptInfos(service, openInfos.concat(closedInfos)); + checkWatchedFiles(host, closedInfos.concat(otherWatchedFiles).map(f => f.toLowerCase())); } + + it("can go to definition correctly", () => { + const host = createHost(files, [mainConfig.path]); + const session = createSession(host); + const service = session.getProjectService(); + openFilesForSession([mainTs], session); + checkNumberOfProjects(service, { configuredProjects: 1 }); + checkProjectActualFiles(service.configuredProjects.get(mainConfig.path)!, [mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`]); + for (let i = 0; i < 5; i++) { + const startSpan = { line: i + 5, offset: 1 }; + const response = session.executeCommandSeq({ + command: protocol.CommandTypes.DefinitionAndBoundSpan, + arguments: { file: mainTs.path, ...startSpan } + }).response as protocol.DefinitionInfoAndBoundSpan; + assert.deepEqual(response, { + definitions: [{ file: dependencyTs.path, start: { line: i + 1, offset: 17 }, end: { line: i + 1, offset: 20 } }], + textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } + }); + } + checkNumberOfProjects(service, { configuredProjects: 1 }); + const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, `${dependecyLocation}/fns.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + verifyInfos(service, host, [mainTs.path], closedInfos, [mainConfig.path]); + + openFilesForSession([randomFile], session); + verifyInfos(service, host, [mainTs.path, randomFile.path], closedInfos, [mainConfig.path, randomConfig.path]); + }); + + it("rename locations from depedency", () => { + const host = createHost(files, [mainConfig.path]); + const session = createSession(host); + const service = session.getProjectService(); + openFilesForSession([dependencyTs, randomFile], session); + checkNumberOfProjects(service, { configuredProjects: 2 }); + checkProjectActualFiles(service.configuredProjects.get(dependencyConfig.path)!, [dependencyTs.path, libFile.path, dependencyConfig.path]); + debugger; + for (let i = 0; i < 5; i++) { + const startSpan = { line: i + 1, offset: 17 }; + const response = session.executeCommandSeq({ + command: protocol.CommandTypes.Rename, + arguments: { file: dependencyTs.path, ...startSpan } + }).response as protocol.RenameResponseBody; + assert.deepEqual(response.locs, [{ + file: dependencyTs.path, + locs: [{ start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } }] + }]); + } + checkNumberOfProjects(service, { configuredProjects: 2 }); + const openInfos = [dependencyTs.path, randomFile.path]; + const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + const otherWatchedFiles = [dependencyConfig.path, randomConfig.path]; + verifyInfos(service, host, openInfos, closedInfos, otherWatchedFiles); + + // Collect the orphan projects and infos + closeFilesForSession([randomFile], session); + openFilesForSession([randomFile], session); + + verifyInfos(service, host, openInfos, closedInfos, otherWatchedFiles); + }); }); }); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 2b78178587e..2bf91fac4f4 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8497,10 +8497,6 @@ declare namespace ts.server { syntaxOnly?: boolean; } class ProjectService { - /** - * Container of all known scripts - */ - private readonly filenameToScriptInfo; private readonly scriptInfoInNodeModulesWatchers; /** * Contains all the deleted script info's version information so that From 56a39b754c655f726ff7efbaebf8cec8c9c2452d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 29 Nov 2018 14:14:45 -0800 Subject: [PATCH 10/23] Keep alive declaration script info and map file info if source file info is alive --- src/server/editorServices.ts | 36 +++++++++++++------ src/server/project.ts | 4 +-- src/server/scriptInfo.ts | 15 +++++--- src/services/services.ts | 2 +- src/services/sourcemaps.ts | 14 ++++---- src/services/types.ts | 2 +- .../unittests/tsserverProjectSystem.ts | 1 - 7 files changed, 47 insertions(+), 27 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index d98aa948113..5c70c7be8bf 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2197,8 +2197,8 @@ namespace ts.server { } /*@internal*/ - getDocumentPositionMapper(fileName: string, project: Project): DocumentPositionMapper | undefined { - const declarationInfo = this.getOrCreateScriptInfoNotOpenedByClient(fileName, project.currentDirectory, project.directoryStructureHost); + getDocumentPositionMapper(project: Project, generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined { + const declarationInfo = this.getOrCreateScriptInfoNotOpenedByClient(generatedFileName, project.currentDirectory, project.directoryStructureHost); if (!declarationInfo) return undefined; declarationInfo.getSnapshot(); // Ensure synchronized @@ -2210,7 +2210,6 @@ namespace ts.server { // Create the mapper declarationInfo.mapInfo = undefined; - // TODO: shkamat Lifetime of declarationInfo and mapInfo let readMapFile: ((fileName: string) => string | undefined) | undefined = fileName => { const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(fileName, project.currentDirectory, project.directoryStructureHost); if (!mapInfo) return undefined; @@ -2227,6 +2226,11 @@ namespace ts.server { ); readMapFile = undefined; // Remove ref to project declarationInfo.mapper = mapper || false; + if (sourceFileName && mapper) { + // Attach as source + const sourceInfo = this.getOrCreateScriptInfoNotOpenedByClient(sourceFileName, project.currentDirectory, project.directoryStructureHost)!; + (declarationInfo.mapInfo!.sourceInfos || (declarationInfo.mapInfo!.sourceInfos = createMap())).set(sourceInfo.path, true); + } return mapper; } @@ -2609,13 +2613,25 @@ namespace ts.server { private removeOrphanScriptInfos() { const toRemoveScriptInfos = cloneMap(this.filenameToScriptInfo); this.filenameToScriptInfo.forEach(info => { - if (info.isScriptOpen() || !info.isOrphan()) { - toRemoveScriptInfos.delete(info.path); - if (info.mapInfo) { - toRemoveScriptInfos.delete(info.mapInfo.path); - if (info.mapInfo.sourceInfos) { - info.mapInfo.sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); - } + // If script info is open or orphan, retain it and its dependencies + if (!info.isScriptOpen() && info.isOrphan()) { + // Otherwise if there is any source info that is alive, this alive too + if (!info.mapInfo || !info.mapInfo.sourceInfos) return; + if (!forEachKey(info.mapInfo.sourceInfos, path => { + const info = this.getScriptInfoForPath(path as Path)!; + return info.isScriptOpen() || !info.isOrphan(); + })) { + return; + } + } + + // Retain this script info + toRemoveScriptInfos.delete(info.path); + if (info.mapInfo) { + // And map file info and source infos + toRemoveScriptInfos.delete(info.mapInfo.path); + if (info.mapInfo.sourceInfos) { + info.mapInfo.sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); } } }); diff --git a/src/server/project.ts b/src/server/project.ts index 6ea9589cb4b..a28e9c35be5 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -504,8 +504,8 @@ namespace ts.server { } /*@internal*/ - getDocumentPositionMapper(fileName: string): DocumentPositionMapper | undefined { - return this.projectService.getDocumentPositionMapper(fileName, this); + getDocumentPositionMapper(generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined { + return this.projectService.getDocumentPositionMapper(this, generatedFileName, sourceFileName); } /*@internal*/ diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 3d8c737064f..d2f230178d3 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -64,14 +64,20 @@ namespace ts.server { this.switchToScriptVersionCache(); } + private resetSourceMapInfo() { + this.info.mapper = undefined; + this.info.sourceFileLike = undefined; + this.info.mapInfo = undefined; + this.info.sourceInfos = undefined; + } + /** Public for testing */ public useText(newText?: string) { this.svc = undefined; this.text = newText; this.lineMap = undefined; this.fileSize = undefined; - this.info.mapper = undefined; - this.info.sourceFileLike = undefined; + this.resetSourceMapInfo(); this.version.text++; } @@ -81,8 +87,7 @@ namespace ts.server { this.text = undefined; this.lineMap = undefined; this.fileSize = undefined; - this.info.mapper = undefined; - this.info.sourceFileLike = undefined; + this.resetSourceMapInfo(); } /** @@ -304,7 +309,7 @@ namespace ts.server { /*@internal*/ sourceInfos?: Map; /*@internal*/ - mapper: DocumentPositionMapper | false | undefined = false; + mapper?: DocumentPositionMapper | false; /*@internal*/ sourceFileLike: SourceFileLike | undefined; diff --git a/src/services/services.ts b/src/services/services.ts index a0c41ffe94d..e8dfb8640e0 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1145,7 +1145,7 @@ namespace ts { getProgram, fileExists: host.fileExists && (f => host.fileExists!(f)), readFile: host.readFile && ((f, encoding) => host.readFile!(f, encoding)), - getDocumentPositionMapper: host.getDocumentPositionMapper && (f => host.getDocumentPositionMapper!(f)), + getDocumentPositionMapper: host.getDocumentPositionMapper && ((generatedFileName, sourceFileName) => host.getDocumentPositionMapper!(generatedFileName, sourceFileName)), getSourceFileLike: host.getSourceFileLike && (f => host.getSourceFileLike!(f)), log }); diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 00d2696abf5..ce692fc2184 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -16,7 +16,7 @@ namespace ts { fileExists?(path: string): boolean; readFile?(path: string, encoding?: string): string | undefined; getSourceFileLike?(fileName: string): SourceFileLike | undefined; - getDocumentPositionMapper?(fileName: string): DocumentPositionMapper | undefined; + getDocumentPositionMapper?(generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined; log(s: string): void; } @@ -31,20 +31,20 @@ namespace ts { return ts.toPath(fileName, currentDirectory, getCanonicalFileName); } - function getDocumentPositionMapper(fileName: string) { - const path = toPath(fileName); + function getDocumentPositionMapper(generatedFileName: string, sourceFileName?: string) { + const path = toPath(generatedFileName); const value = documentPositionMappers.get(path); if (value) return value; let mapper: DocumentPositionMapper | undefined; if (host.getDocumentPositionMapper) { - mapper = host.getDocumentPositionMapper(fileName); + mapper = host.getDocumentPositionMapper(generatedFileName, sourceFileName); } else if (host.readFile) { - const file = getSourceFileLike(fileName); + const file = getSourceFileLike(generatedFileName); mapper = file && ts.getDocumentPositionMapper( { getSourceFileLike, getCanonicalFileName, log: s => host.log(s) }, - fileName, + generatedFileName, getLineInfo(file.text, getLineStarts(file)), f => !host.fileExists || host.fileExists(f) ? host.readFile!(f) : undefined ); @@ -78,7 +78,7 @@ namespace ts { getDeclarationEmitOutputFilePathWorker(info.fileName, program.getCompilerOptions(), currentDirectory, program.getCommonSourceDirectory(), getCanonicalFileName); if (declarationPath === undefined) return undefined; - const newLoc = getDocumentPositionMapper(declarationPath).getGeneratedPosition(info); + const newLoc = getDocumentPositionMapper(declarationPath, info.fileName).getGeneratedPosition(info); return newLoc === info ? undefined : newLoc; } diff --git a/src/services/types.ts b/src/services/types.ts index d3a8f8cdd52..5100a999306 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -234,7 +234,7 @@ namespace ts { writeFile?(fileName: string, content: string): void; /* @internal */ - getDocumentPositionMapper?(fileName: string): DocumentPositionMapper | undefined; + getDocumentPositionMapper?(generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined; /* @internal */ getSourceFileLike?(fileName: string): SourceFileLike | undefined; } diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 217149efd88..59a6f6019d1 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10757,7 +10757,6 @@ fn5();` openFilesForSession([dependencyTs, randomFile], session); checkNumberOfProjects(service, { configuredProjects: 2 }); checkProjectActualFiles(service.configuredProjects.get(dependencyConfig.path)!, [dependencyTs.path, libFile.path, dependencyConfig.path]); - debugger; for (let i = 0; i < 5; i++) { const startSpan = { line: i + 1, offset: 17 }; const response = session.executeCommandSeq({ From 751cb9e2c3e63fdbb7f373427744ce5a266be60f Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 30 Nov 2018 16:29:21 -0800 Subject: [PATCH 11/23] Update source and declaration projects on update to declaration file or map file TODO: add tests --- src/server/editorServices.ts | 89 +++++++++++++++---- src/server/scriptInfo.ts | 16 ++-- .../reference/api/tsserverlibrary.d.ts | 4 + 3 files changed, 86 insertions(+), 23 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 5c70c7be8bf..5eb009fcb52 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -945,10 +945,39 @@ namespace ts.server { // this file and set of inferred projects info.delayReloadNonMixedContentFile(); this.delayUpdateProjectGraphs(info.containingProjects); + this.handleSourceMapProjects(info); } } } + private handleSourceMapProjects(info: ScriptInfo) { + // Change in d.ts, update source projects as well + if (info.sourceMapFilePath) { + const sourceMapFileInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + if (sourceMapFileInfo && sourceMapFileInfo.sourceInfos) { + this.delayUpdateSourceInfoProjects(sourceMapFileInfo.sourceInfos); + } + } + // Change in mapInfo, update declarationProjects and source projects + if (info.sourceInfos) { + this.delayUpdateSourceInfoProjects(info.sourceInfos); + } + if (info.declarationInfoPath) { + this.delayUpdateProjectsOfScriptInfoPath(info.declarationInfoPath); + } + } + + private delayUpdateSourceInfoProjects(sourceInfos: Map) { + sourceInfos.forEach((_value, path) => this.delayUpdateProjectsOfScriptInfoPath(path as Path)); + } + + private delayUpdateProjectsOfScriptInfoPath(path: Path) { + const info = this.getScriptInfoForPath(path); + if (info) { + this.delayUpdateProjectGraphs(info.containingProjects); + } + } + private handleDeletedFile(info: ScriptInfo) { this.stopWatchingScriptInfo(info); @@ -962,6 +991,7 @@ namespace ts.server { // update projects to make sure that set of referenced files is correct this.delayUpdateProjectGraphs(containingProjects); + this.handleSourceMapProjects(info); } } @@ -2201,19 +2231,30 @@ namespace ts.server { const declarationInfo = this.getOrCreateScriptInfoNotOpenedByClient(generatedFileName, project.currentDirectory, project.directoryStructureHost); if (!declarationInfo) return undefined; + // Try to get from cache declarationInfo.getSnapshot(); // Ensure synchronized - const existingMapper = declarationInfo.mapper; - if (existingMapper !== undefined) { - return existingMapper ? existingMapper : undefined; + if (declarationInfo.sourceMapFilePath !== undefined) { + // Doesnt have sourceMap + if (!declarationInfo.sourceMapFilePath) return undefined; + + // Ensure mapper is synchronized + const mapFileInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); + if (mapFileInfo) { + mapFileInfo.getSnapshot(); + if (mapFileInfo.mapper !== undefined) { + return mapFileInfo.mapper ? mapFileInfo.mapper : undefined; + } + } } // Create the mapper - declarationInfo.mapInfo = undefined; + declarationInfo.sourceMapFilePath = undefined; + let sourceMapFileInfo: ScriptInfo | undefined; let readMapFile: ((fileName: string) => string | undefined) | undefined = fileName => { const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(fileName, project.currentDirectory, project.directoryStructureHost); if (!mapInfo) return undefined; - declarationInfo.mapInfo = mapInfo; + sourceMapFileInfo = mapInfo; const snap = mapInfo.getSnapshot(); return snap.getText(0, snap.getLength()); }; @@ -2225,11 +2266,17 @@ namespace ts.server { readMapFile ); readMapFile = undefined; // Remove ref to project - declarationInfo.mapper = mapper || false; - if (sourceFileName && mapper) { - // Attach as source - const sourceInfo = this.getOrCreateScriptInfoNotOpenedByClient(sourceFileName, project.currentDirectory, project.directoryStructureHost)!; - (declarationInfo.mapInfo!.sourceInfos || (declarationInfo.mapInfo!.sourceInfos = createMap())).set(sourceInfo.path, true); + if (sourceMapFileInfo) { + declarationInfo.sourceMapFilePath = sourceMapFileInfo.path; + sourceMapFileInfo.mapper = mapper || false; + if (sourceFileName && mapper) { + // Attach as source + const sourceInfo = this.getOrCreateScriptInfoNotOpenedByClient(sourceFileName, project.currentDirectory, project.directoryStructureHost)!; + (sourceMapFileInfo.sourceInfos || (sourceMapFileInfo.sourceInfos = createMap())).set(sourceInfo.path, true); + } + } + else { + declarationInfo.sourceMapFilePath = false; } return mapper; } @@ -2248,8 +2295,11 @@ namespace ts.server { if (!info) return undefined; // Attach as source - if (declarationInfo && declarationInfo.mapInfo && info !== declarationInfo) { - (declarationInfo.mapInfo.sourceInfos || (declarationInfo.mapInfo.sourceInfos = createMap())).set(info.path, true); + if (declarationInfo && declarationInfo.sourceMapFilePath && info !== declarationInfo) { + const sourceMapInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); + if (sourceMapInfo) { + (sourceMapInfo.sourceInfos || (sourceMapInfo.sourceInfos = createMap())).set(info.path, true); + } } // Key doesnt matter since its only for text and lines @@ -2616,8 +2666,10 @@ namespace ts.server { // If script info is open or orphan, retain it and its dependencies if (!info.isScriptOpen() && info.isOrphan()) { // Otherwise if there is any source info that is alive, this alive too - if (!info.mapInfo || !info.mapInfo.sourceInfos) return; - if (!forEachKey(info.mapInfo.sourceInfos, path => { + if (!info.sourceMapFilePath) return; + const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + if (!sourceMapInfo || !sourceMapInfo.sourceInfos) return; + if (!forEachKey(sourceMapInfo.sourceInfos, path => { const info = this.getScriptInfoForPath(path as Path)!; return info.isScriptOpen() || !info.isOrphan(); })) { @@ -2627,11 +2679,12 @@ namespace ts.server { // Retain this script info toRemoveScriptInfos.delete(info.path); - if (info.mapInfo) { + if (info.sourceMapFilePath) { // And map file info and source infos - toRemoveScriptInfos.delete(info.mapInfo.path); - if (info.mapInfo.sourceInfos) { - info.mapInfo.sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); + toRemoveScriptInfos.delete(info.sourceMapFilePath); + const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + if (sourceMapInfo && sourceMapInfo.sourceInfos) { + sourceMapInfo.sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); } } }); diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index d2f230178d3..29685112188 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -65,10 +65,11 @@ namespace ts.server { } private resetSourceMapInfo() { - this.info.mapper = undefined; this.info.sourceFileLike = undefined; - this.info.mapInfo = undefined; + this.info.sourceMapFilePath = undefined; + this.info.declarationInfoPath = undefined; this.info.sourceInfos = undefined; + this.info.mapper = undefined; } /** Public for testing */ @@ -305,13 +306,18 @@ namespace ts.server { mTime?: number; /*@internal*/ - mapInfo?: ScriptInfo; + sourceFileLike?: SourceFileLike; + + /*@internal*/ + sourceMapFilePath?: Path | false; + + // Present on sourceMapFile info + /*@internal*/ + declarationInfoPath?: Path; /*@internal*/ sourceInfos?: Map; /*@internal*/ mapper?: DocumentPositionMapper | false; - /*@internal*/ - sourceFileLike: SourceFileLike | undefined; constructor( private readonly host: ServerHost, diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 2bf91fac4f4..fa6990c07f4 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8595,6 +8595,9 @@ declare namespace ts.server { getHostFormatCodeOptions(): FormatCodeSettings; getHostPreferences(): protocol.UserPreferences; private onSourceFileChanged; + private handleSourceMapProjects; + private delayUpdateSourceInfoProjects; + private delayUpdateProjectsOfScriptInfoPath; private handleDeletedFile; private onConfigChangedForConfiguredProject; /** @@ -8722,6 +8725,7 @@ declare namespace ts.server { private findExternalProjectContainingOpenScriptInfo; openClientFileWithNormalizedPath(fileName: NormalizedPath, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean, projectRootPath?: NormalizedPath): OpenConfiguredProjectResult; private removeOrphanConfiguredProjects; + private removeOrphanScriptInfos; private telemetryOnOpenFile; /** * Close file whose contents is managed by the client From 0113f43632b2d3a0de3f25e308b64abed7cf27e6 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 3 Dec 2018 12:31:00 -0800 Subject: [PATCH 12/23] Fix the edits clamping from #28583 after rebasing to master --- src/compiler/scanner.ts | 15 ++++++--------- src/compiler/sourcemap.ts | 4 ++-- src/compiler/types.ts | 2 +- src/server/editorServices.ts | 2 +- src/server/scriptInfo.ts | 12 ++++++++---- src/services/services.ts | 4 ++-- src/services/sourcemaps.ts | 1 - 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index e9ccc23d6eb..42589c4228e 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -337,17 +337,14 @@ namespace ts { return result; } - export function getPositionOfLineAndCharacter(sourceFile: SourceFileLike, line: number, character: number): number { - return sourceFile.getPositionOfLineAndCharacter ? - sourceFile.getPositionOfLineAndCharacter(line, character) : - computePositionOfLineAndCharacter(getLineStarts(sourceFile), line, character, sourceFile.text); - } - + export function getPositionOfLineAndCharacter(sourceFile: SourceFileLike, line: number, character: number): number; /* @internal */ - export function getPositionOfLineAndCharacterWithEdits(sourceFile: SourceFileLike, line: number, character: number): number { + // tslint:disable-next-line:unified-signatures + export function getPositionOfLineAndCharacter(sourceFile: SourceFileLike, line: number, character: number, allowEdits?: true): number; + export function getPositionOfLineAndCharacter(sourceFile: SourceFileLike, line: number, character: number, allowEdits?: true): number { return sourceFile.getPositionOfLineAndCharacter ? - sourceFile.getPositionOfLineAndCharacter(line, character) : - computePositionOfLineAndCharacter(getLineStarts(sourceFile), line, character, sourceFile.text, /*allowEdits*/ true); + sourceFile.getPositionOfLineAndCharacter(line, character, allowEdits) : + computePositionOfLineAndCharacter(getLineStarts(sourceFile), line, character, sourceFile.text, allowEdits); } /* @internal */ diff --git a/src/compiler/sourcemap.ts b/src/compiler/sourcemap.ts index 190cac3b761..00a78b0a93c 100644 --- a/src/compiler/sourcemap.ts +++ b/src/compiler/sourcemap.ts @@ -616,7 +616,7 @@ namespace ts { function processMapping(mapping: Mapping): MappedPosition { const generatedPosition = generatedFile !== undefined - ? getPositionOfLineAndCharacterWithEdits(generatedFile, mapping.generatedLine, mapping.generatedCharacter) + ? getPositionOfLineAndCharacter(generatedFile, mapping.generatedLine, mapping.generatedCharacter, /*allowEdits*/ true) : -1; let source: string | undefined; let sourcePosition: number | undefined; @@ -624,7 +624,7 @@ namespace ts { const sourceFile = host.getSourceFileLike(sourceFileAbsolutePaths[mapping.sourceIndex]); source = map.sources[mapping.sourceIndex]; sourcePosition = sourceFile !== undefined - ? getPositionOfLineAndCharacterWithEdits(sourceFile, mapping.sourceLine, mapping.sourceCharacter) + ? getPositionOfLineAndCharacter(sourceFile, mapping.sourceLine, mapping.sourceCharacter, /*allowEdits*/ true) : -1; } return { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index e654f4faab7..772c42e5aa7 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2615,7 +2615,7 @@ namespace ts { readonly text: string; lineMap?: ReadonlyArray; /* @internal */ - getPositionOfLineAndCharacter?(line: number, character: number): number; + getPositionOfLineAndCharacter?(line: number, character: number, allowEdits?: true): number; } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 5eb009fcb52..4d3cf160707 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2316,7 +2316,7 @@ namespace ts.server { const lineOffset = info.positionToLineOffset(pos); return { line: lineOffset.line - 1, character: lineOffset.offset - 1 }; }, - getPositionOfLineAndCharacter: (line, character) => info.lineOffsetToPosition(line + 1, character + 1) + getPositionOfLineAndCharacter: (line, character, allowEdits) => info.lineOffsetToPosition(line + 1, character + 1, allowEdits) }; } return info.sourceFileLike; diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 29685112188..326df8c2219 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -186,9 +186,9 @@ namespace ts.server { * @param line 1 based index * @param offset 1 based index */ - lineOffsetToPosition(line: number, offset: number): number { + lineOffsetToPosition(line: number, offset: number, allowEdits?: true): number { if (!this.useScriptVersionCacheIfValidOrOpen()) { - return computePositionOfLineAndCharacter(this.getLineMap(), line - 1, offset - 1, this.text); + return computePositionOfLineAndCharacter(this.getLineMap(), line - 1, offset - 1, this.text, allowEdits); } // TODO: assert this offset is actually on the line @@ -586,8 +586,12 @@ namespace ts.server { * @param line 1 based index * @param offset 1 based index */ - lineOffsetToPosition(line: number, offset: number): number { - return this.textStorage.lineOffsetToPosition(line, offset); + lineOffsetToPosition(line: number, offset: number): number; + /*@internal*/ + // tslint:disable-next-line:unified-signatures + lineOffsetToPosition(line: number, offset: number, allowEdits?: true): number; + lineOffsetToPosition(line: number, offset: number, allowEdits?: true): number { + return this.textStorage.lineOffsetToPosition(line, offset, allowEdits); } positionToLineOffset(position: number): protocol.Location { diff --git a/src/services/services.ts b/src/services/services.ts index e8dfb8640e0..15f9709cb38 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -602,8 +602,8 @@ namespace ts { return getLineStarts(this); } - public getPositionOfLineAndCharacter(line: number, character: number): number { - return computePositionOfLineAndCharacter(getLineStarts(this), line, character, this.text); + public getPositionOfLineAndCharacter(line: number, character: number, allowEdits?: true): number { + return computePositionOfLineAndCharacter(getLineStarts(this), line, character, this.text, allowEdits); } public getLineEndOfPosition(pos: number): number { diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index ce692fc2184..12991d49802 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -97,7 +97,6 @@ namespace ts { const fileFromCache = sourceFileLike.get(path); if (fileFromCache !== undefined) return fileFromCache ? fileFromCache : undefined; - // TODO: should ask host instead? if (!host.readFile || host.fileExists && !host.fileExists(path)) { sourceFileLike.set(path, false); return undefined; From d0976509c943048ebb30931081b1b110b9aa0b72 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 3 Dec 2018 14:13:09 -0800 Subject: [PATCH 13/23] Add tests and fix DocumentPositionMapper creation on updates to d.ts, source file, map file etc --- src/server/editorServices.ts | 18 +- src/server/scriptInfo.ts | 4 +- src/services/sourcemaps.ts | 13 +- .../unittests/tsserverProjectSystem.ts | 227 +++++++++++++----- 4 files changed, 196 insertions(+), 66 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 4d3cf160707..a3ce0989b0c 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2241,8 +2241,8 @@ namespace ts.server { const mapFileInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); if (mapFileInfo) { mapFileInfo.getSnapshot(); - if (mapFileInfo.mapper !== undefined) { - return mapFileInfo.mapper ? mapFileInfo.mapper : undefined; + if (mapFileInfo.documentPositionMapper !== undefined) { + return mapFileInfo.documentPositionMapper ? mapFileInfo.documentPositionMapper : undefined; } } } @@ -2251,15 +2251,16 @@ namespace ts.server { declarationInfo.sourceMapFilePath = undefined; let sourceMapFileInfo: ScriptInfo | undefined; - let readMapFile: ((fileName: string) => string | undefined) | undefined = fileName => { - const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(fileName, project.currentDirectory, project.directoryStructureHost); + let readMapFile: ReadMapFile | undefined = mapFileName => { + const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(mapFileName, project.currentDirectory, project.directoryStructureHost); if (!mapInfo) return undefined; sourceMapFileInfo = mapInfo; const snap = mapInfo.getSnapshot(); + if (mapInfo.documentPositionMapper !== undefined) return mapInfo.documentPositionMapper; return snap.getText(0, snap.getLength()); }; const projectName = project.projectName; - const mapper = getDocumentPositionMapper( + const documentPositionMapper = getDocumentPositionMapper( { getCanonicalFileName: this.toCanonicalFileName, log: s => this.logger.info(s), getSourceFileLike: f => this.getSourceFileLike(f, projectName, declarationInfo) }, declarationInfo.fileName, declarationInfo.getLineInfo(), @@ -2268,8 +2269,9 @@ namespace ts.server { readMapFile = undefined; // Remove ref to project if (sourceMapFileInfo) { declarationInfo.sourceMapFilePath = sourceMapFileInfo.path; - sourceMapFileInfo.mapper = mapper || false; - if (sourceFileName && mapper) { + sourceMapFileInfo.declarationInfoPath = declarationInfo.path; + sourceMapFileInfo.documentPositionMapper = documentPositionMapper || false; + if (sourceFileName && documentPositionMapper) { // Attach as source const sourceInfo = this.getOrCreateScriptInfoNotOpenedByClient(sourceFileName, project.currentDirectory, project.directoryStructureHost)!; (sourceMapFileInfo.sourceInfos || (sourceMapFileInfo.sourceInfos = createMap())).set(sourceInfo.path, true); @@ -2278,7 +2280,7 @@ namespace ts.server { else { declarationInfo.sourceMapFilePath = false; } - return mapper; + return documentPositionMapper; } /*@internal*/ diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 326df8c2219..1cd00cdca45 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -69,7 +69,7 @@ namespace ts.server { this.info.sourceMapFilePath = undefined; this.info.declarationInfoPath = undefined; this.info.sourceInfos = undefined; - this.info.mapper = undefined; + this.info.documentPositionMapper = undefined; } /** Public for testing */ @@ -317,7 +317,7 @@ namespace ts.server { /*@internal*/ sourceInfos?: Map; /*@internal*/ - mapper?: DocumentPositionMapper | false; + documentPositionMapper?: DocumentPositionMapper | false; constructor( private readonly host: ServerHost, diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 12991d49802..42b00b8a8d7 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -127,11 +127,17 @@ namespace ts { } } + /** + * string | undefined to contents of map file to create DocumentPositionMapper from it + * DocumentPositionMapper | false to give back cached DocumentPositionMapper + */ + export type ReadMapFile = (mapFileName: string) => string | undefined | DocumentPositionMapper | false; + export function getDocumentPositionMapper( host: DocumentPositionMapperHost, generatedFileName: string, generatedFileLineInfo: LineInfo, - readMapFile: (fileName: string) => string | undefined) { + readMapFile: ReadMapFile) { let mapFileName = tryGetSourceMappingURL(generatedFileLineInfo); if (mapFileName) { const match = base64UrlRegExp.exec(mapFileName); @@ -152,9 +158,12 @@ namespace ts { for (const location of possibleMapLocations) { const mapFileName = getNormalizedAbsolutePath(location, getDirectoryPath(generatedFileName)); const mapFileContents = readMapFile(mapFileName); - if (mapFileContents) { + if (isString(mapFileContents)) { return convertDocumentToSourceMapper(host, mapFileContents, mapFileName); } + if (mapFileContents !== undefined) { + return mapFileContents || undefined; + } } return undefined; } diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 59a6f6019d1..b36783d519f 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10681,7 +10681,8 @@ declare class TestLib { export function fn2() { } export function fn3() { } export function fn4() { } -export function fn5() { }` +export function fn5() { } +` }; const dependencyConfig: File = { path: `${dependecyLocation}/tsconfig.json`, @@ -10698,7 +10699,8 @@ fn1(); fn2(); fn3(); fn4(); -fn5();` +fn5(); +` }; const mainConfig: File = { path: `${mainLocation}/tsconfig.json`, @@ -10719,66 +10721,183 @@ fn5();` const files = [dependencyTs, dependencyConfig, mainTs, mainConfig, libFile, randomFile, randomConfig]; - function verifyInfos(service: server.ProjectService, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { + function verifyScriptInfos(service: server.ProjectService, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { checkScriptInfos(service, openInfos.concat(closedInfos)); checkWatchedFiles(host, closedInfos.concat(otherWatchedFiles).map(f => f.toLowerCase())); } - it("can go to definition correctly", () => { - const host = createHost(files, [mainConfig.path]); - const session = createSession(host); - const service = session.getProjectService(); - openFilesForSession([mainTs], session); - checkNumberOfProjects(service, { configuredProjects: 1 }); - checkProjectActualFiles(service.configuredProjects.get(mainConfig.path)!, [mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`]); - for (let i = 0; i < 5; i++) { - const startSpan = { line: i + 5, offset: 1 }; - const response = session.executeCommandSeq({ - command: protocol.CommandTypes.DefinitionAndBoundSpan, - arguments: { file: mainTs.path, ...startSpan } - }).response as protocol.DefinitionInfoAndBoundSpan; - assert.deepEqual(response, { - definitions: [{ file: dependencyTs.path, start: { line: i + 1, offset: 17 }, end: { line: i + 1, offset: 20 } }], - textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } - }); - } - checkNumberOfProjects(service, { configuredProjects: 1 }); - const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, `${dependecyLocation}/fns.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; - verifyInfos(service, host, [mainTs.path], closedInfos, [mainConfig.path]); + function verifyInfosWithRandom(service: server.ProjectService, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { + verifyScriptInfos(service, host, openInfos.concat(randomFile.path), closedInfos, otherWatchedFiles.concat(randomConfig.path)); + } - openFilesForSession([randomFile], session); - verifyInfos(service, host, [mainTs.path, randomFile.path], closedInfos, [mainConfig.path, randomConfig.path]); + function verifyOnlyRandomInfos(service: server.ProjectService, host: TestServerHost) { + verifyScriptInfos(service, host, [randomFile.path], [libFile.path], [randomConfig.path]); + } + + function verifyGotoDefintinionFromMainTs(fn: number, session: TestSession) { + const startSpan = { line: fn + 4, offset: 1 }; + const response = session.executeCommandSeq({ + command: protocol.CommandTypes.DefinitionAndBoundSpan, + arguments: { file: mainTs.path, ...startSpan } + }).response as protocol.DefinitionInfoAndBoundSpan; + assert.deepEqual(response, { + definitions: [{ file: dependencyTs.path, start: { line: fn, offset: 17 }, end: { line: fn, offset: 20 } }], + textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } + }); + } + + function verifyRenameFromDependencyTs(fn: number, session: TestSession) { + const startSpan = { line: fn, offset: 17 }; + const response = session.executeCommandSeq({ + command: protocol.CommandTypes.Rename, + arguments: { file: dependencyTs.path, ...startSpan } + }).response as protocol.RenameResponseBody; + assert.deepEqual(response.locs, [{ + file: dependencyTs.path, + locs: [{ start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } }] + }]); + } + + function verifyAllFnAction(action: (fn: number, session: TestSession) => void, session: TestSession) { + for (let fn = 1; fn <= 5; fn++) { + action(fn, session); + } + } + + function verifyDocumentPositionMapperUpdates( + mainScenario: string, + openFile: File, + expectedProjectActualFiles: ReadonlyArray, + action: (fn: number, session: TestSession) => void, + closedInfos: ReadonlyArray, + openFileLastLine: number) { + const configFile = `${getDirectoryPath(openFile.path)}/tsconfig.json`; + const openInfos = [openFile.path]; + const otherWatchedFiles = [configFile]; + function openTsFile() { + const host = createHost(files, [mainConfig.path]); + const session = createSession(host); + const service = session.getProjectService(); + openFilesForSession([openFile, randomFile], session); + return { host, session, service }; + } + + function checkProject(service: server.ProjectService) { + checkNumberOfProjects(service, { configuredProjects: 2 }); + checkProjectActualFiles(service.configuredProjects.get(configFile)!, expectedProjectActualFiles); + } + + function verifyInfos(service: server.ProjectService, host: TestServerHost) { + verifyInfosWithRandom(service, host, openInfos, closedInfos, otherWatchedFiles); + } + + function verifyDocumentPositionMapper(service: server.ProjectService, dependencyMap: server.ScriptInfo, documentPositionMapper: server.ScriptInfo["documentPositionMapper"], notEqual?: true) { + assert.strictEqual(service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`), dependencyMap); + if (notEqual) { + assert.notStrictEqual(dependencyMap.documentPositionMapper, documentPositionMapper); + } + else { + assert.strictEqual(dependencyMap.documentPositionMapper, documentPositionMapper); + } + } + + function verifyScenarioWithChanges( + change: (host: TestServerHost, session: TestSession) => void, + afterActionDocumentPositionMapperNotEquals?: true + ) { + const { host, session, service } = openTsFile(); + + // Create DocumentPositionMapper + action(1, session); + const dependencyMap = service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`)!; + const documentPositionMapper = dependencyMap.documentPositionMapper; + + // change + change(host, session); + host.runQueuedTimeoutCallbacks(); + checkProject(service); + verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper); + + // action + verifyAllFnAction(action, session); + verifyInfos(service, host); + verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper, afterActionDocumentPositionMapperNotEquals); + } + + it(mainScenario, () => { + const { host, session, service } = openTsFile(); + checkProject(service); + + // Main scenario action + verifyAllFnAction(action, session); + checkProject(service); + verifyInfos(service, host); + const dependencyMap = service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`)!; + const documentPositionMapper = dependencyMap.documentPositionMapper; + + // Collecting at this point retains dependency.d.ts and map + closeFilesForSession([randomFile], session); + openFilesForSession([randomFile], session); + verifyInfos(service, host); + verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper); + + // Closing open file, removes dependencies too + closeFilesForSession([openFile, randomFile], session); + openFilesForSession([randomFile], session); + verifyOnlyRandomInfos(service, host); + }); + + it("when usage file changes, document position mapper doesnt change", () => { + // Edit + verifyScenarioWithChanges((_host, session) => session.executeCommandSeq({ + command: protocol.CommandTypes.Change, + arguments: { file: openFile.path, line: openFileLastLine, offset: 1, endLine: openFileLastLine, endOffset: 1, insertString: "const x = 10;" } + })); + }); + + it("when dependency file changes, document position mapper doesnt change", () => { + // Edit dts to add new fn + verifyScenarioWithChanges(host => host.writeFile( + `${dependecyLocation}/fns.d.ts`, + host.readFile(`${dependecyLocation}/fns.d.ts`)!.replace( + "//# sourceMappingURL=FnS.d.ts.map", + `export declare function fn6(): void; +//# sourceMappingURL=FnS.d.ts.map` + ) + )); + }); + + it("when dependency file's map changes", () => { + // Edit map file to represent added new line + verifyScenarioWithChanges(host => host.writeFile( + `${dependecyLocation}/FnS.d.ts.map`, + `{"version":3,"file":"FnS.d.ts","sourceRoot":"","sources":["FnS.ts"],"names":[],"mappings":"AAAA,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM"}` + ), /*afterActionDocumentPositionMapperNotEquals*/ true); + }); + } + + describe("from project that uses dependency", () => { + const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, `${dependecyLocation}/fns.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + verifyDocumentPositionMapperUpdates( + "can go to definition correctly", + /*openFile*/ mainTs, + /*expectedProjectActualFiles*/ [mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`], + /*action*/ verifyGotoDefintinionFromMainTs, + closedInfos, + /*openFileLastLine*/ 10 + ); }); - it("rename locations from depedency", () => { - const host = createHost(files, [mainConfig.path]); - const session = createSession(host); - const service = session.getProjectService(); - openFilesForSession([dependencyTs, randomFile], session); - checkNumberOfProjects(service, { configuredProjects: 2 }); - checkProjectActualFiles(service.configuredProjects.get(dependencyConfig.path)!, [dependencyTs.path, libFile.path, dependencyConfig.path]); - for (let i = 0; i < 5; i++) { - const startSpan = { line: i + 1, offset: 17 }; - const response = session.executeCommandSeq({ - command: protocol.CommandTypes.Rename, - arguments: { file: dependencyTs.path, ...startSpan } - }).response as protocol.RenameResponseBody; - assert.deepEqual(response.locs, [{ - file: dependencyTs.path, - locs: [{ start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } }] - }]); - } - checkNumberOfProjects(service, { configuredProjects: 2 }); - const openInfos = [dependencyTs.path, randomFile.path]; + describe("from defining project", () => { const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; - const otherWatchedFiles = [dependencyConfig.path, randomConfig.path]; - verifyInfos(service, host, openInfos, closedInfos, otherWatchedFiles); - - // Collect the orphan projects and infos - closeFilesForSession([randomFile], session); - openFilesForSession([randomFile], session); - - verifyInfos(service, host, openInfos, closedInfos, otherWatchedFiles); + verifyDocumentPositionMapperUpdates( + "rename locations from dependency", + /*openFile*/ dependencyTs, + /*expectedProjectActualFiles*/ [dependencyTs.path, libFile.path, dependencyConfig.path], + /*action*/ verifyRenameFromDependencyTs, + closedInfos, + /*openFileLastLine*/ 6 + ); }); }); }); From d86aeb2f004d4ec7be60f5837dae1a6c1de33c54 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 3 Dec 2018 16:18:11 -0800 Subject: [PATCH 14/23] TODOs --- src/testRunner/unittests/tsserverProjectSystem.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index b36783d519f..1f595d95f99 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10899,6 +10899,9 @@ fn5(); /*openFileLastLine*/ 6 ); }); + + // TODO: test project changes when both projects are open + // TODO: project change when dependency is not built }); }); From 60ae299f0cf1d30fd935a9b2307bf30d8e1be1b5 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 4 Dec 2018 12:14:33 -0800 Subject: [PATCH 15/23] Add test to verify when depedency and main project is open --- .../unittests/tsserverProjectSystem.ts | 154 ++++++++++++------ 1 file changed, 106 insertions(+), 48 deletions(-) diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 1f595d95f99..4c611ea735c 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10758,33 +10758,34 @@ fn5(); }]); } - function verifyAllFnAction(action: (fn: number, session: TestSession) => void, session: TestSession) { - for (let fn = 1; fn <= 5; fn++) { - action(fn, session); - } - } - + // Open File, expectedProjectActualFiles, action, openFileLastLine + type DocumentPositionMapperVerifier = [File, ReadonlyArray, (fn: number, session: TestSession) => void, number]; function verifyDocumentPositionMapperUpdates( mainScenario: string, - openFile: File, - expectedProjectActualFiles: ReadonlyArray, - action: (fn: number, session: TestSession) => void, - closedInfos: ReadonlyArray, - openFileLastLine: number) { - const configFile = `${getDirectoryPath(openFile.path)}/tsconfig.json`; - const openInfos = [openFile.path]; - const otherWatchedFiles = [configFile]; + verifier: ReadonlyArray, + closedInfos: ReadonlyArray) { + + const openFiles = verifier.map(v => v[0]); + const expectedProjectActualFiles = verifier.map(v => v[1]); + const actions = verifier.map(v => v[2]); + const openFileLastLines = verifier.map(v => v[3]); + + const configFiles = openFiles.map(openFile => `${getDirectoryPath(openFile.path)}/tsconfig.json`); + const openInfos = openFiles.map(f => f.path); + const otherWatchedFiles = configFiles; function openTsFile() { const host = createHost(files, [mainConfig.path]); const session = createSession(host); const service = session.getProjectService(); - openFilesForSession([openFile, randomFile], session); + openFilesForSession([...openFiles, randomFile], session); return { host, session, service }; } function checkProject(service: server.ProjectService) { - checkNumberOfProjects(service, { configuredProjects: 2 }); - checkProjectActualFiles(service.configuredProjects.get(configFile)!, expectedProjectActualFiles); + checkNumberOfProjects(service, { configuredProjects: 1 + verifier.length }); + configFiles.forEach((configFile, index) => { + checkProjectActualFiles(service.configuredProjects.get(configFile)!, expectedProjectActualFiles[index]); + }); } function verifyInfos(service: server.ProjectService, host: TestServerHost) { @@ -10801,27 +10802,74 @@ fn5(); } } - function verifyScenarioWithChanges( + function verifyAllFnAction( + session: TestSession, + service: server.ProjectService, + host: TestServerHost, + firstDocumentPositionMapperNotEquals?: true, + dependencyMap?: server.ScriptInfo, + documentPositionMapper?: server.ScriptInfo["documentPositionMapper"] + ) { + // action + let isFirst = true; + for (const action of actions) { + for (let fn = 1; fn <= 5; fn++) { + action(fn, session); + verifyInfos(service, host); + if (isFirst) { + isFirst = false; + if (dependencyMap) { + verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper, firstDocumentPositionMapperNotEquals); + documentPositionMapper = dependencyMap.documentPositionMapper; + } + else { + dependencyMap = service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`)!; + documentPositionMapper = dependencyMap.documentPositionMapper; + } + } + else { + verifyDocumentPositionMapper(service, dependencyMap!, documentPositionMapper); + } + } + } + return { dependencyMap: dependencyMap!, documentPositionMapper }; + } + + function verifyScenarioWithChangesWorker( change: (host: TestServerHost, session: TestSession) => void, - afterActionDocumentPositionMapperNotEquals?: true + afterActionDocumentPositionMapperNotEquals: true | undefined, + timeoutBeforeAction: boolean ) { const { host, session, service } = openTsFile(); // Create DocumentPositionMapper - action(1, session); + actions.forEach(action => action(1, session)); const dependencyMap = service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`)!; const documentPositionMapper = dependencyMap.documentPositionMapper; // change change(host, session); - host.runQueuedTimeoutCallbacks(); - checkProject(service); - verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper); + if (timeoutBeforeAction) { + host.runQueuedTimeoutCallbacks(); + checkProject(service); + verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper); + } // action - verifyAllFnAction(action, session); - verifyInfos(service, host); - verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper, afterActionDocumentPositionMapperNotEquals); + verifyAllFnAction(session, service, host, afterActionDocumentPositionMapperNotEquals, dependencyMap, documentPositionMapper); + } + + function verifyScenarioWithChanges( + change: (host: TestServerHost, session: TestSession) => void, + afterActionDocumentPositionMapperNotEquals?: true + ) { + it("when timeout occurs before request", () => { + verifyScenarioWithChangesWorker(change, afterActionDocumentPositionMapperNotEquals, /*timeoutBeforeAction*/ true); + }); + + it("when timeout does not occur before request", () => { + verifyScenarioWithChangesWorker(change, afterActionDocumentPositionMapperNotEquals, /*timeoutBeforeAction*/ false); + }); } it(mainScenario, () => { @@ -10829,11 +10877,9 @@ fn5(); checkProject(service); // Main scenario action - verifyAllFnAction(action, session); + const { dependencyMap, documentPositionMapper } = verifyAllFnAction(session, service, host); checkProject(service); verifyInfos(service, host); - const dependencyMap = service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`)!; - const documentPositionMapper = dependencyMap.documentPositionMapper; // Collecting at this point retains dependency.d.ts and map closeFilesForSession([randomFile], session); @@ -10842,20 +10888,20 @@ fn5(); verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper); // Closing open file, removes dependencies too - closeFilesForSession([openFile, randomFile], session); + closeFilesForSession([...openFiles, randomFile], session); openFilesForSession([randomFile], session); verifyOnlyRandomInfos(service, host); }); - it("when usage file changes, document position mapper doesnt change", () => { + describe("when usage file changes, document position mapper doesnt change", () => { // Edit - verifyScenarioWithChanges((_host, session) => session.executeCommandSeq({ + verifyScenarioWithChanges((_host, session) => openFiles.forEach((openFile, index) => session.executeCommandSeq({ command: protocol.CommandTypes.Change, - arguments: { file: openFile.path, line: openFileLastLine, offset: 1, endLine: openFileLastLine, endOffset: 1, insertString: "const x = 10;" } - })); + arguments: { file: openFile.path, line: openFileLastLines[index], offset: 1, endLine: openFileLastLines[index], endOffset: 1, insertString: "const x = 10;" } + }))); }); - it("when dependency file changes, document position mapper doesnt change", () => { + describe("when dependency file changes, document position mapper doesnt change", () => { // Edit dts to add new fn verifyScenarioWithChanges(host => host.writeFile( `${dependecyLocation}/fns.d.ts`, @@ -10867,7 +10913,7 @@ fn5(); )); }); - it("when dependency file's map changes", () => { + describe("when dependency file's map changes", () => { // Edit map file to represent added new line verifyScenarioWithChanges(host => host.writeFile( `${dependecyLocation}/FnS.d.ts.map`, @@ -10876,32 +10922,44 @@ fn5(); }); } + const usageVerifier: DocumentPositionMapperVerifier = [ + /*openFile*/ mainTs, + /*expectedProjectActualFiles*/[mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`], + /*action*/ verifyGotoDefintinionFromMainTs, + /*openFileLastLine*/ 10 + ]; describe("from project that uses dependency", () => { const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, `${dependecyLocation}/fns.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; verifyDocumentPositionMapperUpdates( "can go to definition correctly", - /*openFile*/ mainTs, - /*expectedProjectActualFiles*/ [mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`], - /*action*/ verifyGotoDefintinionFromMainTs, - closedInfos, - /*openFileLastLine*/ 10 + [usageVerifier], + closedInfos ); }); + const definingVerifier: DocumentPositionMapperVerifier = [ + /*openFile*/ dependencyTs, + /*expectedProjectActualFiles*/[dependencyTs.path, libFile.path, dependencyConfig.path], + /*action*/ verifyRenameFromDependencyTs, + /*openFileLastLine*/ 6 + ]; describe("from defining project", () => { const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; verifyDocumentPositionMapperUpdates( "rename locations from dependency", - /*openFile*/ dependencyTs, - /*expectedProjectActualFiles*/ [dependencyTs.path, libFile.path, dependencyConfig.path], - /*action*/ verifyRenameFromDependencyTs, - closedInfos, - /*openFileLastLine*/ 6 + [definingVerifier], + closedInfos ); }); - // TODO: test project changes when both projects are open - // TODO: project change when dependency is not built + describe("when opening depedency and usage project", () => { + const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + verifyDocumentPositionMapperUpdates( + "goto Definition in usage and rename locations from defining project", + [definingVerifier], + closedInfos + ); + }); }); }); From 34a12839ba62511ddd33f29e9bd2078708deddde Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 4 Dec 2018 14:42:48 -0800 Subject: [PATCH 16/23] Test that when map file is created the changes are reflected. --- .../unittests/tsserverProjectSystem.ts | 206 ++++++++++++------ 1 file changed, 135 insertions(+), 71 deletions(-) diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 4c611ea735c..8f7328e6fc4 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10718,48 +10718,72 @@ fn5(); path: `${projectLocation}/random/tsconfig.json`, content: "{}" }; + const dtsMapLocation = `${dependecyLocation}/FnS.d.ts.map`; + const dtsMapPath = dtsMapLocation.toLowerCase() as Path; const files = [dependencyTs, dependencyConfig, mainTs, mainConfig, libFile, randomFile, randomConfig]; - function verifyScriptInfos(service: server.ProjectService, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { - checkScriptInfos(service, openInfos.concat(closedInfos)); + function verifyScriptInfos(session: TestSession, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { + checkScriptInfos(session.getProjectService(), openInfos.concat(closedInfos)); checkWatchedFiles(host, closedInfos.concat(otherWatchedFiles).map(f => f.toLowerCase())); } - function verifyInfosWithRandom(service: server.ProjectService, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { - verifyScriptInfos(service, host, openInfos.concat(randomFile.path), closedInfos, otherWatchedFiles.concat(randomConfig.path)); + function verifyInfosWithRandom(session: TestSession, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { + verifyScriptInfos(session, host, openInfos.concat(randomFile.path), closedInfos, otherWatchedFiles.concat(randomConfig.path)); } - function verifyOnlyRandomInfos(service: server.ProjectService, host: TestServerHost) { - verifyScriptInfos(service, host, [randomFile.path], [libFile.path], [randomConfig.path]); + function verifyOnlyRandomInfos(session: TestSession, host: TestServerHost) { + verifyScriptInfos(session, host, [randomFile.path], [libFile.path], [randomConfig.path]); } - function verifyGotoDefintinionFromMainTs(fn: number, session: TestSession) { + // Returns request and expected Response + type SessionAction = [Partial, Response]; + function gotoDefintinionFromMainTs(fn: number): SessionAction { const startSpan = { line: fn + 4, offset: 1 }; - const response = session.executeCommandSeq({ - command: protocol.CommandTypes.DefinitionAndBoundSpan, - arguments: { file: mainTs.path, ...startSpan } - }).response as protocol.DefinitionInfoAndBoundSpan; - assert.deepEqual(response, { - definitions: [{ file: dependencyTs.path, start: { line: fn, offset: 17 }, end: { line: fn, offset: 20 } }], - textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } - }); + return [ + { + command: protocol.CommandTypes.DefinitionAndBoundSpan, + arguments: { file: mainTs.path, ...startSpan } + }, + { + definitions: [{ file: dependencyTs.path, start: { line: fn, offset: 17 }, end: { line: fn, offset: 20 } }], + textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } + } + ]; } - function verifyRenameFromDependencyTs(fn: number, session: TestSession) { + function renameFromDependencyTs(fn: number): SessionAction { const startSpan = { line: fn, offset: 17 }; - const response = session.executeCommandSeq({ - command: protocol.CommandTypes.Rename, - arguments: { file: dependencyTs.path, ...startSpan } - }).response as protocol.RenameResponseBody; - assert.deepEqual(response.locs, [{ - file: dependencyTs.path, - locs: [{ start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } }] - }]); + const triggerSpan = { + start: startSpan, + end: { line: startSpan.line, offset: startSpan.offset + 3 } + }; + return [ + { + command: protocol.CommandTypes.Rename, + arguments: { file: dependencyTs.path, ...startSpan } + }, + { + info: { + canRename: true, + fileToRename: undefined, + displayName: `fn${fn}`, + fullDisplayName: `"${dependecyLocation}/FnS".fn${fn}`, + kind: ScriptElementKind.functionElement, + kindModifiers: "export", + triggerSpan + }, + locs: [ + { file: dependencyTs.path, locs: [triggerSpan] } + ] + } + ]; } - // Open File, expectedProjectActualFiles, action, openFileLastLine - type DocumentPositionMapperVerifier = [File, ReadonlyArray, (fn: number, session: TestSession) => void, number]; + // Returns request and expected Response + type SessionActionGetter = (fn: number) => SessionAction; + // Open File, expectedProjectActualFiles, actionGetter, openFileLastLine + type DocumentPositionMapperVerifier = [File, ReadonlyArray, SessionActionGetter, number]; function verifyDocumentPositionMapperUpdates( mainScenario: string, verifier: ReadonlyArray, @@ -10767,7 +10791,7 @@ fn5(); const openFiles = verifier.map(v => v[0]); const expectedProjectActualFiles = verifier.map(v => v[1]); - const actions = verifier.map(v => v[2]); + const actionGetters = verifier.map(v => v[2]); const openFileLastLines = verifier.map(v => v[3]); const configFiles = openFiles.map(openFile => `${getDirectoryPath(openFile.path)}/tsconfig.json`); @@ -10776,24 +10800,24 @@ fn5(); function openTsFile() { const host = createHost(files, [mainConfig.path]); const session = createSession(host); - const service = session.getProjectService(); openFilesForSession([...openFiles, randomFile], session); - return { host, session, service }; + return { host, session }; } - function checkProject(service: server.ProjectService) { + function checkProject(session: TestSession) { + const service = session.getProjectService(); checkNumberOfProjects(service, { configuredProjects: 1 + verifier.length }); configFiles.forEach((configFile, index) => { checkProjectActualFiles(service.configuredProjects.get(configFile)!, expectedProjectActualFiles[index]); }); } - function verifyInfos(service: server.ProjectService, host: TestServerHost) { - verifyInfosWithRandom(service, host, openInfos, closedInfos, otherWatchedFiles); + function verifyInfos(session: TestSession, host: TestServerHost, minusDtsMap?: true) { + verifyInfosWithRandom(session, host, openInfos, minusDtsMap ? closedInfos.filter(f => f.toLowerCase() !== dtsMapPath) : closedInfos, otherWatchedFiles); } - function verifyDocumentPositionMapper(service: server.ProjectService, dependencyMap: server.ScriptInfo, documentPositionMapper: server.ScriptInfo["documentPositionMapper"], notEqual?: true) { - assert.strictEqual(service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`), dependencyMap); + function verifyDocumentPositionMapper(session: TestSession, dependencyMap: server.ScriptInfo, documentPositionMapper: server.ScriptInfo["documentPositionMapper"], notEqual?: true) { + assert.strictEqual(session.getProjectService().filenameToScriptInfo.get(dtsMapPath), dependencyMap); if (notEqual) { assert.notStrictEqual(dependencyMap.documentPositionMapper, documentPositionMapper); } @@ -10802,9 +10826,26 @@ fn5(); } } + function action(actionGetter: SessionActionGetter, fn: number, session: TestSession) { + const [req, expectedResponse] = actionGetter(fn); + const { response } = session.executeCommandSeq(req); + return { response, expectedResponse }; + } + + function verifyAllFnActionWorker(session: TestSession, verifyAction: (result: ReturnType, dtsInfo: server.ScriptInfo) => void) { + // action + for (const actionGetter of actionGetters) { + for (let fn = 1; fn <= 5; fn++) { + const result = action(actionGetter, fn, session); + const dtsInfo = session.getProjectService().filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts`); + assert.isDefined(dtsInfo); + verifyAction(result, dtsInfo!); + } + } + } + function verifyAllFnAction( session: TestSession, - service: server.ProjectService, host: TestServerHost, firstDocumentPositionMapperNotEquals?: true, dependencyMap?: server.ScriptInfo, @@ -10812,51 +10853,63 @@ fn5(); ) { // action let isFirst = true; - for (const action of actions) { - for (let fn = 1; fn <= 5; fn++) { - action(fn, session); - verifyInfos(service, host); - if (isFirst) { - isFirst = false; - if (dependencyMap) { - verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper, firstDocumentPositionMapperNotEquals); - documentPositionMapper = dependencyMap.documentPositionMapper; - } - else { - dependencyMap = service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`)!; - documentPositionMapper = dependencyMap.documentPositionMapper; - } + verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo) => { + assert.deepEqual(response, expectedResponse); + verifyInfos(session, host); + assert.equal(dtsInfo.sourceMapFilePath, dtsMapPath); + if (isFirst) { + isFirst = false; + if (dependencyMap) { + verifyDocumentPositionMapper(session, dependencyMap, documentPositionMapper, firstDocumentPositionMapperNotEquals); + documentPositionMapper = dependencyMap.documentPositionMapper; } else { - verifyDocumentPositionMapper(service, dependencyMap!, documentPositionMapper); + dependencyMap = session.getProjectService().filenameToScriptInfo.get(dtsMapPath)!; + documentPositionMapper = dependencyMap.documentPositionMapper; } } - } + else { + verifyDocumentPositionMapper(session, dependencyMap!, documentPositionMapper); + } + }); return { dependencyMap: dependencyMap!, documentPositionMapper }; } + function verifyAllFnActionWithNoMap( + session: TestSession, + host: TestServerHost + ) { + // action + verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo) => { + assert.deepEqual(response, expectedResponse); + verifyInfos(session, host); + assert.isFalse(dtsInfo.sourceMapFilePath); + assert.isUndefined(session.getProjectService().filenameToScriptInfo.get(dtsMapPath)); + }); + } + function verifyScenarioWithChangesWorker( change: (host: TestServerHost, session: TestSession) => void, afterActionDocumentPositionMapperNotEquals: true | undefined, timeoutBeforeAction: boolean ) { - const { host, session, service } = openTsFile(); + const { host, session } = openTsFile(); // Create DocumentPositionMapper - actions.forEach(action => action(1, session)); - const dependencyMap = service.filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts.map`)!; + actionGetters.forEach(actionGetter => action(actionGetter, 1, session)); + const dependencyMap = session.getProjectService().filenameToScriptInfo.get(dtsMapPath)!; const documentPositionMapper = dependencyMap.documentPositionMapper; // change change(host, session); if (timeoutBeforeAction) { host.runQueuedTimeoutCallbacks(); - checkProject(service); - verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper); + checkProject(session); + verifyDocumentPositionMapper(session, dependencyMap, documentPositionMapper); } // action - verifyAllFnAction(session, service, host, afterActionDocumentPositionMapperNotEquals, dependencyMap, documentPositionMapper); + verifyAllFnAction(session, host, afterActionDocumentPositionMapperNotEquals, dependencyMap, documentPositionMapper); } function verifyScenarioWithChanges( @@ -10873,24 +10926,24 @@ fn5(); } it(mainScenario, () => { - const { host, session, service } = openTsFile(); - checkProject(service); + const { host, session } = openTsFile(); + checkProject(session); // Main scenario action - const { dependencyMap, documentPositionMapper } = verifyAllFnAction(session, service, host); - checkProject(service); - verifyInfos(service, host); + const { dependencyMap, documentPositionMapper } = verifyAllFnAction(session, host); + checkProject(session); + verifyInfos(session, host); // Collecting at this point retains dependency.d.ts and map closeFilesForSession([randomFile], session); openFilesForSession([randomFile], session); - verifyInfos(service, host); - verifyDocumentPositionMapper(service, dependencyMap, documentPositionMapper); + verifyInfos(session, host); + verifyDocumentPositionMapper(session, dependencyMap, documentPositionMapper); // Closing open file, removes dependencies too closeFilesForSession([...openFiles, randomFile], session); openFilesForSession([randomFile], session); - verifyOnlyRandomInfos(service, host); + verifyOnlyRandomInfos(session, host); }); describe("when usage file changes, document position mapper doesnt change", () => { @@ -10916,20 +10969,31 @@ fn5(); describe("when dependency file's map changes", () => { // Edit map file to represent added new line verifyScenarioWithChanges(host => host.writeFile( - `${dependecyLocation}/FnS.d.ts.map`, + dtsMapLocation, `{"version":3,"file":"FnS.d.ts","sourceRoot":"","sources":["FnS.ts"],"names":[],"mappings":"AAAA,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM"}` ), /*afterActionDocumentPositionMapperNotEquals*/ true); }); + + it("when map file is not present", () => { + const host = createHost(files, [mainConfig.path]); + host.deleteFile(dtsMapLocation); + + const session = createSession(host); + openFilesForSession([...openFiles, randomFile], session); + checkProject(session); + + verifyAllFnActionWithNoMap(session, host); + }); } const usageVerifier: DocumentPositionMapperVerifier = [ /*openFile*/ mainTs, /*expectedProjectActualFiles*/[mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`], - /*action*/ verifyGotoDefintinionFromMainTs, + /*actionGetter*/ gotoDefintinionFromMainTs, /*openFileLastLine*/ 10 ]; describe("from project that uses dependency", () => { - const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, `${dependecyLocation}/fns.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, `${dependecyLocation}/fns.d.ts`, dtsMapLocation]; verifyDocumentPositionMapperUpdates( "can go to definition correctly", [usageVerifier], @@ -10940,11 +11004,11 @@ fn5(); const definingVerifier: DocumentPositionMapperVerifier = [ /*openFile*/ dependencyTs, /*expectedProjectActualFiles*/[dependencyTs.path, libFile.path, dependencyConfig.path], - /*action*/ verifyRenameFromDependencyTs, + /*actionGetter*/ renameFromDependencyTs, /*openFileLastLine*/ 6 ]; describe("from defining project", () => { - const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, dtsMapLocation]; verifyDocumentPositionMapperUpdates( "rename locations from dependency", [definingVerifier], @@ -10953,7 +11017,7 @@ fn5(); }); describe("when opening depedency and usage project", () => { - const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, dtsMapLocation]; verifyDocumentPositionMapperUpdates( "goto Definition in usage and rename locations from defining project", [definingVerifier], From 8f3d2d9f76ed1ebc7cea9ab0e4dd6905ef53c703 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 4 Dec 2018 15:42:10 -0800 Subject: [PATCH 17/23] Goto defintion will not go to source if map is present --- .../unittests/tsserverProjectSystem.ts | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 8f7328e6fc4..077f0128adc 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10718,7 +10718,9 @@ fn5(); path: `${projectLocation}/random/tsconfig.json`, content: "{}" }; - const dtsMapLocation = `${dependecyLocation}/FnS.d.ts.map`; + const dtsLocation = `${dependecyLocation}/FnS.d.ts`; + const dtsPath = dtsLocation.toLowerCase() as Path; + const dtsMapLocation = `${dtsLocation}.map`; const dtsMapPath = dtsMapLocation.toLowerCase() as Path; const files = [dependencyTs, dependencyConfig, mainTs, mainConfig, libFile, randomFile, randomConfig]; @@ -10736,18 +10738,25 @@ fn5(); verifyScriptInfos(session, host, [randomFile.path], [libFile.path], [randomConfig.path]); } - // Returns request and expected Response - type SessionAction = [Partial, Response]; + // Returns request and expected Response, expected response when no map file + type SessionAction = [Partial, Response, Response?]; function gotoDefintinionFromMainTs(fn: number): SessionAction { const startSpan = { line: fn + 4, offset: 1 }; + const textSpan: protocol.TextSpan = { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } }; + const definitionSpan: protocol.FileSpan = { file: dependencyTs.path, start: { line: fn, offset: 17 }, end: { line: fn, offset: 20 } }; + const declareSpaceLength = "declare ".length; return [ { command: protocol.CommandTypes.DefinitionAndBoundSpan, arguments: { file: mainTs.path, ...startSpan } }, { - definitions: [{ file: dependencyTs.path, start: { line: fn, offset: 17 }, end: { line: fn, offset: 20 } }], - textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } + definitions: [definitionSpan], + textSpan + }, + { + definitions: [{ file: dtsPath, start: { line: fn, offset: definitionSpan.start.offset + declareSpaceLength }, end: { line: fn, offset: definitionSpan.end.offset + declareSpaceLength } }], + textSpan } ]; } @@ -10812,8 +10821,8 @@ fn5(); }); } - function verifyInfos(session: TestSession, host: TestServerHost, minusDtsMap?: true) { - verifyInfosWithRandom(session, host, openInfos, minusDtsMap ? closedInfos.filter(f => f.toLowerCase() !== dtsMapPath) : closedInfos, otherWatchedFiles); + function verifyInfos(session: TestSession, host: TestServerHost, minusDtsMapAndSource?: true) { + verifyInfosWithRandom(session, host, openInfos, minusDtsMapAndSource ? closedInfos.filter(f => f !== dependencyTs.path && f.toLowerCase() !== dtsMapPath) : closedInfos, otherWatchedFiles); } function verifyDocumentPositionMapper(session: TestSession, dependencyMap: server.ScriptInfo, documentPositionMapper: server.ScriptInfo["documentPositionMapper"], notEqual?: true) { @@ -10827,9 +10836,9 @@ fn5(); } function action(actionGetter: SessionActionGetter, fn: number, session: TestSession) { - const [req, expectedResponse] = actionGetter(fn); + const [req, expectedResponse, expectedNoMapResponse] = actionGetter(fn); const { response } = session.executeCommandSeq(req); - return { response, expectedResponse }; + return { response, expectedResponse, expectedNoMapResponse }; } function verifyAllFnActionWorker(session: TestSession, verifyAction: (result: ReturnType, dtsInfo: server.ScriptInfo) => void) { @@ -10837,7 +10846,7 @@ fn5(); for (const actionGetter of actionGetters) { for (let fn = 1; fn <= 5; fn++) { const result = action(actionGetter, fn, session); - const dtsInfo = session.getProjectService().filenameToScriptInfo.get(`${dependecyLocation}/fns.d.ts`); + const dtsInfo = session.getProjectService().filenameToScriptInfo.get(dtsPath); assert.isDefined(dtsInfo); verifyAction(result, dtsInfo!); } @@ -10880,9 +10889,9 @@ fn5(); host: TestServerHost ) { // action - verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo) => { - assert.deepEqual(response, expectedResponse); - verifyInfos(session, host); + verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoMapResponse }, dtsInfo) => { + assert.deepEqual(response, expectedNoMapResponse && verifier.length ? expectedNoMapResponse : expectedResponse); + verifyInfos(session, host, /*minusDtsMapAndSource*/ true); assert.isFalse(dtsInfo.sourceMapFilePath); assert.isUndefined(session.getProjectService().filenameToScriptInfo.get(dtsMapPath)); }); @@ -10957,8 +10966,8 @@ fn5(); describe("when dependency file changes, document position mapper doesnt change", () => { // Edit dts to add new fn verifyScenarioWithChanges(host => host.writeFile( - `${dependecyLocation}/fns.d.ts`, - host.readFile(`${dependecyLocation}/fns.d.ts`)!.replace( + dtsLocation, + host.readFile(dtsLocation)!.replace( "//# sourceMappingURL=FnS.d.ts.map", `export declare function fn6(): void; //# sourceMappingURL=FnS.d.ts.map` @@ -10988,12 +10997,12 @@ fn5(); const usageVerifier: DocumentPositionMapperVerifier = [ /*openFile*/ mainTs, - /*expectedProjectActualFiles*/[mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`], + /*expectedProjectActualFiles*/[mainTs.path, libFile.path, mainConfig.path, dtsPath], /*actionGetter*/ gotoDefintinionFromMainTs, /*openFileLastLine*/ 10 ]; describe("from project that uses dependency", () => { - const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, `${dependecyLocation}/fns.d.ts`, dtsMapLocation]; + const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, dtsPath, dtsMapLocation]; verifyDocumentPositionMapperUpdates( "can go to definition correctly", [usageVerifier], @@ -11008,7 +11017,7 @@ fn5(); /*openFileLastLine*/ 6 ]; describe("from defining project", () => { - const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, dtsMapLocation]; + const closedInfos = [libFile.path, dtsLocation, dtsMapLocation]; verifyDocumentPositionMapperUpdates( "rename locations from dependency", [definingVerifier], @@ -11017,7 +11026,7 @@ fn5(); }); describe("when opening depedency and usage project", () => { - const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, dtsMapLocation]; + const closedInfos = [libFile.path, dtsLocation, dtsMapLocation]; verifyDocumentPositionMapperUpdates( "goto Definition in usage and rename locations from defining project", [definingVerifier], From 3dc0d5a77c950e4757c17fd4a17959bb5f36b495 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 5 Dec 2018 12:18:14 -0800 Subject: [PATCH 18/23] Watch missing map file and update the source mapping accordingly --- src/server/editorServices.ts | 150 +++++++++++++----- src/server/scriptInfo.ts | 18 ++- src/services/sourcemaps.ts | 5 +- .../unittests/tsserverProjectSystem.ts | 117 +++++++++++--- 4 files changed, 224 insertions(+), 66 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index a3ce0989b0c..b970a8fc716 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -342,6 +342,7 @@ namespace ts.server { FailedLookupLocation = "Directory of Failed lookup locations in module resolution", TypeRoots = "Type root directory", NodeModulesForClosedScriptInfo = "node_modules for closed script infos in them", + MissingSourceMapFile = "Missing source map file" } const enum ConfigFileWatcherStatus { @@ -953,22 +954,25 @@ namespace ts.server { private handleSourceMapProjects(info: ScriptInfo) { // Change in d.ts, update source projects as well if (info.sourceMapFilePath) { - const sourceMapFileInfo = this.getScriptInfoForPath(info.sourceMapFilePath); - if (sourceMapFileInfo && sourceMapFileInfo.sourceInfos) { - this.delayUpdateSourceInfoProjects(sourceMapFileInfo.sourceInfos); + if (isString(info.sourceMapFilePath)) { + const sourceMapFileInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + this.delayUpdateSourceInfoProjects(sourceMapFileInfo && sourceMapFileInfo.sourceInfos); + } + else { + this.delayUpdateSourceInfoProjects(info.sourceMapFilePath.sourceInfos); } } // Change in mapInfo, update declarationProjects and source projects - if (info.sourceInfos) { - this.delayUpdateSourceInfoProjects(info.sourceInfos); - } + this.delayUpdateSourceInfoProjects(info.sourceInfos); if (info.declarationInfoPath) { this.delayUpdateProjectsOfScriptInfoPath(info.declarationInfoPath); } } - private delayUpdateSourceInfoProjects(sourceInfos: Map) { - sourceInfos.forEach((_value, path) => this.delayUpdateProjectsOfScriptInfoPath(path as Path)); + private delayUpdateSourceInfoProjects(sourceInfos: Map | undefined) { + if (sourceInfos) { + sourceInfos.forEach((_value, path) => this.delayUpdateProjectsOfScriptInfoPath(path as Path)); + } } private delayUpdateProjectsOfScriptInfoPath(path: Path) { @@ -992,6 +996,14 @@ namespace ts.server { // update projects to make sure that set of referenced files is correct this.delayUpdateProjectGraphs(containingProjects); this.handleSourceMapProjects(info); + info.closeSourceMapFileWatcher(); + // need to recalculate source map from declaration file + if (info.declarationInfoPath) { + const declarationInfo = this.getScriptInfoForPath(info.declarationInfoPath); + if (declarationInfo) { + declarationInfo.sourceMapFilePath = undefined; + } + } } } @@ -2228,32 +2240,43 @@ namespace ts.server { /*@internal*/ getDocumentPositionMapper(project: Project, generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined { - const declarationInfo = this.getOrCreateScriptInfoNotOpenedByClient(generatedFileName, project.currentDirectory, project.directoryStructureHost); + // Since declaration info and map file watches arent updating project's directory structure host (which can cache file structure) use host + const declarationInfo = this.getOrCreateScriptInfoNotOpenedByClient(generatedFileName, project.currentDirectory, this.host); if (!declarationInfo) return undefined; // Try to get from cache declarationInfo.getSnapshot(); // Ensure synchronized - if (declarationInfo.sourceMapFilePath !== undefined) { - // Doesnt have sourceMap - if (!declarationInfo.sourceMapFilePath) return undefined; - + if (isString(declarationInfo.sourceMapFilePath)) { // Ensure mapper is synchronized - const mapFileInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); - if (mapFileInfo) { - mapFileInfo.getSnapshot(); - if (mapFileInfo.documentPositionMapper !== undefined) { - return mapFileInfo.documentPositionMapper ? mapFileInfo.documentPositionMapper : undefined; + const sourceMapFileInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); + if (sourceMapFileInfo) { + sourceMapFileInfo.getSnapshot(); + if (sourceMapFileInfo.documentPositionMapper !== undefined) { + sourceMapFileInfo.sourceInfos = this.addSourceInfoToSourceMap(sourceFileName, project, sourceMapFileInfo.sourceInfos); + return sourceMapFileInfo.documentPositionMapper ? sourceMapFileInfo.documentPositionMapper : undefined; } } + declarationInfo.sourceMapFilePath = undefined; + } + else if (declarationInfo.sourceMapFilePath) { + declarationInfo.sourceMapFilePath.sourceInfos = this.addSourceInfoToSourceMap(sourceFileName, project, declarationInfo.sourceMapFilePath.sourceInfos); + return undefined; + } + else if (declarationInfo.sourceMapFilePath !== undefined) { + // Doesnt have sourceMap + return undefined; } // Create the mapper - declarationInfo.sourceMapFilePath = undefined; let sourceMapFileInfo: ScriptInfo | undefined; + let mapFileNameFromDeclarationInfo: string | undefined; - let readMapFile: ReadMapFile | undefined = mapFileName => { - const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(mapFileName, project.currentDirectory, project.directoryStructureHost); - if (!mapInfo) return undefined; + let readMapFile: ReadMapFile | undefined = (mapFileName, mapFileNameFromDts) => { + const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(mapFileName, project.currentDirectory, this.host); + if (!mapInfo) { + mapFileNameFromDeclarationInfo = mapFileNameFromDts; + return undefined; + } sourceMapFileInfo = mapInfo; const snap = mapInfo.getSnapshot(); if (mapInfo.documentPositionMapper !== undefined) return mapInfo.documentPositionMapper; @@ -2271,11 +2294,19 @@ namespace ts.server { declarationInfo.sourceMapFilePath = sourceMapFileInfo.path; sourceMapFileInfo.declarationInfoPath = declarationInfo.path; sourceMapFileInfo.documentPositionMapper = documentPositionMapper || false; - if (sourceFileName && documentPositionMapper) { - // Attach as source - const sourceInfo = this.getOrCreateScriptInfoNotOpenedByClient(sourceFileName, project.currentDirectory, project.directoryStructureHost)!; - (sourceMapFileInfo.sourceInfos || (sourceMapFileInfo.sourceInfos = createMap())).set(sourceInfo.path, true); - } + sourceMapFileInfo.sourceInfos = this.addSourceInfoToSourceMap(sourceFileName, project, sourceMapFileInfo.sourceInfos); + } + else if (mapFileNameFromDeclarationInfo) { + declarationInfo.sourceMapFilePath = { + declarationInfoPath: declarationInfo.path, + watcher: this.addMissingSourceMapFile( + project.currentDirectory === this.currentDirectory ? + mapFileNameFromDeclarationInfo : + getNormalizedAbsolutePath(mapFileNameFromDeclarationInfo, project.currentDirectory), + declarationInfo.path + ), + sourceInfos: this.addSourceInfoToSourceMap(sourceFileName, project) + }; } else { declarationInfo.sourceMapFilePath = false; @@ -2283,6 +2314,34 @@ namespace ts.server { return documentPositionMapper; } + private addSourceInfoToSourceMap(sourceFileName: string | undefined, project: Project, sourceInfos?: Map) { + if (sourceFileName) { + // Attach as source + const sourceInfo = this.getOrCreateScriptInfoNotOpenedByClient(sourceFileName, project.currentDirectory, project.directoryStructureHost)!; + (sourceInfos || (sourceInfos = createMap())).set(sourceInfo.path, true); + } + return sourceInfos; + } + + private addMissingSourceMapFile(mapFileName: string, declarationInfoPath: Path) { + const fileWatcher = this.watchFactory.watchFile( + this.host, + mapFileName, + () => { + const declarationInfo = this.getScriptInfoForPath(declarationInfoPath); + if (declarationInfo && declarationInfo.sourceMapFilePath && !isString(declarationInfo.sourceMapFilePath)) { + // Update declaration and source projects + this.delayUpdateProjectGraphs(declarationInfo.containingProjects); + this.delayUpdateSourceInfoProjects(declarationInfo.sourceMapFilePath.sourceInfos); + declarationInfo.closeSourceMapFileWatcher(); + } + }, + PollingInterval.High, + WatchType.MissingSourceMapFile, + ); + return fileWatcher; + } + /*@internal*/ getSourceFileLike(fileName: string, projectNameOrProject: string | Project, declarationInfo?: ScriptInfo) { const project = (projectNameOrProject as Project).projectName ? projectNameOrProject as Project : this.findProject(projectNameOrProject as string); @@ -2297,7 +2356,7 @@ namespace ts.server { if (!info) return undefined; // Attach as source - if (declarationInfo && declarationInfo.sourceMapFilePath && info !== declarationInfo) { + if (declarationInfo && isString(declarationInfo.sourceMapFilePath) && info !== declarationInfo) { const sourceMapInfo = this.getScriptInfoForPath(declarationInfo.sourceMapFilePath); if (sourceMapInfo) { (sourceMapInfo.sourceInfos || (sourceMapInfo.sourceInfos = createMap())).set(info.path, true); @@ -2669,11 +2728,18 @@ namespace ts.server { if (!info.isScriptOpen() && info.isOrphan()) { // Otherwise if there is any source info that is alive, this alive too if (!info.sourceMapFilePath) return; - const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); - if (!sourceMapInfo || !sourceMapInfo.sourceInfos) return; - if (!forEachKey(sourceMapInfo.sourceInfos, path => { - const info = this.getScriptInfoForPath(path as Path)!; - return info.isScriptOpen() || !info.isOrphan(); + let sourceInfos: Map | undefined; + if (isString(info.sourceMapFilePath)) { + const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + sourceInfos = sourceMapInfo && sourceMapInfo.sourceInfos; + } + else { + sourceInfos = info.sourceMapFilePath.sourceInfos; + } + if (!sourceInfos) return; + if (!forEachKey(sourceInfos, path => { + const info = this.getScriptInfoForPath(path as Path); + return !!info && (info.isScriptOpen() || !info.isOrphan()); })) { return; } @@ -2682,11 +2748,18 @@ namespace ts.server { // Retain this script info toRemoveScriptInfos.delete(info.path); if (info.sourceMapFilePath) { - // And map file info and source infos - toRemoveScriptInfos.delete(info.sourceMapFilePath); - const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); - if (sourceMapInfo && sourceMapInfo.sourceInfos) { - sourceMapInfo.sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); + let sourceInfos: Map | undefined; + if (isString(info.sourceMapFilePath)) { + // And map file info and source infos + toRemoveScriptInfos.delete(info.sourceMapFilePath); + const sourceMapInfo = this.getScriptInfoForPath(info.sourceMapFilePath); + sourceInfos = sourceMapInfo && sourceMapInfo.sourceInfos; + } + else { + sourceInfos = info.sourceMapFilePath.sourceInfos; + } + if (sourceInfos) { + sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); } } }); @@ -2695,6 +2768,7 @@ namespace ts.server { // if there are not projects that include this script info - delete it this.stopWatchingScriptInfo(info); this.deleteScriptInfo(info); + info.closeSourceMapFileWatcher(); }); } diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 1cd00cdca45..21c011d7261 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -66,6 +66,7 @@ namespace ts.server { private resetSourceMapInfo() { this.info.sourceFileLike = undefined; + this.info.closeSourceMapFileWatcher(); this.info.sourceMapFilePath = undefined; this.info.declarationInfoPath = undefined; this.info.sourceInfos = undefined; @@ -280,6 +281,13 @@ namespace ts.server { sourceFile: SourceFile; } + /*@internal*/ + export interface SourceMapFileWatcher { + declarationInfoPath: Path; + watcher: FileWatcher; + sourceInfos?: Map; + } + export class ScriptInfo { /** * All projects that include this file @@ -309,7 +317,7 @@ namespace ts.server { sourceFileLike?: SourceFileLike; /*@internal*/ - sourceMapFilePath?: Path | false; + sourceMapFilePath?: Path | SourceMapFileWatcher | false; // Present on sourceMapFile info /*@internal*/ @@ -606,5 +614,13 @@ namespace ts.server { getLineInfo(): LineInfo { return this.textStorage.getLineInfo(); } + + /*@internal*/ + closeSourceMapFileWatcher() { + if (this.sourceMapFilePath && !isString(this.sourceMapFilePath)) { + closeFileWatcherOf(this.sourceMapFilePath); + this.sourceMapFilePath = undefined; + } + } } } diff --git a/src/services/sourcemaps.ts b/src/services/sourcemaps.ts index 42b00b8a8d7..d07c21a9f43 100644 --- a/src/services/sourcemaps.ts +++ b/src/services/sourcemaps.ts @@ -131,7 +131,7 @@ namespace ts { * string | undefined to contents of map file to create DocumentPositionMapper from it * DocumentPositionMapper | false to give back cached DocumentPositionMapper */ - export type ReadMapFile = (mapFileName: string) => string | undefined | DocumentPositionMapper | false; + export type ReadMapFile = (mapFileName: string, mapFileNameFromDts: string | undefined) => string | undefined | DocumentPositionMapper | false; export function getDocumentPositionMapper( host: DocumentPositionMapperHost, @@ -155,9 +155,10 @@ namespace ts { possibleMapLocations.push(mapFileName); } possibleMapLocations.push(generatedFileName + ".map"); + const originalMapFileName = mapFileName && getNormalizedAbsolutePath(mapFileName, getDirectoryPath(generatedFileName)); for (const location of possibleMapLocations) { const mapFileName = getNormalizedAbsolutePath(location, getDirectoryPath(generatedFileName)); - const mapFileContents = readMapFile(mapFileName); + const mapFileContents = readMapFile(mapFileName, originalMapFileName); if (isString(mapFileContents)) { return convertDocumentToSourceMapper(host, mapFileContents, mapFileName); } diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 077f0128adc..b5909d3b3d5 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10806,8 +10806,11 @@ fn5(); const configFiles = openFiles.map(openFile => `${getDirectoryPath(openFile.path)}/tsconfig.json`); const openInfos = openFiles.map(f => f.path); const otherWatchedFiles = configFiles; - function openTsFile() { + function openTsFile(onHostCreate?: (host: TestServerHost) => void) { const host = createHost(files, [mainConfig.path]); + if (onHostCreate) { + onHostCreate(host); + } const session = createSession(host); openFilesForSession([...openFiles, randomFile], session); return { host, session }; @@ -10821,8 +10824,19 @@ fn5(); }); } - function verifyInfos(session: TestSession, host: TestServerHost, minusDtsMapAndSource?: true) { - verifyInfosWithRandom(session, host, openInfos, minusDtsMapAndSource ? closedInfos.filter(f => f !== dependencyTs.path && f.toLowerCase() !== dtsMapPath) : closedInfos, otherWatchedFiles); + function verifyInfos(session: TestSession, host: TestServerHost) { + verifyInfosWithRandom(session, host, openInfos, closedInfos, otherWatchedFiles); + } + + function verifyInfosWhenNoMapFile(session: TestSession, host: TestServerHost, dependencyTsOK?: true) { + const dtsMapClosedInfo = firstDefined(closedInfos, f => f.toLowerCase() === dtsMapPath ? f : undefined); + verifyInfosWithRandom( + session, + host, + openInfos, + closedInfos.filter(f => f !== dtsMapClosedInfo && (dependencyTsOK || f !== dependencyTs.path)), + dtsMapClosedInfo ? otherWatchedFiles.concat(dtsMapClosedInfo) : otherWatchedFiles + ); } function verifyDocumentPositionMapper(session: TestSession, dependencyMap: server.ScriptInfo, documentPositionMapper: server.ScriptInfo["documentPositionMapper"], notEqual?: true) { @@ -10841,14 +10855,20 @@ fn5(); return { response, expectedResponse, expectedNoMapResponse }; } - function verifyAllFnActionWorker(session: TestSession, verifyAction: (result: ReturnType, dtsInfo: server.ScriptInfo) => void) { + function firstAction(session: TestSession) { + actionGetters.forEach(actionGetter => action(actionGetter, 1, session)); + } + + function verifyAllFnActionWorker(session: TestSession, verifyAction: (result: ReturnType, dtsInfo: server.ScriptInfo, isFirst: boolean) => void) { // action + let isFirst = true; for (const actionGetter of actionGetters) { for (let fn = 1; fn <= 5; fn++) { const result = action(actionGetter, fn, session); const dtsInfo = session.getProjectService().filenameToScriptInfo.get(dtsPath); assert.isDefined(dtsInfo); - verifyAction(result, dtsInfo!); + verifyAction(result, dtsInfo!, isFirst); + isFirst = false; } } } @@ -10861,13 +10881,11 @@ fn5(); documentPositionMapper?: server.ScriptInfo["documentPositionMapper"] ) { // action - let isFirst = true; - verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo) => { + verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo, isFirst) => { assert.deepEqual(response, expectedResponse); verifyInfos(session, host); assert.equal(dtsInfo.sourceMapFilePath, dtsMapPath); if (isFirst) { - isFirst = false; if (dependencyMap) { verifyDocumentPositionMapper(session, dependencyMap, documentPositionMapper, firstDocumentPositionMapperNotEquals); documentPositionMapper = dependencyMap.documentPositionMapper; @@ -10886,15 +10904,26 @@ fn5(); function verifyAllFnActionWithNoMap( session: TestSession, - host: TestServerHost + host: TestServerHost, + dependencyTsOK?: true ) { + let sourceMapFilePath: server.ScriptInfo["sourceMapFilePath"]; // action - verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoMapResponse }, dtsInfo) => { + verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoMapResponse }, dtsInfo, isFirst) => { assert.deepEqual(response, expectedNoMapResponse && verifier.length ? expectedNoMapResponse : expectedResponse); - verifyInfos(session, host, /*minusDtsMapAndSource*/ true); - assert.isFalse(dtsInfo.sourceMapFilePath); + verifyInfosWhenNoMapFile(session, host, dependencyTsOK); assert.isUndefined(session.getProjectService().filenameToScriptInfo.get(dtsMapPath)); + if (isFirst) { + assert.isNotString(dtsInfo.sourceMapFilePath); + assert.isNotFalse(dtsInfo.sourceMapFilePath); + assert.isDefined(dtsInfo.sourceMapFilePath); + sourceMapFilePath = dtsInfo.sourceMapFilePath; + } + else { + assert.equal(dtsInfo.sourceMapFilePath, sourceMapFilePath); + } }); + return sourceMapFilePath; } function verifyScenarioWithChangesWorker( @@ -10905,7 +10934,7 @@ fn5(); const { host, session } = openTsFile(); // Create DocumentPositionMapper - actionGetters.forEach(actionGetter => action(actionGetter, 1, session)); + firstAction(session); const dependencyMap = session.getProjectService().filenameToScriptInfo.get(dtsMapPath)!; const documentPositionMapper = dependencyMap.documentPositionMapper; @@ -10934,10 +10963,7 @@ fn5(); }); } - it(mainScenario, () => { - const { host, session } = openTsFile(); - checkProject(session); - + function verifyMainScenarioAndScriptInfoCollection(session: TestSession, host: TestServerHost) { // Main scenario action const { dependencyMap, documentPositionMapper } = verifyAllFnAction(session, host); checkProject(session); @@ -10953,6 +10979,28 @@ fn5(); closeFilesForSession([...openFiles, randomFile], session); openFilesForSession([randomFile], session); verifyOnlyRandomInfos(session, host); + } + + function verifyMainScenarioAndScriptInfoCollectionWithNoMap(session: TestSession, host: TestServerHost, dependencyTsOKInScenario?: true) { + // Main scenario action + verifyAllFnActionWithNoMap(session, host, dependencyTsOKInScenario); + + // Collecting at this point retains dependency.d.ts and map watcher + closeFilesForSession([randomFile], session); + openFilesForSession([randomFile], session); + verifyInfosWhenNoMapFile(session, host); + + // Closing open file, removes dependencies too + closeFilesForSession([...openFiles, randomFile], session); + openFilesForSession([randomFile], session); + verifyOnlyRandomInfos(session, host); + } + + it(mainScenario, () => { + const { host, session } = openTsFile(); + checkProject(session); + + verifyMainScenarioAndScriptInfoCollection(session, host); }); describe("when usage file changes, document position mapper doesnt change", () => { @@ -10963,7 +11011,7 @@ fn5(); }))); }); - describe("when dependency file changes, document position mapper doesnt change", () => { + describe("when dependency .d.ts changes, document position mapper doesnt change", () => { // Edit dts to add new fn verifyScenarioWithChanges(host => host.writeFile( dtsLocation, @@ -10983,15 +11031,34 @@ fn5(); ), /*afterActionDocumentPositionMapperNotEquals*/ true); }); - it("when map file is not present", () => { - const host = createHost(files, [mainConfig.path]); - host.deleteFile(dtsMapLocation); + describe("when map file is not present", () => { + it(mainScenario, () => { + const { host, session } = openTsFile(host => host.deleteFile(dtsMapLocation)); + checkProject(session); - const session = createSession(host); - openFilesForSession([...openFiles, randomFile], session); - checkProject(session); + verifyMainScenarioAndScriptInfoCollectionWithNoMap(session, host); + }); - verifyAllFnActionWithNoMap(session, host); + it("when map file is created", () => { + let dtsMapContents: string | undefined; + const { host, session } = openTsFile(host => { + dtsMapContents = host.readFile(dtsMapLocation)!; + host.deleteFile(dtsMapLocation); + }); + firstAction(session); + + host.writeFile(dtsMapLocation, dtsMapContents!); + verifyMainScenarioAndScriptInfoCollection(session, host); + }); + + it("when map file is deleted", () => { + const { host, session } = openTsFile(); + firstAction(session); + + // The dependency file is deleted when orphan files are collected + host.deleteFile(dtsMapLocation); + verifyMainScenarioAndScriptInfoCollectionWithNoMap(session, host, /*dependencyTsOKInScenario*/ true); + }); }); } From 6cb30651944458fd7a094326d705b68a01917b29 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 5 Dec 2018 15:31:55 -0800 Subject: [PATCH 19/23] Test to verify presence of .d.ts file --- src/compiler/program.ts | 21 +++- .../unittests/tsserverProjectSystem.ts | 107 +++++++++++++++--- 2 files changed, 104 insertions(+), 24 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 1789b161e39..080b17b8cbe 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -787,7 +787,13 @@ namespace ts { // Key is a file name. Value is the (non-empty, or undefined) list of files that redirect to it. let redirectTargetsMap = createMultiMap(); - const filesByName = createMap(); + /** + * map with + * - SourceFile if present + * - false if sourceFile missing for source of project reference redirect + * - undefined otherwise + */ + const filesByName = createMap(); let missingFilePaths: ReadonlyArray | undefined; // stores 'filename -> file association' ignoring case // used to track cases when two file names differ only in casing @@ -854,7 +860,7 @@ namespace ts { } } - missingFilePaths = arrayFrom(filesByName.keys(), p => p).filter(p => !filesByName.get(p)); + missingFilePaths = arrayFrom(mapDefinedIterator(filesByName.entries(), ([path, file]) => file === undefined ? path as Path : undefined)); files = stableSort(processingDefaultLibFiles, compareDefaultLibFiles).concat(processingOtherFiles); processingDefaultLibFiles = undefined; processingOtherFiles = undefined; @@ -1567,7 +1573,7 @@ namespace ts { } function getSourceFileByPath(path: Path): SourceFile | undefined { - return filesByName.get(path); + return filesByName.get(path) || undefined; } function getDiagnosticsHelper( @@ -2104,7 +2110,7 @@ namespace ts { /** This should have similar behavior to 'processSourceFile' without diagnostics or mutation. */ function getSourceFileFromReference(referencingFile: SourceFile, ref: FileReference): SourceFile | undefined { - return getSourceFileFromReferenceWorker(resolveTripleslashReference(ref.fileName, referencingFile.fileName), fileName => filesByName.get(toPath(fileName))); + return getSourceFileFromReferenceWorker(resolveTripleslashReference(ref.fileName, referencingFile.fileName), fileName => filesByName.get(toPath(fileName)) || undefined); } function getSourceFileFromReferenceWorker( @@ -2235,7 +2241,7 @@ namespace ts { } } - return file; + return file || undefined; } let redirectedPath: Path | undefined; @@ -2327,9 +2333,12 @@ namespace ts { } function addFileToFilesByName(file: SourceFile | undefined, path: Path, redirectedPath: Path | undefined) { - filesByName.set(path, file); if (redirectedPath) { filesByName.set(redirectedPath, file); + filesByName.set(path, file || false); + } + else { + filesByName.set(path, file); } } diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index b5909d3b3d5..ba7c7761886 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10692,7 +10692,11 @@ export function fn5() { } const mainTs: File = { path: `${mainLocation}/main.ts`, content: `import { - fn1, fn2, fn3, fn4, fn5 + fn1, + fn2, + fn3, + fn4, + fn5 } from '../dependency/fns' fn1(); @@ -10739,9 +10743,9 @@ fn5(); } // Returns request and expected Response, expected response when no map file - type SessionAction = [Partial, Response, Response?]; + type SessionAction = [Partial, Response, Response?, Response?]; function gotoDefintinionFromMainTs(fn: number): SessionAction { - const startSpan = { line: fn + 4, offset: 1 }; + const startSpan = { line: fn + 8, offset: 1 }; const textSpan: protocol.TextSpan = { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } }; const definitionSpan: protocol.FileSpan = { file: dependencyTs.path, start: { line: fn, offset: 17 }, end: { line: fn, offset: 20 } }; const declareSpaceLength = "declare ".length; @@ -10751,12 +10755,19 @@ fn5(); arguments: { file: mainTs.path, ...startSpan } }, { + // To dependency definitions: [definitionSpan], textSpan }, { + // To the dts definitions: [{ file: dtsPath, start: { line: fn, offset: definitionSpan.start.offset + declareSpaceLength }, end: { line: fn, offset: definitionSpan.end.offset + declareSpaceLength } }], textSpan + }, + { + // To import declaration + definitions: [{ file: mainTs.path, start: { line: fn + 1, offset: 5 }, end: { line: fn + 1, offset: 8 } }], + textSpan } ]; } @@ -10816,11 +10827,16 @@ fn5(); return { host, session }; } - function checkProject(session: TestSession) { + function checkProject(session: TestSession, noDts?: true) { const service = session.getProjectService(); checkNumberOfProjects(service, { configuredProjects: 1 + verifier.length }); configFiles.forEach((configFile, index) => { - checkProjectActualFiles(service.configuredProjects.get(configFile)!, expectedProjectActualFiles[index]); + checkProjectActualFiles( + service.configuredProjects.get(configFile)!, + noDts ? + expectedProjectActualFiles[index].filter(f => f.toLowerCase() !== dtsPath) : + expectedProjectActualFiles[index] + ); }); } @@ -10839,6 +10855,21 @@ fn5(); ); } + function verifyInfosWhenNoDtsFile(session: TestSession, host: TestServerHost) { + const dtsMapClosedInfo = firstDefined(closedInfos, f => f.toLowerCase() === dtsMapPath ? f : undefined); + const dtsClosedInfo = firstDefined(closedInfos, f => f.toLowerCase() === dtsPath ? f : undefined); + verifyInfosWithRandom( + session, + host, + openInfos, + closedInfos.filter(f => f !== dtsMapClosedInfo && f !== dtsClosedInfo && f !== dependencyTs.path), + // When project actual file contains dts, it needs to be watched + dtsClosedInfo && verifier.length === 1 && expectedProjectActualFiles[0].some(f => f.toLowerCase() === dtsPath) ? + otherWatchedFiles.concat(dtsClosedInfo) : + otherWatchedFiles + ); + } + function verifyDocumentPositionMapper(session: TestSession, dependencyMap: server.ScriptInfo, documentPositionMapper: server.ScriptInfo["documentPositionMapper"], notEqual?: true) { assert.strictEqual(session.getProjectService().filenameToScriptInfo.get(dtsMapPath), dependencyMap); if (notEqual) { @@ -10850,24 +10881,29 @@ fn5(); } function action(actionGetter: SessionActionGetter, fn: number, session: TestSession) { - const [req, expectedResponse, expectedNoMapResponse] = actionGetter(fn); + const [req, expectedResponse, expectedNoMapResponse, expectedNoDtsResponse] = actionGetter(fn); const { response } = session.executeCommandSeq(req); - return { response, expectedResponse, expectedNoMapResponse }; + return { response, expectedResponse, expectedNoMapResponse, expectedNoDtsResponse }; } function firstAction(session: TestSession) { actionGetters.forEach(actionGetter => action(actionGetter, 1, session)); } - function verifyAllFnActionWorker(session: TestSession, verifyAction: (result: ReturnType, dtsInfo: server.ScriptInfo, isFirst: boolean) => void) { + function verifyAllFnActionWorker(session: TestSession, verifyAction: (result: ReturnType, dtsInfo: server.ScriptInfo | undefined, isFirst: boolean) => void, dtsAbsent?: true) { // action let isFirst = true; for (const actionGetter of actionGetters) { for (let fn = 1; fn <= 5; fn++) { const result = action(actionGetter, fn, session); const dtsInfo = session.getProjectService().filenameToScriptInfo.get(dtsPath); - assert.isDefined(dtsInfo); - verifyAction(result, dtsInfo!, isFirst); + if (dtsAbsent) { + assert.isUndefined(dtsInfo); + } + else { + assert.isDefined(dtsInfo); + } + verifyAction(result, dtsInfo, isFirst); isFirst = false; } } @@ -10884,7 +10920,7 @@ fn5(); verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo, isFirst) => { assert.deepEqual(response, expectedResponse); verifyInfos(session, host); - assert.equal(dtsInfo.sourceMapFilePath, dtsMapPath); + assert.equal(dtsInfo!.sourceMapFilePath, dtsMapPath); if (isFirst) { if (dependencyMap) { verifyDocumentPositionMapper(session, dependencyMap, documentPositionMapper, firstDocumentPositionMapperNotEquals); @@ -10910,22 +10946,33 @@ fn5(); let sourceMapFilePath: server.ScriptInfo["sourceMapFilePath"]; // action verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoMapResponse }, dtsInfo, isFirst) => { - assert.deepEqual(response, expectedNoMapResponse && verifier.length ? expectedNoMapResponse : expectedResponse); + assert.deepEqual(response, expectedNoMapResponse || expectedResponse); verifyInfosWhenNoMapFile(session, host, dependencyTsOK); assert.isUndefined(session.getProjectService().filenameToScriptInfo.get(dtsMapPath)); if (isFirst) { - assert.isNotString(dtsInfo.sourceMapFilePath); - assert.isNotFalse(dtsInfo.sourceMapFilePath); - assert.isDefined(dtsInfo.sourceMapFilePath); - sourceMapFilePath = dtsInfo.sourceMapFilePath; + assert.isNotString(dtsInfo!.sourceMapFilePath); + assert.isNotFalse(dtsInfo!.sourceMapFilePath); + assert.isDefined(dtsInfo!.sourceMapFilePath); + sourceMapFilePath = dtsInfo!.sourceMapFilePath; } else { - assert.equal(dtsInfo.sourceMapFilePath, sourceMapFilePath); + assert.equal(dtsInfo!.sourceMapFilePath, sourceMapFilePath); } }); return sourceMapFilePath; } + function verifyAllFnActionWithNoDts( + session: TestSession, + host: TestServerHost + ) { + // action + verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoDtsResponse }) => { + assert.deepEqual(response, expectedNoDtsResponse || expectedResponse); + verifyInfosWhenNoDtsFile(session, host); + }, /*dtsAbsent*/ true); + } + function verifyScenarioWithChangesWorker( change: (host: TestServerHost, session: TestSession) => void, afterActionDocumentPositionMapperNotEquals: true | undefined, @@ -10996,6 +11043,21 @@ fn5(); verifyOnlyRandomInfos(session, host); } + function verifyMainScenarioAndScriptInfoCollectionWithNoDts(session: TestSession, host: TestServerHost) { + // Main scenario action + verifyAllFnActionWithNoDts(session, host); + + // Collecting at this point retains dependency.d.ts and map watcher + closeFilesForSession([randomFile], session); + openFilesForSession([randomFile], session); + verifyInfosWhenNoDtsFile(session, host); + + // Closing open file, removes dependencies too + closeFilesForSession([...openFiles, randomFile], session); + openFilesForSession([randomFile], session); + verifyOnlyRandomInfos(session, host); + } + it(mainScenario, () => { const { host, session } = openTsFile(); checkProject(session); @@ -11060,13 +11122,22 @@ fn5(); verifyMainScenarioAndScriptInfoCollectionWithNoMap(session, host, /*dependencyTsOKInScenario*/ true); }); }); + + describe("when .d.ts file is not present", () => { + it(mainScenario, () => { + const { host, session } = openTsFile(host => host.deleteFile(dtsLocation)); + checkProject(session, /*noDts*/ true); + + verifyMainScenarioAndScriptInfoCollectionWithNoDts(session, host); + }); + }); } const usageVerifier: DocumentPositionMapperVerifier = [ /*openFile*/ mainTs, /*expectedProjectActualFiles*/[mainTs.path, libFile.path, mainConfig.path, dtsPath], /*actionGetter*/ gotoDefintinionFromMainTs, - /*openFileLastLine*/ 10 + /*openFileLastLine*/ 14 ]; describe("from project that uses dependency", () => { const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, dtsPath, dtsMapLocation]; From fa7f5cb5a0ab4068235dfa186bb1ee81ab772137 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 6 Dec 2018 12:18:10 -0800 Subject: [PATCH 20/23] Actually verify when dependency and main project is open --- src/compiler/sourcemap.ts | 3 +- src/server/editorServices.ts | 2 +- src/server/project.ts | 5 +- .../unittests/tsserverProjectSystem.ts | 305 +++++++++++------- 4 files changed, 192 insertions(+), 123 deletions(-) diff --git a/src/compiler/sourcemap.ts b/src/compiler/sourcemap.ts index 00a78b0a93c..7c47449da10 100644 --- a/src/compiler/sourcemap.ts +++ b/src/compiler/sourcemap.ts @@ -583,7 +583,8 @@ namespace ts { } function compareSourcePositions(left: SourceMappedPosition, right: SourceMappedPosition) { - return compareValues(left.sourceIndex, right.sourceIndex); + Debug.assert(left.sourceIndex === right.sourceIndex); + return compareValues(left.sourcePosition, right.sourcePosition); } function compareGeneratedPositions(left: MappedPosition, right: MappedPosition) { diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index b970a8fc716..0bdff39e49a 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2604,7 +2604,7 @@ namespace ts.server { /** @internal */ fileExists(fileName: NormalizedPath): boolean { - return this.filenameToScriptInfo.has(fileName) || this.host.fileExists(fileName); + return !!this.getScriptInfoForNormalizedPath(fileName) || this.host.fileExists(fileName); } private findExternalProjectContainingOpenScriptInfo(info: ScriptInfo): ExternalProject | undefined { diff --git a/src/server/project.ts b/src/server/project.ts index a28e9c35be5..e3069e1d183 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -759,7 +759,10 @@ namespace ts.server { } containsScriptInfo(info: ScriptInfo): boolean { - return this.isRoot(info) || (!!this.program && this.program.getSourceFileByPath(info.path) !== undefined); + if (this.isRoot(info)) return true; + if (!this.program) return false; + const file = this.program.getSourceFileByPath(info.path); + return !!file && file.resolvedPath === info.path; } containsFile(filename: NormalizedPath, requireOpen?: boolean): boolean { diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index ba7c7761886..e6fc1fa3726 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -10743,47 +10743,60 @@ fn5(); } // Returns request and expected Response, expected response when no map file - type SessionAction = [Partial, Response, Response?, Response?]; + interface SessionAction { + reqName: string; + request: Partial; + expectedResponse: Response; + expectedResponseNoMap?: Response; + expectedResponseNoDts?: Response; + } function gotoDefintinionFromMainTs(fn: number): SessionAction { - const startSpan = { line: fn + 8, offset: 1 }; - const textSpan: protocol.TextSpan = { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } }; - const definitionSpan: protocol.FileSpan = { file: dependencyTs.path, start: { line: fn, offset: 17 }, end: { line: fn, offset: 20 } }; + const textSpan = usageSpan(fn); + const definition: protocol.FileSpan = { file: dependencyTs.path, ...definitionSpan(fn) }; const declareSpaceLength = "declare ".length; - return [ - { + return { + reqName: "goToDef", + request: { command: protocol.CommandTypes.DefinitionAndBoundSpan, - arguments: { file: mainTs.path, ...startSpan } + arguments: { file: mainTs.path, ...textSpan.start } }, - { + expectedResponse: { // To dependency - definitions: [definitionSpan], + definitions: [definition], textSpan }, - { + expectedResponseNoMap: { // To the dts - definitions: [{ file: dtsPath, start: { line: fn, offset: definitionSpan.start.offset + declareSpaceLength }, end: { line: fn, offset: definitionSpan.end.offset + declareSpaceLength } }], + definitions: [{ file: dtsPath, start: { line: fn, offset: definition.start.offset + declareSpaceLength }, end: { line: fn, offset: definition.end.offset + declareSpaceLength } }], textSpan }, - { + expectedResponseNoDts: { // To import declaration - definitions: [{ file: mainTs.path, start: { line: fn + 1, offset: 5 }, end: { line: fn + 1, offset: 8 } }], + definitions: [{ file: mainTs.path, ...importSpan(fn) }], textSpan } - ]; + }; + } + + function definitionSpan(fn: number): protocol.TextSpan { + return { start: { line: fn, offset: 17 }, end: { line: fn, offset: 20 } }; + } + function importSpan(fn: number): protocol.TextSpan { + return { start: { line: fn + 1, offset: 5 }, end: { line: fn + 1, offset: 8 } }; + } + function usageSpan(fn: number): protocol.TextSpan { + return { start: { line: fn + 8, offset: 1 }, end: { line: fn + 8, offset: 4 } }; } function renameFromDependencyTs(fn: number): SessionAction { - const startSpan = { line: fn, offset: 17 }; - const triggerSpan = { - start: startSpan, - end: { line: startSpan.line, offset: startSpan.offset + 3 } - }; - return [ - { + const triggerSpan = definitionSpan(fn); + return { + reqName: "rename", + request: { command: protocol.CommandTypes.Rename, - arguments: { file: dependencyTs.path, ...startSpan } + arguments: { file: dependencyTs.path, ...triggerSpan.start } }, - { + expectedResponse: { info: { canRename: true, fileToRename: undefined, @@ -10797,26 +10810,57 @@ fn5(); { file: dependencyTs.path, locs: [triggerSpan] } ] } - ]; + }; + } + + function renameFromDependencyTsWithBothProjectsOpen(fn: number): SessionAction { + const { reqName, request, expectedResponse } = renameFromDependencyTs(fn); + const { info, locs } = expectedResponse; + return { + reqName, + request, + expectedResponse: { + info, + locs: [ + locs[0], + { + file: mainTs.path, + locs: [ + importSpan(fn), + usageSpan(fn) + ] + } + ] + }, + // Only dependency result + expectedResponseNoMap: expectedResponse, + expectedResponseNoDts: expectedResponse + }; } // Returns request and expected Response type SessionActionGetter = (fn: number) => SessionAction; // Open File, expectedProjectActualFiles, actionGetter, openFileLastLine - type DocumentPositionMapperVerifier = [File, ReadonlyArray, SessionActionGetter, number]; + interface DocumentPositionMapperVerifier { + openFile: File; + expectedProjectActualFiles: ReadonlyArray; + actionGetter: SessionActionGetter; + openFileLastLine: number; + } function verifyDocumentPositionMapperUpdates( mainScenario: string, verifier: ReadonlyArray, closedInfos: ReadonlyArray) { - const openFiles = verifier.map(v => v[0]); - const expectedProjectActualFiles = verifier.map(v => v[1]); - const actionGetters = verifier.map(v => v[2]); - const openFileLastLines = verifier.map(v => v[3]); + const openFiles = verifier.map(v => v.openFile); + const expectedProjectActualFiles = verifier.map(v => v.expectedProjectActualFiles); + const actionGetters = verifier.map(v => v.actionGetter); + const openFileLastLines = verifier.map(v => v.openFileLastLine); const configFiles = openFiles.map(openFile => `${getDirectoryPath(openFile.path)}/tsconfig.json`); const openInfos = openFiles.map(f => f.path); - const otherWatchedFiles = configFiles; + // When usage and dependency are used, dependency config is part of closedInfo so ignore + const otherWatchedFiles = verifier.length > 1 ? [configFiles[0]] : configFiles; function openTsFile(onHostCreate?: (host: TestServerHost) => void) { const host = createHost(files, [mainConfig.path]); if (onHostCreate) { @@ -10855,16 +10899,16 @@ fn5(); ); } - function verifyInfosWhenNoDtsFile(session: TestSession, host: TestServerHost) { + function verifyInfosWhenNoDtsFile(session: TestSession, host: TestServerHost, dependencyTsAndMapOk?: true) { const dtsMapClosedInfo = firstDefined(closedInfos, f => f.toLowerCase() === dtsMapPath ? f : undefined); const dtsClosedInfo = firstDefined(closedInfos, f => f.toLowerCase() === dtsPath ? f : undefined); verifyInfosWithRandom( session, host, openInfos, - closedInfos.filter(f => f !== dtsMapClosedInfo && f !== dtsClosedInfo && f !== dependencyTs.path), + closedInfos.filter(f => (dependencyTsAndMapOk || f !== dtsMapClosedInfo) && f !== dtsClosedInfo && (dependencyTsAndMapOk || f !== dependencyTs.path)), // When project actual file contains dts, it needs to be watched - dtsClosedInfo && verifier.length === 1 && expectedProjectActualFiles[0].some(f => f.toLowerCase() === dtsPath) ? + dtsClosedInfo && expectedProjectActualFiles.some(expectedProjectActualFiles => expectedProjectActualFiles.some(f => f.toLowerCase() === dtsPath)) ? otherWatchedFiles.concat(dtsClosedInfo) : otherWatchedFiles ); @@ -10881,9 +10925,9 @@ fn5(); } function action(actionGetter: SessionActionGetter, fn: number, session: TestSession) { - const [req, expectedResponse, expectedNoMapResponse, expectedNoDtsResponse] = actionGetter(fn); - const { response } = session.executeCommandSeq(req); - return { response, expectedResponse, expectedNoMapResponse, expectedNoDtsResponse }; + const { reqName, request, expectedResponse, expectedResponseNoMap, expectedResponseNoDts } = actionGetter(fn); + const { response } = session.executeCommandSeq(request); + return { reqName, response, expectedResponse, expectedResponseNoMap, expectedResponseNoDts }; } function firstAction(session: TestSession) { @@ -10917,8 +10961,8 @@ fn5(); documentPositionMapper?: server.ScriptInfo["documentPositionMapper"] ) { // action - verifyAllFnActionWorker(session, ({ response, expectedResponse }, dtsInfo, isFirst) => { - assert.deepEqual(response, expectedResponse); + verifyAllFnActionWorker(session, ({ reqName, response, expectedResponse }, dtsInfo, isFirst) => { + assert.deepEqual(response, expectedResponse, `Failed on ${reqName}`); verifyInfos(session, host); assert.equal(dtsInfo!.sourceMapFilePath, dtsMapPath); if (isFirst) { @@ -10945,8 +10989,8 @@ fn5(); ) { let sourceMapFilePath: server.ScriptInfo["sourceMapFilePath"]; // action - verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoMapResponse }, dtsInfo, isFirst) => { - assert.deepEqual(response, expectedNoMapResponse || expectedResponse); + verifyAllFnActionWorker(session, ({ reqName, response, expectedResponse, expectedResponseNoMap }, dtsInfo, isFirst) => { + assert.deepEqual(response, expectedResponseNoMap || expectedResponse, `Failed on ${reqName}`); verifyInfosWhenNoMapFile(session, host, dependencyTsOK); assert.isUndefined(session.getProjectService().filenameToScriptInfo.get(dtsMapPath)); if (isFirst) { @@ -10964,12 +11008,13 @@ fn5(); function verifyAllFnActionWithNoDts( session: TestSession, - host: TestServerHost + host: TestServerHost, + dependencyTsAndMapOk?: true ) { // action - verifyAllFnActionWorker(session, ({ response, expectedResponse, expectedNoDtsResponse }) => { - assert.deepEqual(response, expectedNoDtsResponse || expectedResponse); - verifyInfosWhenNoDtsFile(session, host); + verifyAllFnActionWorker(session, ({ reqName, response, expectedResponse, expectedResponseNoDts }) => { + assert.deepEqual(response, expectedResponseNoDts || expectedResponse, `Failed on ${reqName}`); + verifyInfosWhenNoDtsFile(session, host, dependencyTsAndMapOk); }, /*dtsAbsent*/ true); } @@ -10998,15 +11043,18 @@ fn5(); } function verifyScenarioWithChanges( + scenarioName: string, change: (host: TestServerHost, session: TestSession) => void, afterActionDocumentPositionMapperNotEquals?: true ) { - it("when timeout occurs before request", () => { - verifyScenarioWithChangesWorker(change, afterActionDocumentPositionMapperNotEquals, /*timeoutBeforeAction*/ true); - }); + describe(scenarioName, () => { + it("when timeout occurs before request", () => { + verifyScenarioWithChangesWorker(change, afterActionDocumentPositionMapperNotEquals, /*timeoutBeforeAction*/ true); + }); - it("when timeout does not occur before request", () => { - verifyScenarioWithChangesWorker(change, afterActionDocumentPositionMapperNotEquals, /*timeoutBeforeAction*/ false); + it("when timeout does not occur before request", () => { + verifyScenarioWithChangesWorker(change, afterActionDocumentPositionMapperNotEquals, /*timeoutBeforeAction*/ false); + }); }); } @@ -11043,9 +11091,9 @@ fn5(); verifyOnlyRandomInfos(session, host); } - function verifyMainScenarioAndScriptInfoCollectionWithNoDts(session: TestSession, host: TestServerHost) { + function verifyMainScenarioAndScriptInfoCollectionWithNoDts(session: TestSession, host: TestServerHost, dependencyTsAndMapOk?: true) { // Main scenario action - verifyAllFnActionWithNoDts(session, host); + verifyAllFnActionWithNoDts(session, host, dependencyTsAndMapOk); // Collecting at this point retains dependency.d.ts and map watcher closeFilesForSession([randomFile], session); @@ -11058,6 +11106,43 @@ fn5(); verifyOnlyRandomInfos(session, host); } + function verifyScenarioWhenFileNotPresent( + scenarioName: string, + fileLocation: string, + verifyScenarioAndScriptInfoCollection: (session: TestSession, host: TestServerHost, dependencyTsOk?: true) => void, + noDts?: true + ) { + describe(scenarioName, () => { + it(mainScenario, () => { + const { host, session } = openTsFile(host => host.deleteFile(fileLocation)); + checkProject(session, noDts); + + verifyScenarioAndScriptInfoCollection(session, host); + }); + + it("when file is created", () => { + let fileContents: string | undefined; + const { host, session } = openTsFile(host => { + fileContents = host.readFile(fileLocation); + host.deleteFile(fileLocation); + }); + firstAction(session); + + host.writeFile(fileLocation, fileContents!); + verifyMainScenarioAndScriptInfoCollection(session, host); + }); + + it("when file is deleted", () => { + const { host, session } = openTsFile(); + firstAction(session); + + // The dependency file is deleted when orphan files are collected + host.deleteFile(fileLocation); + verifyScenarioAndScriptInfoCollection(session, host, /*dependencyTsOk*/ true); + }); + }); + } + it(mainScenario, () => { const { host, session } = openTsFile(); checkProject(session); @@ -11065,80 +11150,60 @@ fn5(); verifyMainScenarioAndScriptInfoCollection(session, host); }); - describe("when usage file changes, document position mapper doesnt change", () => { - // Edit - verifyScenarioWithChanges((_host, session) => openFiles.forEach((openFile, index) => session.executeCommandSeq({ - command: protocol.CommandTypes.Change, - arguments: { file: openFile.path, line: openFileLastLines[index], offset: 1, endLine: openFileLastLines[index], endOffset: 1, insertString: "const x = 10;" } - }))); - }); + // Edit + verifyScenarioWithChanges( + "when usage file changes, document position mapper doesnt change", + (_host, session) => openFiles.forEach( + (openFile, index) => session.executeCommandSeq({ + command: protocol.CommandTypes.Change, + arguments: { file: openFile.path, line: openFileLastLines[index], offset: 1, endLine: openFileLastLines[index], endOffset: 1, insertString: "const x = 10;" } + }) + ) + ); - describe("when dependency .d.ts changes, document position mapper doesnt change", () => { - // Edit dts to add new fn - verifyScenarioWithChanges(host => host.writeFile( + // Edit dts to add new fn + verifyScenarioWithChanges( + "when dependency .d.ts changes, document position mapper doesnt change", + host => host.writeFile( dtsLocation, host.readFile(dtsLocation)!.replace( "//# sourceMappingURL=FnS.d.ts.map", `export declare function fn6(): void; //# sourceMappingURL=FnS.d.ts.map` ) - )); - }); + ) + ); - describe("when dependency file's map changes", () => { - // Edit map file to represent added new line - verifyScenarioWithChanges(host => host.writeFile( + // Edit map file to represent added new line + verifyScenarioWithChanges( + "when dependency file's map changes", + host => host.writeFile( dtsMapLocation, - `{"version":3,"file":"FnS.d.ts","sourceRoot":"","sources":["FnS.ts"],"names":[],"mappings":"AAAA,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM"}` - ), /*afterActionDocumentPositionMapperNotEquals*/ true); - }); + `{"version":3,"file":"FnS.d.ts","sourceRoot":"","sources":["FnS.ts"],"names":[],"mappings":"AAAA,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,wBAAgB,GAAG,SAAM;AACzB,eAAO,MAAM,CAAC,KAAK,CAAC"}` + ), + /*afterActionDocumentPositionMapperNotEquals*/ true + ); - describe("when map file is not present", () => { - it(mainScenario, () => { - const { host, session } = openTsFile(host => host.deleteFile(dtsMapLocation)); - checkProject(session); + verifyScenarioWhenFileNotPresent( + "when map file is not present", + dtsMapLocation, + verifyMainScenarioAndScriptInfoCollectionWithNoMap + ); - verifyMainScenarioAndScriptInfoCollectionWithNoMap(session, host); - }); - - it("when map file is created", () => { - let dtsMapContents: string | undefined; - const { host, session } = openTsFile(host => { - dtsMapContents = host.readFile(dtsMapLocation)!; - host.deleteFile(dtsMapLocation); - }); - firstAction(session); - - host.writeFile(dtsMapLocation, dtsMapContents!); - verifyMainScenarioAndScriptInfoCollection(session, host); - }); - - it("when map file is deleted", () => { - const { host, session } = openTsFile(); - firstAction(session); - - // The dependency file is deleted when orphan files are collected - host.deleteFile(dtsMapLocation); - verifyMainScenarioAndScriptInfoCollectionWithNoMap(session, host, /*dependencyTsOKInScenario*/ true); - }); - }); - - describe("when .d.ts file is not present", () => { - it(mainScenario, () => { - const { host, session } = openTsFile(host => host.deleteFile(dtsLocation)); - checkProject(session, /*noDts*/ true); - - verifyMainScenarioAndScriptInfoCollectionWithNoDts(session, host); - }); - }); + verifyScenarioWhenFileNotPresent( + "when .d.ts file is not present", + dtsLocation, + verifyMainScenarioAndScriptInfoCollectionWithNoDts, + /*noDts*/ true + ); } - const usageVerifier: DocumentPositionMapperVerifier = [ - /*openFile*/ mainTs, - /*expectedProjectActualFiles*/[mainTs.path, libFile.path, mainConfig.path, dtsPath], - /*actionGetter*/ gotoDefintinionFromMainTs, - /*openFileLastLine*/ 14 - ]; + const usageVerifier: DocumentPositionMapperVerifier = { + openFile: mainTs, + expectedProjectActualFiles: [mainTs.path, libFile.path, mainConfig.path, dtsPath], + actionGetter: gotoDefintinionFromMainTs, + openFileLastLine: 14 + }; describe("from project that uses dependency", () => { const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, dtsPath, dtsMapLocation]; verifyDocumentPositionMapperUpdates( @@ -11148,12 +11213,12 @@ fn5(); ); }); - const definingVerifier: DocumentPositionMapperVerifier = [ - /*openFile*/ dependencyTs, - /*expectedProjectActualFiles*/[dependencyTs.path, libFile.path, dependencyConfig.path], - /*actionGetter*/ renameFromDependencyTs, - /*openFileLastLine*/ 6 - ]; + const definingVerifier: DocumentPositionMapperVerifier = { + openFile: dependencyTs, + expectedProjectActualFiles: [dependencyTs.path, libFile.path, dependencyConfig.path], + actionGetter: renameFromDependencyTs, + openFileLastLine: 6 + }; describe("from defining project", () => { const closedInfos = [libFile.path, dtsLocation, dtsMapLocation]; verifyDocumentPositionMapperUpdates( @@ -11164,10 +11229,10 @@ fn5(); }); describe("when opening depedency and usage project", () => { - const closedInfos = [libFile.path, dtsLocation, dtsMapLocation]; + const closedInfos = [libFile.path, dtsPath, dtsMapLocation, dependencyConfig.path]; verifyDocumentPositionMapperUpdates( "goto Definition in usage and rename locations from defining project", - [definingVerifier], + [usageVerifier, { ...definingVerifier, actionGetter: renameFromDependencyTsWithBothProjectsOpen }], closedInfos ); }); From 1ed67c0ebd47800646b46783426c715f98e74e3d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 6 Dec 2018 15:05:23 -0800 Subject: [PATCH 21/23] Accept test changes --- src/testRunner/unittests/textStorage.ts | 18 +++++++++++------- .../reference/api/tsserverlibrary.d.ts | 2 ++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/testRunner/unittests/textStorage.ts b/src/testRunner/unittests/textStorage.ts index 724dc8e4ed3..0bb14aa2d90 100644 --- a/src/testRunner/unittests/textStorage.ts +++ b/src/testRunner/unittests/textStorage.ts @@ -10,12 +10,16 @@ namespace ts.textStorage { }` }; + function getDummyScriptInfo() { + return { closeSourceMapFileWatcher: noop } as server.ScriptInfo; + } + it("text based storage should be have exactly the same as script version cache", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); - const ts2 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, getDummyScriptInfo()); + const ts2 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, getDummyScriptInfo()); ts1.useScriptVersionCache_TestOnly(); ts2.useText(); @@ -49,7 +53,7 @@ namespace ts.textStorage { it("should switch to script version cache if necessary", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, getDummyScriptInfo()); ts1.getSnapshot(); assert.isFalse(ts1.hasScriptVersionCache_TestOnly(), "should not have script version cache - 1"); @@ -67,7 +71,7 @@ namespace ts.textStorage { it("should be able to return the file size immediately after construction", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, getDummyScriptInfo()); assert.strictEqual(f.content.length, ts1.getTelemetryFileSize()); }); @@ -75,7 +79,7 @@ namespace ts.textStorage { it("should be able to return the file size when backed by text", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, getDummyScriptInfo()); ts1.useText(f.content); assert.isFalse(ts1.hasScriptVersionCache_TestOnly()); @@ -86,7 +90,7 @@ namespace ts.textStorage { it("should be able to return the file size when backed by a script version cache", () => { const host = projectSystem.createServerHost([f]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path), /*initialVersion*/ undefined, getDummyScriptInfo()); ts1.useScriptVersionCache_TestOnly(); assert.isTrue(ts1.hasScriptVersionCache_TestOnly()); @@ -126,7 +130,7 @@ namespace ts.textStorage { const host = projectSystem.createServerHost([changingFile]); // Since script info is not used in these tests, just cheat by passing undefined - const ts1 = new server.TextStorage(host, server.asNormalizedPath(changingFile.path), /*initialVersion*/ undefined, {} as server.ScriptInfo); + const ts1 = new server.TextStorage(host, server.asNormalizedPath(changingFile.path), /*initialVersion*/ undefined, getDummyScriptInfo()); assert.isTrue(ts1.reloadFromDisk()); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index fa6990c07f4..91f492ddd1d 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8688,6 +8688,8 @@ declare namespace ts.server { */ getScriptInfoForNormalizedPath(fileName: NormalizedPath): ScriptInfo | undefined; getScriptInfoForPath(fileName: Path): ScriptInfo | undefined; + private addSourceInfoToSourceMap; + private addMissingSourceMapFile; setHostConfiguration(args: protocol.ConfigureRequestArguments): void; closeLog(): void; /** From 2f84741725a1db41db794163fed6215259cfd57b Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 6 Dec 2018 15:13:21 -0800 Subject: [PATCH 22/23] Remove unnecessary fields --- src/server/editorServices.ts | 1 - src/server/scriptInfo.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 0bdff39e49a..3050c976231 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2298,7 +2298,6 @@ namespace ts.server { } else if (mapFileNameFromDeclarationInfo) { declarationInfo.sourceMapFilePath = { - declarationInfoPath: declarationInfo.path, watcher: this.addMissingSourceMapFile( project.currentDirectory === this.currentDirectory ? mapFileNameFromDeclarationInfo : diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 21c011d7261..68c448a7e76 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -283,7 +283,6 @@ namespace ts.server { /*@internal*/ export interface SourceMapFileWatcher { - declarationInfoPath: Path; watcher: FileWatcher; sourceInfos?: Map; } From c156c9377a410ce50457ed8f5243bfbe063445d7 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 7 Dec 2018 08:56:22 -0800 Subject: [PATCH 23/23] Add note about comparing source positions --- src/compiler/sourcemap.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/compiler/sourcemap.ts b/src/compiler/sourcemap.ts index 7c47449da10..c2597d688d8 100644 --- a/src/compiler/sourcemap.ts +++ b/src/compiler/sourcemap.ts @@ -583,6 +583,8 @@ namespace ts { } function compareSourcePositions(left: SourceMappedPosition, right: SourceMappedPosition) { + // Compares sourcePosition without comparing sourceIndex + // since the mappings are grouped by sourceIndex Debug.assert(left.sourceIndex === right.sourceIndex); return compareValues(left.sourcePosition, right.sourcePosition); }