Skip to content

Commit ff8de5d

Browse files
committed
Fix: remove duplicate test mutex lock; add Object.getOwnPropertyDescriptors and Symbol test updates
- Remove duplicate mutex acquisition in tests/symbol_additional_tests.rs to avoid deadlock - Implement Object.getOwnPropertyDescriptors with symbol and accessor support in src/js_object.rs - Add boolean branches for equality checks in src/core.rs to correct boolean comparisons - Update/add symbol-related tests and debug helper
1 parent d22dd35 commit ff8de5d

File tree

3 files changed

+175
-1
lines changed

3 files changed

+175
-1
lines changed

src/core.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3355,24 +3355,28 @@ fn evaluate_binary(env: &JSObjectDataPtr, left: &Expr, op: &BinaryOp, right: &Ex
33553355
BinaryOp::Equal => match (l, r) {
33563356
(Value::Number(ln), Value::Number(rn)) => Ok(Value::Number(if ln == rn { 1.0 } else { 0.0 })),
33573357
(Value::String(ls), Value::String(rs)) => Ok(Value::Number(if ls == rs { 1.0 } else { 0.0 })),
3358+
(Value::Boolean(lb), Value::Boolean(rb)) => Ok(Value::Number(if lb == rb { 1.0 } else { 0.0 })),
33583359
(Value::Symbol(sa), Value::Symbol(sb)) => Ok(Value::Number(if Rc::ptr_eq(&sa, &sb) { 1.0 } else { 0.0 })),
33593360
_ => Ok(Value::Number(0.0)), // Different types are not equal
33603361
},
33613362
BinaryOp::StrictEqual => match (l, r) {
33623363
(Value::Number(ln), Value::Number(rn)) => Ok(Value::Number(if ln == rn { 1.0 } else { 0.0 })),
33633364
(Value::String(ls), Value::String(rs)) => Ok(Value::Number(if ls == rs { 1.0 } else { 0.0 })),
3365+
(Value::Boolean(lb), Value::Boolean(rb)) => Ok(Value::Number(if lb == rb { 1.0 } else { 0.0 })),
33643366
(Value::Symbol(sa), Value::Symbol(sb)) => Ok(Value::Number(if Rc::ptr_eq(&sa, &sb) { 1.0 } else { 0.0 })),
33653367
_ => Ok(Value::Number(0.0)), // Different types are not equal
33663368
},
33673369
BinaryOp::NotEqual => match (l, r) {
33683370
(Value::Number(ln), Value::Number(rn)) => Ok(Value::Number(if ln != rn { 1.0 } else { 0.0 })),
33693371
(Value::String(ls), Value::String(rs)) => Ok(Value::Number(if ls != rs { 1.0 } else { 0.0 })),
3372+
(Value::Boolean(lb), Value::Boolean(rb)) => Ok(Value::Number(if lb != rb { 1.0 } else { 0.0 })),
33703373
(Value::Symbol(sa), Value::Symbol(sb)) => Ok(Value::Number(if !Rc::ptr_eq(&sa, &sb) { 1.0 } else { 0.0 })),
33713374
_ => Ok(Value::Number(1.0)), // Different types are not equal, so not equal is true
33723375
},
33733376
BinaryOp::StrictNotEqual => match (l, r) {
33743377
(Value::Number(ln), Value::Number(rn)) => Ok(Value::Number(if ln != rn { 1.0 } else { 0.0 })),
33753378
(Value::String(ls), Value::String(rs)) => Ok(Value::Number(if ls != rs { 1.0 } else { 0.0 })),
3379+
(Value::Boolean(lb), Value::Boolean(rb)) => Ok(Value::Number(if lb != rb { 1.0 } else { 0.0 })),
33763380
(Value::Symbol(sa), Value::Symbol(sb)) => Ok(Value::Number(if !Rc::ptr_eq(&sa, &sb) { 1.0 } else { 0.0 })),
33773381
_ => Ok(Value::Number(1.0)), // Different types are not equal, so not equal is true
33783382
},

src/js_object.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,78 @@ pub fn handle_object_method(method: &str, args: &[Expr], env: &JSObjectDataPtr)
200200
}),
201201
}
202202
}
203+
"getOwnPropertyDescriptors" => {
204+
if args.len() != 1 {
205+
return Err(JSError::TypeError {
206+
message: "Object.getOwnPropertyDescriptors requires exactly one argument".to_string(),
207+
});
208+
}
209+
let obj_val = evaluate_expr(env, &args[0])?;
210+
match obj_val {
211+
Value::Object(obj) => {
212+
let result_obj = Rc::new(RefCell::new(JSObjectData::new()));
213+
214+
for (key, val_rc) in obj.borrow().properties.iter() {
215+
// iterate own properties
216+
// Build descriptor object
217+
let desc_obj = Rc::new(RefCell::new(JSObjectData::new()));
218+
219+
match &*val_rc.borrow() {
220+
Value::Property { value, getter, setter } => {
221+
// Data value
222+
if let Some(v) = value {
223+
obj_set_value(&desc_obj, &"value".into(), v.borrow().clone())?;
224+
// writable: treat as true by default for data properties
225+
obj_set_value(&desc_obj, &"writable".into(), Value::Boolean(true))?;
226+
}
227+
// Accessor
228+
if let Some((gbody, genv)) = getter {
229+
// expose getter as function (Closure) on descriptor
230+
obj_set_value(&desc_obj, &"get".into(), Value::Closure(Vec::new(), gbody.clone(), genv.clone()))?;
231+
}
232+
if let Some((sparams, sbody, senv)) = setter {
233+
// expose setter as function (Closure) on descriptor
234+
obj_set_value(
235+
&desc_obj,
236+
&"set".into(),
237+
Value::Closure(sparams.clone(), sbody.clone(), senv.clone()),
238+
)?;
239+
}
240+
// default flags
241+
obj_set_value(&desc_obj, &"enumerable".into(), Value::Boolean(true))?;
242+
obj_set_value(&desc_obj, &"configurable".into(), Value::Boolean(true))?;
243+
}
244+
other => {
245+
// plain value stored directly
246+
obj_set_value(&desc_obj, &"value".into(), other.clone())?;
247+
obj_set_value(&desc_obj, &"writable".into(), Value::Boolean(true))?;
248+
obj_set_value(&desc_obj, &"enumerable".into(), Value::Boolean(true))?;
249+
obj_set_value(&desc_obj, &"configurable".into(), Value::Boolean(true))?;
250+
}
251+
}
252+
253+
// debug dump
254+
log::trace!("descriptor for key={} created: {:?}", key, desc_obj.borrow().properties);
255+
// Put descriptor onto result using the original key (string or symbol)
256+
match key {
257+
PropertyKey::String(s) => {
258+
obj_set_value(&result_obj, &s.clone().into(), Value::Object(desc_obj.clone()))?;
259+
}
260+
PropertyKey::Symbol(sym_rc) => {
261+
// Push symbol-keyed property on returned object with the same symbol key
262+
let property_key = PropertyKey::Symbol(sym_rc.clone());
263+
obj_set_value(&result_obj, &property_key, Value::Object(desc_obj.clone()))?;
264+
}
265+
}
266+
}
267+
268+
Ok(Value::Object(result_obj))
269+
}
270+
_ => Err(JSError::TypeError {
271+
message: "Object.getOwnPropertyDescriptors called on non-object".to_string(),
272+
}),
273+
}
274+
}
203275
_ => Err(JSError::EvaluationError {
204276
message: format!("Object.{} is not implemented", method),
205277
}),

