Skip to content
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

Skip verifyCompilerOptions when possible on program updates #60754

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Skip verifyCompilerOptions when possible on program updates #60754

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2432b01
Reuse config diagnostics between programs with reused structure and i…
andrewbranch Dec 13, 2024
88149ba
Remove debug change
andrewbranch Dec 13, 2024
233f344
Extract reusable diagnostic state to separate object
andrewbranch Dec 18, 2024
2341668
Fix API
andrewbranch Dec 18, 2024
1221256
Knip
andrewbranch Dec 18, 2024
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
63 changes: 44 additions & 19 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
DiagnosticArguments,
DiagnosticCategory,
diagnosticCategoryName,
DiagnosticCollection,
DiagnosticMessage,
DiagnosticMessageChain,
DiagnosticReporter,
Expand Down Expand Up @@ -246,6 +247,7 @@ import {
noTransformers,
ObjectLiteralExpression,
OperationCanceledException,
optionDeclarations,
optionsHaveChanges,
PackageId,
packageIdToPackageName,
Expand Down Expand Up @@ -1666,7 +1668,9 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
* Only add diagnostics directly if it always would be done irrespective of program structure reuse.
* Otherwise fileProcessingDiagnostics is correct locations so that the diagnostics can be reported in all structure use scenarios
*/
const programDiagnostics = createDiagnosticCollection();
let programDiagnostics: DiagnosticCollection | undefined;
let reuseConfigDiagnostics = false;
let configDiagnostics: DiagnosticCollection | undefined;
let lazyProgramDiagnosticExplainingFile: LazyProgramDiagnosticExplainingFile[] | undefined = [];
const currentDirectory = host.getCurrentDirectory();
const supportedExtensions = getSupportedExtensions(options);
Expand Down Expand Up @@ -2006,6 +2010,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
resolvedModules,
resolvedTypeReferenceDirectiveNames,
resolvedLibReferences,
getConfigDiagnostics: () => configDiagnostics,
getResolvedModule,
getResolvedModuleFromModuleSpecifier,
getResolvedTypeReferenceDirective,
Expand Down Expand Up @@ -2046,20 +2051,28 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg

onProgramCreateComplete();

verifyCompilerOptions();
if (!reuseConfigDiagnostics) {
verifyCompilerOptions();
}
performance.mark("afterProgram");
performance.measure("Program", "beforeProgram", "afterProgram");
tracing?.pop();

return program;

function updateAndGetProgramDiagnostics() {
if (programDiagnostics) {
return programDiagnostics;
}

programDiagnostics = createDiagnosticCollection();
configDiagnostics?.getDiagnostics().forEach(d => programDiagnostics!.add(d));
if (lazyProgramDiagnosticExplainingFile) {
// Add file processingDiagnostics
fileProcessingDiagnostics?.forEach(diagnostic => {
switch (diagnostic.kind) {
case FilePreprocessingDiagnosticsKind.FilePreprocessingFileExplainingDiagnostic:
return programDiagnostics.add(
return programDiagnostics!.add(
createDiagnosticExplainingFile(
diagnostic.file && getSourceFileByPath(diagnostic.file),
diagnostic.fileProcessingReason,
Expand All @@ -2068,15 +2081,15 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
),
);
case FilePreprocessingDiagnosticsKind.FilePreprocessingLibReferenceDiagnostic:
return programDiagnostics.add(filePreprocessingLibreferenceDiagnostic(diagnostic));
return programDiagnostics!.add(filePreprocessingLibreferenceDiagnostic(diagnostic));
case FilePreprocessingDiagnosticsKind.ResolutionDiagnostics:
return diagnostic.diagnostics.forEach(d => programDiagnostics.add(d));
return diagnostic.diagnostics.forEach(d => programDiagnostics!.add(d));
default:
Debug.assertNever(diagnostic);
}
});
lazyProgramDiagnosticExplainingFile.forEach(({ file, diagnostic, args }) =>
programDiagnostics.add(
programDiagnostics!.add(
createDiagnosticExplainingFile(file, /*fileProcessingReason*/ undefined, diagnostic, args),
)
);
Expand Down Expand Up @@ -2722,6 +2735,14 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
filesByName.set(path, filesByName.get(oldFile.path));
});

if (
oldOptions.configFile && oldOptions.configFile === options.configFile ||
!oldOptions.configFile && !options.configFile && !optionsHaveChanges(oldOptions, options, optionDeclarations)
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to check that the program's file set are the same as well, because a new or deleted file can invalidate/introduce some errors. Maybe it's done elsewhere, but it would be good to add a test for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Early returns above handle that (we wouldn’t be at StructureIsReused.Completely if that happened)

) {
configDiagnostics = oldProgram.getConfigDiagnostics();
reuseConfigDiagnostics = true;
}

files = newSourceFiles;
fileReasons = oldProgram.getFileIncludeReasons();
fileProcessingDiagnostics = oldProgram.getFileProcessingDiagnostics();
Expand Down Expand Up @@ -4308,7 +4329,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg

const outputFile = options.outFile;
if (!options.tsBuildInfoFile && options.incremental && !outputFile && !options.configFilePath) {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Option_incremental_can_only_be_specified_using_tsconfig_emitting_to_single_file_or_when_option_tsBuildInfoFile_is_specified));
addConfigDiagnostic(createCompilerDiagnostic(Diagnostics.Option_incremental_can_only_be_specified_using_tsconfig_emitting_to_single_file_or_when_option_tsBuildInfoFile_is_specified));
}

verifyDeprecatedCompilerOptions();
Expand Down Expand Up @@ -4410,7 +4431,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
else if (firstNonAmbientExternalModuleSourceFile && languageVersion < ScriptTarget.ES2015 && options.module === ModuleKind.None) {
// We cannot use createDiagnosticFromNode because nodes do not have parents yet
const span = getErrorSpanForNode(firstNonAmbientExternalModuleSourceFile, typeof firstNonAmbientExternalModuleSourceFile.externalModuleIndicator === "boolean" ? firstNonAmbientExternalModuleSourceFile : firstNonAmbientExternalModuleSourceFile.externalModuleIndicator!);
programDiagnostics.add(createFileDiagnostic(firstNonAmbientExternalModuleSourceFile, span.start, span.length, Diagnostics.Cannot_use_imports_exports_or_module_augmentations_when_module_is_none));
addConfigDiagnostic(createFileDiagnostic(firstNonAmbientExternalModuleSourceFile, span.start, span.length, Diagnostics.Cannot_use_imports_exports_or_module_augmentations_when_module_is_none));
}

// Cannot specify module gen that isn't amd or system with --out
Expand All @@ -4420,7 +4441,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
}
else if (options.module === undefined && firstNonAmbientExternalModuleSourceFile) {
const span = getErrorSpanForNode(firstNonAmbientExternalModuleSourceFile, typeof firstNonAmbientExternalModuleSourceFile.externalModuleIndicator === "boolean" ? firstNonAmbientExternalModuleSourceFile : firstNonAmbientExternalModuleSourceFile.externalModuleIndicator!);
programDiagnostics.add(createFileDiagnostic(firstNonAmbientExternalModuleSourceFile, span.start, span.length, Diagnostics.Cannot_compile_modules_using_option_0_unless_the_module_flag_is_amd_or_system, "outFile"));
addConfigDiagnostic(createFileDiagnostic(firstNonAmbientExternalModuleSourceFile, span.start, span.length, Diagnostics.Cannot_compile_modules_using_option_0_unless_the_module_flag_is_amd_or_system, "outFile"));
}
}

