Skip to content

Commit d0517fe

Browse files
authored
Merge branch 'main' into feat/res2
2 parents 201be32 + 0253916 commit d0517fe

File tree

22 files changed

+635
-299
lines changed

22 files changed

+635
-299
lines changed

PR_MESSAGE_EXPECT_EXPRESSION.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# fix(parser): replace expect_expression panic with try_into_expression error handling
2+
3+
## Summary
4+
5+
Replaces all uses of `expect_expression()` with `try_into_expression()` in the parser, and removes the panicking `expect_expression()` method. When the parser encounters arrow-function parameter list syntax in a context that requires an expression (e.g. conditional operator `a ? b : c`, call expression `foo()`, optional chaining `a?.b`), it now returns a recoverable parse error instead of panicking.
6+
7+
## Motivation
8+
9+
The `FormalParameterListOrExpression` type represents the grammar ambiguity between `(a, b)` as a comma expression vs `(a, b)` as arrow-function parameters. In contexts like the conditional operator (`x ? y : z`), call expressions (`f()`), and optional chaining (`a?.b`), the left-hand side must be an expression—arrow params are invalid. Previously, call sites used `expect_expression()`, which panics with "Unexpected arrow-function arguments" when the parser produced a `FormalParameterList` instead of an `Expression`. This can occur with edge-case input such as `(a) ? 1 : 2` when `(a)` is parsed as a single arrow param. For Boa as an embedded engine or in tooling, panics are unacceptable; the host expects a parse error. The `try_into_expression()` method already existed and returns `Err(Error::General { ... })` with a helpful message ("invalid arrow-function arguments (parentheses around the arrow-function may help)"). Replacing `expect_expression()` with `try_into_expression()?` propagates that error instead of panicking, aligning with Boa's stability goals.
10+
11+
## Changes
12+
13+
| Category | Description |
14+
| ------------ | ----------------------------------------------------------------------------------------------------- |
15+
| **Replaced** | `lhs.expect_expression()` with `lhs.try_into_expression()?` in conditional expression parser |
16+
| **Replaced** | `member.expect_expression()` with `member.try_into_expression()?` in left-hand side (call expression) |
17+
| **Replaced** | `lhs.expect_expression()` with `lhs.try_into_expression()?` in left-hand side (optional expression) |
18+
| **Removed** | `expect_expression()` method from `FormalParameterListOrExpression` to prevent future misuse |
19+
20+
## Technical Details
21+
22+
- **Files modified:** `core/parser/src/parser/expression/assignment/conditional.rs`, `core/parser/src/parser/expression/left_hand_side/mod.rs`, `core/parser/src/parser/expression/fpl_or_exp.rs`
23+
- **Lines changed:** ~15 lines across 3 files
24+
- **Behavioral impact:** Invalid input that previously caused a panic now produces `Err(Error::General { message: "invalid arrow-function arguments (parentheses around the arrow-function may help)", position })`. Valid input is unchanged.
25+
26+
## Testing
27+
28+
- [x] `cargo test -p boa_parser` — all 296 tests pass
29+
- [x] `cargo test -p boa_engine --lib` — all 921 tests pass
30+
- [x] `cargo clippy` — no warnings
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
const kIterationCount = 1000;
2+
const fmt = new Intl.DateTimeFormat("en-US", {
3+
dateStyle: "full",
4+
timeStyle: "short",
5+
timeZone: "UTC",
6+
});
7+
8+
function main() {
9+
for (let i = 0; i < kIterationCount; i++) {
10+
fmt.resolvedOptions();
11+
}
12+
}

core/engine/src/builtins/bigint/mod.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt
1414
1515
use crate::{
16-
Context, JsArgs, JsBigInt, JsResult, JsString, JsValue,
16+
Context, JsArgs, JsBigInt, JsExpect, JsResult, JsString, JsValue,
1717
builtins::BuiltInObject,
1818
context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
1919
error::JsNativeError,
@@ -122,7 +122,12 @@ impl BigInt {
122122
}
123123

124124
// 2. Return the BigInt value that represents ℝ(number).
125-
Ok(JsBigInt::from(number.to_bigint().expect("This conversion must be safe")).into())
125+
Ok(JsBigInt::from(
126+
number
127+
.to_bigint()
128+
.js_expect("This conversion must be safe")?,
129+
)
130+
.into())
126131
}
127132