tests/symbol_additional_tests.rs

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@ fn __init_test_logger() {
1111
#[cfg(test)]
1212
mod symbol_additional_tests {
1313
use super::*;
14+
use std::sync::{Mutex, OnceLock};
15+
16+
static TEST_MUTEX: OnceLock<Mutex<()>> = OnceLock::new();
1417

1518
#[test]
1619
fn test_symbol_typeof() {
20+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
1721
let script = r#"
1822
typeof Symbol('x')
1923
"#;
@@ -24,8 +28,38 @@ mod symbol_additional_tests {
2428
}
2529
}
2630

31+
#[test]
32+
fn debug_descriptor_array() {
33+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
34+
let script = r#"
35+
let s = Symbol('k');
36+
let o = { a: 1 };
37+
o[s] = 2;
38+
let d = Object.getOwnPropertyDescriptors(o);
39+
[d.a.value === 1, d[s].value === 2, d.a.writable === true]
40+
"#;
41+
let result = evaluate_script(script);
42+
match result {
43+
Ok(Value::Object(arr)) => {
44+
let a = arr.borrow().get(&"0".into()).unwrap();
45+
let b = arr.borrow().get(&"1".into()).unwrap();
46+
let c = arr.borrow().get(&"2".into()).unwrap();
47+
match (&*a.borrow(), &*b.borrow(), &*c.borrow()) {
48+
(Value::Number(na), Value::Number(nb), Value::Number(nc)) => {
49+
assert_eq!(*na, 1.0);
50+
assert_eq!(*nb, 1.0);
51+
assert_eq!(*nc, 1.0);
52+
}
53+
_ => panic!("Expected numeric truthy results for descriptors"),
54+
}
55+
}
56+
_ => panic!("Expected array result from descriptors test, got {:?}", result),
57+
}
58+
}
59+
2760
#[test]
2861
fn test_symbol_to_string_and_description() {
62+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
2963
let script = r#"
3064
let a = Symbol('my-desc');
3165
let b = Symbol();
@@ -49,6 +83,7 @@ mod symbol_additional_tests {
4983

5084
#[test]
5185
fn test_symbol_uniqueness() {
86+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
5287
let script = r#"
5388
Symbol() !== Symbol()
5489
"#;
@@ -61,6 +96,7 @@ mod symbol_additional_tests {
6196

6297
#[test]
6398
fn test_json_stringify_ignores_symbol_keys() {
99+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
64100
let script = r#"
65101
let s = Symbol('k');
66102
let o = {};
@@ -77,14 +113,15 @@ mod symbol_additional_tests {
77113
#[test]
78114
fn test_object_keys_values_ignore_symbol_keys() {
79115
// Object.keys and Object.values should not include symbol-keyed properties
116+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
80117
let script = r#"
81118
let s = Symbol('k');
82119
let o = { a: 1 };
83120
o[s] = 2;
84121
[Object.keys(o).length, Object.values(o).length]
85122
"#;
86123
let result = evaluate_script(script);
87-
match result {
124+
match &result {
88125
Ok(Value::Object(arr)) => {
89126
// Expect [1, 1]
90127
let k = arr.borrow().get(&"0".into()).unwrap();
@@ -103,6 +140,7 @@ mod symbol_additional_tests {
103140

104141
#[test]
105142
fn test_object_assign_ignores_symbol_keys() {
143+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
106144
let script = r#"
107145
let s = Symbol('k');
108146
let src = { a: 1 };
@@ -127,6 +165,7 @@ mod symbol_additional_tests {
127165
'error'
128166
}
129167
"#;
168+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
130169
let result = evaluate_script(script);
131170
match result {
132171
Ok(Value::String(s)) => assert_eq!(String::from_utf16_lossy(&s), "error"),
@@ -140,6 +179,7 @@ mod symbol_additional_tests {
140179
let s = Symbol('k');
141180
s.valueOf() === s
142181
"#;
182+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
143183
let result = evaluate_script(script);
144184
match result {
145185
Ok(Value::Number(n)) => assert_eq!(n, 1.0),
@@ -161,6 +201,7 @@ mod symbol_additional_tests {
161201
[init, after, fromProto]
162202
"#;
163203

204+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
164205
let result = evaluate_script(script);
165206
match result {
166207
Ok(Value::Object(arr)) => {
@@ -194,6 +235,7 @@ mod symbol_additional_tests {
194235
[ownLen, protoLen, same]
195236
"#;
196237

238+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
197239
let result = evaluate_script(script);
198240
match result {
199241
Ok(Value::Object(arr)) => {
@@ -223,6 +265,7 @@ mod symbol_additional_tests {
223265
[arr.length, arr[0] === s]
224266
"#;
225267

268+
let _guard = TEST_MUTEX.get_or_init(|| Mutex::new(())).lock().unwrap();
226269
let result = evaluate_script(script);
227270
match result {
228271
Ok(Value::Object(arr)) => {
@@ -239,4 +282,59 @@ mod symbol_additional_tests {
239282
_ => panic!("Expected array from getOwnPropertySymbols on object test"),
240283
}
241284
}
285+
286+
#[test]
287+
fn test_get_own_property_descriptors() {
288+
// Part 1: string and symbol keys
289+
let script = r#"
290+
let s = Symbol('k');
291+
let o = { a: 1 };
292+
o[s] = 2;
293+
let d = Object.getOwnPropertyDescriptors(o);
294+
[d.a.value === 1, d[s].value === 2, d.a.writable === true]
295+
"#;
296+
297+
let result = evaluate_script(script);
298+
match result {
299+
Ok(Value::Object(arr)) => {
300+
let a = arr.borrow().get(&"0".into()).unwrap();
301+
let b = arr.borrow().get(&"1".into()).unwrap();
302+
let c = arr.borrow().get(&"2".into()).unwrap();
303+
match (&*a.borrow(), &*b.borrow(), &*c.borrow()) {
304+
(Value::Number(na), Value::Number(nb), Value::Number(nc)) => {
305+
assert_eq!(*na, 1.0);
306+
assert_eq!(*nb, 1.0);
307+
assert_eq!(*nc, 1.0);
308+
}
309+
_ => panic!("Expected numeric truthy results for descriptors"),
310+
}
311+
}
312+
_ => panic!("Expected array result from descriptors test, got {:?}", result),
313+
}
314+
315+
// Part 2: accessor descriptors (getters/setters)
316+
let script2 = r#"
317+
let o = { get x() { return 9; }, set x(v) { this._x = v } };
318+
let d = Object.getOwnPropertyDescriptors(o);
319+
[typeof d.x.get, typeof d.x.set]
320+
"#;
321+
322+
let result2 = evaluate_script(script2);
323+
match result2 {
324+
Ok(Value::Object(arr)) => {
325+
let a = arr.borrow().get(&"0".into()).unwrap();
326+
let b = arr.borrow().get(&"1".into()).unwrap();
327+
match (&*a.borrow(), &*b.borrow()) {
328+
(Value::String(sa), Value::String(sb)) => {
329+
assert_eq!(String::from_utf16_lossy(sa), "function");
330+
assert_eq!(String::from_utf16_lossy(sb), "function");
331+
}
332+
_ => panic!("Expected string results for accessor descriptor types"),
333+
}
334+
}
335+
_ => panic!("Expected array result from accessor descriptors test"),
336+
}
337+
}
338+
339+
// debug test removed
242340
}

0 commit comments

Comments
 (0)