Skip to content

Commit 7397f7b

Browse files
sheetalkamatsheetalkamat
committed
Add checkPending for deciding whether to rebuild or report errors
1 parent d48c85d commit 7397f7b

File tree

54 files changed

+5673
-6105
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+5673
-6105
lines changed

‎src/compiler/builder.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ import {
7676
SemanticDiagnosticsBuilderProgram,
7777
SignatureInfo,
7878
skipAlias,
79-
skipTypeChecking,
79+
skipTypeCheckingIgnoringNoCheck,
8080
some,
8181
SourceFile,
8282
sourceFileMayBeEmitted,
@@ -158,6 +158,8 @@ export interface ReusableBuilderProgramState extends BuilderState {
158158
* emitKind pending for a program with --out
159159
*/
160160
programEmitPending?: BuilderFileEmit;
161+
/** If semantic diagnsotic check is pending */
162+
checkPending?: true;
161163
/*
162164
* true if semantic diagnostics are ReusableDiagnostic instead of Diagnostic
163165
*/
@@ -329,6 +331,7 @@ function createBuilderProgramState(
329331
}
330332
state.changedFilesSet = new Set();
331333
state.latestChangedDtsFile = compilerOptions.composite ? oldState?.latestChangedDtsFile : undefined;
334+
state.checkPending = state.compilerOptions.noCheck ? true : undefined;
332335

333336
const useOldState = BuilderState.canReuseOldState(state.referencedMap, oldState);
334337
const oldCompilerOptions = useOldState ? oldState!.compilerOptions : undefined;
@@ -470,9 +473,15 @@ function createBuilderProgramState(
470473
state.programEmitPending | pendingEmitKind :
471474
pendingEmitKind;
472475
}
476+
// if (!state.compilerOptions.noCheck)
473477
state.buildInfoEmitPending = true;
474478
}
475479
}
480+
if (
481+
useOldState &&
482+
state.semanticDiagnosticsPerFile.size !== state.fileInfos.size &&
483+
oldState!.checkPending !== state.checkPending
484+
) state.buildInfoEmitPending = true;
476485
return state;
477486

