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

Allows emitting buildInfo when --noEmit is specified #39122

Merged
merged 2 commits into from
Jun 18, 2020
Merged

Allows emitting buildInfo when --noEmit is specified #39122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a741bc
Some tests
sheetalkamat Jun 15, 2020
e18dd33
Allow noEmit with incremental and composite
sheetalkamat Jun 16, 2020
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
7 changes: 5 additions & 2 deletions src/compiler/builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ namespace ts {
reportsUnnecessary?: {};
source?: string;
relatedInformation?: ReusableDiagnosticRelatedInformation[];
skippedOn?: keyof CompilerOptions;
}

export interface ReusableDiagnosticRelatedInformation {
Expand Down Expand Up @@ -268,6 +269,7 @@ namespace ts {
const result: Diagnostic = convertToDiagnosticRelatedInformation(diagnostic, newProgram, toPath);
result.reportsUnnecessary = diagnostic.reportsUnnecessary;
result.source = diagnostic.source;
result.skippedOn = diagnostic.skippedOn;
const { relatedInformation } = diagnostic;
result.relatedInformation = relatedInformation ?
relatedInformation.length ?
Expand Down Expand Up @@ -676,7 +678,7 @@ namespace ts {
const cachedDiagnostics = state.semanticDiagnosticsPerFile.get(path);
// Report the bind and check diagnostics from the cache if we already have those diagnostics present
if (cachedDiagnostics) {
return cachedDiagnostics;
return filterSemanticDiagnotics(cachedDiagnostics, state.compilerOptions);
}
}

Expand All @@ -685,7 +687,7 @@ namespace ts {
if (state.semanticDiagnosticsPerFile) {
state.semanticDiagnosticsPerFile.set(path, diagnostics);
}
return diagnostics;
return filterSemanticDiagnotics(diagnostics, state.compilerOptions);
}

export type ProgramBuildInfoDiagnostic = string | [string, readonly ReusableDiagnostic[]];
Expand Down Expand Up @@ -816,6 +818,7 @@ namespace ts {
const result: ReusableDiagnostic = convertToReusableDiagnosticRelatedInformation(diagnostic, relativeToBuildInfo);
result.reportsUnnecessary = diagnostic.reportsUnnecessary;
result.source = diagnostic.source;
result.skippedOn = diagnostic.skippedOn;
const { relatedInformation } = diagnostic;
result.relatedInformation = relatedInformation ?
relatedInformation.length ?
Expand Down
35 changes: 25 additions & 10 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,12 @@ namespace ts {
}
}

function errorSkippedOn(key: keyof CompilerOptions, location: Node | undefined, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number, arg3?: string | number): Diagnostic {
Copy link
Member

Choose a reason for hiding this comment

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

I'm having trouble wrapping my head around what this does and would find an explanation helpful.

Copy link
Member

Choose a reason for hiding this comment

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

location, message, arg0, ... are the existing error logging parameters.

key: keyof CompilerOptions is a typesafe way to assert that key, while a string, actually refers to a property in CompilerOptions. That way the string can safely be used as a property name of CompilerOptions in filterSemanticDiagnostics:

filter(diagnostics, d => !d.skippedOn || !options[d.skippedOn])

This keeps diagnostics that

  1. were not issued by errorSkippedOn, so didn't set skippedOn
    -OR-
  2. do have skippedOn, which is guaranteed to be the name of a property in compiler options, say "noEmit". For the "noEmit" example, !options[d.skippedOn] is equivalent to !options.noEmit.

This meta-programming JS would require an enum plus a switch-case conversion in C# I think. Or the meta-programming facilities there, though the result wouldn't be as typesafe as in Typescript.

const diagnostic = error(location, message, arg0, arg1, arg2, arg3);
diagnostic.skippedOn = key;
return diagnostic;
}

function error(location: Node | undefined, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number, arg3?: string | number): Diagnostic {
const diagnostic = location
? createDiagnosticForNode(location, message, arg0, arg1, arg2, arg3)
Expand Down Expand Up @@ -32030,13 +32036,13 @@ namespace ts {

function checkCollisionWithArgumentsInGeneratedCode(node: SignatureDeclaration) {
// no rest parameters \ declaration context \ overload - no codegen impact
if (languageVersion >= ScriptTarget.ES2015 || compilerOptions.noEmit || !hasRestParameter(node) || node.flags & NodeFlags.Ambient || nodeIsMissing((<FunctionLikeDeclaration>node).body)) {
if (languageVersion >= ScriptTarget.ES2015 || !hasRestParameter(node) || node.flags & NodeFlags.Ambient || nodeIsMissing((<FunctionLikeDeclaration>node).body)) {
return;
}

forEach(node.parameters, p => {
if (p.name && !isBindingPattern(p.name) && p.name.escapedText === argumentsSymbol.escapedName) {
error(p, Diagnostics.Duplicate_identifier_arguments_Compiler_uses_arguments_to_initialize_rest_parameters);
errorSkippedOn("noEmit", p, Diagnostics.Duplicate_identifier_arguments_Compiler_uses_arguments_to_initialize_rest_parameters);
}
});
}
Expand Down Expand Up @@ -32106,13 +32112,13 @@ namespace ts {
function checkWeakMapCollision(node: Node) {
const enclosingBlockScope = getEnclosingBlockScopeContainer(node);
if (getNodeCheckFlags(enclosingBlockScope) & NodeCheckFlags.ContainsClassWithPrivateIdentifiers) {
error(node, Diagnostics.Compiler_reserves_name_0_when_emitting_private_identifier_downlevel, "WeakMap");
errorSkippedOn("noEmit", node, Diagnostics.Compiler_reserves_name_0_when_emitting_private_identifier_downlevel, "WeakMap");
}
}

function checkCollisionWithRequireExportsInGeneratedCode(node: Node, name: Identifier) {
// No need to check for require or exports for ES6 modules and later
if (moduleKind >= ModuleKind.ES2015 || compilerOptions.noEmit) {
if (moduleKind >= ModuleKind.ES2015) {
return;
}

Expand All @@ -32129,13 +32135,13 @@ namespace ts {
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent)) {
// If the declaration happens to be in external module, report error that require and exports are reserved keywords
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module,
errorSkippedOn("noEmit", name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (languageVersion >= ScriptTarget.ES2017 || compilerOptions.noEmit || !needCollisionCheckForIdentifier(node, name, "Promise")) {
if (languageVersion >= ScriptTarget.ES2017 || !needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

Expand All @@ -32148,7 +32154,7 @@ namespace ts {
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
errorSkippedOn("noEmit", name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}
Expand Down Expand Up @@ -32366,7 +32372,7 @@ namespace ts {
}
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
if (!compilerOptions.noEmit && languageVersion < ScriptTarget.ESNext && needCollisionCheckForIdentifier(node, node.name, "WeakMap")) {
if (languageVersion < ScriptTarget.ESNext && needCollisionCheckForIdentifier(node, node.name, "WeakMap")) {
potentialWeakMapCollisions.push(node);
}
}
Expand Down Expand Up @@ -38267,7 +38273,7 @@ namespace ts {

const moduleKind = getEmitModuleKind(compilerOptions);

if (moduleKind < ModuleKind.ES2015 && moduleKind !== ModuleKind.System && !compilerOptions.noEmit &&
if (moduleKind < ModuleKind.ES2015 && moduleKind !== ModuleKind.System &&
!(node.parent.parent.flags & NodeFlags.Ambient) && hasSyntacticModifier(node.parent.parent, ModifierFlags.Export)) {
checkESModuleMarker(node.name);
}
Expand All @@ -38287,7 +38293,7 @@ namespace ts {
function checkESModuleMarker(name: Identifier | BindingPattern): boolean {
if (name.kind === SyntaxKind.Identifier) {
if (idText(name) === "__esModule") {
return grammarErrorOnNode(name, Diagnostics.Identifier_expected_esModule_is_reserved_as_an_exported_marker_when_transforming_ECMAScript_modules);
return grammarErrorOnNodeSkippedOn("noEmit", name, Diagnostics.Identifier_expected_esModule_is_reserved_as_an_exported_marker_when_transforming_ECMAScript_modules);
}
}
else {
Expand Down Expand Up @@ -38397,6 +38403,15 @@ namespace ts {
return false;
}

function grammarErrorOnNodeSkippedOn(key: keyof CompilerOptions, node: Node, message: DiagnosticMessage, arg0?: any, arg1?: any, arg2?: any): boolean {
const sourceFile = getSourceFileOfNode(node);
if (!hasParseDiagnostics(sourceFile)) {
errorSkippedOn(key, node, message, arg0, arg1, arg2);
return true;
}
return false;
}

function grammarErrorOnNode(node: Node, message: DiagnosticMessage, arg0?: any, arg1?: any, arg2?: any): boolean {
const sourceFile = getSourceFileOfNode(node);
if (!hasParseDiagnostics(sourceFile)) {
Expand Down
1 change: 0 additions & 1 deletion src/compiler/commandLineParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,6 @@ namespace ts {
{
name: "noEmit",
type: "boolean",
affectsEmit: true,
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I find this surprising enough that I'd leave it present but commented out and with an explanation.

showInSimplifiedHelpView: true,
category: Diagnostics.Basic_Options,
description: Diagnostics.Do_not_emit_outputs,
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4458,6 +4458,10 @@
"category": "Error",
"code": 6309
},
"Referenced project '{0}' may not disable emit.": {
"category": "Error",
"code": 6310
},
"Project '{0}' is out of date because oldest output '{1}' is older than newest input '{2}'": {
"category": "Message",
"code": 6350
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ namespace ts {
// Write build information if applicable
if (!buildInfoPath || targetSourceFile || emitSkipped) return;
const program = host.getProgramBuildInfo();
if (host.isEmitBlocked(buildInfoPath) || compilerOptions.noEmit) {
if (host.isEmitBlocked(buildInfoPath)) {
emitSkipped = true;
return;
}
Expand Down
26 changes: 17 additions & 9 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1719,7 +1719,7 @@ namespace ts {

function getSemanticDiagnosticsForFile(sourceFile: SourceFile, cancellationToken: CancellationToken | undefined): readonly Diagnostic[] {
return concatenate(
getBindAndCheckDiagnosticsForFile(sourceFile, cancellationToken),
filterSemanticDiagnotics(getBindAndCheckDiagnosticsForFile(sourceFile, cancellationToken), options),
getProgramDiagnostics(sourceFile)
);
}
Expand Down Expand Up @@ -3011,10 +3011,6 @@ namespace ts {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Option_incremental_can_only_be_specified_using_tsconfig_emitting_to_single_file_or_when_option_tsBuildInfoFile_is_specified));
}

if (!options.listFilesOnly && options.noEmit && isIncrementalCompilation(options)) {
createDiagnosticForOptionName(Diagnostics.Option_0_cannot_be_specified_with_option_1, "noEmit", options.incremental ? "incremental" : "composite");
}

verifyProjectReferences();

// List of collected files is complete; validate exhautiveness if this is a project with a file list
Expand Down Expand Up @@ -3269,7 +3265,7 @@ namespace ts {
}

function verifyProjectReferences() {
const buildInfoPath = !options.noEmit && !options.suppressOutputPathCheck ? getTsBuildInfoEmitOutputFilePath(options) : undefined;
const buildInfoPath = !options.suppressOutputPathCheck ? getTsBuildInfoEmitOutputFilePath(options) : undefined;
forEachProjectReference(projectReferences, resolvedProjectReferences, (resolvedRef, index, parent) => {
const ref = (parent ? parent.commandLine.projectReferences : projectReferences)![index];
const parentFile = parent && parent.sourceFile as JsonSourceFile;
Expand All @@ -3278,11 +3274,12 @@ namespace ts {
return;
}
const options = resolvedRef.commandLine.options;
if (!options.composite) {
if (!options.composite || options.noEmit) {
// ok to not have composite if the current program is container only
const inputs = parent ? parent.commandLine.fileNames : rootNames;
if (inputs.length) {
createDiagnosticForReference(parentFile, index, Diagnostics.Referenced_project_0_must_have_setting_composite_Colon_true, ref.path);
if (!options.composite) createDiagnosticForReference(parentFile, index, Diagnostics.Referenced_project_0_must_have_setting_composite_Colon_true, ref.path);
if (options.noEmit) createDiagnosticForReference(parentFile, index, Diagnostics.Referenced_project_0_may_not_disable_emit, ref.path);
}
}
if (ref.prepend) {
Expand Down Expand Up @@ -3648,7 +3645,13 @@ namespace ts {
cancellationToken: CancellationToken | undefined
): EmitResult | undefined {
const options = program.getCompilerOptions();
if (options.noEmit) return emitSkippedWithNoDiagnostics;
if (options.noEmit) {
// Cache the semantic diagnostics
program.getSemanticDiagnostics(sourceFile, cancellationToken);
return sourceFile || outFile(options) ?
emitSkippedWithNoDiagnostics :
program.emitBuildInfo(writeFile, cancellationToken);
}

// If the noEmitOnError flag is set, then check if we have any errors so far. If so,
// immediately bail out. Note that we pass 'undefined' for 'sourceFile' so that we
Expand All @@ -3675,6 +3678,11 @@ namespace ts {
return { diagnostics, sourceMaps: undefined, emittedFiles, emitSkipped: true };
}

/*@internal*/
export function filterSemanticDiagnotics(diagnostic: readonly Diagnostic[], option: CompilerOptions): readonly Diagnostic[] {
Copy link
Member

@sandersn sandersn Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

should probably be named diagnostics and options

Copy link

Choose a reason for hiding this comment

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

There's also a typo in the function name.

return filter(diagnostic, d => !d.skippedOn || !option[d.skippedOn]);
}

/*@internal*/
interface CompilerHostLike {
useCaseSensitiveFileNames(): 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 @@ -5522,6 +5522,7 @@ namespace ts {
reportsUnnecessary?: {};
source?: string;
relatedInformation?: DiagnosticRelatedInformation[];
/* @internal */ skippedOn?: keyof CompilerOptions;
}

export interface DiagnosticRelatedInformation {
Expand Down
28 changes: 16 additions & 12 deletions src/testRunner/unittests/tsbuild/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ interface Symbol {
}
else if (actualText !== expectedText) {
// Verify build info without affectedFilesPendingEmit
const { text: actualBuildInfoText, affectedFilesPendingEmit: actualAffectedFilesPendingEmit } = getBuildInfoWithoutAffectedFilesPendingEmit(actualText);
const { text: expectedBuildInfoText, affectedFilesPendingEmit: expectedAffectedFilesPendingEmit } = getBuildInfoWithoutAffectedFilesPendingEmit(expectedText);
assert.equal(actualBuildInfoText, expectedBuildInfoText, `TsBuild info text without affectedFilesPendingEmit: ${outputFile}::\nIncremental buildInfoText:: ${actualText}\nClean buildInfoText:: ${expectedText}`);
const { buildInfo: actualBuildInfo, affectedFilesPendingEmit: actualAffectedFilesPendingEmit } = getBuildInfoForIncrementalCorrectnessCheck(actualText);
const { buildInfo: expectedBuildInfo, affectedFilesPendingEmit: expectedAffectedFilesPendingEmit } = getBuildInfoForIncrementalCorrectnessCheck(expectedText);
assert.deepEqual(actualBuildInfo, expectedBuildInfo, `TsBuild info text without affectedFilesPendingEmit: ${outputFile}::\nIncremental buildInfoText:: ${actualText}\nClean buildInfoText:: ${expectedText}`);
// Verify that incrementally pending affected file emit are in clean build since clean build can contain more files compared to incremental depending of noEmitOnError option
if (actualAffectedFilesPendingEmit) {
assert.isDefined(expectedAffectedFilesPendingEmit, `Incremental build contains affectedFilesPendingEmit, clean build should also have it: ${outputFile}::\nIncremental buildInfoText:: ${actualText}\nClean buildInfoText:: ${expectedText}`);
Expand All @@ -314,15 +314,19 @@ interface Symbol {
});
}

function getBuildInfoWithoutAffectedFilesPendingEmit(text: string | undefined): { text: string | undefined; affectedFilesPendingEmit?: ProgramBuildInfo["affectedFilesPendingEmit"]; } {
function getBuildInfoForIncrementalCorrectnessCheck(text: string | undefined): { buildInfo: BuildInfo | undefined; affectedFilesPendingEmit?: ProgramBuildInfo["affectedFilesPendingEmit"]; } {
const buildInfo = text ? getBuildInfo(text) : undefined;
if (!buildInfo?.program?.affectedFilesPendingEmit) return { text };
const { program: { affectedFilesPendingEmit, ...programRest }, ...rest } = buildInfo;
if (!buildInfo?.program) return { buildInfo };
// Ignore noEmit since that shouldnt be reason to emit the tsbuild info and presence of it in the buildinfo file does not matter
const { program: { affectedFilesPendingEmit, options: { noEmit, ...optionsRest}, ...programRest }, ...rest } = buildInfo;
return {
text: getBuildInfoText({
buildInfo: {
...rest,
program: programRest
}),
program: {
options: optionsRest,
...programRest
}
},
affectedFilesPendingEmit
};
}
Expand Down Expand Up @@ -423,7 +427,7 @@ interface Symbol {
subScenario,
baseFs,
newSys,
commandLineArgs,
commandLineArgs: incrementalCommandLineArgs || commandLineArgs,
incrementalModifyFs,
modifyFs,
tick
Expand Down Expand Up @@ -515,12 +519,12 @@ interface Symbol {
}));
});
describe("incremental correctness", () => {
incrementalScenarios.forEach((_, index) => verifyIncrementalCorrectness(() => ({
incrementalScenarios.forEach(({ commandLineArgs: incrementalCommandLineArgs }, index) => verifyIncrementalCorrectness(() => ({
scenario,
subScenario,
baseFs,
newSys: incrementalSys[index],
commandLineArgs,
commandLineArgs: incrementalCommandLineArgs || commandLineArgs,
incrementalModifyFs: fs => {
for (let i = 0; i <= index; i++) {
incrementalScenarios[i].modifyFs(fs);
Expand Down
Loading