From cec1b0a7179983eb880e806120ae25ac207029a9 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 21 Aug 2018 16:25:57 -0700 Subject: [PATCH] Report error summary from the queue. --- src/compiler/program.ts | 2 +- src/compiler/tsbuild.ts | 166 ++++++++++++++----- src/testRunner/unittests/tsbuild.ts | 4 +- src/testRunner/unittests/tsbuildWatchMode.ts | 80 ++++++++- src/testRunner/unittests/tscWatchMode.ts | 4 +- 5 files changed, 206 insertions(+), 50 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 08bb4727781..d1168046b8b 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -175,7 +175,7 @@ namespace ts { return getDirectoryPath(normalizePath(system.getExecutingFilePath())); } - const newLine = getNewLineCharacter(options); + const newLine = getNewLineCharacter(options, () => system.newLine); const realpath = system.realpath && ((path: string) => system.realpath!(path)); return { diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 22b2237d1ae..5fdab168951 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -35,9 +35,11 @@ namespace ts { * Map from config file name to up-to-date status */ projectStatus: FileMap; + diagnostics?: FileMap; // TODO(shkamat): this should be really be diagnostics but thats for later time - invalidatedProjects: FileMap; - queuedProjects: FileMap; + invalidateProject(project: ResolvedConfigFileName, dependencyGraph: DependencyGraph | undefined): void; + getNextInvalidatedProject(): ResolvedConfigFileName | undefined; + pendingInvalidatedProjects(): boolean; missingRoots: Map; } @@ -194,6 +196,7 @@ namespace ts { hasKey(fileName: string): boolean; removeKey(fileName: string): void; getKeys(): string[]; + getSize(): number; } /** @@ -209,7 +212,8 @@ namespace ts { getValueOrUndefined, removeKey, getKeys, - hasKey + hasKey, + getSize }; function getKeys(): string[] { @@ -242,6 +246,10 @@ namespace ts { const f = normalizePath(fileName); return lookup.get(f); } + + function getSize() { + return lookup.size; + } } function createDependencyMapper() { @@ -375,18 +383,64 @@ namespace ts { } export function createBuildContext(options: BuildOptions): BuildContext { - const invalidatedProjects = createFileMap(); - const queuedProjects = createFileMap(); + const invalidatedProjectQueue = [] as ResolvedConfigFileName[]; + let nextIndex = 0; + const projectPendingBuild = createFileMap(); const missingRoots = createMap(); + const diagnostics = options.watch ? createFileMap() : undefined; return { options, projectStatus: createFileMap(), + diagnostics, unchangedOutputs: createFileMap(), - invalidatedProjects, - missingRoots, - queuedProjects + invalidateProject, + getNextInvalidatedProject, + pendingInvalidatedProjects, + missingRoots }; + + function invalidateProject(proj: ResolvedConfigFileName, dependancyGraph: DependencyGraph | undefined) { + if (!projectPendingBuild.hasKey(proj)) { + addProjToQueue(proj); + if (dependancyGraph) { + queueBuildForDownstreamReferences(proj, dependancyGraph); + } + } + } + + function addProjToQueue(proj: ResolvedConfigFileName) { + projectPendingBuild.setValue(proj, true); + invalidatedProjectQueue.push(proj); + } + + function getNextInvalidatedProject() { + if (nextIndex < invalidatedProjectQueue.length) { + const proj = invalidatedProjectQueue[nextIndex]; + nextIndex++; + projectPendingBuild.removeKey(proj); + if (!projectPendingBuild.getSize()) { + invalidatedProjectQueue.length = 0; + } + return proj; + } + } + + function pendingInvalidatedProjects() { + return !!projectPendingBuild.getSize(); + } + + // Mark all downstream projects of this one needing to be built "later" + function queueBuildForDownstreamReferences(root: ResolvedConfigFileName, dependancyGraph: DependencyGraph) { + const deps = dependancyGraph.dependencyMap.getReferencesTo(root); + for (const ref of deps) { + // Can skip circular references + if (!projectPendingBuild.hasKey(ref)) { + addProjToQueue(ref); + queueBuildForDownstreamReferences(ref, dependancyGraph); + } + } + } } export interface SolutionBuilderHost extends CompilerHost { @@ -443,6 +497,7 @@ namespace ts { const hostWithWatch = host as SolutionBuilderWithWatchHost; const configFileCache = createConfigFileCache(host); let context = createBuildContext(defaultOptions); + let timerToBuildInvalidatedProject: any; const existingWatchersForWildcards = createMap(); return { @@ -454,8 +509,7 @@ namespace ts { getBuildGraph, invalidateProject, - buildInvalidatedProjects, - buildDependentInvalidatedProjects, + buildInvalidatedProject, resolveProjectName, @@ -466,7 +520,19 @@ namespace ts { host.reportSolutionBuilderStatus(createCompilerDiagnostic(message, ...args)); } - function reportWatchStatus(message: DiagnosticMessage, ...args: string[]) { + function storeErrors(proj: ResolvedConfigFileName, diagnostics: ReadonlyArray) { + if (context.options.watch) { + storeErrorSummary(proj, diagnostics.filter(diagnostic => diagnostic.category === DiagnosticCategory.Error).length); + } + } + + function storeErrorSummary(proj: ResolvedConfigFileName, errorCount: number) { + if (context.options.watch) { + context.diagnostics!.setValue(proj, errorCount); + } + } + + function reportWatchStatus(message: DiagnosticMessage, ...args: (string | number | undefined)[]) { if (hostWithWatch.onWatchStatusChange) { hostWithWatch.onWatchStatusChange(createCompilerDiagnostic(message, ...args), host.getNewLine(), { preserveWatchOutput: context.options.preserveWatchOutput }); } @@ -506,15 +572,12 @@ namespace ts { } } - function invalidateProjectAndScheduleBuilds(resolved: ResolvedConfigFileName) { - reportWatchStatus(Diagnostics.File_change_detected_Starting_incremental_compilation); - invalidateProject(resolved); - if (!hostWithWatch.setTimeout) { - return; - } - hostWithWatch.setTimeout(buildInvalidatedProjects, 100); - hostWithWatch.setTimeout(buildDependentInvalidatedProjects, 3000); - } + } + + function invalidateProjectAndScheduleBuilds(resolved: ResolvedConfigFileName) { + reportWatchStatus(Diagnostics.File_change_detected_Starting_incremental_compilation); + invalidateProject(resolved); + scheduleBuildInvalidatedProject(); } function resetBuildContext(opts = defaultOptions) { @@ -725,33 +788,44 @@ namespace ts { } configFileCache.removeKey(resolved); - context.invalidatedProjects.setValue(resolved, true); context.projectStatus.removeKey(resolved); - - const graph = getGlobalDependencyGraph()!; - if (graph) { - queueBuildForDownstreamReferences(resolved); + if (context.options.watch) { + context.diagnostics!.removeKey(resolved); } - // Mark all downstream projects of this one needing to be built "later" - function queueBuildForDownstreamReferences(root: ResolvedConfigFileName) { - const deps = graph.dependencyMap.getReferencesTo(root); - for (const ref of deps) { - // Can skip circular references - if (!context.queuedProjects.hasKey(ref)) { - context.queuedProjects.setValue(ref, true); - queueBuildForDownstreamReferences(ref); - } + context.invalidateProject(resolved, getGlobalDependencyGraph()); + } + + function scheduleBuildInvalidatedProject() { + if (!hostWithWatch.setTimeout || !hostWithWatch.clearTimeout) { + return; + } + if (timerToBuildInvalidatedProject) { + hostWithWatch.clearTimeout(timerToBuildInvalidatedProject); + } + timerToBuildInvalidatedProject = hostWithWatch.setTimeout(buildInvalidatedProject, 250); + } + + function buildInvalidatedProject() { + timerToBuildInvalidatedProject = undefined; + const buildProject = context.getNextInvalidatedProject(); + buildSomeProjects(p => p === buildProject); + if (context.pendingInvalidatedProjects()) { + if (!timerToBuildInvalidatedProject) { + scheduleBuildInvalidatedProject(); } } + else { + reportErrorSummary(); + } } - function buildInvalidatedProjects() { - buildSomeProjects(p => context.invalidatedProjects.hasKey(p)); - } - - function buildDependentInvalidatedProjects() { - buildSomeProjects(p => context.queuedProjects.hasKey(p)); + function reportErrorSummary() { + if (context.options.watch) { + let errorCount = 0; + context.diagnostics!.getKeys().forEach(resolved => errorCount += context.diagnostics!.getValue(resolved)); + reportWatchStatus(errorCount === 1 ? Diagnostics.Found_1_error_Watching_for_file_changes : Diagnostics.Found_0_errors_Watching_for_file_changes, errorCount); + } } function buildSomeProjects(predicate: (projName: ResolvedConfigFileName) => boolean) { @@ -808,6 +882,7 @@ namespace ts { if (temporaryMarks[projPath]) { if (!inCircularContext) { hadError = true; + // TODO(shkamat): Account for this error reportStatus(Diagnostics.Project_references_may_not_form_a_circular_graph_Cycle_detected_Colon_0, circularityReportStack.join("\r\n")); return; } @@ -853,6 +928,7 @@ namespace ts { if (!configFile) { // Failed to read the config file resultFlags |= BuildResultFlags.ConfigFileErrors; + storeErrorSummary(proj, 1); context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Config file errors" }); return resultFlags; } @@ -880,6 +956,7 @@ namespace ts { for (const diag of syntaxDiagnostics) { host.reportDiagnostic(diag); } + storeErrors(proj, syntaxDiagnostics); context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Syntactic errors" }); return resultFlags; } @@ -892,6 +969,7 @@ namespace ts { for (const diag of declDiagnostics) { host.reportDiagnostic(diag); } + storeErrors(proj, declDiagnostics); context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Declaration file errors" }); return resultFlags; } @@ -904,6 +982,7 @@ namespace ts { for (const diag of semanticDiagnostics) { host.reportDiagnostic(diag); } + storeErrors(proj, semanticDiagnostics); context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Semantic errors" }); return resultFlags; } @@ -1029,6 +1108,7 @@ namespace ts { if (host.fileExists(fullPathWithTsconfig)) { return fullPathWithTsconfig as ResolvedConfigFileName; } + // TODO(shkamat): right now this is accounted as 1 error in config file, but we need to do better host.reportDiagnostic(createCompilerDiagnostic(Diagnostics.File_0_not_found, relName(fullPath))); return undefined; } @@ -1048,7 +1128,10 @@ namespace ts { function buildAllProjects(): ExitStatus { if (context.options.watch) { reportWatchStatus(Diagnostics.Starting_compilation_in_watch_mode); } const graph = getGlobalDependencyGraph(); - if (graph === undefined) return ExitStatus.DiagnosticsPresent_OutputsSkipped; + if (graph === undefined) { + reportErrorSummary(); + return ExitStatus.DiagnosticsPresent_OutputsSkipped; + } const queue = graph.buildQueue; reportBuildQueue(graph); @@ -1092,6 +1175,7 @@ namespace ts { const buildResult = buildSingleProject(next); anyFailed = anyFailed || !!(buildResult & BuildResultFlags.AnyErrors); } + reportErrorSummary(); return anyFailed ? ExitStatus.DiagnosticsPresent_OutputsSkipped : ExitStatus.Success; } diff --git a/src/testRunner/unittests/tsbuild.ts b/src/testRunner/unittests/tsbuild.ts index f8813ecd1ae..b7d708446c1 100644 --- a/src/testRunner/unittests/tsbuild.ts +++ b/src/testRunner/unittests/tsbuild.ts @@ -205,14 +205,14 @@ namespace ts { // Rebuild this project tick(); builder.invalidateProject("/src/logic"); - builder.buildInvalidatedProjects(); + builder.buildInvalidatedProject(); // The file should be updated assert.equal(fs.statSync("/src/logic/index.js").mtimeMs, time(), "JS file should have been rebuilt"); assert.isBelow(fs.statSync("/src/tests/index.js").mtimeMs, time(), "Downstream JS file should *not* have been rebuilt"); // Build downstream projects should update 'tests', but not 'core' tick(); - builder.buildDependentInvalidatedProjects(); + builder.buildInvalidatedProject(); assert.equal(fs.statSync("/src/tests/index.js").mtimeMs, time(), "Downstream JS file should have been rebuilt"); assert.isBelow(fs.statSync("/src/core/index.js").mtimeMs, time(), "Upstream JS file should not have been rebuilt"); }); diff --git a/src/testRunner/unittests/tsbuildWatchMode.ts b/src/testRunner/unittests/tsbuildWatchMode.ts index d43a24a0b9c..d1405fa9e2c 100644 --- a/src/testRunner/unittests/tsbuildWatchMode.ts +++ b/src/testRunner/unittests/tsbuildWatchMode.ts @@ -25,12 +25,18 @@ namespace ts.tscWatch { /** [tsconfig, index] | [tsconfig, index, anotherModule, someDecl] */ type SubProjectFiles = [ReadonlyFile, ReadonlyFile] | [ReadonlyFile, ReadonlyFile, ReadonlyFile, ReadonlyFile]; const root = Harness.IO.getWorkspaceRoot(); + + function projectFilePath(subProject: SubProject, baseFileName: string) { + return `${projectsLocation}/${project}/${subProject}/${baseFileName.toLowerCase()}`; + } + function projectFile(subProject: SubProject, baseFileName: string): File { return { - path: `${projectsLocation}/${project}/${subProject}/${baseFileName.toLowerCase()}`, + path: projectFilePath(subProject, baseFileName), content: Harness.IO.readFile(`${root}/tests/projects/${project}/${subProject}/${baseFileName}`)! }; } + function subProjectFiles(subProject: SubProject, anotherModuleAndSomeDecl?: true): SubProjectFiles { const tsconfig = projectFile(subProject, "tsconfig.json"); const index = projectFile(subProject, "index.ts"); @@ -42,20 +48,86 @@ namespace ts.tscWatch { return [tsconfig, index, anotherModule, someDecl]; } + function getOutputFileNames(subProject: SubProject, baseFileNameWithoutExtension: string) { + const file = projectFilePath(subProject, baseFileNameWithoutExtension); + return [`${file}.js`, `${file}.d.ts`]; + } + + type OutputFileStamp = [string, Date | undefined]; + function getOutputStamps(host: WatchedSystem, subProject: SubProject, baseFileNameWithoutExtension: string): OutputFileStamp[] { + return getOutputFileNames(subProject, baseFileNameWithoutExtension).map(f => [f, host.getModifiedTime(f)] as OutputFileStamp); + } + + function getOutputFileStamps(host: WatchedSystem): OutputFileStamp[] { + return [ + ...getOutputStamps(host, SubProject.core, "anotherModule"), + ...getOutputStamps(host, SubProject.core, "index"), + ...getOutputStamps(host, SubProject.logic, "index"), + ...getOutputStamps(host, SubProject.tests, "index"), + ]; + } + + function verifyChangedFiles(actualStamps: OutputFileStamp[], oldTimeStamps: OutputFileStamp[], changedFiles: string[]) { + for (let i = 0; i < oldTimeStamps.length; i++) { + const actual = actualStamps[i]; + const old = oldTimeStamps[i]; + if (contains(changedFiles, actual[0])) { + assert.isTrue((actual[1] || 0) > (old[1] || 0), `${actual[0]} expected to written`); + } + else { + assert.equal(actual[1], old[1], `${actual[0]} expected to not change`); + } + } + } + const core = subProjectFiles(SubProject.core, /*anotherModuleAndSomeDecl*/ true); const logic = subProjectFiles(SubProject.logic); const tests = subProjectFiles(SubProject.tests); const ui = subProjectFiles(SubProject.ui); const allFiles: ReadonlyArray = [libFile, ...core, ...logic, ...tests, ...ui]; const testProjectExpectedWatchedFiles = [core[0], core[1], core[2], ...logic, ...tests].map(f => f.path); - it("creates solution in watch mode", () => { + + function createSolutionInWatchMode() { const host = createWatchedSystem(allFiles, { currentDirectory: projectsLocation }); - const originalWrite = host.write; - host.write = s => { console.log(s); originalWrite.call(host, s); }; createSolutionBuilderWithWatch(host, [`${project}/${SubProject.tests}`]); checkWatchedFiles(host, testProjectExpectedWatchedFiles); checkWatchedDirectories(host, emptyArray, /*recursive*/ false); checkWatchedDirectories(host, emptyArray, /*recursive*/ true); // TODO: #26524 + checkOutputErrorsInitial(host, emptyArray); + const outputFileStamps = getOutputFileStamps(host); + for (const stamp of outputFileStamps) { + assert.isDefined(stamp[1], `${stamp[0]} expected to be present`); + } + return { host, outputFileStamps }; + } + it("creates solution in watch mode", () => { + createSolutionInWatchMode(); }); + + it("change builds changes and reports found errors message", () => { + const { host, outputFileStamps } = createSolutionInWatchMode(); + host.writeFile(core[1].path, `${core[1].content} +export class someClass { }`); + host.checkTimeoutQueueLengthAndRun(1); // Builds core + const changedCore = getOutputFileStamps(host); + verifyChangedFiles(changedCore, outputFileStamps, [ + ...getOutputFileNames(SubProject.core, "anotherModule"), // This should not be written really + ...getOutputFileNames(SubProject.core, "index") + ]); + host.checkTimeoutQueueLengthAndRun(1); // Builds tests + const changedTests = getOutputFileStamps(host); + verifyChangedFiles(changedTests, changedCore, [ + ...getOutputFileNames(SubProject.tests, "index") // Again these need not be written + ]); + host.checkTimeoutQueueLengthAndRun(1); // Builds logic + const changedLogic = getOutputFileStamps(host); + verifyChangedFiles(changedLogic, changedTests, [ + ...getOutputFileNames(SubProject.logic, "index") // Again these need not be written + ]); + host.checkTimeoutQueueLength(0); + checkOutputErrorsIncremental(host, emptyArray); + }); + + // TODO: write tests reporting errors but that will have more involved work since file }); } diff --git a/src/testRunner/unittests/tscWatchMode.ts b/src/testRunner/unittests/tscWatchMode.ts index c881e9c14da..da1c4fd0d70 100644 --- a/src/testRunner/unittests/tscWatchMode.ts +++ b/src/testRunner/unittests/tscWatchMode.ts @@ -136,7 +136,7 @@ namespace ts.tscWatch { : createCompilerDiagnostic(Diagnostics.Found_0_errors_Watching_for_file_changes, errors.length); } - function checkOutputErrorsInitial(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeErrors?: string[]) { + export function checkOutputErrorsInitial(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeErrors?: string[]) { checkOutputErrors( host, /*logsBeforeWatchDiagnostic*/ undefined, @@ -147,7 +147,7 @@ namespace ts.tscWatch { createErrorsFoundCompilerDiagnostic(errors)); } - function checkOutputErrorsIncremental(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeWatchDiagnostic?: string[], logsBeforeErrors?: string[]) { + export function checkOutputErrorsIncremental(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeWatchDiagnostic?: string[], logsBeforeErrors?: string[]) { checkOutputErrors( host, logsBeforeWatchDiagnostic,