Skip to content

Fix bug: validate <field> before <operation> to prevent <error/crash> #35597

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

Open
harshilparekh874 wants to merge 1 commit into
base: main
Choose a base branch
from
+93 −44
Open

Fix bug: validate <field> before <operation> to prevent <error/crash> #35597

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
137 changes: 93 additions & 44 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 @@ -1260,50 +1260,99 @@ function applyEffect(
console.log(prettyFormat(state.debugAbstractValue(value)));
}

if (
mutationKind === 'mutate-frozen' &&
context.hoistedContextDeclarations.has(
effect.value.identifier.declarationId,
)
) {
const variable =
effect.value.identifier.name !== null &&
effect.value.identifier.name.kind === 'named'
? `\`${effect.value.identifier.name.value}\``
: null;
const hoistedAccess = context.hoistedContextDeclarations.get(
effect.value.identifier.declarationId,
);
const diagnostic = CompilerDiagnostic.create({
category: ErrorCategory.Immutability,
reason: 'Cannot access variable before it is declared',
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.withDetails({
kind: 'error',
loc: hoistedAccess.loc,
message: `${variable ?? 'variable'} accessed before it is declared`,
});
}
diagnostic.withDetails({
kind: 'error',
loc: effect.value.loc,
message: `${variable ?? 'variable'} is declared here`,
});

applyEffect(
context,
state,
{
kind: 'MutateFrozen',
place: effect.value,
error: diagnostic,
},
initialized,
effects,
);
} else {
+ if (
+ mutationKind === 'mutate-frozen' &&
+ context.hoistedContextDeclarations.has(
+ effect.value.identifier.declarationId,
+ )
+ ) {
+ const variable =
+ effect.value.identifier.name !== null &&
+ effect.value.identifier.name.kind === 'named'
+ ? `\`${effect.value.identifier.name.value}\``
+ : null;
+ const hoistedAccess = context.hoistedContextDeclarations.get(
+ effect.value.identifier.declarationId,
+ );
+
+ // Only produce the specialized "accessed before it is declared" diagnostic
+ // when we actually observed a prior distinct hoisted access location.
+ // If we don't have a recorded prior access location (hoistedAccess === null)
+ // or the prior access location is the same as the declaration, fall back to
+ // the generic "This value cannot be modified" diagnostic below.
+ if (hoistedAccess != null && hoistedAccess.loc != effect.value.loc) {
+ const diagnostic = CompilerDiagnostic.create({
+ category: ErrorCategory.Immutability,
+ reason: 'Cannot access variable before it is declared',
+ description: `${variable ?? 'This variable'} is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
+ });
+ diagnostic.withDetails({
+ kind: 'error',
+ loc: hoistedAccess.loc,
+ message: `${variable ?? 'variable'} accessed before it is declared`,
+ });
+ diagnostic.withDetails({
+ kind: 'error',
+ loc: effect.value.loc,
+ message: `${variable ?? 'variable'} is declared here`,
+ });
+
+ applyEffect(
+ context,
+ state,
+ {
+ kind: 'MutateFrozen',
+ place: effect.value,
+ error: diagnostic,
+ },
+ initialized,
+ effects,
+ );
+ } else {
+ // No prior distinct hoisted access recorded: fall back to the generic diagnostic
+ // that explains why the value cannot be modified.
+ const reason = getWriteErrorReason({
+ kind: value.kind,
+ reason: value.reason,
+ });
+ const variableName =
+ effect.value.identifier.name !== null &&
+ effect.value.identifier.name.kind === 'named'
+ ? `\`${effect.value.identifier.name.value}\``
+ : 'value';
+ const diagnostic = CompilerDiagnostic.create({
+ category: ErrorCategory.Immutability,
+ reason: 'This value cannot be modified',
+ description: reason,
+ }).withDetails({
+ kind: 'error',
+ loc: effect.value.loc,
+ message: `${variableName} cannot be modified`,
+ });
+ if (
+ effect.kind === 'Mutate' &&
+ effect.reason?.kind === 'AssignCurrentProperty'
+ ) {
+ diagnostic.withDetails({
+ kind: 'hint',
+ message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`,
+ });
+ }
+
+ applyEffect(
+ context,
+ state,
+ {
+ kind:
+ value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
+ place: effect.value,
+ error: diagnostic,
+ },
+ initialized,
+ effects,
+ );
+ }
+ } else {
const reason = getWriteErrorReason({
kind: value.kind,
reason: value.reason,
Expand Down