128133
/// The abstract operation `thisBigIntValue` takes argument value.

core/engine/src/builtins/error/aggregate.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/AggregateError
99
1010
use crate::{
11-
Context, JsArgs, JsResult, JsString, JsValue,
11+
Context, JsArgs, JsExpect, JsResult, JsString, JsValue,
1212
builtins::{
1313
Array, BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject,
1414
iterable::IteratorHint,
@@ -128,7 +128,7 @@ impl BuiltInConstructor for AggregateError {
128128
.build(),
129129
context,
130130
)
131-
.expect("should not fail according to spec");
131+
.js_expect("should not fail according to spec")?;
132132

133133
// 5. Return O.
134134
Ok(o.into())

core/engine/src/builtins/eval/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval
1111
1212
use crate::{
13-
Context, JsArgs, JsResult, JsString, JsValue, SpannedSourceText,
13+
Context, JsArgs, JsExpect, JsResult, JsString, JsValue, SpannedSourceText,
1414
builtins::{BuiltInObject, function::OrdinaryFunction},
1515
bytecompiler::{ByteCompiler, prepare_eval_declaration_instantiation},
1616
context::intrinsics::Intrinsics,
@@ -152,7 +152,7 @@ impl Eval {
152152
.slots()
153153
.function_object()
154154
.downcast_ref::<OrdinaryFunction>()
155-
.expect("must be function object");
155+
.js_expect("must be function object")?;
156156

157157
// ii. Set inFunction to true.
158158
let mut flags = Flags::IN_FUNCTION;

core/engine/src/builtins/generator/mod.rs

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,11 @@
1010
//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator
1111
1212
use crate::{
13-
Context, JsArgs, JsData, JsError, JsResult, JsString,
13+
Context, JsArgs, JsData, JsError, JsExpect, JsResult, JsString,
1414
builtins::iterable::create_iter_result_object,
1515
context::intrinsics::Intrinsics,
1616
error::JsNativeError,
17+
error::PanicError,
1718
js_string,
1819
object::{CONSTRUCTOR, JsObject},
1920
property::Attribute,
@@ -101,7 +102,9 @@ impl GeneratorContext {
101102
context: &mut Context,
102103
) -> CompletionRecord {
103104
std::mem::swap(&mut context.vm.stack, &mut self.stack);
104-
let frame = self.call_frame.take().expect("should have a call frame");
105+
let Some(frame) = self.call_frame.take() else {
106+
return CompletionRecord::Throw(PanicError::new("should have a call frame").into());
107+
};
105108
let fp = frame.fp;
106109
let rp = frame.rp;
107110
context.vm.push_frame(frame);
@@ -125,16 +128,21 @@ impl GeneratorContext {
125128
}
126129

127130
/// Returns the async generator object, if the function that this [`GeneratorContext`] is from an async generator, [`None`] otherwise.
128-
pub(crate) fn async_generator_object(&self) -> Option<JsObject> {
129-
let frame = self.call_frame.as_ref()?;
131+
pub(crate) fn async_generator_object(&self) -> JsResult<Option<JsObject>> {
132+
let Some(frame) = self.call_frame.as_ref() else {
133+
return Ok(None);
134+
};
135+
130136
if !frame.code_block().is_async_generator() {
131-
return None;
137+
return Ok(None);
132138
}
133139

134-
self.stack
140+
let val = self
141+
.stack
135142
.get_register(frame, CallFrame::ASYNC_GENERATOR_OBJECT_REGISTER_INDEX)
136-
.expect("registers must have an async generator object")
137-
.as_object()
143+
.js_expect("registers must have an async generator object")?;
144+
145+
Ok(val.as_object())
138146
}
139147
}
140148

@@ -306,7 +314,7 @@ impl Generator {
306314

307315
let mut r#gen = generator_obj
308316
.downcast_mut::<Self>()
309-
.expect("already checked this object type");
317+
.js_expect("already checked this object type")?;
310318

311319
// 8. Push genContext onto the execution context stack; genContext is now the running execution context.
312320
// 9. Resume the suspended evaluation of genContext using NormalCompletion(value) as the result of the operation that suspended it. Let result be the value returned by the resumed computation.

0 commit comments

Comments
 (0)