Skip to content

Commit

Permalink
Add checkPending for deciding whether to rebuild or report errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@sheetalkamat
sheetalkamat committed Jun 13, 2024
1 parent 5360b7b commit 5a84e20
Show file tree
Hide file tree
Showing 54 changed files with 5,673 additions and 6,105 deletions.
34 changes: 31 additions & 3 deletions src/compiler/builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ import {
SemanticDiagnosticsBuilderProgram,
SignatureInfo,
skipAlias,
skipTypeChecking,
skipTypeCheckingIgnoringNoCheck,
some,
SourceFile,
sourceFileMayBeEmitted,
Expand Down Expand Up @@ -158,6 +158,8 @@ export interface ReusableBuilderProgramState extends BuilderState {
* emitKind pending for a program with --out
*/
programEmitPending?: BuilderFileEmit;
/** If semantic diagnsotic check is pending */
checkPending?: true;
/*
* true if semantic diagnostics are ReusableDiagnostic instead of Diagnostic
*/
Expand Down Expand Up @@ -329,6 +331,7 @@ function createBuilderProgramState(
}
state.changedFilesSet = new Set();
state.latestChangedDtsFile = compilerOptions.composite ? oldState?.latestChangedDtsFile : undefined;
state.checkPending = state.compilerOptions.noCheck ? true : undefined;

const useOldState = BuilderState.canReuseOldState(state.referencedMap, oldState);
const oldCompilerOptions = useOldState ? oldState!.compilerOptions : undefined;
Expand Down Expand Up @@ -470,9 +473,15 @@ function createBuilderProgramState(
state.programEmitPending | pendingEmitKind :
pendingEmitKind;
}
// if (!state.compilerOptions.noCheck)
state.buildInfoEmitPending = true;
}
}
if (
useOldState &&
state.semanticDiagnosticsPerFile.size !== state.fileInfos.size &&
oldState!.checkPending !== state.checkPending
) state.buildInfoEmitPending = true;
return state;