478487
function addFileToChangeSet(path: Path) {
@@ -741,7 +750,7 @@ function removeDiagnosticsOfLibraryFiles(state: BuilderProgramStateWithDefinedPr
741750
const options = state.program.getCompilerOptions();
742751
forEach(state.program.getSourceFiles(), f =>
743752
state.program.isSourceFileDefaultLibrary(f) &&
744-
!skipTypeChecking(f, options, state.program) &&
753+
!skipTypeCheckingIgnoringNoCheck(f, options, state.program) &&
745754
removeSemanticDiagnosticsOf(state, f.resolvedPath));
746755
}
747756
}
@@ -986,6 +995,7 @@ function getSemanticDiagnosticsOfFile(
986995
cancellationToken?: CancellationToken,
987996
semanticDiagnosticsPerFile?: BuilderProgramState["semanticDiagnosticsPerFile"],
988997
): readonly Diagnostic[] {
998+
if (state.compilerOptions.noCheck) return emptyArray;
989999
return concatenate(
9901000
getBinderAndCheckerDiagnosticsOfFile(state, sourceFile, cancellationToken, semanticDiagnosticsPerFile),
9911001
state.program.getProgramDiagnostics(sourceFile),
@@ -1084,6 +1094,7 @@ export interface IncrementalBuildInfoBase extends BuildInfo {
10841094
// Because this is only output file in the program, we dont need fileId to deduplicate name
10851095
latestChangedDtsFile?: string | undefined;
10861096
errors: true | undefined;
1097+
checkPending: true | undefined;
10871098
}
10881099

10891100
/** @internal */
@@ -1131,6 +1142,7 @@ export function isIncrementalBuildInfo(info: BuildInfo): info is IncrementalBuil
11311142
export interface NonIncrementalBuildInfo extends BuildInfo {
11321143
root: readonly string[];
11331144
errors: true | undefined;
1145+
checkPending: true | undefined;
11341146
}
11351147

11361148
/** @internal */
@@ -1190,6 +1202,7 @@ function getBuildInfo(state: BuilderProgramStateWithDefinedProgram): BuildInfo {
11901202
const buildInfo: NonIncrementalBuildInfo = {
11911203
root: arrayFrom(rootFileNames, r => relativeToBuildInfo(r)),
11921204
errors: state.hasErrors ? true : undefined,
1205+
checkPending: state.checkPending,
11931206
version,
11941207
};
11951208
return buildInfo;
@@ -1223,6 +1236,7 @@ function getBuildInfo(state: BuilderProgramStateWithDefinedProgram): BuildInfo {
12231236
false : // Pending emit is same as deteremined by compilerOptions
12241237
state.programEmitPending, // Actual value
12251238
errors: state.hasErrors ? true : undefined,
1239+
checkPending: state.checkPending,
12261240
version,
12271241
};
12281242
return buildInfo;
@@ -1313,6 +1327,7 @@ function getBuildInfo(state: BuilderProgramStateWithDefinedProgram): BuildInfo {
13131327
emitSignatures,
13141328
latestChangedDtsFile,
13151329
errors: state.hasErrors ? true : undefined,
1330+
checkPending: state.checkPending,
13161331
version,
13171332
};
13181333
return buildInfo;
@@ -1952,7 +1967,13 @@ export function createBuilderProgram(
19521967
while (true) {
19531968
const affected = getNextAffectedFile(state, cancellationToken, host);
19541969
let result;
1955-
if (!affected) return undefined; // Done
1970+
if (!affected) {
1971+
if (state.checkPending && !state.compilerOptions.noCheck) {
1972+
state.checkPending = undefined;
1973+
state.buildInfoEmitPending = true;
1974+
}
1975+
return undefined; // Done
1976+
}
19561977
else if (affected !== state.program) {
19571978
// Get diagnostics for the affected file if its not ignored
19581979
const affectedSourceFile = affected as SourceFile;
@@ -1984,6 +2005,7 @@ export function createBuilderProgram(
19842005
result = diagnostics || emptyArray;
19852006
state.changedFilesSet.clear();
19862007
state.programEmitPending = getBuilderFileEmit(state.compilerOptions);
2008+
if (!state.compilerOptions.noCheck) state.checkPending = undefined;
19872009
state.buildInfoEmitPending = true;
19882010
}
19892011
return { result, affected };
@@ -2021,6 +2043,10 @@ export function createBuilderProgram(
20212043
for (const sourceFile of state.program.getSourceFiles()) {
20222044
diagnostics = addRange(diagnostics, getSemanticDiagnosticsOfFile(state, sourceFile, cancellationToken));
20232045
}
2046+
if (state.checkPending && !state.compilerOptions.noCheck) {
2047+
state.checkPending = undefined;
2048+
state.buildInfoEmitPending = true;
2049+
}
20242050
return diagnostics || emptyArray;
20252051
}
20262052
}
@@ -2089,6 +2115,7 @@ export function createBuilderProgramUsingIncrementalBuildInfo(
20892115
outSignature: buildInfo.outSignature,
20902116
programEmitPending: buildInfo.pendingEmit === undefined ? undefined : toProgramEmitPending(buildInfo.pendingEmit, buildInfo.options),
20912117
hasErrors: buildInfo.errors,
2118+
checkPending: buildInfo.checkPending,
20922119
};
20932120
}
20942121
else {
@@ -2125,6 +2152,7 @@ export function createBuilderProgramUsingIncrementalBuildInfo(
21252152
latestChangedDtsFile,
21262153
emitSignatures: emitSignatures?.size ? emitSignatures : undefined,
21272154
hasErrors: buildInfo.errors,
2155+
checkPending: buildInfo.checkPending,
21282156
};
21292157
}
21302158

‎src/compiler/commandLineParser.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -510,8 +510,7 @@ export const commonOptionsWithBuild: CommandLineOption[] = [
510510
description: Diagnostics.Disable_full_type_checking_only_critical_parse_and_emit_errors_will_be_reported,
511511
transpileOptionValue: true,
512512
defaultValueDescription: false,
513-
affectsSemanticDiagnostics: true,
514-
affectsBuildInfo: true,
513+
// Not setting affectsSemanticDiagnostics or affectsBuildInfo because we dont want all diagnostics to go away, its handled in builder
515514
},
516515
{
517516
name: "noEmit",

‎src/compiler/tsbuildPublic.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1535,7 +1535,11 @@ function getUpToDateStatusWorker<T extends BuilderProgram>(state: SolutionBuilde
15351535
};
15361536
}
15371537

1538-
if ((buildInfo as IncrementalBuildInfo | NonIncrementalBuildInfo).errors) {
1538+
if (
1539+
!project.options.noCheck &&
1540+
((buildInfo as IncrementalBuildInfo | NonIncrementalBuildInfo).errors || // TODO: syntax errors????
1541+
(buildInfo as IncrementalBuildInfo | NonIncrementalBuildInfo).checkPending)
1542+
) {
15391543
return {
15401544
type: UpToDateStatusType.OutOfDateBuildInfoWithErrors,
15411545
buildInfoFile: buildInfoPath,
@@ -1545,8 +1549,9 @@ function getUpToDateStatusWorker<T extends BuilderProgram>(state: SolutionBuilde
15451549
if (incrementalBuildInfo) {
15461550
// If there are errors, we need to build project again to report it
15471551
if (
1548-
incrementalBuildInfo.semanticDiagnosticsPerFile?.length ||
1549-
(!project.options.noEmit && getEmitDeclarations(project.options) && incrementalBuildInfo.emitDiagnosticsPerFile?.length)
1552+
!project.options.noCheck &&
1553+
(incrementalBuildInfo.semanticDiagnosticsPerFile?.length ||
1554+
(!project.options.noEmit && getEmitDeclarations(project.options) && incrementalBuildInfo.emitDiagnosticsPerFile?.length))
15501555
) {
15511556
return {
15521557
type: UpToDateStatusType.OutOfDateBuildInfoWithErrors,

‎src/compiler/utilities.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10108,13 +10108,35 @@ export interface HostWithIsSourceOfProjectReferenceRedirect {
1010810108
isSourceOfProjectReferenceRedirect(fileName: string): boolean;
1010910109
}
1011010110
/** @internal */
10111-
export function skipTypeChecking(sourceFile: SourceFile, options: CompilerOptions, host: HostWithIsSourceOfProjectReferenceRedirect) {
10111+
export function skipTypeChecking(
10112+
sourceFile: SourceFile,
10113+
options: CompilerOptions,
10114+
host: HostWithIsSourceOfProjectReferenceRedirect,
10115+
) {
10116+
return skipTypeCheckingWorker(sourceFile, options, host, /*ignoreNoCheck*/ false);
10117+
}
10118+
10119+
/** @internal */
10120+
export function skipTypeCheckingIgnoringNoCheck(
10121+
sourceFile: SourceFile,
10122+
options: CompilerOptions,
10123+
host: HostWithIsSourceOfProjectReferenceRedirect,
10124+
) {
10125+
return skipTypeCheckingWorker(sourceFile, options, host, /*ignoreNoCheck*/ true);
10126+
}
10127+
10128+
function skipTypeCheckingWorker(
10129+
sourceFile: SourceFile,
10130+
options: CompilerOptions,
10131+
host: HostWithIsSourceOfProjectReferenceRedirect,
10132+
ignoreNoCheck: boolean,
10133+
) {
1011210134
// If skipLibCheck is enabled, skip reporting errors if file is a declaration file.
1011310135
// If skipDefaultLibCheck is enabled, skip reporting errors if file contains a
1011410136
// '/// <reference no-default-lib="true"/>' directive.
1011510137
return (options.skipLibCheck && sourceFile.isDeclarationFile ||
1011610138
options.skipDefaultLibCheck && sourceFile.hasNoDefaultLib) ||
10117-
options.noCheck ||
10139+
(!ignoreNoCheck && options.noCheck) ||
1011810140
host.isSourceOfProjectReferenceRedirect(sourceFile.fileName) ||
1011910141
!canIncludeBindAndCheckDiagnostics(sourceFile, options);
1012010142
}

‎src/testRunner/unittests/helpers/noCheck.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ export function forEachTscScenarioWithNoCheck(buildType: "-b" | "-p") {
2828
caption: "Introduce error with noCheck",
2929
edit: fs => fs.writeFileSync("/src/a.ts", aText),
3030
};
31+
const noChangeRunWithCheckPendingDiscrepancy: TestTscEdit = {
32+
...noChangeRun,
33+
discrepancyExplanation: () => [
34+
"Clean build will have check pending since it didnt type check",
35+
"Incremental build has typechecked before this so wont have checkPending",
36+
],
37+
};
3138

3239
[undefined, true].forEach(incremental => {
3340
[{}, { module: "amd", outFile: "../outFile.js" }].forEach(options => {
@@ -53,7 +60,9 @@ export function forEachTscScenarioWithNoCheck(buildType: "-b" | "-p") {
5360
noChangeRun, // Should be no op
5461
checkNoChangeRun, // Check errors - should not report any errors - update buildInfo
5562
checkNoChangeRun, // Should be no op
56-
noChangeRun, // Should be no op
63+
incremental || buildType === "-b" ?
64+
noChangeRunWithCheckPendingDiscrepancy : // Should be no op
65+
noChangeRun, // Should be no op
5766
noCheckError,
5867
noChangeRun, // Should be no op
5968
checkNoChangeRun, // Should check errors and update buildInfo
@@ -67,7 +76,9 @@ export function forEachTscScenarioWithNoCheck(buildType: "-b" | "-p") {
6776
noCheckError,
6877
noCheckFixError,
6978
checkNoChangeRun,
70-
noChangeRun, // Should be no op
79+
incremental || buildType === "-b" ?
80+
noChangeRunWithCheckPendingDiscrepancy : // Should be no op
81+
noChangeRun, // Should be no op
7182
checkNoChangeRun, // Should be no op
7283
],
7384
baselinePrograms: true,

‎src/testRunner/unittests/helpers/tsc.ts

Lines changed: 68 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -352,31 +352,69 @@ function verifyTscEditDiscrepancies({
352352
}
353353
}
354354
}
355-
if ((incrementalReadableBuildInfo as ReadableIncrementalBuildInfo)?.emitDiagnosticsPerFile) {
356-
(incrementalReadableBuildInfo as ReadableIncrementalBuildInfo).emitDiagnosticsPerFile!.forEach(([actualFileOrArray]) => {
357-
const actualFile = ts.isString(actualFileOrArray) ? actualFileOrArray : actualFileOrArray[0];
358-
if (
359-
// Does not have emit diagnostics in clean buildInfo
360-
!ts.find(
361-
(cleanReadableBuildInfo as ReadableIncrementalBuildInfo).emitDiagnosticsPerFile,
362-
([expectedFileOrArray]) => actualFile === (ts.isString(expectedFileOrArray) ? expectedFileOrArray : expectedFileOrArray[0]),
363-
) &&
364-
// Is not marked as affectedFilesPendingEmit in clean buildInfo
365-
(!ts.find(
366-
(cleanReadableBuildInfo as ReadableIncrementalMultiFileEmitBuildInfo).affectedFilesPendingEmit,
367-
([expectedFileOrArray]) => actualFile === (ts.isString(expectedFileOrArray) ? expectedFileOrArray : expectedFileOrArray[0]),
368-
)) &&
369-
// Program emit is not pending in clean buildInfo
370-
!(cleanReadableBuildInfo as ReadableIncrementalBundleEmitBuildInfo).pendingEmit
371-
) {
372-
addBaseline(
373-
`Incremental build contains ${actualFile} file has errors, clean build does not have errors or does not mark is as pending emit: ${outputFile}::`,
374-
`Incremental buildInfoText:: ${incrementalBuildText}`,
375-
`Clean buildInfoText:: ${cleanBuildText}`,
376-
);
377-
}
378-
});
379-
}
355+
const readableIncrementalBuildInfo = incrementalReadableBuildInfo as ReadableIncrementalBuildInfo | undefined;
356+
const readableCleanBuildInfo = cleanReadableBuildInfo as ReadableIncrementalBuildInfo | undefined;
357+
readableIncrementalBuildInfo?.semanticDiagnosticsPerFile?.forEach((
358+
[actualFile, notCachedORDiagnostics],
359+
) => {
360+
const cleanFileDiagnostics = ts.find(
361+
readableCleanBuildInfo?.semanticDiagnosticsPerFile,
362+
([expectedFile]) => actualFile === expectedFile,
363+
);
364+
// Incremental build should have same diagnostics as clean build
365+
if (cleanFileDiagnostics && JSON.stringify(notCachedORDiagnostics) === JSON.stringify(cleanFileDiagnostics[1])) return;
366+
// Otherwise marked as "not Cached" in clean build with errors in incremental build is ok
367+
if (
368+
ts.isString(cleanFileDiagnostics?.[1]) &&
369+
!ts.isString(notCachedORDiagnostics)
370+
) return;
371+
addBaseline(
372+
`Incremental build contains ${actualFile} file ${!ts.isString(notCachedORDiagnostics) ? "has" : "does not have"} semantic errors, it does not match with clean build: ${outputFile}::`,
373+
`Incremental buildInfoText:: ${incrementalBuildText}`,
374+
`Clean buildInfoText:: ${cleanBuildText}`,
375+
);
376+
});
377+
readableCleanBuildInfo?.semanticDiagnosticsPerFile?.forEach((
378+
[expectedFile, cleanFileDiagnostics],
379+
) => {
380+
if (
381+
// if there are errors in the file
382+
!ts.isString(cleanFileDiagnostics?.[1]) &&
383+
// and its not already verified, its issue with the error caching
384+
!ts.find(
385+
readableIncrementalBuildInfo?.semanticDiagnosticsPerFile,
386+
([actualFile]) => actualFile === expectedFile,
387+
)
388+
) {
389+
addBaseline(
390+
`Incremental build does not contain ${expectedFile} file for semantic errors, clean build has semantic errors: ${outputFile}::`,
391+
`Incremental buildInfoText:: ${incrementalBuildText}`,
392+
`Clean buildInfoText:: ${cleanBuildText}`,
393+
);
394+
}
395+
});
396+
readableIncrementalBuildInfo?.emitDiagnosticsPerFile?.forEach(([actualFile]) => {
397+
if (
398+
// Does not have emit diagnostics in clean buildInfo
399+
!ts.find(
400+
readableCleanBuildInfo!.emitDiagnosticsPerFile,
401+
([expectedFile]) => actualFile === expectedFile,
402+
) &&
403+
// Is not marked as affectedFilesPendingEmit in clean buildInfo
404+
(!ts.find(
405+
(readableCleanBuildInfo as ReadableIncrementalMultiFileEmitBuildInfo).affectedFilesPendingEmit,
406+
([expectedFileOrArray]) => actualFile === (ts.isString(expectedFileOrArray) ? expectedFileOrArray : expectedFileOrArray[0]),
407+
)) &&
408+
// Program emit is not pending in clean buildInfo
409+
!(readableCleanBuildInfo as ReadableIncrementalBundleEmitBuildInfo).pendingEmit
410+
) {
411+
addBaseline(
412+
`Incremental build contains ${actualFile} file has emit errors, clean build does not have errors or does not mark is as pending emit: ${outputFile}::`,
413+
`Incremental buildInfoText:: ${incrementalBuildText}`,
414+
`Clean buildInfoText:: ${cleanBuildText}`,
415+
);
416+
}
417+
});
380418
}
381419
}
382420
if (!headerAdded && discrepancyExplanation) addBaseline("*** Supplied discrepancy explanation but didnt find any difference");
@@ -462,11 +500,15 @@ function getBuildInfoForIncrementalCorrectnessCheck(text: string | undefined): {
462500
fileIdsList: undefined,
463501
fileInfos: sanitizedFileInfos,
464502
// Ignore noEmit since that shouldnt be reason to emit the tsbuild info and presence of it in the buildinfo file does not matter
465-
options: readableBuildInfo.options ? { ...readableBuildInfo.options, noEmit: undefined } : undefined,
503+
options: readableBuildInfo.options && {
504+
...readableBuildInfo.options,
505+
noEmit: undefined,
506+
},
466507
affectedFilesPendingEmit: undefined,
467508
pendingEmit: undefined,
468509
emitDiagnosticsPerFile: undefined,
469510
latestChangedDtsFile: readableBuildInfo.latestChangedDtsFile ? "FakeFileName" : undefined,
511+
semanticDiagnosticsPerFile: undefined,
470512
} : undefined),
471513
size: undefined, // Size doesnt need to be equal
472514
}),

0 commit comments

Comments
 (0)