Skip to content

Commit 2e6d209

Browse files
mrhapilemrhapile
authored
Improve TypeError diagnostics in WeakMap, WeakSet and Object builtins and update tests (#4863)
<!--- Thank you for contributing to Boa! Please fill out the template below, and remove or add any information as you feel necessary. ---> Improve TypeError diagnostics in WeakMap, WeakSet and Object builtins and update tests This Pull Request fixes/closes #. It changes the following: * Improve TypeError messages in `WeakMap`, `WeakSet`, and `Object` builtins to provide clearer diagnostics. * Replace generic error messages such as `"called with non-object value"` and `"Expected an object"` with explicit messages indicating the expected receiver type. * Standardize error message format to: `<Builtin>.<method>: expected 'this' to be a <Type> object` * Update unit tests to match the improved error messages. ### Before Some errors used generic messages that did not clearly indicate which builtin method failed or what type was expected. Examples: ``` WeakMap.get: called with non-object value WeakMap.getOrInsertComputed: called with non-object value Object prototype may only be an Object or null: undefined Expected an object ``` These messages lacked clear context about the failing method or the expected `this` value. ### After Error messages now explicitly describe the expected receiver type and the builtin method where the error occurred. Examples: ``` WeakMap.prototype.get: expected 'this' to be a WeakMap object WeakMap.prototype.getOrInsert: expected 'this' to be a WeakMap object WeakMap.prototype.getOrInsertComputed: expected 'this' to be a WeakMap object Object.create: expected 'proto' to be an Object or null, got `undefined` Object.defineProperty: expected 'this' to be an Object Object.defineProperties: expected 'this' to be an Object ``` ### Files Modified **Engine changes** * `core/engine/src/builtins/weak_map/mod.rs` * `core/engine/src/builtins/weak_set/mod.rs` * `core/engine/src/builtins/object/mod.rs` These files were updated to replace generic `TypeError` messages with more descriptive diagnostics. **Test updates** To keep the test suite consistent with the improved diagnostics, expected error strings were updated in: * `core/engine/src/builtins/weak_map/tests.rs` * `core/engine/src/builtins/object/tests.rs` No behavioral changes were introduced; only error message diagnostics were improved and tests updated accordingly. --------- Signed-off-by: mrhapile <allinonegaming3456@gmail.com>
1 parent 92b7565 commit 2e6d209

File tree

5 files changed

+86
-111
lines changed

5 files changed

+86
-111
lines changed

‎core/engine/src/builtins/object/mod.rs‎

Lines changed: 33 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ use crate::{
2222
Context, JsArgs, JsData, JsExpect, JsResult, JsString,
2323
builtins::{BuiltInObject, iterable::IteratorHint, map},
2424
context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
25-
error::JsNativeError,
26-
js_string,
25+
js_error, js_string,
2726
native_function::NativeFunction,
2827
object::{
2928
FunctionObjectBuilder, IntegrityLevel, JsObject,
@@ -255,9 +254,7 @@ impl OrdinaryObject {
255254

256255
// 5. If status is false, throw a TypeError exception.
257256
if !status {
258-
return Err(JsNativeError::typ()
259-
.with_message("__proto__ called on null or undefined")
260-
.into());
257+
return Err(js_error!(TypeError: "__proto__ called on null or undefined"));
261258
}
262259

263260
// 6. Return undefined.
@@ -286,9 +283,9 @@ impl OrdinaryObject {
286283

287284
// 2. If IsCallable(getter) is false, throw a TypeError exception.
288285
if !getter.is_callable() {
289-
return Err(JsNativeError::typ()
290-
.with_message("Object.prototype.__defineGetter__: Expecting function")
291-
.into());
286+
return Err(js_error!(TypeError:
287+
"Object.prototype.__defineGetter__: expected 'getter' to be a Function object",
288+
));
292289
}
293290

294291
// 3. Let desc be PropertyDescriptor { [[Get]]: getter, [[Enumerable]]: true, [[Configurable]]: true }.
@@ -329,9 +326,9 @@ impl OrdinaryObject {
329326

330327
// 2. If IsCallable(setter) is false, throw a TypeError exception.
331328
if !setter.is_callable() {
332-
return Err(JsNativeError::typ()
333-
.with_message("Object.prototype.__defineSetter__: Expecting function")
334-
.into());
329+
return Err(js_error!(TypeError:
330+
"Object.prototype.__defineSetter__: expected 'setter' to be a Function object",
331+
));
335332
}
336333

337334
// 3. Let desc be PropertyDescriptor { [[Set]]: setter, [[Enumerable]]: true, [[Configurable]]: true }.
@@ -467,12 +464,10 @@ impl OrdinaryObject {
467464
.upcast()
468465
}
469466
_ => {
470-
return Err(JsNativeError::typ()
471-
.with_message(format!(
472-
"Object prototype may only be an Object or null: {}",
473-
prototype.display()
474-
))
475-
.into());
467+
return Err(js_error!(TypeError:
468+
"Object.create: expected 'proto' to be an Object or null, got `{}`",
469+
prototype.type_of()
470+
));
476471
}
477472
};
478473

@@ -645,11 +640,9 @@ impl OrdinaryObject {
645640
context: &mut Context,
646641
) -> JsResult<JsValue> {
647642
if args.is_empty() {
648-
return Err(JsNativeError::typ()
649-
.with_message(
650-
"Object.getPrototypeOf: At least 1 argument required, but only 0 passed",
651-
)
652-
.into());
643+
return Err(js_error!(TypeError:
644+
"Object.getPrototypeOf: At least 1 argument required, but only 0 passed",
645+
));
653646
}
654647

655648
// 1. Let obj be ? ToObject(O).
@@ -672,12 +665,10 @@ impl OrdinaryObject {
672665
context: &mut Context,
673666
) -> JsResult<JsValue> {
674667
if args.len() < 2 {
675-
return Err(JsNativeError::typ()
676-
.with_message(format!(
677-
"Object.setPrototypeOf: At least 2 arguments required, but only {} passed",
678-
args.len()
679-
))
680-
.into());
668+
return Err(js_error!(TypeError:
669+
"Object.setPrototypeOf: At least 2 arguments required, but only {} passed",
670+
args.len()
671+
));
681672
}
682673

683674
// 1. Set O to ? RequireObjectCoercible(O).
@@ -693,12 +684,10 @@ impl OrdinaryObject {
693684
JsVariant::Null => None,
694685
// 2. If Type(proto) is neither Object nor Null, throw a TypeError exception.
695686
val => {
696-
return Err(JsNativeError::typ()
697-
.with_message(format!(
698-
"expected an object or null, got `{}`",
699-
val.type_of()
700-
))
701-
.into());
687+
return Err(js_error!(TypeError:
688+
"Object.setPrototypeOf: expected 'proto' to be an Object or null, got `{}`",
689+
val.type_of()
690+
));
702691
}
703692
};
704693

@@ -711,11 +700,8 @@ impl OrdinaryObject {
711700
let status =
712701
obj.__set_prototype_of__(proto, &mut InternalMethodPropertyContext::new(context))?;
713702

714-
// 5. If status is false, throw a TypeError exception.
715703
if !status {
716-
return Err(JsNativeError::typ()
717-
.with_message("can't set prototype of this object")
718-
.into());
704+
return Err(js_error!(TypeError: "can't set prototype of this object"));
719705
}
720706

721707
// 6. Return O.
@@ -774,9 +760,7 @@ impl OrdinaryObject {
774760

775761
Ok(object.clone().into())
776762
} else {
777-
Err(JsNativeError::typ()
778-
.with_message("Object.defineProperty called on non-object")
779-
.into())
763+
Err(js_error!(TypeError: "Object.defineProperty: expected 'this' to be an Object"))
780764
}
781765
}
782766

@@ -801,9 +785,7 @@ impl OrdinaryObject {
801785
object_define_properties(&obj, props, context)?;
802786
Ok(arg.clone())
803787
} else {
804-
Err(JsNativeError::typ()
805-
.with_message("Expected an object")
806-
.into())
788+
Err(js_error!(TypeError: "Object.defineProperties: expected 'this' to be an Object"))
807789
}
808790
}
809791

@@ -1122,9 +1104,7 @@ impl OrdinaryObject {
11221104
let status = o.set_integrity_level(IntegrityLevel::Sealed, context)?;
11231105
// 3. If status is false, throw a TypeError exception.
11241106
if !status {
1125-
return Err(JsNativeError::typ()
1126-
.with_message("cannot seal object")
1127-
.into());
1107+
return Err(js_error!(TypeError: "cannot seal object"));
11281108
}
11291109
}
11301110
// 1. If Type(O) is not Object, return O.
@@ -1169,9 +1149,7 @@ impl OrdinaryObject {
11691149
let status = o.set_integrity_level(IntegrityLevel::Frozen, context)?;
11701150
// 3. If status is false, throw a TypeError exception.
11711151
if !status {
1172-
return Err(JsNativeError::typ()
1173-
.with_message("cannot freeze object")
1174-
.into());
1152+
return Err(js_error!(TypeError: "cannot freeze object"));
11751153
}
11761154
}
11771155
// 1. If Type(O) is not Object, return O.
@@ -1221,9 +1199,7 @@ impl OrdinaryObject {
12211199
o.__prevent_extensions__(&mut InternalMethodPropertyContext::new(context))?;
12221200
// 3. If status is false, throw a TypeError exception.
12231201
if !status {
1224-
return Err(JsNativeError::typ()
1225-
.with_message("cannot prevent extensions")
1226-
.into());
1202+
return Err(js_error!(TypeError: "cannot prevent extensions"));
12271203
}
12281204
}
12291205
// 1. If Type(O) is not Object, return O.
@@ -1381,9 +1357,9 @@ impl OrdinaryObject {
13811357
items.require_object_coercible()?;
13821358

13831359
// 2. If IsCallable(callbackfn) is false, throw a TypeError exception.
1384-
let callback = callback.as_callable().ok_or_else(|| {
1385-
JsNativeError::typ().with_message("callback must be a callable object")
1386-
})?;
1360+
let callback = callback
1361+
.as_callable()
1362+
.ok_or_else(|| js_error!(TypeError: "callback must be a callable object"))?;
13871363

13881364
// 3. Let groups be a new empty List.
13891365
let mut groups: IndexMap<PropertyKey, Vec<JsValue>, BuildHasherDefault<FxHasher>> =
@@ -1400,9 +1376,7 @@ impl OrdinaryObject {
14001376
// a. If k ≥ 2^53 - 1, then
14011377
if k >= Number::MAX_SAFE_INTEGER as u64 {
14021378
// i. Let error be ThrowCompletion(a newly created TypeError object).
1403-
let error = JsNativeError::typ()
1404-
.with_message("exceeded maximum safe integer")
1405-
.into();
1379+
let error = js_error!(TypeError: "exceeded maximum safe integer");
14061380

14071381
// ii. Return ? IteratorClose(iteratorRecord, error).
14081382
return iterator.close(Err(error), context);

‎core/engine/src/builtins/object/tests.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ fn object_create_with_undefined() {
1717
run_test_actions([TestAction::assert_native_error(
1818
"Object.create()",
1919
JsNativeErrorKind::Type,
20-
"Object prototype may only be an Object or null: undefined",
20+
"Object.create: expected 'proto' to be an Object or null, got `undefined`",
2121
)]);
2222
}
2323

@@ -26,7 +26,7 @@ fn object_create_with_number() {
2626
run_test_actions([TestAction::assert_native_error(
2727
"Object.create(5)",
2828
JsNativeErrorKind::Type,
29-
"Object prototype may only be an Object or null: 5",
29+
"Object.create: expected 'proto' to be an Object or null, got `number`",
3030
)]);
3131
}
3232

‎core/engine/src/builtins/weak_map/mod.rs‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@
88
//! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakMap
99
1010
use crate::{
11-
Context, JsArgs, JsNativeError, JsResult, JsString, JsValue,
11+
Context, JsArgs, JsResult, JsString, JsValue,
1212
builtins::{
1313
BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject,
1414
map::add_entries_from_iterable,
1515
},
1616
context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
17-
js_string,
17+
js_error, js_string,
1818
object::{ErasedVTableObject, JsObject, internal_methods::get_prototype_from_constructor},
1919
property::Attribute,
2020
realm::Realm,
@@ -87,9 +87,7 @@ impl BuiltInConstructor for WeakMap {
8787
) -> JsResult<JsValue> {
8888
// 1. If NewTarget is undefined, throw a TypeError exception.
8989
if new_target.is_undefined() {
90-
return Err(JsNativeError::typ()
91-
.with_message("WeakMap: cannot call constructor without `new`")
92-
.into());
90+
return Err(js_error!(TypeError: "WeakMap: cannot call constructor without `new`"));
9391
}
9492

9593
// 2. Let map be ? OrdinaryCreateFromConstructor(NewTarget, "%WeakMap.prototype%", « [[WeakMapData]] »).
@@ -114,7 +112,7 @@ impl BuiltInConstructor for WeakMap {
114112
let adder = map
115113
.get(js_string!("set"), context)?
116114
.as_function()
117-
.ok_or_else(|| JsNativeError::typ().with_message("WeakMap: 'add' is not a function"))?;
115+
.ok_or_else(|| js_error!(TypeError: "WeakMap: 'add' is not a function"))?;
118116

119117
// 7. Return ? AddEntriesFromIterable(map, iterable, adder).
120118
add_entries_from_iterable(&map, iterable, &adder, context)
@@ -142,7 +140,9 @@ impl WeakMap {
142140
.as_ref()
143141
.and_then(JsObject::downcast_mut::<NativeWeakMap>)
144142
.ok_or_else(|| {
145-
JsNativeError::typ().with_message("WeakMap.delete: called with non-object value")
143+
js_error!(TypeError:
144+
"WeakMap.prototype.delete: expected 'this' to be a WeakMap object",
145+
)
146146
})?;
147147

148148
// 3. Let entries be M.[[WeakMapData]].
@@ -180,7 +180,8 @@ impl WeakMap {
180180
.as_ref()
181181
.and_then(JsObject::downcast_ref::<NativeWeakMap>)
182182
.ok_or_else(|| {
183-
JsNativeError::typ().with_message("WeakMap.get: called with non-object value")
183+
js_error!(TypeError:
184+
"WeakMap.prototype.get: expected 'this' to be a WeakMap object")
184185
})?;
185186

186187
// 3. Let entries be M.[[WeakMapData]].
@@ -215,7 +216,8 @@ impl WeakMap {
215216
.as_ref()
216217
.and_then(JsObject::downcast_ref::<NativeWeakMap>)
217218
.ok_or_else(|| {
218-
JsNativeError::typ().with_message("WeakMap.has: called with non-object value")
219+
js_error!(TypeError:
220+
"WeakMap.prototype.has: expected 'this' to be a WeakMap object")
219221
})?;
220222

221223
// 3. Let entries be M.[[WeakMapData]].
@@ -250,18 +252,18 @@ impl WeakMap {
250252
.as_ref()
251253
.and_then(JsObject::downcast_mut::<NativeWeakMap>)
252254
.ok_or_else(|| {
253-
JsNativeError::typ().with_message("WeakMap.set: called with non-object value")
255+
js_error!(TypeError:
256+
"WeakMap.prototype.set: expected 'this' to be a WeakMap object")
254257
})?;
255258

256259
// 3. Let entries be M.[[WeakMapData]].
257260
// 4. If key is not an Object, throw a TypeError exception.
258261
let key = args.get_or_undefined(0);
259262
let Some(key) = key.as_object() else {
260-
return Err(JsNativeError::typ()
261-
.with_message(format!(
262-
"WeakMap.set: expected target argument of type `object`, got target of type `{}`",
263-
key.type_of()
264-
)).into());
263+
return Err(js_error!(TypeError:
264+
"WeakMap.set: expected target argument of type `object`, got target of type `{}`",
265+
key.type_of()
266+
));
265267
};
266268

267269
// 5. For each Record { [[Key]], [[Value]] } p of entries, do
@@ -298,8 +300,9 @@ impl WeakMap {
298300
let map = object
299301
.and_then(|obj| obj.clone().downcast::<NativeWeakMap>().ok())
300302
.ok_or_else(|| {
301-
JsNativeError::typ()
302-
.with_message("WeakMap.getOrInsert: called with non-object value")
303+
js_error!(TypeError:
304+
"WeakMap.prototype.getOrInsert: expected 'this' to be a WeakMap object",
305+
)
303306
})?;
304307

305308
// 3. If CanBeHeldWeakly(key) is false, throw a TypeError exception.
@@ -308,12 +311,10 @@ impl WeakMap {
308311
// future according to the proposal.
309312
let key_val = args.get_or_undefined(0);
310313
let Some(key) = key_val.as_object() else {
311-
return Err(JsNativeError::typ()
312-
.with_message(format!(
313-
"WeakMap.getOrInsert: expected target argument of type `object`, got target of type `{}`",
314-
key_val.type_of()
315-
))
316-
.into());
314+
return Err(js_error!(TypeError:
315+
"WeakMap.getOrInsert: expected target argument of type `object`, got target of type `{}`",
316+
key_val.type_of()
317+
));
317318
};
318319

319320
// 4. For each Record { [[Key]], [[Value]] } p of M.[[WeakMapData]]
@@ -352,8 +353,9 @@ impl WeakMap {
352353
let map = object
353354
.and_then(|obj| obj.clone().downcast::<NativeWeakMap>().ok())
354355
.ok_or_else(|| {
355-
JsNativeError::typ()
356-
.with_message("WeakMap.getOrInsertComputed: called with non-object value")
356+
js_error!(TypeError:
357+
"WeakMap.prototype.getOrInsertComputed: expected 'this' to be a WeakMap object",
358+
)
357359
})?;
358360

359361
// 3. If CanBeHeldWeakly(key) is false, throw a TypeError exception.
@@ -362,19 +364,17 @@ impl WeakMap {
362364
// future according to the proposal.
363365
let key_value = args.get_or_undefined(0).clone();
364366
let Some(key_obj) = key_value.as_object() else {
365-
return Err(JsNativeError::typ()
366-
.with_message(format!(
367-
"WeakMap.getOrInsertComputed: expected target argument of type `object`, got target of type `{}`",
368-
key_value.type_of()
369-
))
370-
.into());
367+
return Err(js_error!(TypeError:
368+
"WeakMap.getOrInsertComputed: expected target argument of type `object`, got target of type `{}`",
369+
key_value.type_of()
370+
));
371371
};
372372

373373
// 4. If IsCallable(callback) is false, throw a TypeError exception.
374374
let Some(callback_fn) = args.get_or_undefined(1).as_callable() else {
375-
return Err(JsNativeError::typ()
376-
.with_message("Method WeakMap.prototype.getOrInsertComputed called with non-callable callback function")
377-
.into());
375+
return Err(js_error!(TypeError:
376+
"Method WeakMap.prototype.getOrInsertComputed called with non-callable callback function"
377+
));
378378
};
379379

380380
// 5. For each Record { [[Key]], [[Value]] } p of M.[[WeakMapData]]

‎core/engine/src/builtins/weak_map/tests.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ fn get_or_insert_this_not_weakmap() {
101101
run_test_actions([TestAction::assert_native_error(
102102
"WeakMap.prototype.getOrInsert.call({}, {}, 1)",
103103
JsNativeErrorKind::Type,
104-
"WeakMap.getOrInsert: called with non-object value",
104+
"WeakMap.prototype.getOrInsert: expected 'this' to be a WeakMap object",
105105
)]);
106106
}
107107

@@ -110,6 +110,6 @@ fn get_or_insert_computed_this_not_weakmap() {
110110
run_test_actions([TestAction::assert_native_error(
111111
"WeakMap.prototype.getOrInsertComputed.call({}, {}, x => x)",
112112
JsNativeErrorKind::Type,
113-
"WeakMap.getOrInsertComputed: called with non-object value",
113+
"WeakMap.prototype.getOrInsertComputed: expected 'this' to be a WeakMap object",
114114
)]);
115115
}

0 commit comments

Comments
 (0)