Expand Down Expand Up @@ -4965,7 +4986,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
forEachPropertyAssignment(pathProp.initializer, key, keyProps => {
const initializer = keyProps.initializer;
if (isArrayLiteralExpression(initializer) && initializer.elements.length > valueIndex) {
programDiagnostics.add(createDiagnosticForNodeInSourceFile(options.configFile!, initializer.elements[valueIndex], message, ...args));
addConfigDiagnostic(createDiagnosticForNodeInSourceFile(options.configFile!, initializer.elements[valueIndex], message, ...args));
needCompilerDiagnostic = false;
}
});
Expand Down Expand Up @@ -5027,10 +5048,10 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
function createDiagnosticForReference(sourceFile: JsonSourceFile | undefined, index: number, message: DiagnosticMessage, ...args: DiagnosticArguments) {
const referencesSyntax = forEachTsConfigPropArray(sourceFile || options.configFile, "references", property => isArrayLiteralExpression(property.initializer) ? property.initializer : undefined);
if (referencesSyntax && referencesSyntax.elements.length > index) {
programDiagnostics.add(createDiagnosticForNodeInSourceFile(sourceFile || options.configFile!, referencesSyntax.elements[index], message, ...args));
addConfigDiagnostic(createDiagnosticForNodeInSourceFile(sourceFile || options.configFile!, referencesSyntax.elements[index], message, ...args));
}
else {
programDiagnostics.add(createCompilerDiagnostic(message, ...args));
addConfigDiagnostic(createCompilerDiagnostic(message, ...args));
}
}

