Skip to content

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

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

Closed
everettbu wants to merge 1 commit into from
Closed

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

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 {
Comment on lines +1263 to +1355

@greptile-apps greptile-apps Bot Jan 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

syntax: Invalid syntax: literal + characters at the start of lines

This entire code block has diff markers (+) that were not removed during merge/patch application. These are literal characters in the source file, not just visual diff indicators. This will cause compilation to fail.

Remove the leading + characters from all lines in this range (92 lines total from line 1263-1355).

Suggested change
+ 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 {
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.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 {
Prompt To Fix With AI 
This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
Line: 1263:1355

Comment:
**syntax:** Invalid syntax: literal `+` characters at the start of lines

This entire code block has diff markers (`+`) that were not removed during merge/patch application. These are literal characters in the source file, not just visual diff indicators. This will cause compilation to fail.

Remove the leading `+` characters from all lines in this range (92 lines total from line 1263-1355).

```suggestion
        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.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 {
```

How can I resolve this? If you propose a fix, please make it concise.

const reason = getWriteErrorReason({
kind: value.kind,
reason: value.reason,
Expand Down
Loading