function addFileToChangeSet(path: Path) {
Expand Down Expand Up @@ -741,7 +750,7 @@ function removeDiagnosticsOfLibraryFiles(state: BuilderProgramStateWithDefinedPr
const options = state.program.getCompilerOptions();
forEach(state.program.getSourceFiles(), f =>
state.program.isSourceFileDefaultLibrary(f) &&
!skipTypeChecking(f, options, state.program) &&
!skipTypeCheckingIgnoringNoCheck(f, options, state.program) &&
removeSemanticDiagnosticsOf(state, f.resolvedPath));
}
}
Expand Down Expand Up @@ -986,6 +995,7 @@ function getSemanticDiagnosticsOfFile(
cancellationToken: CancellationToken | undefined,
semanticDiagnosticsPerFile?: BuilderProgramState["semanticDiagnosticsPerFile"],
): readonly Diagnostic[] {
if (state.compilerOptions.noCheck) return emptyArray;
return concatenate(
getBinderAndCheckerDiagnosticsOfFile(state, sourceFile, cancellationToken, semanticDiagnosticsPerFile),
state.program.getProgramDiagnostics(sourceFile),
Expand Down Expand Up @@ -1084,6 +1094,7 @@ export interface IncrementalBuildInfoBase extends BuildInfo {
// Because this is only output file in the program, we dont need fileId to deduplicate name
latestChangedDtsFile?: string | undefined;
errors: true | undefined;
checkPending: true | undefined;
}

/** @internal */
Expand Down Expand Up @@ -1131,6 +1142,7 @@ export function isIncrementalBuildInfo(info: BuildInfo): info is IncrementalBuil
export interface NonIncrementalBuildInfo extends BuildInfo {
root: readonly string[];
errors: true | undefined;
checkPending: true | undefined;
}

/** @internal */
Expand Down Expand Up @@ -1190,6 +1202,7 @@ function getBuildInfo(state: BuilderProgramStateWithDefinedProgram): BuildInfo {
const buildInfo: NonIncrementalBuildInfo = {
root: arrayFrom(rootFileNames, r => relativeToBuildInfo(r)),
errors: state.hasErrors ? true : undefined,
checkPending: state.checkPending,
version,
};
return buildInfo;
Expand Down Expand Up @@ -1223,6 +1236,7 @@ function getBuildInfo(state: BuilderProgramStateWithDefinedProgram): BuildInfo {
false : // Pending emit is same as deteremined by compilerOptions
state.programEmitPending, // Actual value
errors: state.hasErrors ? true : undefined,
checkPending: state.checkPending,
version,
};
return buildInfo;
Expand Down Expand Up @@ -1313,6 +1327,7 @@ function getBuildInfo(state: BuilderProgramStateWithDefinedProgram): BuildInfo {
emitSignatures,
latestChangedDtsFile,
errors: state.hasErrors ? true : undefined,
checkPending: state.checkPending,
version,
};
return buildInfo;
Expand Down Expand Up @@ -1952,7 +1967,13 @@ export function createBuilderProgram(
while (true) {
const affected = getNextAffectedFile(state, cancellationToken, host);
let result;
if (!affected) return undefined; // Done
if (!affected) {
if (state.checkPending && !state.compilerOptions.noCheck) {
state.checkPending = undefined;
state.buildInfoEmitPending = true;
}
return undefined; // Done
}
else if (affected !== state.program) {
// Get diagnostics for the affected file if its not ignored
const affectedSourceFile = affected as SourceFile;
Expand Down Expand Up @@ -1984,6 +2005,7 @@ export function createBuilderProgram(
result = diagnostics || emptyArray;
state.changedFilesSet.clear();
state.programEmitPending = getBuilderFileEmit(state.compilerOptions);
if (!state.compilerOptions.noCheck) state.checkPending = undefined;
state.buildInfoEmitPending = true;
}
return { result, affected };
Expand Down Expand Up @@ -2021,6 +2043,10 @@ export function createBuilderProgram(
for (const sourceFile of state.program.getSourceFiles()) {
diagnostics = addRange(diagnostics, getSemanticDiagnosticsOfFile(state, sourceFile, cancellationToken));
}
if (state.checkPending && !state.compilerOptions.noCheck) {
state.checkPending = undefined;
state.buildInfoEmitPending = true;
}
return diagnostics || emptyArray;
}
}
Expand Down Expand Up @@ -2089,6 +2115,7 @@ export function createBuilderProgramUsingIncrementalBuildInfo(
outSignature: buildInfo.outSignature,
programEmitPending: buildInfo.pendingEmit === undefined ? undefined : toProgramEmitPending(buildInfo.pendingEmit, buildInfo.options),
hasErrors: buildInfo.errors,
checkPending: buildInfo.checkPending,
};
}
else {
Expand Down Expand Up @@ -2125,6 +2152,7 @@ export function createBuilderProgramUsingIncrementalBuildInfo(
latestChangedDtsFile,
emitSignatures: emitSignatures?.size ? emitSignatures : undefined,
hasErrors: buildInfo.errors,
checkPending: buildInfo.checkPending,
};
}

Expand Down
3 changes: 1 addition & 2 deletions src/compiler/commandLineParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,7 @@ export const commonOptionsWithBuild: CommandLineOption[] = [
description: Diagnostics.Disable_full_type_checking_only_critical_parse_and_emit_errors_will_be_reported,
transpileOptionValue: true,
defaultValueDescription: false,
affectsSemanticDiagnostics: true,
affectsBuildInfo: true,
// Not setting affectsSemanticDiagnostics or affectsBuildInfo because we dont want all diagnostics to go away, its handled in builder
},
{
name: "noEmit",
Expand Down
11 changes: 8 additions & 3 deletions src/compiler/tsbuildPublic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1535,7 +1535,11 @@ function getUpToDateStatusWorker<T extends BuilderProgram>(state: SolutionBuilde
};
}

if ((buildInfo as IncrementalBuildInfo | NonIncrementalBuildInfo).errors) {
if (
!project.options.noCheck &&
((buildInfo as IncrementalBuildInfo | NonIncrementalBuildInfo).errors || // TODO: syntax errors????
(buildInfo as IncrementalBuildInfo | NonIncrementalBuildInfo).checkPending)
) {
return {
type: UpToDateStatusType.OutOfDateBuildInfoWithErrors,
buildInfoFile: buildInfoPath,
Expand All @@ -1545,8 +1549,9 @@ function getUpToDateStatusWorker<T extends BuilderProgram>(state: SolutionBuilde
if (incrementalBuildInfo) {
// If there are errors, we need to build project again to report it
if (
incrementalBuildInfo.semanticDiagnosticsPerFile?.length ||
(!project.options.noEmit && getEmitDeclarations(project.options) && incrementalBuildInfo.emitDiagnosticsPerFile?.length)
!project.options.noCheck &&
(incrementalBuildInfo.semanticDiagnosticsPerFile?.length ||
(!project.options.noEmit && getEmitDeclarations(project.options) && incrementalBuildInfo.emitDiagnosticsPerFile?.length))
) {
return {
type: UpToDateStatusType.OutOfDateBuildInfoWithErrors,
Expand Down
26 changes: 24 additions & 2 deletions src/compiler/utilities.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10108,13 +10108,35 @@ export interface HostWithIsSourceOfProjectReferenceRedirect {
isSourceOfProjectReferenceRedirect(fileName: string): boolean;
}
/** @internal */
export function skipTypeChecking(sourceFile: SourceFile, options: CompilerOptions, host: HostWithIsSourceOfProjectReferenceRedirect) {
export function skipTypeChecking(
sourceFile: SourceFile,
options: CompilerOptions,
host: HostWithIsSourceOfProjectReferenceRedirect,
) {
return skipTypeCheckingWorker(sourceFile, options, host, /*ignoreNoCheck*/ false);
}

/** @internal */
export function skipTypeCheckingIgnoringNoCheck(
sourceFile: SourceFile,
options: CompilerOptions,
host: HostWithIsSourceOfProjectReferenceRedirect,
) {
return skipTypeCheckingWorker(sourceFile, options, host, /*ignoreNoCheck*/ true);
}

function skipTypeCheckingWorker(
sourceFile: SourceFile,
options: CompilerOptions,
host: HostWithIsSourceOfProjectReferenceRedirect,
ignoreNoCheck: boolean,
) {
// If skipLibCheck is enabled, skip reporting errors if file is a declaration file.
// If skipDefaultLibCheck is enabled, skip reporting errors if file contains a
// '/// <reference no-default-lib="true"/>' directive.
return (options.skipLibCheck && sourceFile.isDeclarationFile ||
options.skipDefaultLibCheck && sourceFile.hasNoDefaultLib) ||
options.noCheck ||
(!ignoreNoCheck && options.noCheck) ||
host.isSourceOfProjectReferenceRedirect(sourceFile.fileName) ||
!canIncludeBindAndCheckDiagnostics(sourceFile, options);
}
Expand Down
15 changes: 13 additions & 2 deletions src/testRunner/unittests/helpers/noCheck.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ export function forEachTscScenarioWithNoCheck(buildType: "-b" | "-p") {
caption: "Introduce error with noCheck",
edit: fs => fs.writeFileSync("/src/a.ts", aText),
};
const noChangeRunWithCheckPendingDiscrepancy: TestTscEdit = {
...noChangeRun,
discrepancyExplanation: () => [
"Clean build will have check pending since it didnt type check",
"Incremental build has typechecked before this so wont have checkPending",
],
};

[undefined, true].forEach(incremental => {
[{}, { module: "amd", outFile: "../outFile.js" }].forEach(options => {
Expand All @@ -53,7 +60,9 @@ export function forEachTscScenarioWithNoCheck(buildType: "-b" | "-p") {
noChangeRun, // Should be no op
checkNoChangeRun, // Check errors - should not report any errors - update buildInfo
checkNoChangeRun, // Should be no op
noChangeRun, // Should be no op
incremental || buildType === "-b" ?
noChangeRunWithCheckPendingDiscrepancy : // Should be no op
noChangeRun, // Should be no op
noCheckError,
noChangeRun, // Should be no op
checkNoChangeRun, // Should check errors and update buildInfo
Expand All @@ -67,7 +76,9 @@ export function forEachTscScenarioWithNoCheck(buildType: "-b" | "-p") {
noCheckError,
noCheckFixError,
checkNoChangeRun,
noChangeRun, // Should be no op
incremental || buildType === "-b" ?
noChangeRunWithCheckPendingDiscrepancy : // Should be no op
noChangeRun, // Should be no op
checkNoChangeRun, // Should be no op
],
baselinePrograms: true,
Expand Down
94 changes: 68 additions & 26 deletions src/testRunner/unittests/helpers/tsc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,31 +352,69 @@ function verifyTscEditDiscrepancies({
}
}
}
if ((incrementalReadableBuildInfo as ReadableIncrementalBuildInfo)?.emitDiagnosticsPerFile) {
(incrementalReadableBuildInfo as ReadableIncrementalBuildInfo).emitDiagnosticsPerFile!.forEach(([actualFileOrArray]) => {
const actualFile = ts.isString(actualFileOrArray) ? actualFileOrArray : actualFileOrArray[0];
if (
// Does not have emit diagnostics in clean buildInfo
!ts.find(
(cleanReadableBuildInfo as ReadableIncrementalBuildInfo).emitDiagnosticsPerFile,
([expectedFileOrArray]) => actualFile === (ts.isString(expectedFileOrArray) ? expectedFileOrArray : expectedFileOrArray[0]),
) &&
// Is not marked as affectedFilesPendingEmit in clean buildInfo
(!ts.find(
(cleanReadableBuildInfo as ReadableIncrementalMultiFileEmitBuildInfo).affectedFilesPendingEmit,
([expectedFileOrArray]) => actualFile === (ts.isString(expectedFileOrArray) ? expectedFileOrArray : expectedFileOrArray[0]),
)) &&
// Program emit is not pending in clean buildInfo
!(cleanReadableBuildInfo as ReadableIncrementalBundleEmitBuildInfo).pendingEmit
) {
addBaseline(
`Incremental build contains ${actualFile} file has errors, clean build does not have errors or does not mark is as pending emit: ${outputFile}::`,
`Incremental buildInfoText:: ${incrementalBuildText}`,
`Clean buildInfoText:: ${cleanBuildText}`,
);
}
});
}
const readableIncrementalBuildInfo = incrementalReadableBuildInfo as ReadableIncrementalBuildInfo | undefined;
const readableCleanBuildInfo = cleanReadableBuildInfo as ReadableIncrementalBuildInfo | undefined;
readableIncrementalBuildInfo?.semanticDiagnosticsPerFile?.forEach((
[actualFile, notCachedORDiagnostics],
) => {
const cleanFileDiagnostics = ts.find(
readableCleanBuildInfo?.semanticDiagnosticsPerFile,
([expectedFile]) => actualFile === expectedFile,
);
// Incremental build should have same diagnostics as clean build
if (cleanFileDiagnostics && JSON.stringify(notCachedORDiagnostics) === JSON.stringify(cleanFileDiagnostics[1])) return;
// Otherwise marked as "not Cached" in clean build with errors in incremental build is ok
if (
ts.isString(cleanFileDiagnostics?.[1]) &&
!ts.isString(notCachedORDiagnostics)
) return;
addBaseline(
`Incremental build contains ${actualFile} file ${!ts.isString(notCachedORDiagnostics) ? "has" : "does not have"} semantic errors, it does not match with clean build: ${outputFile}::`,
`Incremental buildInfoText:: ${incrementalBuildText}`,
`Clean buildInfoText:: ${cleanBuildText}`,
);
});
readableCleanBuildInfo?.semanticDiagnosticsPerFile?.forEach((
[expectedFile, cleanFileDiagnostics],
) => {
if (
// if there are errors in the file
!ts.isString(cleanFileDiagnostics?.[1]) &&
// and its not already verified, its issue with the error caching
!ts.find(
readableIncrementalBuildInfo?.semanticDiagnosticsPerFile,
([actualFile]) => actualFile === expectedFile,
)
) {
addBaseline(
`Incremental build does not contain ${expectedFile} file for semantic errors, clean build has semantic errors: ${outputFile}::`,
`Incremental buildInfoText:: ${incrementalBuildText}`,
`Clean buildInfoText:: ${cleanBuildText}`,
);
}
});
readableIncrementalBuildInfo?.emitDiagnosticsPerFile?.forEach(([actualFile]) => {
if (
// Does not have emit diagnostics in clean buildInfo
!ts.find(
readableCleanBuildInfo!.emitDiagnosticsPerFile,
([expectedFile]) => actualFile === expectedFile,
) &&
// Is not marked as affectedFilesPendingEmit in clean buildInfo
(!ts.find(
(readableCleanBuildInfo as ReadableIncrementalMultiFileEmitBuildInfo).affectedFilesPendingEmit,
([expectedFileOrArray]) => actualFile === (ts.isString(expectedFileOrArray) ? expectedFileOrArray : expectedFileOrArray[0]),
)) &&
// Program emit is not pending in clean buildInfo
!(readableCleanBuildInfo as ReadableIncrementalBundleEmitBuildInfo).pendingEmit
) {
addBaseline(
`Incremental build contains ${actualFile} file has emit errors, clean build does not have errors or does not mark is as pending emit: ${outputFile}::`,
`Incremental buildInfoText:: ${incrementalBuildText}`,
`Clean buildInfoText:: ${cleanBuildText}`,
);
}
});
}
}
if (!headerAdded && discrepancyExplanation) addBaseline("*** Supplied discrepancy explanation but didnt find any difference");
Expand Down Expand Up @@ -462,11 +500,15 @@ function getBuildInfoForIncrementalCorrectnessCheck(text: string | undefined): {
fileIdsList: undefined,
fileInfos: sanitizedFileInfos,
// Ignore noEmit since that shouldnt be reason to emit the tsbuild info and presence of it in the buildinfo file does not matter
options: readableBuildInfo.options ? { ...readableBuildInfo.options, noEmit: undefined } : undefined,
options: readableBuildInfo.options && {
...readableBuildInfo.options,
noEmit: undefined,
},
affectedFilesPendingEmit: undefined,
pendingEmit: undefined,
emitDiagnosticsPerFile: undefined,
latestChangedDtsFile: readableBuildInfo.latestChangedDtsFile ? "FakeFileName" : undefined,
semanticDiagnosticsPerFile: undefined,
} : undefined),
size: undefined, // Size doesnt need to be equal
}),
Expand Down
Loading

0 comments on commit 5a84e20

Please sign in to comment.