Make build info tolerant to json errors (#50265)

* Make build info tolerant to json errors Fixes #49754 * Fix incorrect code
microsoft · Aug 10, 2022 · 90cfbae · 90cfbae
1 parent 8a24fe7
commit 90cfbae
Show file tree

Hide file tree

Showing 17 changed files with 521 additions and 40 deletions.
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -5398,6 +5398,10 @@
         "category": "Message",
         "code": 6400
     },
+    "Project '{0}' is out of date because there was error reading file '{1}'": {
+        "category": "Message",
+        "code": 6401
+    },
 
     "The expected type comes from property '{0}' which is declared here on type '{1}'": {
         "category": "Message",

diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -652,8 +652,8 @@ namespace ts {
     }
 
     /*@internal*/
-    export function getBuildInfo(buildInfoText: string) {
-        return JSON.parse(buildInfoText) as BuildInfo;
+    export function getBuildInfo(buildInfoFile: string, buildInfoText: string) {
+        return readJsonOrUndefined(buildInfoFile, buildInfoText) as BuildInfo | undefined;
     }
 
     /*@internal*/
@@ -751,18 +751,17 @@ namespace ts {
     ): EmitUsingBuildInfoResult {
         const createHash = maybeBind(host, host.createHash);
         const { buildInfoPath, jsFilePath, sourceMapFilePath, declarationFilePath, declarationMapPath } = getOutputPathsForBundle(config.options, /*forceDtsPaths*/ false);
-        let buildInfo: BuildInfo;
+        let buildInfo: BuildInfo | undefined;
         if (host.getBuildInfo) {
             // If host directly provides buildinfo we can get it directly. This allows host to cache the buildinfo
-            const hostBuildInfo = host.getBuildInfo(buildInfoPath!, config.options.configFilePath);
-            if (!hostBuildInfo) return buildInfoPath!;
-            buildInfo = hostBuildInfo;
+            buildInfo = host.getBuildInfo(buildInfoPath!, config.options.configFilePath);
         }
         else {
             const buildInfoText = host.readFile(buildInfoPath!);
             if (!buildInfoText) return buildInfoPath!;
-            buildInfo = getBuildInfo(buildInfoText);
+            buildInfo = getBuildInfo(buildInfoPath!, buildInfoText);
         }
+        if (!buildInfo) return buildInfoPath!;
         if (!buildInfo.bundle || !buildInfo.bundle.js || (declarationFilePath && !buildInfo.bundle.dts)) return buildInfoPath!;
 
         const jsFileText = host.readFile(Debug.checkDefined(jsFilePath));
@@ -805,7 +804,7 @@ namespace ts {
         const emitHost: EmitHost = {
             getPrependNodes: memoize(() => [...prependNodes, ownPrependInput]),
             getCanonicalFileName: host.getCanonicalFileName,
-            getCommonSourceDirectory: () => getNormalizedAbsolutePath(buildInfo.bundle!.commonSourceDirectory, buildInfoDirectory),
+            getCommonSourceDirectory: () => getNormalizedAbsolutePath(buildInfo!.bundle!.commonSourceDirectory, buildInfoDirectory),
             getCompilerOptions: () => config.options,
             getCurrentDirectory: () => host.getCurrentDirectory(),
             getNewLine: () => host.getNewLine(),
@@ -827,13 +826,13 @@ namespace ts {
                         break;
                     case buildInfoPath:
                         const newBuildInfo = data!.buildInfo!;
-                        newBuildInfo.program = buildInfo.program;
+                        newBuildInfo.program = buildInfo!.program;
                         if (newBuildInfo.program && changedDtsText !== undefined && config.options.composite) {
                             // Update the output signature
                             (newBuildInfo.program as ProgramBundleEmitBuildInfo).outSignature = computeSignature(changedDtsText, createHash, changedDtsData);
                         }
                         // Update sourceFileInfo
-                        const { js, dts, sourceFiles } = buildInfo.bundle!;
+                        const { js, dts, sourceFiles } = buildInfo!.bundle!;
                         newBuildInfo.bundle!.js!.sources = js!.sources;
                         if (dts) {
                             newBuildInfo.bundle!.dts!.sources = dts.sources;

diff --git a/src/compiler/factory/nodeFactory.ts b/src/compiler/factory/nodeFactory.ts
@@ -6734,7 +6734,7 @@ namespace ts {
             const getAndCacheBuildInfo = (getText: () => string | undefined) => {
                 if (buildInfo === undefined) {
                     const result = getText();
-                    buildInfo = result !== undefined ? getBuildInfo(result) : false;
+                    buildInfo = result !== undefined ? getBuildInfo(node.buildInfoPath!, result) ?? false : false;
                 }
                 return buildInfo || undefined;
             };

diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts
@@ -16,6 +16,7 @@ namespace ts {
          */
         OutOfDateWithPrepend,
         OutputMissing,
+        ErrorReadingFile,
         OutOfDateWithSelf,
         OutOfDateWithUpstream,
         OutOfDateBuildInfo,
@@ -37,6 +38,7 @@ namespace ts {
         | Status.UpToDate
         | Status.OutOfDateWithPrepend
         | Status.OutputMissing
+        | Status.ErrorReadingFile
         | Status.OutOfDateWithSelf
         | Status.OutOfDateWithUpstream
         | Status.OutOfDateBuildInfo
@@ -95,6 +97,12 @@ namespace ts {
             missingOutputFileName: string;
         }
 
+        /** Error reading file */
+        export interface ErrorReadingFile {
+            type: UpToDateStatusType.ErrorReadingFile;
+            fileName: string;
+        }
+
         /**
          * One or more of the project's outputs is older than its newest input.
          */

diff --git a/src/compiler/tsbuildPublic.ts b/src/compiler/tsbuildPublic.ts
@@ -1110,11 +1110,11 @@ namespace ts {
             const emitterDiagnostics = createDiagnosticCollection();
             const emittedOutputs = new Map<Path, string>();
             let resultFlags = BuildResultFlags.DeclarationOutputUnchanged;
-            const existingBuildInfo = state.buildInfoCache.get(projectPath)!.buildInfo as BuildInfo;
+            const existingBuildInfo = state.buildInfoCache.get(projectPath)!.buildInfo || undefined;
             outputFiles.forEach(({ name, text, writeByteOrderMark, buildInfo }) => {
                 emittedOutputs.set(toPath(state, name), name);
                 if (buildInfo) {
-                    if ((buildInfo.program as ProgramBundleEmitBuildInfo)?.outSignature !== (existingBuildInfo.program as ProgramBundleEmitBuildInfo)?.outSignature) {
+                    if ((buildInfo.program as ProgramBundleEmitBuildInfo)?.outSignature !== (existingBuildInfo?.program as ProgramBundleEmitBuildInfo)?.outSignature) {
                         resultFlags &= ~BuildResultFlags.DeclarationOutputUnchanged;
                     }
                     setBuildInfo(state, buildInfo, projectPath, config.options, resultFlags);
@@ -1496,8 +1496,7 @@ namespace ts {
             return existing.buildInfo || undefined;
         }
         const value = state.readFileWithCache(buildInfoPath);
-        const buildInfo = value ? ts.getBuildInfo(value) : undefined;
-        Debug.assert(modifiedTime || !buildInfo);
+        const buildInfo = value ? ts.getBuildInfo(buildInfoPath, value) : undefined;
         state.buildInfoCache.set(resolvedConfigPath, { path, buildInfo: buildInfo || false, modifiedTime: modifiedTime || missingFileModifiedTime });
         return buildInfo;
     }
@@ -1587,7 +1586,14 @@ namespace ts {
                 };
             }
 
-            const buildInfo = Debug.checkDefined(getBuildInfo(state, buildInfoPath, resolvedPath, buildInfoTime));
+            const buildInfo = getBuildInfo(state, buildInfoPath, resolvedPath, buildInfoTime);
+            if (!buildInfo) {
+                // Error reading buildInfo
+                return {
+                    type: UpToDateStatusType.ErrorReadingFile,
+                    fileName: buildInfoPath
+                };
+            }
             if ((buildInfo.bundle || buildInfo.program) && buildInfo.version !== version) {
                 return {
                     type: UpToDateStatusType.TsVersionOutputOfDate,
@@ -2344,6 +2350,13 @@ namespace ts {
                     relName(state, configFileName),
                     relName(state, status.missingOutputFileName)
                 );
+            case UpToDateStatusType.ErrorReadingFile:
+                return reportStatus(
+                    state,
+                    Diagnostics.Project_0_is_out_of_date_because_there_was_error_reading_file_1,
+                    relName(state, configFileName),
+                    relName(state, status.fileName)
+                );
             case UpToDateStatusType.OutOfDateBuildInfo:
                 return reportStatus(
                     state,

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -5360,20 +5360,16 @@ namespace ts {
         return getStringFromExpandedCharCodes(expandedCharCodes);
     }
 
+    export function readJsonOrUndefined(path: string, hostOrText: { readFile(fileName: string): string | undefined } | string): object | undefined {
+        const jsonText = isString(hostOrText) ? hostOrText : hostOrText.readFile(path);
+        if (!jsonText) return undefined;
+        // gracefully handle if readFile fails or returns not JSON
+        const result = parseConfigFileTextToJson(path, jsonText);
+        return !result.error ? result.config : undefined;
+    }
+
     export function readJson(path: string, host: { readFile(fileName: string): string | undefined }): object {
-        try {
-            const jsonText = host.readFile(path);
-            if (!jsonText) return {};
-            const result = parseConfigFileTextToJson(path, jsonText);
-            if (result.error) {
-                return {};
-            }
-            return result.config;
-        }
-        catch (e) {
-            // gracefully handle if readFile fails or returns not JSON
-            return {};
-        }
+        return readJsonOrUndefined(path, host) || {};
     }
 
     export function directoryProbablyExists(directoryName: string, host: { directoryExists?: (directoryName: string) => boolean }): boolean {

diff --git a/src/compiler/watchPublic.ts b/src/compiler/watchPublic.ts
@@ -13,15 +13,13 @@ namespace ts {
         if (host.getBuildInfo) {
             // host provides buildinfo, get it from there. This allows host to cache it
             buildInfo = host.getBuildInfo(buildInfoPath, compilerOptions.configFilePath);
-            if (!buildInfo) return undefined;
         }
         else {
             const content = host.readFile(buildInfoPath);
             if (!content) return undefined;
-            buildInfo = getBuildInfo(content);
+            buildInfo = getBuildInfo(buildInfoPath, content);
         }
-        if (buildInfo.version !== version) return undefined;
-        if (!buildInfo.program) return undefined;
+        if (!buildInfo || buildInfo.version !== version || !buildInfo.program) return undefined;
         return createBuilderProgramUsingProgramBuildInfo(buildInfo.program, buildInfoPath, host);
     }
 

diff --git a/src/harness/fakesHosts.ts b/src/harness/fakesHosts.ts
@@ -506,7 +506,8 @@ ${indentText}${text}`;
         sys.readFile = (path, encoding) => {
             const value = originalReadFile.call(sys, path, encoding);
             if (!value || !ts.isBuildInfoFile(path)) return value;
-            const buildInfo = ts.getBuildInfo(value);
+            const buildInfo = ts.getBuildInfo(path, value);
+            if (!buildInfo) return value;
             ts.Debug.assert(buildInfo.version === version);
             buildInfo.version = ts.version;
             return ts.getBuildInfoText(buildInfo);
@@ -519,10 +520,14 @@ ${indentText}${text}`;
         sys.write = msg => originalWrite.call(sys, msg.replace(ts.version, version));
         const originalWriteFile = sys.writeFile;
         sys.writeFile = (fileName: string, content: string, writeByteOrderMark: boolean) => {
-            if (!ts.isBuildInfoFile(fileName)) return originalWriteFile.call(sys, fileName, content, writeByteOrderMark);
-            const buildInfo = ts.getBuildInfo(content);
-            buildInfo.version = version;
-            originalWriteFile.call(sys, fileName, ts.getBuildInfoText(buildInfo), writeByteOrderMark);
+            if (ts.isBuildInfoFile(fileName)) {
+                const buildInfo = ts.getBuildInfo(fileName, content);
+                if (buildInfo) {
+                    buildInfo.version = version;
+                    return originalWriteFile.call(sys, fileName, ts.getBuildInfoText(buildInfo), writeByteOrderMark);
+                }
+            }
+            return originalWriteFile.call(sys, fileName, content, writeByteOrderMark);
         };
         return sys;
     }

diff --git a/src/testRunner/unittests/tsbuild/helpers.ts b/src/testRunner/unittests/tsbuild/helpers.ts
@@ -12,7 +12,8 @@ namespace ts {
         host.readFile = path => {
             const value = originalReadFile.call(host, path);
             if (!value || !isBuildInfoFile(path)) return value;
-            const buildInfo = getBuildInfo(value);
+            const buildInfo = getBuildInfo(path, value);
+            if (!buildInfo) return value;
             buildInfo.version = fakes.version;
             return getBuildInfoText(buildInfo);
         };
@@ -330,7 +331,8 @@ interface Symbol {
         if (!buildInfoPath || !sys.writtenFiles!.has(toPathWithSystem(sys, buildInfoPath))) return;
         if (!sys.fileExists(buildInfoPath)) return;
 
-        const buildInfo = getBuildInfo((originalReadCall || sys.readFile).call(sys, buildInfoPath, "utf8")!);
+        const buildInfo = getBuildInfo(buildInfoPath, (originalReadCall || sys.readFile).call(sys, buildInfoPath, "utf8")!);
+        if (!buildInfo) return sys.writeFile(`${buildInfoPath}.baseline.txt`, "Error reading valid buildinfo file");
         generateBuildInfoProgramBaseline(sys, buildInfoPath, buildInfo);
 
         if (!outFile(options)) return;

diff --git a/src/testRunner/unittests/tsbuild/sample.ts b/src/testRunner/unittests/tsbuild/sample.ts
@@ -168,6 +168,21 @@ namespace ts {
                 commandLineArgs: ["--b", "/src/tests", "--verbose", "--force"],
             });
 
+            verifyTscWithEdits({
+                scenario: "sample1",
+                subScenario: "tsbuildinfo has error",
+                fs: () => loadProjectFromFiles({
+                    "/src/project/main.ts": "export const x = 10;",
+                    "/src/project/tsconfig.json": "{}",
+                    "/src/project/tsconfig.tsbuildinfo": "Some random string",
+                }),
+                commandLineArgs: ["--b", "src/project", "-i", "-v"],
+                edits: [{
+                    subScenario: "tsbuildinfo written has error",
+                    modifyFs: fs => prependText(fs, "/src/project/tsconfig.tsbuildinfo", "Some random string"),
+                }]
+            });
+
             verifyTscCompileLike(testTscCompileLike, {
                 scenario: "sample1",
                 subScenario: "rebuilds completely when version in tsbuildinfo doesnt match ts version",

diff --git a/src/testRunner/unittests/tsbuildWatch/programUpdates.ts b/src/testRunner/unittests/tsbuildWatch/programUpdates.ts
@@ -729,5 +729,18 @@ export function someFn() { }`),
                 }
             ]
         });
+
+        verifyTscWatch({
+            scenario: "programUpdates",
+            subScenario: "tsbuildinfo has error",
+            sys: () => createWatchedSystem({
+                "/src/project/main.ts": "export const x = 10;",
+                "/src/project/tsconfig.json": "{}",
+                "/src/project/tsconfig.tsbuildinfo": "Some random string",
+                [libFile.path]: libFile.content,
+            }),
+            commandLineArgs: ["--b", "src/project", "-i", "-w"],
+            changes: emptyArray
+        });
     });
 }
diff --git a/src/testRunner/unittests/tsc/incremental.ts b/src/testRunner/unittests/tsc/incremental.ts
@@ -73,6 +73,21 @@ namespace ts {
             edits: noChangeOnlyRuns
         });
 
+        verifyTscWithEdits({
+            scenario: "incremental",
+            subScenario: "tsbuildinfo has error",
+            fs: () => loadProjectFromFiles({
+                "/src/project/main.ts": "export const x = 10;",
+                "/src/project/tsconfig.json": "{}",
+                "/src/project/tsconfig.tsbuildinfo": "Some random string",
+            }),
+            commandLineArgs: ["--p", "src/project", "-i"],
+            edits: [{
+                subScenario: "tsbuildinfo written has error",
+                modifyFs: fs => prependText(fs, "/src/project/tsconfig.tsbuildinfo", "Some random string"),
+            }]
+        });
+
         describe("with noEmitOnError", () => {
             let projFs: vfs.FileSystem;
             before(() => {

diff --git a/src/testRunner/unittests/tscWatch/incremental.ts b/src/testRunner/unittests/tscWatch/incremental.ts
@@ -374,5 +374,18 @@ export const Fragment: unique symbol;
                 },
             });
         });
+
+        verifyTscWatch({
+            scenario: "incremental",
+            subScenario: "tsbuildinfo has error",
+            sys: () => createWatchedSystem({
+                "/src/project/main.ts": "export const x = 10;",
+                "/src/project/tsconfig.json": "{}",
+                "/src/project/tsconfig.tsbuildinfo": "Some random string",
+                [libFile.path]: libFile.content,
+            }),
+            commandLineArgs: ["--p", "src/project", "-i", "-w"],
+            changes: emptyArray
+        });
     });
 }