From 65521bc25949a724fc5e53d43ea8dc6d23a5966d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 7 Aug 2017 14:47:32 -0700 Subject: [PATCH] Feedback from the PR --- src/compiler/commandLineParser.ts | 6 +- src/compiler/types.ts | 8 +- src/harness/unittests/projectErrors.ts | 18 ++-- .../unittests/tsserverProjectSystem.ts | 84 ++++++++++++++----- src/server/editorServices.ts | 9 +- src/server/project.ts | 10 +++ 6 files changed, 91 insertions(+), 44 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index efb574ac971..21cf4275cca 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1478,7 +1478,11 @@ namespace ts { } } - export function getErrorForNoInputFiles({ includeSpecs, excludeSpecs }: ConfigFileSpecs, configFileName?: string) { + export function isErrorNoInputFiles(error: Diagnostic) { + return error.code === Diagnostics.No_inputs_were_found_in_config_file_0_Specified_include_paths_were_1_and_exclude_paths_were_2.code; + } + + export function getErrorForNoInputFiles({ includeSpecs, excludeSpecs }: ConfigFileSpecs, configFileName: string | undefined) { return createCompilerDiagnostic( Diagnostics.No_inputs_were_found_in_config_file_0_Specified_include_paths_were_1_and_exclude_paths_were_2, configFileName || "tsconfig.json", diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 8929b19bfe3..ed0a8128458 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3930,9 +3930,9 @@ namespace ts { } export interface ResolvedModuleWithFailedLookupLocations { - resolvedModule: ResolvedModuleFull | undefined; + readonly resolvedModule: ResolvedModuleFull | undefined; /* @internal */ - failedLookupLocations: string[]; + readonly failedLookupLocations: string[]; /*@internal*/ isInvalidated?: boolean; } @@ -3945,8 +3945,8 @@ namespace ts { } export interface ResolvedTypeReferenceDirectiveWithFailedLookupLocations { - resolvedTypeReferenceDirective: ResolvedTypeReferenceDirective; - failedLookupLocations: string[]; + readonly resolvedTypeReferenceDirective: ResolvedTypeReferenceDirective; + readonly failedLookupLocations: string[]; /*@internal*/ isInvalidated?: boolean; } diff --git a/src/harness/unittests/projectErrors.ts b/src/harness/unittests/projectErrors.ts index 0143aee7701..4d0757e3c2f 100644 --- a/src/harness/unittests/projectErrors.ts +++ b/src/harness/unittests/projectErrors.ts @@ -23,11 +23,9 @@ namespace ts.projectSystem { function checkDiagnosticsWithLinePos(errors: server.protocol.DiagnosticWithLinePosition[], expectedErrors: string[]) { assert.equal(errors ? errors.length : 0, expectedErrors.length, `expected ${expectedErrors.length} error in the list`); if (expectedErrors.length) { - for (let i = 0; i < errors.length; i++) { - const actualMessage = errors[i].message; - const expectedMessage = expectedErrors[i]; - assert.isTrue(actualMessage.indexOf(errors[i].message) === 0, `error message does not match, expected ${actualMessage} to start with ${expectedMessage}`); - } + zipWith(errors, expectedErrors, ({ message: actualMessage }, expectedMessage) => { + assert.isTrue(startsWith(actualMessage, actualMessage), `error message does not match, expected ${actualMessage} to start with ${expectedMessage}`); + }); } } @@ -40,12 +38,11 @@ namespace ts.projectSystem { path: "/a/b/applib.ts", content: "" }; - // only file1 exists - expect error const host = createServerHost([file1, libFile]); const session = createSession(host); const projectService = session.getProjectService(); const projectFileName = "/a/b/test.csproj"; - const compilerOptionsRequest = { + const compilerOptionsRequest: server.protocol.CompilerOptionsDiagnosticsRequest = { type: "request", command: server.CommandNames.CompilerOptionsDiagnosticsFull, seq: 2, @@ -61,19 +58,20 @@ namespace ts.projectSystem { checkNumberOfProjects(projectService, { externalProjects: 1 }); const diags = session.executeCommand(compilerOptionsRequest).response; + // only file1 exists - expect error checkDiagnosticsWithLinePos(diags, ["File '/a/b/applib.ts' not found."]); } - // only file2 exists - expect error host.reloadFS([file2, libFile]); { + // only file2 exists - expect error checkNumberOfProjects(projectService, { externalProjects: 1 }); const diags = session.executeCommand(compilerOptionsRequest).response; checkDiagnosticsWithLinePos(diags, ["File '/a/b/app.ts' not found."]); } - // both files exist - expect no errors host.reloadFS([file1, file2, libFile]); { + // both files exist - expect no errors checkNumberOfProjects(projectService, { externalProjects: 1 }); const diags = session.executeCommand(compilerOptionsRequest).response; checkDiagnosticsWithLinePos(diags, []); @@ -99,7 +97,7 @@ namespace ts.projectSystem { openFilesForSession([file1], session); checkNumberOfProjects(projectService, { configuredProjects: 1 }); const project = configuredProjectAt(projectService, 0); - const compilerOptionsRequest = { + const compilerOptionsRequest: server.protocol.CompilerOptionsDiagnosticsRequest = { type: "request", command: server.CommandNames.CompilerOptionsDiagnosticsFull, seq: 2, diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index fa6ffc74e65..17953a8bcd5 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -253,29 +253,19 @@ namespace ts.projectSystem { entries: FSEntry[]; } - export function isFolder(s: FSEntry): s is Folder { + export function isFolder(s: FSEntry | undefined): s is Folder { return s && isArray((s).entries); } - export function isFile(s: FSEntry): s is File { + export function isFile(s: FSEntry | undefined): s is File { return s && typeof (s).content === "string"; } - function invokeDirectoryWatcher(callbacks: DirectoryWatcherCallback[], getRelativeFilePath: () => string) { + function invokeWatcherCallbacks(callbacks: T[], invokeCallback: (cb: T) => void): void { if (callbacks) { const cbs = callbacks.slice(); for (const cb of cbs) { - const fileName = getRelativeFilePath(); - cb(fileName); - } - } - } - - function invokeFileWatcher(callbacks: FileWatcherCallback[], fileName: string, eventId: FileWatcherEventKind) { - if (callbacks) { - const cbs = callbacks.slice(); - for (const cb of cbs) { - cb(fileName, eventId); + invokeCallback(cb); } } } @@ -380,7 +370,7 @@ namespace ts.projectSystem { private readonly output: string[] = []; - private fs: Map = createMap(); + private fs = createMap(); private getCanonicalFileName: (s: string) => string; private toPath: (f: string) => Path; private timeoutCallbacks = new Callbacks(); @@ -510,8 +500,12 @@ namespace ts.projectSystem { } else { Debug.assert(fileOrFolder.entries.length === 0); - invokeDirectoryWatcher(this.watchedDirectories.get(fileOrFolder.path), () => this.getRelativePathToDirectory(fileOrFolder.fullPath, fileOrFolder.fullPath)); - invokeDirectoryWatcher(this.watchedDirectoriesRecursive.get(fileOrFolder.path), () => this.getRelativePathToDirectory(fileOrFolder.fullPath, fileOrFolder.fullPath)); + const relativePath = this.getRelativePathToDirectory(fileOrFolder.fullPath, fileOrFolder.fullPath); + // Invoke directory and recursive directory watcher for the folder + // Here we arent invoking recursive directory watchers for the base folders + // since that is something we would want to do for both file as well as folder we are deleting + invokeWatcherCallbacks(this.watchedDirectories.get(fileOrFolder.path), cb => cb(relativePath)); + invokeWatcherCallbacks(this.watchedDirectoriesRecursive.get(fileOrFolder.path), cb => cb(relativePath)); } if (basePath !== fileOrFolder.path) { @@ -524,22 +518,31 @@ namespace ts.projectSystem { } } - private invokeFileWatcher(fileFullPath: string, eventId: FileWatcherEventKind) { + private invokeFileWatcher(fileFullPath: string, eventKind: FileWatcherEventKind) { const callbacks = this.watchedFiles.get(this.toPath(fileFullPath)); - invokeFileWatcher(callbacks, getBaseFileName(fileFullPath), eventId); + const fileName = getBaseFileName(fileFullPath); + invokeWatcherCallbacks(callbacks, cb => cb(fileName, eventKind)); } private getRelativePathToDirectory(directoryFullPath: string, fileFullPath: string) { return getRelativePathToDirectoryOrUrl(directoryFullPath, fileFullPath, this.currentDirectory, this.getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); } + /** + * This will call the directory watcher for the foldeFullPath and recursive directory watchers for this and base folders + */ private invokeDirectoryWatcher(folderFullPath: string, fileName: string) { - invokeDirectoryWatcher(this.watchedDirectories.get(this.toPath(folderFullPath)), () => this.getRelativePathToDirectory(folderFullPath, fileName)); + const relativePath = this.getRelativePathToDirectory(folderFullPath, fileName); + invokeWatcherCallbacks(this.watchedDirectories.get(this.toPath(folderFullPath)), cb => cb(relativePath)); this.invokeRecursiveDirectoryWatcher(folderFullPath, fileName); } + /** + * This will call the recursive directory watcher for this directory as well as all the base directories + */ private invokeRecursiveDirectoryWatcher(fullPath: string, fileName: string) { - invokeDirectoryWatcher(this.watchedDirectoriesRecursive.get(this.toPath(fullPath)), () => this.getRelativePathToDirectory(fullPath, fileName)); + const relativePath = this.getRelativePathToDirectory(fullPath, fileName); + invokeWatcherCallbacks(this.watchedDirectoriesRecursive.get(this.toPath(fullPath)), cb => cb(relativePath)); const basePath = getDirectoryPath(fullPath); if (this.getCanonicalFileName(fullPath) !== this.getCanonicalFileName(basePath)) { this.invokeRecursiveDirectoryWatcher(basePath, fileName); @@ -885,6 +888,45 @@ namespace ts.projectSystem { checkWatchedDirectories(host, [getDirectoryPath(configFile.path)], /*recursive*/ true); }); + it("create configured project with the file list", () => { + const configFile: FileOrFolder = { + path: "/a/b/tsconfig.json", + content: ` + { + "compilerOptions": {}, + "include": ["*.ts"] + }` + }; + const file1: FileOrFolder = { + path: "/a/b/f1.ts", + content: "let x = 1" + }; + const file2: FileOrFolder = { + path: "/a/b/f2.ts", + content: "let y = 1" + }; + const file3: FileOrFolder = { + path: "/a/b/c/f3.ts", + content: "let z = 1" + }; + + const host = createServerHost([configFile, libFile, file1, file2, file3]); + const projectService = createProjectService(host); + const { configFileName, configFileErrors } = projectService.openClientFile(file1.path); + + assert(configFileName, "should find config file"); + assert.isTrue(!configFileErrors, `expect no errors in config file, got ${JSON.stringify(configFileErrors)}`); + checkNumberOfInferredProjects(projectService, 0); + checkNumberOfConfiguredProjects(projectService, 1); + + const project = configuredProjectAt(projectService, 0); + checkProjectActualFiles(project, [file1.path, libFile.path, file2.path, configFile.path]); + checkProjectRootFiles(project, [file1.path, file2.path]); + // watching all files except one that was open + checkWatchedFiles(host, [configFile.path, file2.path, libFile.path]); + checkWatchedDirectories(host, [getDirectoryPath(configFile.path)], /*recursive*/ false); + }); + it("add and then remove a config file in a folder with loose files", () => { const configFile: FileOrFolder = { path: "/a/b/tsconfig.json", diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 003bb87f6a9..b71bc939097 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -665,14 +665,7 @@ namespace ts.server { const configFileSpecs = project.configFileSpecs; const result = getFileNamesFromConfigSpecs(configFileSpecs, getDirectoryPath(configFilename), project.getCompilerOptions(), project.getCachedServerHost(), this.hostConfiguration.extraFileExtensions); - const errors = project.getAllProjectErrors(); - const isErrorNoInputFiles = (error: Diagnostic) => error.code === Diagnostics.No_inputs_were_found_in_config_file_0_Specified_include_paths_were_1_and_exclude_paths_were_2.code; - if (result.fileNames.length !== 0) { - filterMutate(errors, error => !isErrorNoInputFiles(error)); - } - else if (!configFileSpecs.filesSpecs && !some(errors, isErrorNoInputFiles)) { - errors.push(getErrorForNoInputFiles(configFileSpecs, configFilename)); - } + project.updateErrorOnNoInputFiles(result.fileNames.length !== 0); this.updateNonInferredProjectFiles(project, result.fileNames, fileNamePropertyReader, /*clientFileName*/ undefined); this.delayUpdateProjectGraphAndInferredProjectsRefresh(project); } diff --git a/src/server/project.ts b/src/server/project.ts index 62c450f9f4f..d36551bdb5a 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -1226,6 +1226,16 @@ namespace ts.server { getEffectiveTypeRoots() { return getEffectiveTypeRoots(this.getCompilerOptions(), this.lsHost.host) || []; } + + /*@internal*/ + updateErrorOnNoInputFiles(hasFileNames: boolean) { + if (hasFileNames) { + filterMutate(this.projectErrors, error => !isErrorNoInputFiles(error)); + } + else if (!this.configFileSpecs.filesSpecs && !some(this.projectErrors, isErrorNoInputFiles)) { + this.projectErrors.push(getErrorForNoInputFiles(this.configFileSpecs, this.getConfigFilePath())); + } + } } /**