Expand All @@ -5054,18 +5075,18 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
if (compilerOptionsProperty) {
// eslint-disable-next-line local/no-in-operator
if ("messageText" in message) {
programDiagnostics.add(createDiagnosticForNodeFromMessageChain(options.configFile!, compilerOptionsProperty.name, message));
addConfigDiagnostic(createDiagnosticForNodeFromMessageChain(options.configFile!, compilerOptionsProperty.name, message));
}
else {
programDiagnostics.add(createDiagnosticForNodeInSourceFile(options.configFile!, compilerOptionsProperty.name, message, ...args));
addConfigDiagnostic(createDiagnosticForNodeInSourceFile(options.configFile!, compilerOptionsProperty.name, message, ...args));
}
}
// eslint-disable-next-line local/no-in-operator
else if ("messageText" in message) {
programDiagnostics.add(createCompilerDiagnosticFromMessageChain(message));
addConfigDiagnostic(createCompilerDiagnosticFromMessageChain(message));
}
else {
programDiagnostics.add(createCompilerDiagnostic(message, ...args));
addConfigDiagnostic(createCompilerDiagnostic(message, ...args));
}
}

Expand Down Expand Up @@ -5096,10 +5117,10 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
forEachPropertyAssignment(objectLiteral, key1, prop => {
// eslint-disable-next-line local/no-in-operator
if ("messageText" in message) {
programDiagnostics.add(createDiagnosticForNodeFromMessageChain(options.configFile!, onKey ? prop.name : prop.initializer, message));
addConfigDiagnostic(createDiagnosticForNodeFromMessageChain(options.configFile!, onKey ? prop.name : prop.initializer, message));
}
else {
programDiagnostics.add(createDiagnosticForNodeInSourceFile(options.configFile!, onKey ? prop.name : prop.initializer, message, ...args));
addConfigDiagnostic(createDiagnosticForNodeInSourceFile(options.configFile!, onKey ? prop.name : prop.initializer, message, ...args));
}
needsCompilerDiagnostic = true;
}, key2);
Expand All @@ -5108,7 +5129,11 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg

function blockEmittingOfFile(emitFileName: string, diag: Diagnostic) {
hasEmitBlockingDiagnostics.set(toPath(emitFileName), true);
programDiagnostics.add(diag);
addConfigDiagnostic(diag);
}

function addConfigDiagnostic(diag: Diagnostic) {
(configDiagnostics ??= createDiagnosticCollection()).add(diag);
}

function isEmittedFile(file: string): boolean {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4892,6 +4892,7 @@ export interface Program extends ScriptReferenceHost {
* @internal
*/
resolvedLibReferences: Map<string, LibResolution> | undefined;
/** @internal */ getConfigDiagnostics: () => DiagnosticCollection | undefined;
/** @internal */ getCurrentPackagesMap(): Map<string, boolean> | undefined;
/**
* Is the file emitted file
Expand Down
Loading