From 67026f3461443155e1b8975c0f37510a972675bf Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 29 Oct 2015 13:09:46 -0700 Subject: [PATCH 1/7] use resolvedFileName as is when calling methods on host --- src/compiler/program.ts | 5 +---- src/compiler/tsc.ts | 34 +++++++++++++++++++--------------- src/compiler/utilities.ts | 6 ++++++ src/harness/projectsRunner.ts | 17 ++++++++++++----- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index b73afe67edc..e9f659792f9 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -869,10 +869,7 @@ namespace ts { ? resolution.resolvedFileName : getNormalizedAbsolutePath(resolution.resolvedFileName, currentDirectory); - // convert an absolute import path to path that is relative to current directory - // this was host still can locate it but files names in user output will be shorter (and thus look nicer). - const relativePath = getRelativePathToDirectoryOrUrl(currentDirectory, absoluteImportPath, currentDirectory, getCanonicalFileName, false); - const importedFile = findSourceFile(relativePath, absoluteImportPath, /* isDefaultLib */ false, file, skipTrivia(file.text, file.imports[i].pos), file.imports[i].end); + const importedFile = findSourceFile(resolution.resolvedFileName, absoluteImportPath, /* isDefaultLib */ false, file, skipTrivia(file.text, file.imports[i].pos), file.imports[i].end); if (importedFile && resolution.isExternalLibraryImport) { if (!isExternalModule(importedFile)) { diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index c493cf8f87f..fc6d65b45e9 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -81,12 +81,16 @@ namespace ts { return diagnostic.messageText; } - function reportDiagnostic(diagnostic: Diagnostic) { + function reportDiagnostic(diagnostic: Diagnostic, host: CompilerHost) { let output = ""; if (diagnostic.file) { let loc = getLineAndCharacterOfPosition(diagnostic.file, diagnostic.start); - output += `${ diagnostic.file.fileName }(${ loc.line + 1 },${ loc.character + 1 }): `; + const relativeFileName = host + ? toRelativePath(diagnostic.file.fileName, host.getCurrentDirectory(), fileName => host.getCanonicalFileName(fileName)) + : diagnostic.file.fileName; + + output += `${ relativeFileName }(${ loc.line + 1 },${ loc.character + 1 }): `; } let category = DiagnosticCategory[diagnostic.category].toLowerCase(); @@ -95,9 +99,9 @@ namespace ts { sys.write(output); } - function reportDiagnostics(diagnostics: Diagnostic[]) { + function reportDiagnostics(diagnostics: Diagnostic[], host: CompilerHost) { for (let i = 0; i < diagnostics.length; i++) { - reportDiagnostic(diagnostics[i]); + reportDiagnostic(diagnostics[i], host); } } @@ -166,7 +170,7 @@ namespace ts { if (commandLine.options.locale) { if (!isJSONSupported()) { - reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--locale")); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--locale"), /* compilerHost */ undefined); return sys.exit(ExitStatus.DiagnosticsPresent_OutputsSkipped); } validateLocaleAndSetLanguage(commandLine.options.locale, commandLine.errors); @@ -175,7 +179,7 @@ namespace ts { // If there are any errors due to command line parsing and/or // setting up localization, report them and quit. if (commandLine.errors.length > 0) { - reportDiagnostics(commandLine.errors); + reportDiagnostics(commandLine.errors, compilerHost); return sys.exit(ExitStatus.DiagnosticsPresent_OutputsSkipped); } @@ -185,7 +189,7 @@ namespace ts { } if (commandLine.options.version) { - reportDiagnostic(createCompilerDiagnostic(Diagnostics.Version_0, ts.version)); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.Version_0, ts.version), /* compilerHost */ undefined); return sys.exit(ExitStatus.Success); } @@ -197,12 +201,12 @@ namespace ts { if (commandLine.options.project) { if (!isJSONSupported()) { - reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--project")); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--project"), /* compilerHost */ undefined); return sys.exit(ExitStatus.DiagnosticsPresent_OutputsSkipped); } configFileName = normalizePath(combinePaths(commandLine.options.project, "tsconfig.json")); if (commandLine.fileNames.length !== 0) { - reportDiagnostic(createCompilerDiagnostic(Diagnostics.Option_project_cannot_be_mixed_with_source_files_on_a_command_line)); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.Option_project_cannot_be_mixed_with_source_files_on_a_command_line), /* compilerHost */ undefined); return sys.exit(ExitStatus.DiagnosticsPresent_OutputsSkipped); } } @@ -220,7 +224,7 @@ namespace ts { // Firefox has Object.prototype.watch if (commandLine.options.watch && commandLine.options.hasOwnProperty("watch")) { if (!sys.watchFile) { - reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--watch")); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--watch"), /* compilerHost */ undefined); return sys.exit(ExitStatus.DiagnosticsPresent_OutputsSkipped); } if (configFileName) { @@ -256,7 +260,7 @@ namespace ts { let configObject = result.config; let configParseResult = parseJsonConfigFileContent(configObject, sys, getDirectoryPath(configFileName)); if (configParseResult.errors.length > 0) { - reportDiagnostics(configParseResult.errors); + reportDiagnostics(configParseResult.errors, /* compilerHost */ undefined); sys.exit(ExitStatus.DiagnosticsPresent_OutputsSkipped); return; } @@ -463,7 +467,7 @@ namespace ts { } } - reportDiagnostics(diagnostics); + reportDiagnostics(diagnostics, compilerHost); // If the user doesn't want us to emit, then we're done at this point. if (compilerOptions.noEmit) { @@ -474,7 +478,7 @@ namespace ts { // Otherwise, emit and report any errors we ran into. let emitOutput = program.emit(); - reportDiagnostics(emitOutput.diagnostics); + reportDiagnostics(emitOutput.diagnostics, compilerHost); // If the emitter didn't emit anything, then pass that value along. if (emitOutput.emitSkipped) { @@ -587,7 +591,7 @@ namespace ts { let currentDirectory = sys.getCurrentDirectory(); let file = normalizePath(combinePaths(currentDirectory, "tsconfig.json")); if (sys.fileExists(file)) { - reportDiagnostic(createCompilerDiagnostic(Diagnostics.A_tsconfig_json_file_is_already_defined_at_Colon_0, file)); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.A_tsconfig_json_file_is_already_defined_at_Colon_0, file), /* compilerHost */ undefined); } else { let compilerOptions = extend(options, defaultInitCompilerOptions); @@ -602,7 +606,7 @@ namespace ts { } sys.writeFile(file, JSON.stringify(configurations, undefined, 4)); - reportDiagnostic(createCompilerDiagnostic(Diagnostics.Successfully_created_a_tsconfig_json_file)); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.Successfully_created_a_tsconfig_json_file), /* compilerHost */ undefined); } return; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index d8de6e0c303..c019bffc812 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2183,6 +2183,12 @@ namespace ts { return result; } + export function toRelativePath(absoluteOrRelativePath: string, basePath: string, getCanonicalFileName: (path: string) => string): string { + return !isRootedDiskPath(absoluteOrRelativePath) + ? absoluteOrRelativePath + : getRelativePathToDirectoryOrUrl(basePath, absoluteOrRelativePath, basePath, getCanonicalFileName, /* isAbsolutePathAnUrl */ false); + } + const carriageReturnLineFeed = "\r\n"; const lineFeed = "\n"; export function getNewLineCharacter(options: CompilerOptions): string { diff --git a/src/harness/projectsRunner.ts b/src/harness/projectsRunner.ts index 74539b70848..52d4b85f6b8 100644 --- a/src/harness/projectsRunner.ts +++ b/src/harness/projectsRunner.ts @@ -234,12 +234,15 @@ class ProjectRunner extends RunnerBase { } function writeFile(fileName: string, data: string, writeByteOrderMark: boolean) { + // convert file name to rooted name + // if filename is not rooted - concat it with project root and then expand project root relative to current directory let diskFileName = ts.isRootedDiskPath(fileName) ? fileName - : ts.normalizeSlashes(testCase.projectRoot) + "/" + ts.normalizeSlashes(fileName); + : Harness.IO.resolvePath(ts.normalizeSlashes(testCase.projectRoot) + "/" + ts.normalizeSlashes(fileName)); - let diskRelativeName = ts.getRelativePathToDirectoryOrUrl(testCase.projectRoot, diskFileName, - getCurrentDirectory(), Harness.Compiler.getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); + let currentDirectory = getCurrentDirectory(); + // compute file name relative to current directory (expanded project root) + let diskRelativeName = ts.getRelativePathToDirectoryOrUrl(currentDirectory, diskFileName, currentDirectory, Harness.Compiler.getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); if (ts.isRootedDiskPath(diskRelativeName) || diskRelativeName.substr(0, 3) === "../") { // If the generated output file resides in the parent folder or is rooted path, // we need to instead create files that can live in the project reference folder @@ -373,8 +376,12 @@ class ProjectRunner extends RunnerBase { runTest: testCase.runTest, bug: testCase.bug, rootDir: testCase.rootDir, - resolvedInputFiles: ts.map(compilerResult.program.getSourceFiles(), inputFile => inputFile.fileName), - emittedFiles: ts.map(compilerResult.outputFiles, outputFile => outputFile.emittedFileName) + resolvedInputFiles: ts.map(compilerResult.program.getSourceFiles(), inputFile => { + return ts.toRelativePath(inputFile.fileName, getCurrentDirectory(), path => Harness.Compiler.getCanonicalFileName(path)); + }), + emittedFiles: ts.map(compilerResult.outputFiles, outputFile => { + return ts.toRelativePath(outputFile.emittedFileName, getCurrentDirectory(), path => Harness.Compiler.getCanonicalFileName(path)); + }) }; return resolutionInfo; From 93e942a6de8d55f0d7eb7c9c6a5ad8a71eb191e4 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 29 Oct 2015 13:52:38 -0700 Subject: [PATCH 2/7] FileMap now internally stores absolute normalized file names --- src/compiler/core.ts | 4 ++-- src/compiler/program.ts | 4 ++-- src/server/editorServices.ts | 4 ++-- src/services/services.ts | 10 ++++++---- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 68c9bd06825..9ad692f3b6c 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -17,7 +17,7 @@ namespace ts { True = -1 } - export function createFileMap(getCanonicalFileName: (fileName: string) => string): FileMap { + export function createFileMap(getCanonicalFileName: (fileName: string) => string, currentDirectory: string): FileMap { let files: Map = {}; return { get, @@ -50,7 +50,7 @@ namespace ts { } function normalizeKey(key: string) { - return getCanonicalFileName(normalizeSlashes(key)); + return getCanonicalFileName(getNormalizedAbsolutePath(key, currentDirectory)); } function clear() { diff --git a/src/compiler/program.ts b/src/compiler/program.ts index e9f659792f9..5a27aa108dc 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -346,10 +346,10 @@ namespace ts { ? ((moduleNames: string[], containingFile: string) => host.resolveModuleNames(moduleNames, containingFile)) : ((moduleNames: string[], containingFile: string) => map(moduleNames, moduleName => resolveModuleName(moduleName, containingFile, options, host).resolvedModule)); - let filesByName = createFileMap(getCanonicalFileName); + let filesByName = createFileMap(getCanonicalFileName, currentDirectory); // stores 'filename -> file association' ignoring case // used to track cases when two file names differ only in casing - let filesByNameIgnoreCase = host.useCaseSensitiveFileNames() ? createFileMap(fileName => fileName.toLowerCase()) : undefined; + let filesByNameIgnoreCase = host.useCaseSensitiveFileNames() ? createFileMap(fileName => fileName.toLowerCase(), currentDirectory) : undefined; if (oldProgram) { // check properties that can affect structure of the program or module resolution strategy diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index d2e16487d07..9384244c831 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -93,8 +93,8 @@ namespace ts.server { constructor(public host: ServerHost, public project: Project) { const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames); - this.resolvedModuleNames = createFileMap>(getCanonicalFileName); - this.filenameToScript = createFileMap(getCanonicalFileName); + this.resolvedModuleNames = createFileMap>(getCanonicalFileName, host.getCurrentDirectory()); + this.filenameToScript = createFileMap(getCanonicalFileName, host.getCurrentDirectory()); this.moduleResolutionHost = { fileExists: fileName => this.fileExists(fileName), readFile: fileName => this.host.readFile(fileName) diff --git a/src/services/services.ts b/src/services/services.ts index 5815e2ab1d1..dbb208be5f5 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1698,7 +1698,7 @@ namespace ts { constructor(private host: LanguageServiceHost, getCanonicalFileName: (fileName: string) => string) { // script id => script index - this.fileNameToEntry = createFileMap(getCanonicalFileName); + this.fileNameToEntry = createFileMap(getCanonicalFileName, host.getCurrentDirectory()); // Initialize the list with the root file names let rootFileNames = host.getScriptFileNames(); @@ -1993,7 +1993,7 @@ namespace ts { } - export function createDocumentRegistry(useCaseSensitiveFileNames?: boolean): DocumentRegistry { + export function createDocumentRegistry(useCaseSensitiveFileNames?: boolean, currentDirectory = ""): DocumentRegistry { // Maps from compiler setting target (ES3, ES5, etc.) to all the cached documents we have // for those settings. let buckets: Map> = {}; @@ -2007,7 +2007,7 @@ namespace ts { let key = getKeyFromCompilationSettings(settings); let bucket = lookUp(buckets, key); if (!bucket && createIfMissing) { - buckets[key] = bucket = createFileMap(getCanonicalFileName); + buckets[key] = bucket = createFileMap(getCanonicalFileName, currentDirectory); } return bucket; } @@ -2556,7 +2556,9 @@ namespace ts { } } - export function createLanguageService(host: LanguageServiceHost, documentRegistry: DocumentRegistry = createDocumentRegistry()): LanguageService { + export function createLanguageService(host: LanguageServiceHost, + documentRegistry: DocumentRegistry = createDocumentRegistry(host.useCaseSensitiveFileNames && host.useCaseSensitiveFileNames(), host.getCurrentDirectory())): LanguageService { + let syntaxTreeCache: SyntaxTreeCache = new SyntaxTreeCache(host); let ruleProvider: formatting.RulesProvider; let program: Program; From e1b4f01e77062de4ca87feabac34959edbc10627 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 29 Oct 2015 14:54:56 -0700 Subject: [PATCH 3/7] introduce Path as branded string type, switch FileMap to use Path --- src/compiler/core.ts | 56 ++++++++++++++++------------ src/compiler/program.ts | 44 +++++++++++----------- src/compiler/types.ts | 16 +++++--- src/compiler/utilities.ts | 1 - src/server/editorServices.ts | 42 ++++++++++++--------- src/services/services.ts | 72 +++++++++++++++++++----------------- 6 files changed, 129 insertions(+), 102 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 9ad692f3b6c..c0e5739263e 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -17,45 +17,55 @@ namespace ts { True = -1 } - export function createFileMap(getCanonicalFileName: (fileName: string) => string, currentDirectory: string): FileMap { + export function createFileMap(keyMapper?: (key: string) => string): FileMap { let files: Map = {}; return { - get, - set, - contains, - remove, - clear, - forEachValue: forEachValueInMap + getPath, + setPath, + containsPath, + removePath, + forEachValue: forEachValueInMap, + clear }; - function set(fileName: string, value: T) { - files[normalizeKey(fileName)] = value; + function forEachValueInMap(f: (key: Path, value: T) => void) { + for (let key in files) { + f(key, files[key]); + } } - function get(fileName: string) { - return files[normalizeKey(fileName)]; + // path should already be well-formed so it does not need to be normalized + function getPath(path: Path): T { + return files[toKey(path)]; } - function contains(fileName: string) { - return hasProperty(files, normalizeKey(fileName)); + function setPath(path: Path, value: T) { + files[toKey(path)] = value; } - function remove (fileName: string) { - let key = normalizeKey(fileName); + function containsPath(path: Path) { + return hasProperty(files, toKey(path)); + } + + function removePath(path: Path) { + const key = toKey(path); delete files[key]; } - function forEachValueInMap(f: (value: T) => void) { - forEachValue(files, f); - } - - function normalizeKey(key: string) { - return getCanonicalFileName(getNormalizedAbsolutePath(key, currentDirectory)); - } - function clear() { files = {}; } + + function toKey(path: Path): string { + return keyMapper ? keyMapper(path) : path; + } + } + + export function toPath(fileName: string, basePath: string, getCanonicalFileName: (path: string) => string): Path { + const nonCanonicalizedPath = isRootedDiskPath(fileName) + ? normalizePath(fileName) + : getNormalizedAbsolutePath(fileName, basePath); + return getCanonicalFileName(nonCanonicalizedPath); } export const enum Comparison { diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 5a27aa108dc..c579f66253b 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -346,10 +346,10 @@ namespace ts { ? ((moduleNames: string[], containingFile: string) => host.resolveModuleNames(moduleNames, containingFile)) : ((moduleNames: string[], containingFile: string) => map(moduleNames, moduleName => resolveModuleName(moduleName, containingFile, options, host).resolvedModule)); - let filesByName = createFileMap(getCanonicalFileName, currentDirectory); + let filesByName = createFileMap(); // stores 'filename -> file association' ignoring case // used to track cases when two file names differ only in casing - let filesByNameIgnoreCase = host.useCaseSensitiveFileNames() ? createFileMap(fileName => fileName.toLowerCase(), currentDirectory) : undefined; + let filesByNameIgnoreCase = host.useCaseSensitiveFileNames() ? createFileMap(fileName => fileName.toLowerCase()) : undefined; if (oldProgram) { // check properties that can affect structure of the program or module resolution strategy @@ -384,7 +384,7 @@ namespace ts { program = { getRootFileNames: () => rootNames, - getSourceFile: getSourceFile, + getSourceFile, getSourceFiles: () => files, getCompilerOptions: () => options, getSyntacticDiagnostics, @@ -435,7 +435,7 @@ namespace ts { // check if program source files has changed in the way that can affect structure of the program let newSourceFiles: SourceFile[] = []; - let normalizedAbsoluteFileNames: string[] = []; + let filePaths: Path[] = []; let modifiedSourceFiles: SourceFile[] = []; for (let oldSourceFile of oldProgram.getSourceFiles()) { @@ -444,8 +444,8 @@ namespace ts { return false; } - const normalizedAbsolutePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory); - normalizedAbsoluteFileNames.push(normalizedAbsolutePath); + newSourceFile.path = oldSourceFile.path; + filePaths.push(newSourceFile.path); if (oldSourceFile !== newSourceFile) { if (oldSourceFile.hasNoDefaultLib !== newSourceFile.hasNoDefaultLib) { @@ -469,7 +469,7 @@ namespace ts { if (resolveModuleNamesWorker) { let moduleNames = map(newSourceFile.imports, name => name.text); - let resolutions = resolveModuleNamesWorker(moduleNames, normalizedAbsolutePath); + let resolutions = resolveModuleNamesWorker(moduleNames, getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory)); // ensure that module resolution results are still correct for (let i = 0; i < moduleNames.length; ++i) { let newResolution = resolutions[i]; @@ -500,7 +500,7 @@ namespace ts { // update fileName -> file mapping for (let i = 0, len = newSourceFiles.length; i < len; ++i) { - filesByName.set(normalizedAbsoluteFileNames[i], newSourceFiles[i]); + filesByName.setPath(filePaths[i], newSourceFiles[i]); } files = newSourceFiles; @@ -570,7 +570,7 @@ namespace ts { } function getSourceFile(fileName: string): SourceFile { - return filesByName.get(getNormalizedAbsolutePath(fileName, currentDirectory)); + return filesByName.getPath(toPath(fileName, currentDirectory, getCanonicalFileName)); } function getDiagnosticsHelper( @@ -741,7 +741,7 @@ namespace ts { diagnostic = Diagnostics.File_0_has_unsupported_extension_The_only_supported_extensions_are_1; diagnosticArgument = [fileName, "'" + supportedExtensions.join("', '") + "'"]; } - else if (!findSourceFile(fileName, getNormalizedAbsolutePath(fileName, currentDirectory), isDefaultLib, refFile, refPos, refEnd)) { + else if (!findSourceFile(fileName, toPath(fileName, currentDirectory, getCanonicalFileName), isDefaultLib, refFile, refPos, refEnd)) { diagnostic = Diagnostics.File_0_not_found; diagnosticArgument = [fileName]; } @@ -751,13 +751,13 @@ namespace ts { } } else { - let nonTsFile: SourceFile = options.allowNonTsExtensions && findSourceFile(fileName, getNormalizedAbsolutePath(fileName, currentDirectory), isDefaultLib, refFile, refPos, refEnd); + let nonTsFile: SourceFile = options.allowNonTsExtensions && findSourceFile(fileName, toPath(fileName, currentDirectory, getCanonicalFileName), isDefaultLib, refFile, refPos, refEnd); if (!nonTsFile) { if (options.allowNonTsExtensions) { diagnostic = Diagnostics.File_0_not_found; diagnosticArgument = [fileName]; } - else if (!forEach(supportedExtensions, extension => findSourceFile(fileName + extension, getNormalizedAbsolutePath(fileName + extension, currentDirectory), isDefaultLib, refFile, refPos, refEnd))) { + else if (!forEach(supportedExtensions, extension => findSourceFile(fileName + extension, toPath(fileName + extension, currentDirectory, getCanonicalFileName), isDefaultLib, refFile, refPos, refEnd))) { diagnostic = Diagnostics.File_0_not_found; fileName += ".ts"; diagnosticArgument = [fileName]; @@ -786,9 +786,9 @@ namespace ts { } // Get source file from normalized fileName - function findSourceFile(fileName: string, normalizedAbsolutePath: string, isDefaultLib: boolean, refFile?: SourceFile, refPos?: number, refEnd?: number): SourceFile { - if (filesByName.contains(normalizedAbsolutePath)) { - const file = filesByName.get(normalizedAbsolutePath); + function findSourceFile(fileName: string, normalizedAbsolutePath: Path, isDefaultLib: boolean, refFile?: SourceFile, refPos?: number, refEnd?: number): SourceFile { + if (filesByName.containsPath(normalizedAbsolutePath)) { + const file = filesByName.getPath(normalizedAbsolutePath); // try to check if we've already seen this file but with a different casing in path // NOTE: this only makes sense for case-insensitive file systems if (file && options.forceConsistentCasingInFileNames && getNormalizedAbsolutePath(file.fileName, currentDirectory) !== normalizedAbsolutePath) { @@ -809,16 +809,18 @@ namespace ts { } }); - filesByName.set(normalizedAbsolutePath, file); + filesByName.setPath(normalizedAbsolutePath, file); if (file) { + file.path = normalizedAbsolutePath; + if (host.useCaseSensitiveFileNames()) { // for case-sensitive file systems check if we've already seen some file with similar filename ignoring case - const existingFile = filesByNameIgnoreCase.get(normalizedAbsolutePath); + const existingFile = filesByNameIgnoreCase.getPath(normalizedAbsolutePath); if (existingFile) { reportFileNamesDifferOnlyInCasingError(fileName, existingFile.fileName, refFile, refPos, refEnd); } else { - filesByNameIgnoreCase.set(normalizedAbsolutePath, file); + filesByNameIgnoreCase.setPath(normalizedAbsolutePath, file); } } @@ -865,11 +867,7 @@ namespace ts { let resolution = resolutions[i]; setResolvedModule(file, moduleNames[i], resolution); if (resolution && !options.noResolve) { - const absoluteImportPath = isRootedDiskPath(resolution.resolvedFileName) - ? resolution.resolvedFileName - : getNormalizedAbsolutePath(resolution.resolvedFileName, currentDirectory); - - const importedFile = findSourceFile(resolution.resolvedFileName, absoluteImportPath, /* isDefaultLib */ false, file, skipTrivia(file.text, file.imports[i].pos), file.imports[i].end); + const importedFile = findSourceFile(resolution.resolvedFileName, toPath(resolution.resolvedFileName, currentDirectory, getCanonicalFileName), /* isDefaultLib */ false, file, skipTrivia(file.text, file.imports[i].pos), file.imports[i].end); if (importedFile && resolution.isExternalLibraryImport) { if (!isExternalModule(importedFile)) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index ae08ba7ff32..1e0b8129ac4 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3,12 +3,17 @@ namespace ts { [index: string]: T; } + // branded string type used to store absolute, normalized and canonicalized paths + // arbitrary file name can be converted to Path via toPath function + export type Path = string & { __pathBrand: any }; + export interface FileMap { - get(fileName: string): T; - set(fileName: string, value: T): void; - contains(fileName: string): boolean; - remove(fileName: string): void; - forEachValue(f: (v: T) => void): void; + getPath(fileName: Path): T; + setPath(fileName: Path, value: T): void; + containsPath(fileName: Path): boolean; + removePath(fileName: Path): void; + + forEachValue(f: (key: Path, v: T) => void): void; clear(): void; } @@ -1250,6 +1255,7 @@ namespace ts { endOfFileToken: Node; fileName: string; + /* internal */ path: Path; text: string; amdDependencies: {path: string; name: string}[]; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index c019bffc812..fb6d2686b15 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1357,7 +1357,6 @@ namespace ts { export function tryResolveScriptReference(host: ScriptReferenceHost, sourceFile: SourceFile, reference: FileReference) { if (!host.getCompilerOptions().noResolve) { let referenceFileName = isRootedDiskPath(reference.fileName) ? reference.fileName : combinePaths(getDirectoryPath(sourceFile.fileName), reference.fileName); - referenceFileName = getNormalizedAbsolutePath(referenceFileName, host.getCurrentDirectory()); return host.getSourceFile(referenceFileName); } } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 9384244c831..3bc69b9a179 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -33,8 +33,10 @@ namespace ts.server { defaultProject: Project; // project to use by default for file fileWatcher: FileWatcher; formatCodeOptions = ts.clone(CompilerService.defaultFormatCodeOptions); + path: Path; constructor(private host: ServerHost, public fileName: string, public content: string, public isOpen = false) { + this.path = toPath(fileName, host.getCurrentDirectory(), createGetCanonicalFileName(host.useCaseSensitiveFileNames)); this.svc = ScriptVersionCache.fromString(host, content); } @@ -90,11 +92,12 @@ namespace ts.server { roots: ScriptInfo[] = []; private resolvedModuleNames: ts.FileMap>; private moduleResolutionHost: ts.ModuleResolutionHost; + private getCanonicalFileName: (fileName: string) => string; constructor(public host: ServerHost, public project: Project) { - const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames); - this.resolvedModuleNames = createFileMap>(getCanonicalFileName, host.getCurrentDirectory()); - this.filenameToScript = createFileMap(getCanonicalFileName, host.getCurrentDirectory()); + this.getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames); + this.resolvedModuleNames = createFileMap>(); + this.filenameToScript = createFileMap(); this.moduleResolutionHost = { fileExists: fileName => this.fileExists(fileName), readFile: fileName => this.host.readFile(fileName) @@ -102,7 +105,8 @@ namespace ts.server { } resolveModuleNames(moduleNames: string[], containingFile: string): ResolvedModule[] { - let currentResolutionsInFile = this.resolvedModuleNames.get(containingFile); + let path = toPath(containingFile, this.host.getCurrentDirectory(), this.getCanonicalFileName); + let currentResolutionsInFile = this.resolvedModuleNames.getPath(path); let newResolutions: Map = {}; let resolvedModules: ResolvedModule[] = []; @@ -131,7 +135,7 @@ namespace ts.server { } // replace old results with a new one - this.resolvedModuleNames.set(containingFile, newResolutions); + this.resolvedModuleNames.setPath(path, newResolutions); return resolvedModules; function moduleResolutionIsValid(resolution: TimestampedResolvedModule): boolean { @@ -201,34 +205,35 @@ namespace ts.server { removeReferencedFile(info: ScriptInfo) { if (!info.isOpen) { - this.filenameToScript.remove(info.fileName); - this.resolvedModuleNames.remove(info.fileName); + this.filenameToScript.removePath(info.path); + this.resolvedModuleNames.removePath(info.path); } } getScriptInfo(filename: string): ScriptInfo { - let scriptInfo = this.filenameToScript.get(filename); + let path = toPath(filename, this.host.getCurrentDirectory(), this.getCanonicalFileName); + let scriptInfo = this.filenameToScript.getPath(path); if (!scriptInfo) { scriptInfo = this.project.openReferencedFile(filename); if (scriptInfo) { - this.filenameToScript.set(scriptInfo.fileName, scriptInfo); + this.filenameToScript.setPath(path, scriptInfo); } } return scriptInfo; } addRoot(info: ScriptInfo) { - if (!this.filenameToScript.contains(info.fileName)) { - this.filenameToScript.set(info.fileName, info); + if (!this.filenameToScript.containsPath(info.path)) { + this.filenameToScript.setPath(info.path, info); this.roots.push(info); } } removeRoot(info: ScriptInfo) { - if (!this.filenameToScript.contains(info.fileName)) { - this.filenameToScript.remove(info.fileName); + if (!this.filenameToScript.containsPath(info.path)) { + this.filenameToScript.removePath(info.path); this.roots = copyListRemovingItem(info, this.roots); - this.resolvedModuleNames.remove(info.fileName); + this.resolvedModuleNames.removePath(info.path); } } @@ -277,7 +282,8 @@ namespace ts.server { * @param line 1 based index */ lineToTextSpan(filename: string, line: number): ts.TextSpan { - const script: ScriptInfo = this.filenameToScript.get(filename); + let path = toPath(filename, this.host.getCurrentDirectory(), this.getCanonicalFileName); + const script: ScriptInfo = this.filenameToScript.getPath(path); const index = script.snap().index; const lineInfo = index.lineNumberToInfo(line + 1); @@ -297,7 +303,8 @@ namespace ts.server { * @param offset 1 based index */ lineOffsetToPosition(filename: string, line: number, offset: number): number { - const script: ScriptInfo = this.filenameToScript.get(filename); + let path = toPath(filename, this.host.getCurrentDirectory(), this.getCanonicalFileName); + const script: ScriptInfo = this.filenameToScript.getPath(path); const index = script.snap().index; const lineInfo = index.lineNumberToInfo(line); @@ -310,7 +317,8 @@ namespace ts.server { * @param offset 1-based index */ positionToLineOffset(filename: string, position: number): ILineInfo { - const script: ScriptInfo = this.filenameToScript.get(filename); + let path = toPath(filename, this.host.getCurrentDirectory(), this.getCanonicalFileName); + const script: ScriptInfo = this.filenameToScript.getPath(path); const index = script.snap().index; const lineOffset = index.charOffsetToLineNumberAndPos(position); return { line: lineOffset.line, offset: lineOffset.offset + 1 }; diff --git a/src/services/services.ts b/src/services/services.ts index dbb208be5f5..954a0a37dc9 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -773,6 +773,7 @@ namespace ts { class SourceFileObject extends NodeObject implements SourceFile { public _declarationBrand: any; public fileName: string; + public path: Path; public text: string; public scriptSnapshot: IScriptSnapshot; public lineMap: number[]; @@ -1695,15 +1696,17 @@ namespace ts { class HostCache { private fileNameToEntry: FileMap; private _compilationSettings: CompilerOptions; + private currentDirectory: string; - constructor(private host: LanguageServiceHost, getCanonicalFileName: (fileName: string) => string) { + constructor(private host: LanguageServiceHost, private getCanonicalFileName: (fileName: string) => string) { // script id => script index - this.fileNameToEntry = createFileMap(getCanonicalFileName, host.getCurrentDirectory()); + this.currentDirectory = host.getCurrentDirectory(); + this.fileNameToEntry = createFileMap(); // Initialize the list with the root file names let rootFileNames = host.getScriptFileNames(); for (let fileName of rootFileNames) { - this.createEntry(fileName); + this.createEntry(fileName, toPath(fileName, this.currentDirectory, getCanonicalFileName)); } // store the compilation settings @@ -1714,7 +1717,7 @@ namespace ts { return this._compilationSettings; } - private createEntry(fileName: string) { + private createEntry(fileName: string, path: Path) { let entry: HostFileInformation; let scriptSnapshot = this.host.getScriptSnapshot(fileName); if (scriptSnapshot) { @@ -1725,30 +1728,31 @@ namespace ts { }; } - this.fileNameToEntry.set(fileName, entry); + this.fileNameToEntry.setPath(path, entry); return entry; } - private getEntry(fileName: string): HostFileInformation { - return this.fileNameToEntry.get(fileName); + private getEntry(path: Path): HostFileInformation { + return this.fileNameToEntry.getPath(path); } - private contains(fileName: string): boolean { - return this.fileNameToEntry.contains(fileName); + private contains(path: Path): boolean { + return this.fileNameToEntry.containsPath(path); } public getOrCreateEntry(fileName: string): HostFileInformation { - if (this.contains(fileName)) { - return this.getEntry(fileName); + let path = toPath(fileName, this.currentDirectory, this.getCanonicalFileName) + if (this.contains(path)) { + return this.getEntry(path); } - return this.createEntry(fileName); + return this.createEntry(fileName, path); } public getRootFileNames(): string[] { let fileNames: string[] = []; - this.fileNameToEntry.forEachValue(value => { + this.fileNameToEntry.forEachValue((path, value) => { if (value) { fileNames.push(value.hostFileName); } @@ -1757,13 +1761,13 @@ namespace ts { return fileNames; } - public getVersion(fileName: string): string { - let file = this.getEntry(fileName); + public getVersion(path: Path): string { + let file = this.getEntry(path); return file && file.version; } - public getScriptSnapshot(fileName: string): IScriptSnapshot { - let file = this.getEntry(fileName); + public getScriptSnapshot(path: Path): IScriptSnapshot { + let file = this.getEntry(path); return file && file.scriptSnapshot; } } @@ -2007,7 +2011,7 @@ namespace ts { let key = getKeyFromCompilationSettings(settings); let bucket = lookUp(buckets, key); if (!bucket && createIfMissing) { - buckets[key] = bucket = createFileMap(getCanonicalFileName, currentDirectory); + buckets[key] = bucket = createFileMap(); } return bucket; } @@ -2016,14 +2020,13 @@ namespace ts { let bucketInfoArray = Object.keys(buckets).filter(name => name && name.charAt(0) === '_').map(name => { let entries = lookUp(buckets, name); let sourceFiles: { name: string; refCount: number; references: string[]; }[] = []; - for (let i in entries) { - let entry = entries.get(i); + entries.forEachValue((key, entry) => { sourceFiles.push({ - name: i, + name: key, refCount: entry.languageServiceRefCount, references: entry.owners.slice(0) }); - } + }); sourceFiles.sort((x, y) => y.refCount - x.refCount); return { bucket: name, @@ -2049,7 +2052,8 @@ namespace ts { acquiring: boolean): SourceFile { let bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ true); - let entry = bucket.get(fileName); + let path = toPath(fileName, currentDirectory, getCanonicalFileName); + let entry = bucket.getPath(path); if (!entry) { Debug.assert(acquiring, "How could we be trying to update a document that the registry doesn't have?"); @@ -2061,7 +2065,7 @@ namespace ts { languageServiceRefCount: 0, owners: [] }; - bucket.set(fileName, entry); + bucket.setPath(path, entry); } else { // We have an entry for this file. However, it may be for a different version of @@ -2089,12 +2093,14 @@ namespace ts { let bucket = getBucketForCompilationSettings(compilationSettings, false); Debug.assert(bucket !== undefined); - let entry = bucket.get(fileName); + let path = toPath(fileName, currentDirectory, getCanonicalFileName); + + let entry = bucket.getPath(path); entry.languageServiceRefCount--; Debug.assert(entry.languageServiceRefCount >= 0); if (entry.languageServiceRefCount === 0) { - bucket.remove(fileName); + bucket.removePath(path); } } @@ -2567,6 +2573,7 @@ namespace ts { let useCaseSensitivefileNames = false; let cancellationToken = new CancellationTokenObject(host.getCancellationToken && host.getCancellationToken()); + let currentDirectory = host.getCurrentDirectory(); // Check if the localized messages json is set, otherwise query the host for it if (!localizedDiagnosticMessages && host.getLocalizedDiagnosticMessages) { localizedDiagnosticMessages = host.getLocalizedDiagnosticMessages(); @@ -2581,8 +2588,7 @@ namespace ts { let getCanonicalFileName = createGetCanonicalFileName(useCaseSensitivefileNames); function getValidSourceFile(fileName: string): SourceFile { - fileName = normalizeSlashes(fileName); - let sourceFile = program.getSourceFile(getCanonicalFileName(fileName)); + let sourceFile = program.getSourceFile(fileName); if (!sourceFile) { throw new Error("Could not find file: '" + fileName + "'."); } @@ -2643,7 +2649,7 @@ namespace ts { getNewLine: () => getNewLineOrDefaultFromHost(host), getDefaultLibFileName: (options) => host.getDefaultLibFileName(options), writeFile: (fileName, data, writeByteOrderMark) => { }, - getCurrentDirectory: () => host.getCurrentDirectory(), + getCurrentDirectory: () => currentDirectory, fileExists: (fileName): boolean => { // stub missing host functionality Debug.assert(!host.resolveModuleNames); @@ -2667,9 +2673,8 @@ namespace ts { if (program) { let oldSourceFiles = program.getSourceFiles(); for (let oldSourceFile of oldSourceFiles) { - let fileName = oldSourceFile.fileName; - if (!newProgram.getSourceFile(fileName) || changesInCompilationSettingsAffectSyntax) { - documentRegistry.releaseDocument(fileName, oldSettings); + if (!newProgram.getSourceFile(oldSourceFile.fileName) || changesInCompilationSettingsAffectSyntax) { + documentRegistry.releaseDocument(oldSourceFile.fileName, oldSettings); } } } @@ -2734,7 +2739,8 @@ namespace ts { } function sourceFileUpToDate(sourceFile: SourceFile): boolean { - return sourceFile && sourceFile.version === hostCache.getVersion(sourceFile.fileName); + let path = sourceFile.path || toPath(sourceFile.fileName, currentDirectory, getCanonicalFileName); + return sourceFile && sourceFile.version === hostCache.getVersion(path); } function programUpToDate(): boolean { From 534bb62c592cb2f53c178515e9b592a710d86197 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 29 Oct 2015 16:43:12 -0700 Subject: [PATCH 4/7] remove 'path' suffix from FileMap methods --- src/compiler/core.ts | 16 ++++++++-------- src/compiler/program.ts | 14 +++++++------- src/compiler/types.ts | 8 ++++---- src/server/editorServices.ts | 28 ++++++++++++++-------------- src/services/services.ts | 14 +++++++------- 5 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index c0e5739263e..068ab1865bb 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -20,10 +20,10 @@ namespace ts { export function createFileMap(keyMapper?: (key: string) => string): FileMap { let files: Map = {}; return { - getPath, - setPath, - containsPath, - removePath, + get, + set, + contains, + remove, forEachValue: forEachValueInMap, clear }; @@ -35,19 +35,19 @@ namespace ts { } // path should already be well-formed so it does not need to be normalized - function getPath(path: Path): T { + function get(path: Path): T { return files[toKey(path)]; } - function setPath(path: Path, value: T) { + function set(path: Path, value: T) { files[toKey(path)] = value; } - function containsPath(path: Path) { + function contains(path: Path) { return hasProperty(files, toKey(path)); } - function removePath(path: Path) { + function remove(path: Path) { const key = toKey(path); delete files[key]; } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index c579f66253b..4822a86b527 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -500,7 +500,7 @@ namespace ts { // update fileName -> file mapping for (let i = 0, len = newSourceFiles.length; i < len; ++i) { - filesByName.setPath(filePaths[i], newSourceFiles[i]); + filesByName.set(filePaths[i], newSourceFiles[i]); } files = newSourceFiles; @@ -570,7 +570,7 @@ namespace ts { } function getSourceFile(fileName: string): SourceFile { - return filesByName.getPath(toPath(fileName, currentDirectory, getCanonicalFileName)); + return filesByName.get(toPath(fileName, currentDirectory, getCanonicalFileName)); } function getDiagnosticsHelper( @@ -787,8 +787,8 @@ namespace ts { // Get source file from normalized fileName function findSourceFile(fileName: string, normalizedAbsolutePath: Path, isDefaultLib: boolean, refFile?: SourceFile, refPos?: number, refEnd?: number): SourceFile { - if (filesByName.containsPath(normalizedAbsolutePath)) { - const file = filesByName.getPath(normalizedAbsolutePath); + if (filesByName.contains(normalizedAbsolutePath)) { + const file = filesByName.get(normalizedAbsolutePath); // try to check if we've already seen this file but with a different casing in path // NOTE: this only makes sense for case-insensitive file systems if (file && options.forceConsistentCasingInFileNames && getNormalizedAbsolutePath(file.fileName, currentDirectory) !== normalizedAbsolutePath) { @@ -809,18 +809,18 @@ namespace ts { } }); - filesByName.setPath(normalizedAbsolutePath, file); + filesByName.set(normalizedAbsolutePath, file); if (file) { file.path = normalizedAbsolutePath; if (host.useCaseSensitiveFileNames()) { // for case-sensitive file systems check if we've already seen some file with similar filename ignoring case - const existingFile = filesByNameIgnoreCase.getPath(normalizedAbsolutePath); + const existingFile = filesByNameIgnoreCase.get(normalizedAbsolutePath); if (existingFile) { reportFileNamesDifferOnlyInCasingError(fileName, existingFile.fileName, refFile, refPos, refEnd); } else { - filesByNameIgnoreCase.setPath(normalizedAbsolutePath, file); + filesByNameIgnoreCase.set(normalizedAbsolutePath, file); } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 1e0b8129ac4..b58e6307202 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -8,10 +8,10 @@ namespace ts { export type Path = string & { __pathBrand: any }; export interface FileMap { - getPath(fileName: Path): T; - setPath(fileName: Path, value: T): void; - containsPath(fileName: Path): boolean; - removePath(fileName: Path): void; + get(fileName: Path): T; + set(fileName: Path, value: T): void; + contains(fileName: Path): boolean; + remove(fileName: Path): void; forEachValue(f: (key: Path, v: T) => void): void; clear(): void; diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 3bc69b9a179..df3b63903c5 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -106,7 +106,7 @@ namespace ts.server { resolveModuleNames(moduleNames: string[], containingFile: string): ResolvedModule[] { let path = toPath(containingFile, this.host.getCurrentDirectory(), this.getCanonicalFileName); - let currentResolutionsInFile = this.resolvedModuleNames.getPath(path); + let currentResolutionsInFile = this.resolvedModuleNames.get(path); let newResolutions: Map = {}; let resolvedModules: ResolvedModule[] = []; @@ -135,7 +135,7 @@ namespace ts.server { } // replace old results with a new one - this.resolvedModuleNames.setPath(path, newResolutions); + this.resolvedModuleNames.set(path, newResolutions); return resolvedModules; function moduleResolutionIsValid(resolution: TimestampedResolvedModule): boolean { @@ -205,35 +205,35 @@ namespace ts.server { removeReferencedFile(info: ScriptInfo) { if (!info.isOpen) { - this.filenameToScript.removePath(info.path); - this.resolvedModuleNames.removePath(info.path); + this.filenameToScript.remove(info.path); + this.resolvedModuleNames.remove(info.path); } } getScriptInfo(filename: string): ScriptInfo { let path = toPath(filename, this.host.getCurrentDirectory(), this.getCanonicalFileName); - let scriptInfo = this.filenameToScript.getPath(path); + let scriptInfo = this.filenameToScript.get(path); if (!scriptInfo) { scriptInfo = this.project.openReferencedFile(filename); if (scriptInfo) { - this.filenameToScript.setPath(path, scriptInfo); + this.filenameToScript.set(path, scriptInfo); } } return scriptInfo; } addRoot(info: ScriptInfo) { - if (!this.filenameToScript.containsPath(info.path)) { - this.filenameToScript.setPath(info.path, info); + if (!this.filenameToScript.contains(info.path)) { + this.filenameToScript.set(info.path, info); this.roots.push(info); } } removeRoot(info: ScriptInfo) { - if (!this.filenameToScript.containsPath(info.path)) { - this.filenameToScript.removePath(info.path); + if (!this.filenameToScript.contains(info.path)) { + this.filenameToScript.remove(info.path); this.roots = copyListRemovingItem(info, this.roots); - this.resolvedModuleNames.removePath(info.path); + this.resolvedModuleNames.remove(info.path); } } @@ -283,7 +283,7 @@ namespace ts.server { */ lineToTextSpan(filename: string, line: number): ts.TextSpan { let path = toPath(filename, this.host.getCurrentDirectory(), this.getCanonicalFileName); - const script: ScriptInfo = this.filenameToScript.getPath(path); + const script: ScriptInfo = this.filenameToScript.get(path); const index = script.snap().index; const lineInfo = index.lineNumberToInfo(line + 1); @@ -304,7 +304,7 @@ namespace ts.server { */ lineOffsetToPosition(filename: string, line: number, offset: number): number { let path = toPath(filename, this.host.getCurrentDirectory(), this.getCanonicalFileName); - const script: ScriptInfo = this.filenameToScript.getPath(path); + const script: ScriptInfo = this.filenameToScript.get(path); const index = script.snap().index; const lineInfo = index.lineNumberToInfo(line); @@ -318,7 +318,7 @@ namespace ts.server { */ positionToLineOffset(filename: string, position: number): ILineInfo { let path = toPath(filename, this.host.getCurrentDirectory(), this.getCanonicalFileName); - const script: ScriptInfo = this.filenameToScript.getPath(path); + const script: ScriptInfo = this.filenameToScript.get(path); const index = script.snap().index; const lineOffset = index.charOffsetToLineNumberAndPos(position); return { line: lineOffset.line, offset: lineOffset.offset + 1 }; diff --git a/src/services/services.ts b/src/services/services.ts index 954a0a37dc9..6b7aa0ecbff 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1728,16 +1728,16 @@ namespace ts { }; } - this.fileNameToEntry.setPath(path, entry); + this.fileNameToEntry.set(path, entry); return entry; } private getEntry(path: Path): HostFileInformation { - return this.fileNameToEntry.getPath(path); + return this.fileNameToEntry.get(path); } private contains(path: Path): boolean { - return this.fileNameToEntry.containsPath(path); + return this.fileNameToEntry.contains(path); } public getOrCreateEntry(fileName: string): HostFileInformation { @@ -2053,7 +2053,7 @@ namespace ts { let bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ true); let path = toPath(fileName, currentDirectory, getCanonicalFileName); - let entry = bucket.getPath(path); + let entry = bucket.get(path); if (!entry) { Debug.assert(acquiring, "How could we be trying to update a document that the registry doesn't have?"); @@ -2065,7 +2065,7 @@ namespace ts { languageServiceRefCount: 0, owners: [] }; - bucket.setPath(path, entry); + bucket.set(path, entry); } else { // We have an entry for this file. However, it may be for a different version of @@ -2095,12 +2095,12 @@ namespace ts { let path = toPath(fileName, currentDirectory, getCanonicalFileName); - let entry = bucket.getPath(path); + let entry = bucket.get(path); entry.languageServiceRefCount--; Debug.assert(entry.languageServiceRefCount >= 0); if (entry.languageServiceRefCount === 0) { - bucket.removePath(path); + bucket.remove(path); } } From 79f09dab4f6ea788c595692b7fa724ea8778abde Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Fri, 30 Oct 2015 09:14:39 -0700 Subject: [PATCH 5/7] Add more detailed error message When an object literal, for example, is returned that does not match the type of the consturctor, add detail about a field that is required but missing. Do this by passing `node.expression` instead of `undefined` -- the rest of the error reporting infrastructure is already in place. --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 99b3d97b7be..d1dbc966952 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12647,7 +12647,7 @@ namespace ts { error(node.expression, Diagnostics.Setters_cannot_return_a_value); } else if (func.kind === SyntaxKind.Constructor) { - if (!isTypeAssignableTo(exprType, returnType)) { + if (!checkTypeAssignableTo(exprType, returnType, node.expression)) { error(node.expression, Diagnostics.Return_type_of_constructor_signature_must_be_assignable_to_the_instance_type_of_the_class); } } From 1b0bc8a496c9677a0bff772a572cb21255b3f3cf Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Fri, 30 Oct 2015 09:21:23 -0700 Subject: [PATCH 6/7] Accept baselines --- .../constructorReturnsInvalidType.errors.txt | 7 ++++- ...rWithAssignableReturnExpression.errors.txt | 14 +++++++++- .../reference/returnInConstructor1.errors.txt | 26 ++++++++++++++++++- .../typeGuardFunctionErrors.errors.txt | 7 ++++- 4 files changed, 50 insertions(+), 4 deletions(-) diff --git a/tests/baselines/reference/constructorReturnsInvalidType.errors.txt b/tests/baselines/reference/constructorReturnsInvalidType.errors.txt index 2ebd59b3d93..a8fe1de4700 100644 --- a/tests/baselines/reference/constructorReturnsInvalidType.errors.txt +++ b/tests/baselines/reference/constructorReturnsInvalidType.errors.txt @@ -1,11 +1,16 @@ +tests/cases/compiler/constructorReturnsInvalidType.ts(3,16): error TS2322: Type 'number' is not assignable to type 'X'. + Property 'foo' is missing in type 'Number'. tests/cases/compiler/constructorReturnsInvalidType.ts(3,16): error TS2409: Return type of constructor signature must be assignable to the instance type of the class -==== tests/cases/compiler/constructorReturnsInvalidType.ts (1 errors) ==== +==== tests/cases/compiler/constructorReturnsInvalidType.ts (2 errors) ==== class X { constructor() { return 1; ~ +!!! error TS2322: Type 'number' is not assignable to type 'X'. +!!! error TS2322: Property 'foo' is missing in type 'Number'. + ~ !!! error TS2409: Return type of constructor signature must be assignable to the instance type of the class } foo() { } diff --git a/tests/baselines/reference/constructorWithAssignableReturnExpression.errors.txt b/tests/baselines/reference/constructorWithAssignableReturnExpression.errors.txt index f5ef669d31b..1f7d7133bd7 100644 --- a/tests/baselines/reference/constructorWithAssignableReturnExpression.errors.txt +++ b/tests/baselines/reference/constructorWithAssignableReturnExpression.errors.txt @@ -1,8 +1,13 @@ +tests/cases/conformance/classes/constructorDeclarations/constructorWithAssignableReturnExpression.ts(12,16): error TS2322: Type 'number' is not assignable to type 'D'. + Property 'x' is missing in type 'Number'. tests/cases/conformance/classes/constructorDeclarations/constructorWithAssignableReturnExpression.ts(12,16): error TS2409: Return type of constructor signature must be assignable to the instance type of the class +tests/cases/conformance/classes/constructorDeclarations/constructorWithAssignableReturnExpression.ts(26,16): error TS2322: Type '{ x: number; }' is not assignable to type 'F'. + Types of property 'x' are incompatible. + Type 'number' is not assignable to type 'T'. tests/cases/conformance/classes/constructorDeclarations/constructorWithAssignableReturnExpression.ts(26,16): error TS2409: Return type of constructor signature must be assignable to the instance type of the class -==== tests/cases/conformance/classes/constructorDeclarations/constructorWithAssignableReturnExpression.ts (2 errors) ==== +==== tests/cases/conformance/classes/constructorDeclarations/constructorWithAssignableReturnExpression.ts (4 errors) ==== // a class constructor may return an expression, it must be assignable to the class instance type to be valid class C { @@ -16,6 +21,9 @@ tests/cases/conformance/classes/constructorDeclarations/constructorWithAssignabl constructor() { return 1; // error ~ +!!! error TS2322: Type 'number' is not assignable to type 'D'. +!!! error TS2322: Property 'x' is missing in type 'Number'. + ~ !!! error TS2409: Return type of constructor signature must be assignable to the instance type of the class } } @@ -32,6 +40,10 @@ tests/cases/conformance/classes/constructorDeclarations/constructorWithAssignabl constructor() { return { x: 1 }; // error ~~~~~~~~ +!!! error TS2322: Type '{ x: number; }' is not assignable to type 'F'. +!!! error TS2322: Types of property 'x' are incompatible. +!!! error TS2322: Type 'number' is not assignable to type 'T'. + ~~~~~~~~ !!! error TS2409: Return type of constructor signature must be assignable to the instance type of the class } } diff --git a/tests/baselines/reference/returnInConstructor1.errors.txt b/tests/baselines/reference/returnInConstructor1.errors.txt index eca0037b4a3..dbe3b9e5ee8 100644 --- a/tests/baselines/reference/returnInConstructor1.errors.txt +++ b/tests/baselines/reference/returnInConstructor1.errors.txt @@ -1,10 +1,20 @@ +tests/cases/compiler/returnInConstructor1.ts(11,16): error TS2322: Type 'number' is not assignable to type 'B'. + Property 'foo' is missing in type 'Number'. tests/cases/compiler/returnInConstructor1.ts(11,16): error TS2409: Return type of constructor signature must be assignable to the instance type of the class +tests/cases/compiler/returnInConstructor1.ts(25,16): error TS2322: Type 'string' is not assignable to type 'D'. + Property 'foo' is missing in type 'String'. tests/cases/compiler/returnInConstructor1.ts(25,16): error TS2409: Return type of constructor signature must be assignable to the instance type of the class +tests/cases/compiler/returnInConstructor1.ts(39,16): error TS2322: Type '{ foo: number; }' is not assignable to type 'F'. + Types of property 'foo' are incompatible. + Type 'number' is not assignable to type 'string'. tests/cases/compiler/returnInConstructor1.ts(39,16): error TS2409: Return type of constructor signature must be assignable to the instance type of the class +tests/cases/compiler/returnInConstructor1.ts(55,16): error TS2322: Type 'G' is not assignable to type 'H'. + Types of property 'foo' are incompatible. + Type '() => void' is not assignable to type 'string'. tests/cases/compiler/returnInConstructor1.ts(55,16): error TS2409: Return type of constructor signature must be assignable to the instance type of the class -==== tests/cases/compiler/returnInConstructor1.ts (4 errors) ==== +==== tests/cases/compiler/returnInConstructor1.ts (8 errors) ==== class A { foo() { } constructor() { @@ -17,6 +27,9 @@ tests/cases/compiler/returnInConstructor1.ts(55,16): error TS2409: Return type o constructor() { return 1; // error ~ +!!! error TS2322: Type 'number' is not assignable to type 'B'. +!!! error TS2322: Property 'foo' is missing in type 'Number'. + ~ !!! error TS2409: Return type of constructor signature must be assignable to the instance type of the class } } @@ -33,6 +46,9 @@ tests/cases/compiler/returnInConstructor1.ts(55,16): error TS2409: Return type o constructor() { return "test"; // error ~~~~~~ +!!! error TS2322: Type 'string' is not assignable to type 'D'. +!!! error TS2322: Property 'foo' is missing in type 'String'. + ~~~~~~ !!! error TS2409: Return type of constructor signature must be assignable to the instance type of the class } } @@ -49,6 +65,10 @@ tests/cases/compiler/returnInConstructor1.ts(55,16): error TS2409: Return type o constructor() { return { foo: 1 }; //error ~~~~~~~~~~ +!!! error TS2322: Type '{ foo: number; }' is not assignable to type 'F'. +!!! error TS2322: Types of property 'foo' are incompatible. +!!! error TS2322: Type 'number' is not assignable to type 'string'. + ~~~~~~~~~~ !!! error TS2409: Return type of constructor signature must be assignable to the instance type of the class } } @@ -67,6 +87,10 @@ tests/cases/compiler/returnInConstructor1.ts(55,16): error TS2409: Return type o super(); return new G(); //error ~~~~~~~ +!!! error TS2322: Type 'G' is not assignable to type 'H'. +!!! error TS2322: Types of property 'foo' are incompatible. +!!! error TS2322: Type '() => void' is not assignable to type 'string'. + ~~~~~~~ !!! error TS2409: Return type of constructor signature must be assignable to the instance type of the class } } diff --git a/tests/baselines/reference/typeGuardFunctionErrors.errors.txt b/tests/baselines/reference/typeGuardFunctionErrors.errors.txt index b178b02c6b4..a1427dc2526 100644 --- a/tests/baselines/reference/typeGuardFunctionErrors.errors.txt +++ b/tests/baselines/reference/typeGuardFunctionErrors.errors.txt @@ -25,6 +25,8 @@ tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(96,9): tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(97,16): error TS1228: A type predicate is only allowed in return type position for functions and methods. tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(98,20): error TS1228: A type predicate is only allowed in return type position for functions and methods. tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(104,25): error TS1228: A type predicate is only allowed in return type position for functions and methods. +tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(105,16): error TS2322: Type 'boolean' is not assignable to type 'D'. + Property 'm1' is missing in type 'Boolean'. tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(105,16): error TS2409: Return type of constructor signature must be assignable to the instance type of the class tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(107,20): error TS1228: A type predicate is only allowed in return type position for functions and methods. tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(110,20): error TS1228: A type predicate is only allowed in return type position for functions and methods. @@ -37,7 +39,7 @@ tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(133,34 tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(137,39): error TS1230: A type predicate cannot reference element 'p1' in a binding pattern. -==== tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts (31 errors) ==== +==== tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts (32 errors) ==== class A { propA: number; @@ -192,6 +194,9 @@ tests/cases/conformance/expressions/typeGuards/typeGuardFunctionErrors.ts(137,39 !!! error TS1228: A type predicate is only allowed in return type position for functions and methods. return true; ~~~~ +!!! error TS2322: Type 'boolean' is not assignable to type 'D'. +!!! error TS2322: Property 'm1' is missing in type 'Boolean'. + ~~~~ !!! error TS2409: Return type of constructor signature must be assignable to the instance type of the class } get m1(p1: A): p1 is C { From 83919f0a3ec1ffad0d101e98833d7db0baa1efec Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 30 Oct 2015 09:52:14 -0700 Subject: [PATCH 7/7] addressed PR feedback: renamed 'toRelativePath' to 'convertToRelativePath' --- src/compiler/tsc.ts | 2 +- src/compiler/utilities.ts | 2 +- src/harness/projectsRunner.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index fc6d65b45e9..232360116f3 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -87,7 +87,7 @@ namespace ts { if (diagnostic.file) { let loc = getLineAndCharacterOfPosition(diagnostic.file, diagnostic.start); const relativeFileName = host - ? toRelativePath(diagnostic.file.fileName, host.getCurrentDirectory(), fileName => host.getCanonicalFileName(fileName)) + ? convertToRelativePath(diagnostic.file.fileName, host.getCurrentDirectory(), fileName => host.getCanonicalFileName(fileName)) : diagnostic.file.fileName; output += `${ relativeFileName }(${ loc.line + 1 },${ loc.character + 1 }): `; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index fb6d2686b15..fd991039d3e 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2182,7 +2182,7 @@ namespace ts { return result; } - export function toRelativePath(absoluteOrRelativePath: string, basePath: string, getCanonicalFileName: (path: string) => string): string { + export function convertToRelativePath(absoluteOrRelativePath: string, basePath: string, getCanonicalFileName: (path: string) => string): string { return !isRootedDiskPath(absoluteOrRelativePath) ? absoluteOrRelativePath : getRelativePathToDirectoryOrUrl(basePath, absoluteOrRelativePath, basePath, getCanonicalFileName, /* isAbsolutePathAnUrl */ false); diff --git a/src/harness/projectsRunner.ts b/src/harness/projectsRunner.ts index 52d4b85f6b8..e587aad3ddb 100644 --- a/src/harness/projectsRunner.ts +++ b/src/harness/projectsRunner.ts @@ -377,10 +377,10 @@ class ProjectRunner extends RunnerBase { bug: testCase.bug, rootDir: testCase.rootDir, resolvedInputFiles: ts.map(compilerResult.program.getSourceFiles(), inputFile => { - return ts.toRelativePath(inputFile.fileName, getCurrentDirectory(), path => Harness.Compiler.getCanonicalFileName(path)); + return ts.convertToRelativePath(inputFile.fileName, getCurrentDirectory(), path => Harness.Compiler.getCanonicalFileName(path)); }), emittedFiles: ts.map(compilerResult.outputFiles, outputFile => { - return ts.toRelativePath(outputFile.emittedFileName, getCurrentDirectory(), path => Harness.Compiler.getCanonicalFileName(path)); + return ts.convertToRelativePath(outputFile.emittedFileName, getCurrentDirectory(), path => Harness.Compiler.getCanonicalFileName(path)); }) };