Skip to content

[compiler] Cleanup diagnostic messages #33765

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
Jul 24, 2025
Merged

[compiler] Cleanup diagnostic messages #33765

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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
4 changes: 3 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ export default function BabelPluginReactCompiler(
}
} catch (e) {
if (e instanceof CompilerError) {
throw new Error(e.printErrorMessage(pass.file.code));
throw new Error(
e.printErrorMessage(pass.file.code, {eslint: false}),
);
}
throw e;
}
Expand Down
101 changes: 58 additions & 43 deletions compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import * as t from '@babel/types';
import {codeFrameColumns} from '@babel/code-frame';
import type {SourceLocation} from './HIR';
import {Err, Ok, Result} from './Utils/Result';
Expand Down Expand Up @@ -93,6 +94,14 @@ export type CompilerErrorDetailOptions = {
suggestions?: Array<CompilerSuggestion> | null | undefined;
};

export type PrintErrorMessageOptions = {
/**
* ESLint uses 1-indexed columns and prints one error at a time
* So it doesn't require the "Found # error(s)" text
*/
eslint: boolean;
};

export class CompilerDiagnostic {
options: CompilerDiagnosticOptions;

Expand Down Expand Up @@ -128,7 +137,7 @@ export class CompilerDiagnostic {
return this.options.details.filter(d => d.kind === 'error')[0]?.loc ?? null;
}

printErrorMessage(source: string): string {
printErrorMessage(source: string, options: PrintErrorMessageOptions): string {
const buffer = [
printErrorSummary(this.severity, this.category),
'\n\n',
Expand All @@ -143,28 +152,18 @@ export class CompilerDiagnostic {
}
let codeFrame: string;
try {
codeFrame = codeFrameColumns(
source,
{
start: {
line: loc.start.line,
column: loc.start.column + 1,
},
end: {
line: loc.end.line,
column: loc.end.column + 1,
},
},
{
message: detail.message,
},
);
codeFrame = printCodeFrame(source, loc, detail.message);
} catch (e) {
codeFrame = detail.message;
}
buffer.push(
`\n\n${loc.filename}:${loc.start.line}:${loc.start.column}\n`,
);
buffer.push('\n\n');
if (loc.filename != null) {
const line = loc.start.line;
const column = options.eslint
? loc.start.column + 1
: loc.start.column;
buffer.push(`${loc.filename}:${line}:${column}\n`);
}
buffer.push(codeFrame);
break;
}
Expand Down Expand Up @@ -223,7 +222,7 @@ export class CompilerErrorDetail {
return this.loc;
}

printErrorMessage(source: string): string {
printErrorMessage(source: string, options: PrintErrorMessageOptions): string {
const buffer = [printErrorSummary(this.severity, this.reason)];
if (this.description != null) {
buffer.push(`\n\n${this.description}.`);
Expand All @@ -232,28 +231,16 @@ export class CompilerErrorDetail {
if (loc != null && typeof loc !== 'symbol') {
let codeFrame: string;
try {
codeFrame = codeFrameColumns(
source,
{
start: {
line: loc.start.line,
column: loc.start.column + 1,
},
end: {
line: loc.end.line,
column: loc.end.column + 1,
},
},
{
message: this.reason,
},
);
codeFrame = printCodeFrame(source, loc, this.reason);
} catch (e) {
codeFrame = '';
}
buffer.push(
`\n\n${loc.filename}:${loc.start.line}:${loc.start.column}\n`,
);
buffer.push(`\n\n`);
if (loc.filename != null) {
const line = loc.start.line;
const column = options.eslint ? loc.start.column + 1 : loc.start.column;
buffer.push(`${loc.filename}:${line}:${column}\n`);
}
buffer.push(codeFrame);
buffer.push('\n\n');
}
Expand Down Expand Up @@ -372,10 +359,15 @@ export class CompilerError extends Error {
return this.name;
}

printErrorMessage(source: string): string {
printErrorMessage(source: string, options: PrintErrorMessageOptions): string {
if (options.eslint && this.details.length === 1) {
return this.details[0].printErrorMessage(source, options);
}
return (
`Found ${this.details.length} error${this.details.length === 1 ? '' : 's'}:\n` +
this.details.map(detail => detail.printErrorMessage(source)).join('\n')
`Found ${this.details.length} error${this.details.length === 1 ? '' : 's'}:\n\n` +
this.details
.map(detail => detail.printErrorMessage(source, options).trim())
.join('\n\n')
);
}

Expand Down Expand Up @@ -438,6 +430,29 @@ export class CompilerError extends Error {
}
}

function printCodeFrame(
source: string,
loc: t.SourceLocation,
message: string,
): string {
return codeFrameColumns(
source,
{
start: {
line: loc.start.line,
column: loc.start.column + 1,
},
end: {
line: loc.end.line,
column: loc.end.column + 1,
},
},
{
message,
},
);
}

function printErrorSummary(severity: ErrorSeverity, message: string): string {
let severityCategory: string;
switch (severity) {
Expand Down
13 changes: 5 additions & 8 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,9 @@ export function lower(
if (binding.kind !== 'Identifier') {
builder.errors.pushDiagnostic(
CompilerDiagnostic.create({
category: 'Could not find binding',
description: `[BuildHIR] Could not find binding for param \`${param.node.name}\``,
severity: ErrorSeverity.Invariant,
suggestions: null,
category: 'Could not find binding',
description: `[BuildHIR] Could not find binding for param \`${param.node.name}\`.`,
}).withDetail({
kind: 'error',
loc: param.node.loc ?? null,
Expand Down Expand Up @@ -172,10 +171,9 @@ export function lower(
} else {
builder.errors.pushDiagnostic(
CompilerDiagnostic.create({
category: `Handle ${param.node.type} parameters`,
description: `[BuildHIR] Add support for ${param.node.type} parameters`,
severity: ErrorSeverity.Todo,
suggestions: null,
category: `Handle ${param.node.type} parameters`,
description: `[BuildHIR] Add support for ${param.node.type} parameters.`,
}).withDetail({
kind: 'error',
loc: param.node.loc ?? null,
Expand Down Expand Up @@ -205,8 +203,7 @@ export function lower(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidJS,
category: `Unexpected function body kind`,
description: `Expected function body to be an expression or a block statement, got \`${body.type}\``,
suggestions: null,
description: `Expected function body to be an expression or a block statement, got \`${body.type}\`.`,
}).withDetail({
kind: 'error',
loc: body.node.loc ?? null,
Expand Down
41 changes: 19 additions & 22 deletions compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,23 +447,22 @@ function applySignature(
reason: value.reason,
context: new Set(),
});
const message =
const variable =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `\`${effect.value.identifier.name.value}\` cannot be modified`
: 'This value cannot be modified';
? `\`${effect.value.identifier.name.value}\``
: 'value';
effects.push({
kind: 'MutateFrozen',
place: effect.value,
error: CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'This value cannot be modified',
description: reason,
suggestions: null,
description: `${reason}.`,
}).withDetail({
kind: 'error',
loc: effect.value.loc,
message,
message: `${variable} cannot be modified`,
}),
});
}
Expand Down Expand Up @@ -1018,30 +1017,30 @@ function applyEffect(
effect.value.identifier.declarationId,
)
) {
const description =
const variable =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `Variable \`${effect.value.identifier.name.value}\``
: 'This variable';
? `\`${effect.value.identifier.name.value}\``
: null;
const hoistedAccess = context.hoistedContextDeclarations.get(
effect.value.identifier.declarationId,
);
const diagnostic = CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'Cannot access variable before it is declared',
description: `${description} is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
description: `${variable ?? 'This variable'} is accessed before it is declared, which prevents the earlier access from updating when this value changes over time.`,
});
if (hoistedAccess != null && hoistedAccess.loc != effect.value.loc) {
diagnostic.withDetail({
kind: 'error',
loc: hoistedAccess.loc,
message: 'Variable accessed before it is declared',
message: `${variable ?? 'variable'} accessed before it is declared`,
});
}
diagnostic.withDetail({
kind: 'error',
loc: effect.value.loc,
message: 'The variable is declared here',
message: `${variable ?? 'variable'} is declared here`,
});

applyEffect(
Expand All @@ -1061,11 +1060,11 @@ function applyEffect(
reason: value.reason,
context: new Set(),
});
const message =
const variable =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `\`${effect.value.identifier.name.value}\` cannot be modified`
: 'This value cannot be modified';
? `\`${effect.value.identifier.name.value}\``
: 'value';
applyEffect(
context,
state,
Expand All @@ -1078,11 +1077,11 @@ function applyEffect(
error: CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'This value cannot be modified',
description: reason,
description: `${reason}.`,
}).withDetail({
kind: 'error',
loc: effect.value.loc,
message,
message: `${variable} cannot be modified`,
}),
},
initialized,
Expand Down Expand Up @@ -2002,20 +2001,19 @@ function computeSignatureForInstruction(
break;
}
case 'StoreGlobal': {
const variable = `\`${value.name}\``;
effects.push({
kind: 'MutateGlobal',
place: value.value,
error: CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category:
'Cannot reassign variables declared outside of the component/hook',
description:
'Reassigning a variable declared outside of the component/hook is a form of side effect, which can cause unpredictable behavior depending on when the component happens to re-render. If this variable is used in rendering, use useState instead. Otherwise, consider updating it in an effect (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)',
suggestions: null,
description: `Variable ${variable} is declared outside of the component/hook. Reassigning this value during render is a form of side effect, which can cause unpredictable behavior depending on when the component happens to re-render. If this variable is used in rendering, use useState instead. Otherwise, consider updating it in an effect. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)`,
}).withDetail({
kind: 'error',
loc: instr.loc,
message: 'Cannot reassign variable',
message: `${variable} cannot be reassigned`,
}),
});
effects.push({kind: 'Assign', from: value.value, into: lvalue});
Expand Down Expand Up @@ -2114,7 +2112,6 @@ function computeEffectsForLegacySignature(
? `\`${signature.canonicalName}\` is an impure function. `
: '') +
'Calling an impure function can produce unstable results that update unpredictably when the component happens to re-render. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)',
suggestions: null,
}).withDetail({
kind: 'error',
loc,
Expand Down
40 changes: 27 additions & 13 deletions ...ages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,20 @@ export function validateLocalsNotReassignedAfterRender(fn: HIRFunction): void {
);
if (reassignment !== null) {
const errors = new CompilerError();
const variable =
reassignment.identifier.name != null &&
reassignment.identifier.name.kind === 'named'
? `\`${reassignment.identifier.name.value}\``
: 'variable';
errors.pushDiagnostic(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'Cannot reassign a variable after render completes',
description: `Reassigning ${reassignment.identifier.name != null && reassignment.identifier.name.kind === 'named' ? `variable \`${reassignment.identifier.name.value}\`` : 'a variable'} after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead`,
category: 'Cannot reassign variable after render completes',
description: `Reassigning ${variable} after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead.`,
}).withDetail({
kind: 'error',
loc: reassignment.loc,
message: 'Cannot reassign variable after render completes',
message: `Cannot reassign ${variable} after render completes`,
}),
);
throw errors;
Expand Down Expand Up @@ -78,16 +83,25 @@ function getContextReassignment(
// if the function or its depends reassign, propagate that fact on the lvalue
if (reassignment !== null) {
if (isAsync || value.loweredFunc.func.async) {
CompilerError.throwInvalidReact({
reason:
'Reassigning a variable in an async function can cause inconsistent behavior on subsequent renders. Consider using state instead',
description:
reassignment.identifier.name !== null &&
reassignment.identifier.name.kind === 'named'
? `Variable \`${reassignment.identifier.name.value}\` cannot be reassigned after render`
: '',
loc: reassignment.loc,
});
const errors = new CompilerError();
const variable =
reassignment.identifier.name !== null &&
reassignment.identifier.name.kind === 'named'
? `\`${reassignment.identifier.name.value}\``
: 'variable';
errors.pushDiagnostic(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: 'Cannot reassign variable in async function',
description:
'Reassigning a variable in an async function can cause inconsistent behavior on subsequent renders. Consider using state instead',
}).withDetail({
kind: 'error',
loc: reassignment.loc,
message: `Cannot reassign ${variable}`,
}),
);
throw errors;
}
reassigningFunctions.set(lvalue.identifier.id, reassignment);
}
Expand Down
Loading
Loading