eval: SyntaxError for new.target in direct eval outside non-arrow function (#5579)#5766
Merged
Merged
Conversation
…ction
ES2025 §15.2.1.1 makes `new.target` in a direct eval body a SyntaxError
unless the eval is contained in function code that is not an ArrowFunction.
Perry's const-fold path folded eval("new.target") into a completion IIFE
without checking this rule, so at global scope or inside an arrow it returned
undefined instead of throwing.
Fix: add `in_nonarrow_fn: bool` to `LoweringContext`, set it to `true` at
every non-arrow function boundary (fn declarations, fn expressions, methods,
constructors, getters, setters, private members, static blocks, object-literal
methods/accessors), and check it in `check_direct_eval_super_private` — the
same pattern already used by `check_indirect_eval_super_private` for its
unconditional new.target rejection.
Fixes two test262 cases tracked in #5579:
language/eval-code/direct/new.target.js
language/eval-code/direct/new.target-arrow.js
📝 WalkthroughWalkthroughAdds a in_nonarrow_fn tracking and new.target eval check
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-hir/src/lower/eval_super_scan.rs`:
- Around line 186-193: The setter bailout in expr_object lowering is leaving
ctx.in_nonarrow_fn in the wrong state because the early Ok(None) path skips
restoring the saved value. Update the setter-handling flow in expr_object::lower
to always restore saved_in_nonarrow_fn before returning, including the malformed
setter-parameter bailout, so later eval/new.target checks in eval_super_scan are
not misclassified.
In `@crates/perry-hir/src/lower/expr_object.rs`:
- Around line 471-472: The early bailout in the object-expression lowering path
leaves ctx.in_nonarrow_fn set to true, which leaks the non-arrow function
context into the caller. Update the logic around the saved_in_nonarrow_fn
assignment and the Ok(None) return path so ctx.in_nonarrow_fn is always restored
before exiting, just as strict mode and scope are unwound. Make sure the
restoration happens in the expr_object lowering flow regardless of whether the
function exits normally or via the bailout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 824cee42-6eba-4829-a615-924029d1e4de
📒 Files selected for processing (10)
crates/perry-hir/src/lower/context.rscrates/perry-hir/src/lower/eval_super_scan.rscrates/perry-hir/src/lower/expr_function.rscrates/perry-hir/src/lower/expr_object.rscrates/perry-hir/src/lower/lowering_context.rscrates/perry-hir/src/lower_decl/body_stmt/nested_fn_decl.rscrates/perry-hir/src/lower_decl/class_decl.rscrates/perry-hir/src/lower_decl/class_members.rscrates/perry-hir/src/lower_decl/fn_decl.rscrates/perry-hir/src/lower_decl/private_members.rs
Comment on lines
+186
to
+193
| // ES2025 §15.2.1.1: `new.target` in a direct eval body is only legal when | ||
| // the eval is contained in function code that is NOT an ArrowFunction. | ||
| // At module/script top-level and inside arrow functions, it is a SyntaxError. | ||
| if scan.new_target && !ctx.in_nonarrow_fn { | ||
| return Some(throw_eval_syntax_error_expr( | ||
| "new.target expression is not allowed here", | ||
| )); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect every in_nonarrow_fn save site and confirm a matching restore exists on
# all exit paths, with focus on non-error early returns (return Ok(None) / return Ok(_)).
rg -nP --type=rust -C8 'ctx\.in_nonarrow_fn\s*=\s*true' crates/perry-hir/src
echo '--- early returns between save and restore in expr_object.rs ---'
rg -nP --type=rust -C3 'return Ok\(None\)|exit_scope\(scope_mark\)|in_nonarrow_fn' crates/perry-hir/src/lower/expr_object.rsRepository: PerryTS/perry
Length of output: 24930
🏁 Script executed:
#!/bin/bash
sed -n '450,525p' crates/perry-hir/src/lower/expr_object.rs
printf '\n--- caller context ---\n'
rg -n -C4 'lower_accessor_prop\(' crates/perry-hir/srcRepository: PerryTS/perry
Length of output: 5820
Restore in_nonarrow_fn on the early Ok(None) return In crates/perry-hir/src/lower/expr_object.rs:485-490, the setter bailout skips ctx.in_nonarrow_fn = saved_in_nonarrow_fn;, so a malformed setter param can leak true into later lowering and misclassify subsequent new.target/eval checks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-hir/src/lower/eval_super_scan.rs` around lines 186 - 193, The
setter bailout in expr_object lowering is leaving ctx.in_nonarrow_fn in the
wrong state because the early Ok(None) path skips restoring the saved value.
Update the setter-handling flow in expr_object::lower to always restore
saved_in_nonarrow_fn before returning, including the malformed setter-parameter
bailout, so later eval/new.target checks in eval_super_scan are not
misclassified.
Comment on lines
+471
to
+472
| let saved_in_nonarrow_fn = ctx.in_nonarrow_fn; | ||
| ctx.in_nonarrow_fn = true; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore ctx.in_nonarrow_fn on the early Ok(None) path.
The bailout at Line 486 unwinds strict mode and scope, but leaves ctx.in_nonarrow_fn = true. That leaks non-arrow context into the caller, so a later direct eval("new.target") in the enclosing scope can be accepted when it should throw.
Suggested fix
Err(_) => {
ctx.exit_strict_mode();
ctx.exit_scope(scope_mark);
+ ctx.in_nonarrow_fn = saved_in_nonarrow_fn;
return Ok(None);
}Also applies to: 486-489
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-hir/src/lower/expr_object.rs` around lines 471 - 472, The early
bailout in the object-expression lowering path leaves ctx.in_nonarrow_fn set to
true, which leaks the non-arrow function context into the caller. Update the
logic around the saved_in_nonarrow_fn assignment and the Ok(None) return path so
ctx.in_nonarrow_fn is always restored before exiting, just as strict mode and
scope are unwound. Make sure the restoration happens in the expr_object lowering
flow regardless of whether the function exits normally or via the bailout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
ES2025 §15.2.1.1 states that
new.targetin a direct eval body is aSyntaxError unless the eval is contained in function code that is not an
ArrowFunction. Perry's const-fold path (
try_const_fold_eval) foldedeval("new.target")into a completion IIFE without checking this rule, so atglobal scope or inside an arrow the folded code returned
undefinedinstead ofthrowing.
check_indirect_eval_super_privatealready rejectednew.targetunconditionally(indirect eval is always global code).
check_direct_eval_super_privatehad nocorresponding check — this PR closes that gap.
What changes
New field
in_nonarrow_fn: boolonLoweringContext— tracks whether thecurrent lowering site is lexically inside a non-arrow function (ordinary
functiondeclaration/expression, method, constructor, getter, setter, or static block).
ArrowFunction bodies and module/script top-level leave the field
false.Save/restore at every non-arrow function entry point:
lower_fn_decl(fn_decl.rs)lower_fn_expr_anon(expr_function.rs)lower_nested_fn_decl(body_stmt/nested_fn_decl.rs)lower_constructor,lower_class_method_with_name,lower_getter_method_with_name,lower_setter_method_with_name(class_members.rs)lower_private_method,lower_private_getter,lower_private_setter(private_members.rs)StaticBlockarms inclass_decl.rs(both lowering paths)lower_method_prop,lower_accessor_prop(expr_object.rs)New check in
check_direct_eval_super_private(eval_super_scan.rs):Before / after (test262
language/eval-code/direct, tracked in #5579)direct/new.target.jsruntime-fail(returnsundefined, expected SyntaxError)passdirect/new.target-arrow.jsruntime-failpassNet: −2 failures from the
language/eval-codecluster in issue #5579.Verification
cargo build --release -p perry-hir— clean (no new errors)cargo fmt --all -- --check— cleanbash scripts/check_file_size.sh— clean (all files ≤2000 lines)Closes two cases from the eval-code failure cluster tracked in #5579.
Generated by Claude Code
Summary by CodeRabbit
new.targetin evaluated code, preventing invalid usage in places where it isn’t allowed.