Correctly skip upstream-blocked projects

RyanCavanaugh · RyanCavanaugh · commit 8ac795ba01f6 · 2018-05-24T16:21:32.000-07:00
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -3644,6 +3644,14 @@
         "category": "Message",
         "code": 6360
     },
+    "Skipping build of project '{0}' because its upstream project '{1}' has errors": {
+        "category": "Message",
+        "code": 6361
+    },
+    "Project '{0}' can't be built because it depends on a project with errors": {
+        "category": "Message",
+        "code": 6362
+    },
 
     "Variable '{0}' implicitly has an '{1}' type.": {
         "category": "Error",
diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts
@@ -69,7 +69,8 @@ namespace ts {
         OutputMissing,
         OutOfDateWithSelf,
         OutOfDateWithUpstream,
-        UpstreamOutOfDate
+        UpstreamOutOfDate,
+        UpstreamBlocked
     }
 
     type UpToDateStatus =
@@ -78,7 +79,8 @@ namespace ts {
         | StatusOutputMissing
         | StatusOutOfDateWithSelf
         | StatusOutOfDateWithUpstream
-        | StatusUpstreamOutOfDate;
+        | StatusUpstreamOutOfDate
+        | StatusUpstreamBlocked;
 
     /**
      * The project can't be built at all in its current state. For example,
@@ -128,6 +130,14 @@ namespace ts {
         upstreamProjectName: string;
     }
 
+    /**
+     * This project depends an upstream project with build errors
+     */
+    interface StatusUpstreamBlocked {
+        type: UpToDateStatusType.UpstreamBlocked;
+        upstreamProjectName: string;
+    }
+
     /**
      * One or more of the project's outputs is older than the newest output of
      * an upstream project.
@@ -466,33 +476,33 @@ namespace ts {
             const outputs = getAllProjectOutputs(project);
 
             // Now see if all outputs are newer than the newest input
-            let oldestOutputFileName: string = undefined!;
+            let oldestOutputFileName: string | undefined;
             let oldestOutputFileTime: Date = maximumDate;
             let newestOutputFileTime: Date = minimumDate;
             let newestDeclarationFileContentChangedTime: Date = minimumDate;
+            let missingOutputFileName: string | undefined;
+            let isOutOfDateWithInputs = false;
             for (const output of outputs) {
-                // Output is missing
+                // Output is missing; can stop checking
+                // Don't immediately return because we can still be upstream-blocked, which is a higher-priority status
                 if (!host.fileExists(output)) {
-                    return {
-                        type: UpToDateStatusType.OutputMissing,
-                        missingOutputFileName: output
-                    };
+                    missingOutputFileName = output;
+                    break;
                 }
 
                 const outputTime = host.getModifiedTime!(output);
-                // If an output is older than the newest input, we can stop checking
-                if (outputTime < newestInputFileTime) {
-                    return {
-                        type: UpToDateStatusType.OutOfDateWithSelf,
-                        outOfDateOutputFileName: output,
-                        newerInputFileName: newestInputFileName
-                    };
-                }
-
                 if (outputTime < oldestOutputFileTime) {
                     oldestOutputFileTime = outputTime;
                     oldestOutputFileName = output;
                 }
+
+                // If an output is older than the newest input, we can stop checking
+                // Don't immediately return because we can still be upstream-blocked, which is a higher-priority status
+                if (outputTime < newestInputFileTime) {
+                    isOutOfDateWithInputs = true;
+                    break;
+                }
+
                 newestOutputFileTime = newer(newestOutputFileTime, outputTime);
 
                 // Keep track of when the most recent time a .d.ts file was changed.
@@ -511,13 +521,19 @@ namespace ts {
             }
 
             let pseudoUpToDate = false;
-            // By here, we know the project is at least up-to-date with its own inputs.
-            // See if any of its upstream projects are newer than it
             if (project.projectReferences) {
                 for (const ref of project.projectReferences) {
                     const resolvedRef = resolveProjectReferencePath(host, ref) as ResolvedConfigFileName;
                     const refStatus = getUpToDateStatus(configFileCache.parseConfigFile(resolvedRef));
 
+                    // An upstream project is blocked
+                    if (refStatus.type === UpToDateStatusType.Unbuildable) {
+                        return {
+                            type: UpToDateStatusType.UpstreamBlocked,
+                            upstreamProjectName: ref.path
+                        };
+                    }
+
                     // If the upstream project is out of date, then so are we (someone shouldn't have asked, though?)
                     if (refStatus.type !== UpToDateStatusType.UpToDate) {
                         return {
@@ -543,12 +559,27 @@ namespace ts {
                     Debug.assert(oldestOutputFileName !== undefined, "Should have an oldest output filename here");
                     return {
                         type: UpToDateStatusType.OutOfDateWithUpstream,
-                        outOfDateOutputFileName: oldestOutputFileName,
+                        outOfDateOutputFileName: oldestOutputFileName!,
                         newerProjectName: ref.path
                     };
                 }
             }
 
+            if (missingOutputFileName !== undefined) {
+                return {
+                    type: UpToDateStatusType.OutputMissing,
+                    missingOutputFileName
+                };
+            }
+
+            if (isOutOfDateWithInputs) {
+                return {
+                    type: UpToDateStatusType.OutOfDateWithSelf,
+                    outOfDateOutputFileName: oldestOutputFileName!,
+                    newerInputFileName: newestInputFileName
+                };
+            }
+
             // Up to date
             return {
                 type: pseudoUpToDate ? UpToDateStatusType.UpToDateWithUpstreamTypes : UpToDateStatusType.UpToDate,
@@ -637,6 +668,7 @@ namespace ts {
             if (!configFile) {
                 // Failed to read the config file
                 resultFlags |= BuildResultFlags.ConfigFileErrors;
+                context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Config file errors" });
                 return resultFlags;
             }
 
@@ -660,6 +692,7 @@ namespace ts {
                 for (const diag of syntaxDiagnostics) {
                     reportDiagnostic(diag);
                 }
+                context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Syntactic errors" });
                 return resultFlags;
             }
 
@@ -671,6 +704,7 @@ namespace ts {
                     for (const diag of declDiagnostics) {
                         reportDiagnostic(diag);
                     }
+                    context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Declaration file errors" });
                     return resultFlags;
                 }
             }
@@ -681,6 +715,7 @@ namespace ts {
                 for (const diag of semanticDiagnostics) {
                     reportDiagnostic(diag);
                 }
+                context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Semantic errors" });
                 return resultFlags;
             }
 
@@ -704,7 +739,6 @@ namespace ts {
             });
 
             context.projectStatus.setValue(proj, { type: UpToDateStatusType.UpToDate, newestDeclarationFileContentChangedTime } as UpToDateStatus);
-
             return resultFlags;
         }
 
@@ -812,15 +846,15 @@ namespace ts {
                 if (proj === undefined) {
                     break;
                 }
-
                 const status = getUpToDateStatus(proj);
                 reportProjectStatus(next, status);
 
+                const projName = proj.options.configFilePath;
                 if (status.type === UpToDateStatusType.UpToDate && !context.options.force) {
                     // Up to date, skip
                     if (options.dry) {
                         // In a dry build, inform the user of this fact
-                        reportDiagnostic(createCompilerDiagnostic(Diagnostics.Project_0_is_up_to_date));
+                        reportDiagnostic(createCompilerDiagnostic(Diagnostics.Project_0_is_up_to_date, projName));
                     }
                     continue;
                 }
@@ -831,10 +865,12 @@ namespace ts {
                     continue;
                 }
 
-                const result = buildSingleProject(next);
-                if (result & BuildResultFlags.AnyErrors) {
-                    break;
+                if (status.type === UpToDateStatusType.UpstreamBlocked) {
+                    context.verbose(Diagnostics.Skipping_build_of_project_0_because_its_upstream_project_1_has_errors, projName, status.upstreamProjectName);
+                    continue;
                 }
+
+                buildSingleProject(next);
             }
 
             function getNext(): ResolvedConfigFileName | undefined {
@@ -889,9 +925,13 @@ namespace ts {
                 case UpToDateStatusType.UpToDateWithUpstreamTypes:
                     context.verbose(Diagnostics.Project_0_is_up_to_date_with_its_upstream_types, configFileName);
                     return;
-                    case UpToDateStatusType.UpstreamOutOfDate:
+                case UpToDateStatusType.UpstreamOutOfDate:
                     context.verbose(Diagnostics.Project_0_is_up_to_date_with_its_upstream_types, configFileName);
                     return;
+                case UpToDateStatusType.UpstreamBlocked:
+                    context.verbose(Diagnostics.Project_0_can_t_be_built_because_it_depends_on_a_project_with_errors, configFileName);
+                    return;
+
                 default:
                     throw new Error(`Invalid build status - ${UpToDateStatusType[status.type]}`);
             }
diff --git a/src/harness/unittests/tsbuild.ts b/src/harness/unittests/tsbuild.ts
@@ -181,6 +181,31 @@ namespace ts {
         });
     });
 
+    describe("tsbuild - downstream-blocked compilations", () => {
+        it("won't build downstream projects if upstream projects have errors", () => {
+            const fs = bfs.shadow();
+            const host = new fakes.CompilerHost(fs);
+            const builder = createSolutionBuilder(host, reportDiagnostic, { dry: false, force: false, verbose: true });
+
+            clearDiagnostics();
+
+            // Induce an error in the middle project
+            replaceText(fs, "/src/logic/index.ts", "c.multiply(10, 15)", `c.muitply()`);
+            fs.chdir("/src/tests");
+            builder.buildProjects(["."]);
+            assertDiagnosticMessages(
+                Diagnostics.Sorted_list_of_input_projects_Colon_0,
+                Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist,
+                Diagnostics.Building_project_0,
+                Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist,
+                Diagnostics.Building_project_0,
+                Diagnostics.Property_0_does_not_exist_on_type_1,
+                Diagnostics.Project_0_can_t_be_built_because_it_depends_on_a_project_with_errors,
+                Diagnostics.Skipping_build_of_project_0_because_its_upstream_project_1_has_errors
+            );
+        });
+    });
+
     function replaceText(fs: vfs.FileSystem, path: string, oldText: string, newText: string) {
         if (!fs.statSync(path).isFile()) {
             throw new Error(`File ${path} does not exist`);
