Skip to content

Some refactoring so we arent checking undefined on program or asserting it so much #58782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Some refactoring so we arent checking undefined on program or asserting it so much #58782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
335 changes: 231 additions & 104 deletions src/compiler/builder.ts
View file
Open in desktop

Large diffs are not rendered by default.

115 changes: 101 additions & 14 deletions src/compiler/builderPublic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export type HostForComputeHash = Pick<BuilderProgramHost, "createHash" | "storeS
*/
export interface BuilderProgram {
/** @internal */
getState(): ReusableBuilderProgramState;
state: ReusableBuilderProgramState;
/** @internal */
saveEmitState(): SavedBuildProgramEmitState;
/** @internal */
Expand Down Expand Up @@ -164,28 +164,115 @@ export interface EmitAndSemanticDiagnosticsBuilderProgram extends SemanticDiagno
/**
* Create the builder to manage semantic diagnostics and cache them
*/
export function createSemanticDiagnosticsBuilderProgram(newProgram: Program, host: BuilderProgramHost, oldProgram?: SemanticDiagnosticsBuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[]): SemanticDiagnosticsBuilderProgram;
export function createSemanticDiagnosticsBuilderProgram(rootNames: readonly string[] | undefined, options: CompilerOptions | undefined, host?: CompilerHost, oldProgram?: SemanticDiagnosticsBuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[], projectReferences?: readonly ProjectReference[]): SemanticDiagnosticsBuilderProgram;
export function createSemanticDiagnosticsBuilderProgram(newProgramOrRootNames: Program | readonly string[] | undefined, hostOrOptions: BuilderProgramHost | CompilerOptions | undefined, oldProgramOrHost?: CompilerHost | SemanticDiagnosticsBuilderProgram, configFileParsingDiagnosticsOrOldProgram?: readonly Diagnostic[] | SemanticDiagnosticsBuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[], projectReferences?: readonly ProjectReference[]) {
return createBuilderProgram(BuilderProgramKind.SemanticDiagnosticsBuilderProgram, getBuilderCreationParameters(newProgramOrRootNames, hostOrOptions, oldProgramOrHost, configFileParsingDiagnosticsOrOldProgram, configFileParsingDiagnostics, projectReferences));
export function createSemanticDiagnosticsBuilderProgram(
newProgram: Program,
host: BuilderProgramHost,
oldProgram?: SemanticDiagnosticsBuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
): SemanticDiagnosticsBuilderProgram;
export function createSemanticDiagnosticsBuilderProgram(
rootNames: readonly string[] | undefined,
options: CompilerOptions | undefined,
host?: CompilerHost,
oldProgram?: SemanticDiagnosticsBuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
projectReferences?: readonly ProjectReference[],
): SemanticDiagnosticsBuilderProgram;
export function createSemanticDiagnosticsBuilderProgram(
newProgramOrRootNames: Program | readonly string[] | undefined,
hostOrOptions: BuilderProgramHost | CompilerOptions | undefined,
oldProgramOrHost?: CompilerHost | SemanticDiagnosticsBuilderProgram,
configFileParsingDiagnosticsOrOldProgram?: readonly Diagnostic[] | SemanticDiagnosticsBuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
projectReferences?: readonly ProjectReference[],
) {
return createBuilderProgram(
BuilderProgramKind.SemanticDiagnosticsBuilderProgram,
getBuilderCreationParameters(
newProgramOrRootNames,
hostOrOptions,
oldProgramOrHost,
configFileParsingDiagnosticsOrOldProgram,
configFileParsingDiagnostics,
projectReferences,
),
);
}

/**
* Create the builder that can handle the changes in program and iterate through changed files
* to emit the those files and manage semantic diagnostics cache as well
*/
export function createEmitAndSemanticDiagnosticsBuilderProgram(newProgram: Program, host: BuilderProgramHost, oldProgram?: EmitAndSemanticDiagnosticsBuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[]): EmitAndSemanticDiagnosticsBuilderProgram;
export function createEmitAndSemanticDiagnosticsBuilderProgram(rootNames: readonly string[] | undefined, options: CompilerOptions | undefined, host?: CompilerHost, oldProgram?: EmitAndSemanticDiagnosticsBuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[], projectReferences?: readonly ProjectReference[]): EmitAndSemanticDiagnosticsBuilderProgram;
export function createEmitAndSemanticDiagnosticsBuilderProgram(newProgramOrRootNames: Program | readonly string[] | undefined, hostOrOptions: BuilderProgramHost | CompilerOptions | undefined, oldProgramOrHost?: CompilerHost | EmitAndSemanticDiagnosticsBuilderProgram, configFileParsingDiagnosticsOrOldProgram?: readonly Diagnostic[] | EmitAndSemanticDiagnosticsBuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[], projectReferences?: readonly ProjectReference[]) {
return createBuilderProgram(BuilderProgramKind.EmitAndSemanticDiagnosticsBuilderProgram, getBuilderCreationParameters(newProgramOrRootNames, hostOrOptions, oldProgramOrHost, configFileParsingDiagnosticsOrOldProgram, configFileParsingDiagnostics, projectReferences));
export function createEmitAndSemanticDiagnosticsBuilderProgram(
newProgram: Program,
host: BuilderProgramHost,
oldProgram?: EmitAndSemanticDiagnosticsBuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
): EmitAndSemanticDiagnosticsBuilderProgram;
export function createEmitAndSemanticDiagnosticsBuilderProgram(
rootNames: readonly string[] | undefined,
options: CompilerOptions | undefined,
host?: CompilerHost,
oldProgram?: EmitAndSemanticDiagnosticsBuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
projectReferences?: readonly ProjectReference[],
): EmitAndSemanticDiagnosticsBuilderProgram;
export function createEmitAndSemanticDiagnosticsBuilderProgram(
newProgramOrRootNames: Program | readonly string[] | undefined,
hostOrOptions: BuilderProgramHost | CompilerOptions | undefined,
oldProgramOrHost?: CompilerHost | EmitAndSemanticDiagnosticsBuilderProgram,
configFileParsingDiagnosticsOrOldProgram?: readonly Diagnostic[] | EmitAndSemanticDiagnosticsBuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
projectReferences?: readonly ProjectReference[],
) {
return createBuilderProgram(
BuilderProgramKind.EmitAndSemanticDiagnosticsBuilderProgram,
getBuilderCreationParameters(
newProgramOrRootNames,
hostOrOptions,
oldProgramOrHost,
configFileParsingDiagnosticsOrOldProgram,
configFileParsingDiagnostics,
projectReferences,
),
);
}

/**
* Creates a builder thats just abstraction over program and can be used with watch
*/
export function createAbstractBuilder(newProgram: Program, host: BuilderProgramHost, oldProgram?: BuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[]): BuilderProgram;
export function createAbstractBuilder(rootNames: readonly string[] | undefined, options: CompilerOptions | undefined, host?: CompilerHost, oldProgram?: BuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[], projectReferences?: readonly ProjectReference[]): BuilderProgram;
export function createAbstractBuilder(newProgramOrRootNames: Program | readonly string[] | undefined, hostOrOptions: BuilderProgramHost | CompilerOptions | undefined, oldProgramOrHost?: CompilerHost | BuilderProgram, configFileParsingDiagnosticsOrOldProgram?: readonly Diagnostic[] | BuilderProgram, configFileParsingDiagnostics?: readonly Diagnostic[], projectReferences?: readonly ProjectReference[]): BuilderProgram {
const { newProgram, configFileParsingDiagnostics: newConfigFileParsingDiagnostics } = getBuilderCreationParameters(newProgramOrRootNames, hostOrOptions, oldProgramOrHost, configFileParsingDiagnosticsOrOldProgram, configFileParsingDiagnostics, projectReferences);
return createRedirectedBuilderProgram(() => ({ program: newProgram, compilerOptions: newProgram.getCompilerOptions() }), newConfigFileParsingDiagnostics);
export function createAbstractBuilder(
newProgram: Program,
host: BuilderProgramHost,
oldProgram?: BuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
): BuilderProgram;
export function createAbstractBuilder(
rootNames: readonly string[] | undefined,
options: CompilerOptions | undefined,
host?: CompilerHost,
oldProgram?: BuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
projectReferences?: readonly ProjectReference[],
): BuilderProgram;
export function createAbstractBuilder(
newProgramOrRootNames: Program | readonly string[] | undefined,
hostOrOptions: BuilderProgramHost | CompilerOptions | undefined,
oldProgramOrHost?: CompilerHost | BuilderProgram,
configFileParsingDiagnosticsOrOldProgram?: readonly Diagnostic[] | BuilderProgram,
configFileParsingDiagnostics?: readonly Diagnostic[],
projectReferences?: readonly ProjectReference[],
): BuilderProgram {
const { newProgram, configFileParsingDiagnostics: newConfigFileParsingDiagnostics } = getBuilderCreationParameters(
newProgramOrRootNames,
hostOrOptions,
oldProgramOrHost,
configFileParsingDiagnosticsOrOldProgram,
configFileParsingDiagnostics,
projectReferences,
);
return createRedirectedBuilderProgram(
{ program: newProgram, compilerOptions: newProgram.getCompilerOptions() },
newConfigFileParsingDiagnostics,
);
}
4 changes: 2 additions & 2 deletions src/compiler/watch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,8 @@ function createTabularErrorsDisplay(filesInError: (ReportFileInError | undefined
}

/** @internal */
export function isBuilderProgram(program: Program | BuilderProgram): program is BuilderProgram {
return !!(program as BuilderProgram).getState;
export function isBuilderProgram<T extends BuilderProgram>(program: Program | BuilderProgram): program is T {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unsafe; this could conform to anything, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's copied from somewhere else...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

return !!(program as T).state;
}

/** @internal */
Expand Down
7 changes: 2 additions & 5 deletions src/compiler/watchUtilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
identity,
insertSorted,
isArray,
isBuilderProgram,
isDeclarationFileName,
isExcludedFile,
isSupportedSourceFileName,
Expand Down Expand Up @@ -627,7 +628,7 @@ export function isIgnoredFileFromWildCardWatching({
return realProgram ?
!!realProgram.getSourceFileByPath(file) :
builderProgram ?
builderProgram.getState().fileInfos.has(file) :
builderProgram.state.fileInfos.has(file) :
!!find(program as readonly string[], rootFile => toPath(rootFile) === file);
}

Expand All @@ -651,10 +652,6 @@ export function isIgnoredFileFromWildCardWatching({
}
}

function isBuilderProgram<T extends BuilderProgram>(program: Program | T): program is T {
return !!(program as T).getState;
}

/** @internal */
export function isEmittedFileOfProgram(program: Program | undefined, file: string) {
if (!program) {
Expand Down
7 changes: 3 additions & 4 deletions src/testRunner/unittests/helpers/baseline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,8 @@ function baselineProgram(baseline: string[], [program, builderProgram]: CommandL

if (!builderProgram) return;
if (builderProgram !== oldProgram?.[1]) {
const state = builderProgram.getState();
const internalState = state as unknown as ts.BuilderProgramState;
if (state.semanticDiagnosticsPerFile.size) {
const internalState = builderProgram.state as ts.BuilderProgramState;
if (builderProgram.state.semanticDiagnosticsPerFile.size) {
baseline.push("Semantic diagnostics in builder refreshed for::");
for (const file of program.getSourceFiles()) {
if (!internalState.semanticDiagnosticsFromOldState || !internalState.semanticDiagnosticsFromOldState.has(file.resolvedPath)) {
Expand All @@ -90,7 +89,7 @@ function baselineProgram(baseline: string[], [program, builderProgram]: CommandL
if (internalState.hasCalledUpdateShapeSignature?.size) {
baseline.push("Shape signatures in builder refreshed for::");
internalState.hasCalledUpdateShapeSignature.forEach((path: ts.Path) => {
const info = state.fileInfos.get(path);
const info = builderProgram.state.fileInfos.get(path);
const signatureInfo = internalState.signatureInfo?.get(path)!;
switch (signatureInfo) {
case ts.SignatureInfo.ComputedDts:
Expand Down
3 changes: 1 addition & 2 deletions src/testRunner/unittests/helpers/tsc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ function storeDtsSignatures(sys: TscCompileSystem, programs: readonly CommandLin
sys.dtsSignaures ??= new Map();
const dtsSignatureData = new Map<string, string>();
sys.dtsSignaures.set(`${toPathWithSystem(sys, buildInfoPath)}.readable.baseline.txt` as ts.Path, dtsSignatureData);
const state = builderProgram.getState();
state.hasCalledUpdateShapeSignature?.forEach(resolvedPath => {
builderProgram.state.hasCalledUpdateShapeSignature?.forEach(resolvedPath => {
const file = program.getSourceFileByPath(resolvedPath);
if (!file || file.isDeclarationFile) return;
// Compute dts and exported map and store it
Expand Down
25 changes: 12 additions & 13 deletions src/testRunner/unittests/tscWatch/incremental.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,42 +170,41 @@ describe("unittests:: tsc-watch:: emit file --incremental", () => {
host: ts.createIncrementalCompilerHost(command.options, system),
});

const state = builderProgram.getState();
assert.equal(state.changedFilesSet!.size, 0, "changes");
assert.equal(builderProgram.state.changedFilesSet!.size, 0, "changes");

assert.equal(state.fileInfos.size, 3, "FileInfo size");
assert.deepEqual(state.fileInfos.get(libFile.path as ts.Path), {
assert.equal(builderProgram.state.fileInfos.size, 3, "FileInfo size");
assert.deepEqual(builderProgram.state.fileInfos.get(libFile.path as ts.Path), {
version: system.createHash(libFile.content),
signature: system.createHash(libFile.content),
affectsGlobalScope: true,
impliedFormat: ts.ModuleKind.CommonJS,
});
assert.deepEqual(state.fileInfos.get(file1.path as ts.Path), {
assert.deepEqual(builderProgram.state.fileInfos.get(file1.path as ts.Path), {
version: system.createHash(file1.content),
signature: system.createHash(file1.content),
affectsGlobalScope: undefined,
impliedFormat: ts.ModuleKind.CommonJS,
});
assert.deepEqual(state.fileInfos.get(file2.path as ts.Path), {
assert.deepEqual(builderProgram.state.fileInfos.get(file2.path as ts.Path), {
version: system.createHash(fileModified.content),
signature: system.createHash(fileModified.content),
affectsGlobalScope: undefined,
impliedFormat: ts.ModuleKind.CommonJS,
});

assert.deepEqual(state.compilerOptions, {
assert.deepEqual(builderProgram.state.compilerOptions, {
incremental: true,
module: ts.ModuleKind.AMD,
configFilePath: config.path,
});

assert.equal(ts.arrayFrom(state.referencedMap!.keys()).length, 0);
assert.equal(ts.arrayFrom(builderProgram.state.referencedMap!.keys()).length, 0);

assert.equal(state.semanticDiagnosticsPerFile.size, 3);
assert.deepEqual(state.semanticDiagnosticsPerFile.get(libFile.path as ts.Path), ts.emptyArray);
assert.deepEqual(state.semanticDiagnosticsPerFile.get(file1.path as ts.Path), ts.emptyArray);
assert.deepEqual(state.semanticDiagnosticsPerFile.get(file2.path as ts.Path), [{
file: state.program!.getSourceFileByPath(file2.path as ts.Path)!,
assert.equal(builderProgram.state.semanticDiagnosticsPerFile.size, 3);
assert.deepEqual(builderProgram.state.semanticDiagnosticsPerFile.get(libFile.path as ts.Path), ts.emptyArray);
assert.deepEqual(builderProgram.state.semanticDiagnosticsPerFile.get(file1.path as ts.Path), ts.emptyArray);
assert.deepEqual(builderProgram.state.semanticDiagnosticsPerFile.get(file2.path as ts.Path), [{
file: builderProgram.state.program!.getSourceFileByPath(file2.path as ts.Path)!,
start: 13,
length: 1,
code: ts.Diagnostics.Type_0_is_not_assignable_to_type_1.code,
Expand Down