Skip to content

Commit

Permalink
To handle d.ts emit errors that could affect other files, in incremen…
Browse files Browse the repository at this point in the history 
…tal mode use d.ts emit text + diagnostics as signature of the file (#49543)

* Add test when declaration emit has errors and d.ts emit doesnt change which results in incorrect incremental behaviour

* Refactor

* Use declaration diagnostics in the d.ts signature for the file so it can be more accurate for detecting changes to file that could affect other files
Fixes #49527

* Renames and clarifications

* Simplify serialize declaration diagnostics for signature purpose
Do not serialize file name if error is in same file we are emitting. this should avoid having to do file path computation in most cases.
Locations are start and length instead of line and character.
Do not use any indents

* Fix baselines
  • Loading branch information
@sheetalkamat
sheetalkamat committed Jun 27, 2022
1 parent 8ed846c commit df21926
Show file tree
Hide file tree
Showing 11 changed files with 862 additions and 118 deletions.
131 changes: 107 additions & 24 deletions src/compiler/builder.ts
View file

Large diffs are not rendered by default.

86 changes: 61 additions & 25 deletions src/compiler/builderState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ namespace ts {
export function getFileEmitOutput(program: Program, sourceFile: SourceFile, emitOnlyDtsFiles: boolean,
cancellationToken?: CancellationToken, customTransformers?: CustomTransformers, forceDtsEmit?: boolean): EmitOutput {
const outputFiles: OutputFile[] = [];
const { emitSkipped, diagnostics, exportedModulesFromDeclarationEmit } = program.emit(sourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers, forceDtsEmit);
return { outputFiles, emitSkipped, diagnostics, exportedModulesFromDeclarationEmit };
const { emitSkipped, diagnostics } = program.emit(sourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers, forceDtsEmit);
return { outputFiles, emitSkipped, diagnostics };

function writeFile(fileName: string, text: string, writeByteOrderMark: boolean) {
outputFiles.push({ name: fileName, writeByteOrderMark, text });
Expand Down Expand Up @@ -321,24 +321,45 @@ namespace ts {
/**
* Gets the files affected by the path from the program
*/
export function getFilesAffectedBy(state: BuilderState, programOfThisState: Program, path: Path, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash): readonly SourceFile[] {
const result = getFilesAffectedByWithOldState(state, programOfThisState, path, cancellationToken, computeHash);
export function getFilesAffectedBy(
state: BuilderState,
programOfThisState: Program,
path: Path,
cancellationToken: CancellationToken | undefined,
computeHash: ComputeHash,
getCanonicalFileName: GetCanonicalFileName,
): readonly SourceFile[] {
const result = getFilesAffectedByWithOldState(
state,
programOfThisState,
path,
cancellationToken,
computeHash,
getCanonicalFileName,
);
state.oldSignatures?.clear();
state.oldExportedModulesMap?.clear();
return result;
}

export function getFilesAffectedByWithOldState(state: BuilderState, programOfThisState: Program, path: Path, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash): readonly SourceFile[] {
export function getFilesAffectedByWithOldState(
state: BuilderState,
programOfThisState: Program,
path: Path,
cancellationToken: CancellationToken | undefined,
computeHash: ComputeHash,
getCanonicalFileName: GetCanonicalFileName,
): readonly SourceFile[] {
const sourceFile = programOfThisState.getSourceFileByPath(path);
if (!sourceFile) {
return emptyArray;
}

if (!updateShapeSignature(state, programOfThisState, sourceFile, cancellationToken, computeHash)) {
if (!updateShapeSignature(state, programOfThisState, sourceFile, cancellationToken, computeHash, getCanonicalFileName)) {
return [sourceFile];
}

return (state.referencedMap ? getFilesAffectedByUpdatedShapeWhenModuleEmit : getFilesAffectedByUpdatedShapeWhenNonModuleEmit)(state, programOfThisState, sourceFile, cancellationToken, computeHash);
return (state.referencedMap ? getFilesAffectedByUpdatedShapeWhenModuleEmit : getFilesAffectedByUpdatedShapeWhenNonModuleEmit)(state, programOfThisState, sourceFile, cancellationToken, computeHash, getCanonicalFileName);
}

export function updateSignatureOfFile(state: BuilderState, signature: string | undefined, path: Path) {
Expand All @@ -349,30 +370,42 @@ namespace ts {
/**
* Returns if the shape of the signature has changed since last emit
*/
export function updateShapeSignature(state: BuilderState, programOfThisState: Program, sourceFile: SourceFile, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash, useFileVersionAsSignature = state.useFileVersionAsSignature) {
export function updateShapeSignature(
state: BuilderState,
programOfThisState: Program,
sourceFile: SourceFile,
cancellationToken: CancellationToken | undefined,
computeHash: ComputeHash,
getCanonicalFileName: GetCanonicalFileName,
useFileVersionAsSignature = state.useFileVersionAsSignature
) {
// If we have cached the result for this file, that means hence forth we should assume file shape is uptodate
if (state.hasCalledUpdateShapeSignature?.has(sourceFile.resolvedPath)) return false;

const info = state.fileInfos.get(sourceFile.resolvedPath)!;
const prevSignature = info.signature;
let latestSignature: string | undefined;
if (!sourceFile.isDeclarationFile && !useFileVersionAsSignature) {
const emitOutput = getFileEmitOutput(
programOfThisState,
programOfThisState.emit(
sourceFile,
/*emitOnlyDtsFiles*/ true,
(fileName, text, _writeByteOrderMark, _onError, sourceFiles, data) => {
Debug.assert(isDeclarationFileName(fileName), `File extension for signature expected to be dts: Got:: ${fileName}`);
latestSignature = computeSignatureWithDiagnostics(
sourceFile,
text,
computeHash,
getCanonicalFileName,
data,
);
if (latestSignature !== prevSignature) {
updateExportedModules(state, sourceFile, sourceFiles![0].exportedModulesFromDeclarationEmit);
}
},
cancellationToken,
/*emitOnlyDtsFiles*/ true,
/*customTransformers*/ undefined,
/*forceDtsEmit*/ true
);
const firstDts = firstOrUndefined(emitOutput.outputFiles);
if (firstDts) {
Debug.assert(isDeclarationFileName(firstDts.name), "File extension for signature expected to be dts", () => `Found: ${getAnyExtensionFromPath(firstDts.name)} for ${firstDts.name}:: All output files: ${JSON.stringify(emitOutput.outputFiles.map(f => f.name))}`);
latestSignature = computeSignature(firstDts.text, computeHash);
if (latestSignature !== prevSignature) {
updateExportedModules(state, sourceFile, emitOutput.exportedModulesFromDeclarationEmit);
}
}
}
// Default is to use file version as signature
if (latestSignature === undefined) {
Expand All @@ -395,10 +428,6 @@ namespace ts {
return latestSignature !== prevSignature;
}

export function computeSignature(text: string, computeHash: ComputeHash | undefined) {
return (computeHash || generateDjb2Hash)(text);
}

/**
* Coverts the declaration emit result into exported modules map
*/
Expand Down Expand Up @@ -556,7 +585,14 @@ namespace ts {
/**
* When program emits modular code, gets the files affected by the sourceFile whose shape has changed
*/
function getFilesAffectedByUpdatedShapeWhenModuleEmit(state: BuilderState, programOfThisState: Program, sourceFileWithUpdatedShape: SourceFile, cancellationToken: CancellationToken | undefined, computeHash: ComputeHash) {
function getFilesAffectedByUpdatedShapeWhenModuleEmit(
state: BuilderState,
programOfThisState: Program,
sourceFileWithUpdatedShape: SourceFile,
cancellationToken: CancellationToken | undefined,
computeHash: ComputeHash,
getCanonicalFileName: GetCanonicalFileName,
) {
if (isFileAffectingGlobalScope(sourceFileWithUpdatedShape)) {
return getAllFilesExcludingDefaultLibraryFile(state, programOfThisState, sourceFileWithUpdatedShape);
}
Expand All @@ -579,7 +615,7 @@ namespace ts {
if (!seenFileNamesMap.has(currentPath)) {
const currentSourceFile = programOfThisState.getSourceFileByPath(currentPath)!;
seenFileNamesMap.set(currentPath, currentSourceFile);
if (currentSourceFile && updateShapeSignature(state, programOfThisState, currentSourceFile, cancellationToken, computeHash)) {
if (currentSourceFile && updateShapeSignature(state, programOfThisState, currentSourceFile, cancellationToken, computeHash, getCanonicalFileName)) {
queue.push(...getReferencedByPaths(state, currentSourceFile.resolvedPath));
}
}
Expand Down
1 change: 0 additions & 1 deletion src/compiler/builderStatePublic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ namespace ts {
outputFiles: OutputFile[];
emitSkipped: boolean;
/* @internal */ diagnostics: readonly Diagnostic[];
/* @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
}

export interface OutputFile {
Expand Down
33 changes: 14 additions & 19 deletions src/compiler/emitter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,6 @@ namespace ts {
const { enter, exit } = performance.createTimer("printTime", "beforePrint", "afterPrint");
let bundleBuildInfo: BundleBuildInfo | undefined;
let emitSkipped = false;
let exportedModulesFromDeclarationEmit: ExportedModulesFromDeclarationEmit | undefined;

// Emit each output file
enter();
Expand All @@ -308,7 +307,6 @@ namespace ts {
diagnostics: emitterDiagnostics.getDiagnostics(),
emittedFiles: emittedFilesList,
sourceMaps: sourceMapDataList,
exportedModulesFromDeclarationEmit
};

function emitSourceFileOrBundle({ jsFilePath, sourceMapFilePath, declarationFilePath, declarationMapPath, buildInfoPath }: EmitFileNames, sourceFileOrBundle: SourceFile | Bundle | undefined) {
Expand Down Expand Up @@ -414,7 +412,7 @@ namespace ts {
});

Debug.assert(transform.transformed.length === 1, "Should only see one output from the transform");
printSourceFileOrBundle(jsFilePath, sourceMapFilePath, transform.transformed[0], printer, compilerOptions);
printSourceFileOrBundle(jsFilePath, sourceMapFilePath, transform, printer, compilerOptions);

// Clean up emit nodes on parse tree
transform.dispose();
Expand Down Expand Up @@ -453,7 +451,7 @@ namespace ts {
noEmitHelpers: true,
module: compilerOptions.module,
target: compilerOptions.target,
sourceMap: compilerOptions.sourceMap,
sourceMap: !forceDtsEmit && compilerOptions.declarationMap,
inlineSourceMap: compilerOptions.inlineSourceMap,
extendedDiagnostics: compilerOptions.extendedDiagnostics,
onlyPrintJsDocStyle: true,
Expand All @@ -478,20 +476,16 @@ namespace ts {
printSourceFileOrBundle(
declarationFilePath,
declarationMapPath,
declarationTransform.transformed[0],
declarationTransform,
declarationPrinter,
{
sourceMap: !forceDtsEmit && compilerOptions.declarationMap,
sourceMap: printerOptions.sourceMap,
sourceRoot: compilerOptions.sourceRoot,
mapRoot: compilerOptions.mapRoot,
extendedDiagnostics: compilerOptions.extendedDiagnostics,
// Explicitly do not passthru either `inline` option
}
);
if (forceDtsEmit && declarationTransform.transformed[0].kind === SyntaxKind.SourceFile) {
const sourceFile = declarationTransform.transformed[0];
exportedModulesFromDeclarationEmit = sourceFile.exportedModulesFromDeclarationEmit;
}
}
declarationTransform.dispose();
if (bundleBuildInfo) bundleBuildInfo.dts = declarationPrinter.bundleFileInfo;
Expand All @@ -511,7 +505,8 @@ namespace ts {
forEachChild(node, collectLinkedAliases);
}

function printSourceFileOrBundle(jsFilePath: string, sourceMapFilePath: string | undefined, sourceFileOrBundle: SourceFile | Bundle, printer: Printer, mapOptions: SourceMapOptions) {
function printSourceFileOrBundle(jsFilePath: string, sourceMapFilePath: string | undefined, transform: TransformationResult<SourceFile | Bundle>, printer: Printer, mapOptions: SourceMapOptions) {
const sourceFileOrBundle = transform.transformed[0];
const bundle = sourceFileOrBundle.kind === SyntaxKind.Bundle ? sourceFileOrBundle : undefined;
const sourceFile = sourceFileOrBundle.kind === SyntaxKind.SourceFile ? sourceFileOrBundle : undefined;
const sourceFiles = bundle ? bundle.sourceFiles : [sourceFile!];
Expand Down Expand Up @@ -559,7 +554,7 @@ namespace ts {
if (sourceMapFilePath) {
const sourceMap = sourceMapGenerator.toString();
writeFile(host, emitterDiagnostics, sourceMapFilePath, sourceMap, /*writeByteOrderMark*/ false, sourceFiles);
if (printer.bundleFileInfo) printer.bundleFileInfo.mapHash = BuilderState.computeSignature(sourceMap, maybeBind(host, host.createHash));
if (printer.bundleFileInfo) printer.bundleFileInfo.mapHash = computeSignature(sourceMap, maybeBind(host, host.createHash));
}
}
else {
Expand All @@ -568,10 +563,10 @@ namespace ts {

// Write the output file
const text = writer.getText();
writeFile(host, emitterDiagnostics, jsFilePath, text, !!compilerOptions.emitBOM, sourceFiles, { sourceMapUrlPos });
writeFile(host, emitterDiagnostics, jsFilePath, text, !!compilerOptions.emitBOM, sourceFiles, { sourceMapUrlPos, diagnostics: transform.diagnostics });
// We store the hash of the text written in the buildinfo to ensure that text of the referenced d.ts file is same as whats in the buildinfo
// This is needed because incremental can be toggled between two runs and we might use stale file text to do text manipulation in prepend mode
if (printer.bundleFileInfo) printer.bundleFileInfo.hash = BuilderState.computeSignature(text, maybeBind(host, host.createHash));
if (printer.bundleFileInfo) printer.bundleFileInfo.hash = computeSignature(text, maybeBind(host, host.createHash));

// Reset state
writer.clear();
Expand Down Expand Up @@ -774,20 +769,20 @@ namespace ts {
const jsFileText = host.readFile(Debug.checkDefined(jsFilePath));
if (!jsFileText) return jsFilePath!;
// If the jsFileText is not same has what it was created with, tsbuildinfo is stale so dont use it
if (BuilderState.computeSignature(jsFileText, createHash) !== buildInfo.bundle.js.hash) return jsFilePath!;
if (computeSignature(jsFileText, createHash) !== buildInfo.bundle.js.hash) return jsFilePath!;
const sourceMapText = sourceMapFilePath && host.readFile(sourceMapFilePath);
// error if no source map or for now if inline sourcemap
if ((sourceMapFilePath && !sourceMapText) || config.options.inlineSourceMap) return sourceMapFilePath || "inline sourcemap decoding";
if (sourceMapFilePath && BuilderState.computeSignature(sourceMapText!, createHash) !== buildInfo.bundle.js.mapHash) return sourceMapFilePath;
if (sourceMapFilePath && computeSignature(sourceMapText!, createHash) !== buildInfo.bundle.js.mapHash) return sourceMapFilePath;

// read declaration text
const declarationText = declarationFilePath && host.readFile(declarationFilePath);
if (declarationFilePath && !declarationText) return declarationFilePath;
if (declarationFilePath && BuilderState.computeSignature(declarationText!, createHash) !== buildInfo.bundle.dts!.hash) return declarationFilePath;
if (declarationFilePath && computeSignature(declarationText!, createHash) !== buildInfo.bundle.dts!.hash) return declarationFilePath;
const declarationMapText = declarationMapPath && host.readFile(declarationMapPath);
// error if no source map or for now if inline sourcemap
if ((declarationMapPath && !declarationMapText) || config.options.inlineSourceMap) return declarationMapPath || "inline sourcemap decoding";
if (declarationMapPath && BuilderState.computeSignature(declarationMapText!, createHash) !== buildInfo.bundle.dts!.mapHash) return declarationMapPath;
if (declarationMapPath && computeSignature(declarationMapText!, createHash) !== buildInfo.bundle.dts!.mapHash) return declarationMapPath;

const buildInfoDirectory = getDirectoryPath(getNormalizedAbsolutePath(buildInfoPath!, host.getCurrentDirectory()));
const ownPrependInput = createInputFiles(
Expand Down Expand Up @@ -836,7 +831,7 @@ namespace ts {
newBuildInfo.program = buildInfo.program;
if (newBuildInfo.program && changedDtsText !== undefined && config.options.composite) {
// Update the output signature
(newBuildInfo.program as ProgramBundleEmitBuildInfo).outSignature = computeSignature(changedDtsText, changedDtsData, createHash);
(newBuildInfo.program as ProgramBundleEmitBuildInfo).outSignature = computeSignature(changedDtsText, createHash, changedDtsData);
newBuildInfo.program.dtsChangeTime = getCurrentTime(host).getTime();
}
// Update sourceFileInfo
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4026,6 +4026,7 @@ namespace ts {
export interface WriteFileCallbackData {
/*@internal*/ sourceMapUrlPos?: number;
/*@internal*/ buildInfo?: BuildInfo;
/*@internal*/ diagnostics?: readonly DiagnosticWithLocation[];
}
export type WriteFileCallback = (
fileName: string,
Expand Down Expand Up @@ -4335,7 +4336,6 @@ namespace ts {
diagnostics: readonly Diagnostic[];
emittedFiles?: string[]; // Array of files the compiler wrote to disk
/* @internal */ sourceMaps?: SourceMapEmitResult[]; // Array of sourceMapData if compiler emitted sourcemaps
/* @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
}

/* @internal */
Expand Down
3 changes: 2 additions & 1 deletion src/server/project.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,8 @@ namespace ts.server {
this.program!,
scriptInfo.path,
this.cancellationToken,
maybeBind(this.projectService.host, this.projectService.host.createHash)
maybeBind(this.projectService.host, this.projectService.host.createHash),
this.getCanonicalFileName,
),
sourceFile => this.shouldEmitFile(this.projectService.getScriptInfoForPath(sourceFile.path)) ? sourceFile.fileName : undefined
);
Expand Down
2 changes: 0 additions & 2 deletions src/testRunner/unittests/services/languageService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export function Component(x: Config): any;`
emitSkipped: true,
diagnostics: emptyArray,
outputFiles: emptyArray,
exportedModulesFromDeclarationEmit: undefined
}
);

Expand All @@ -80,7 +79,6 @@ export function Component(x: Config): any;`
text: "export {};\r\n",
writeByteOrderMark: false
}],
exportedModulesFromDeclarationEmit: undefined
}
);
});
Expand Down

0 comments on commit df21926

Please sign in to comment.