fix(runtime): throw for non-callable locale string methods

changelog.d/8379-noncallable-locale-string-methods.md

@@ -0,0 +1,3 @@
+Primitive and typed-array `toLocaleString` calls now throw a `TypeError` when
+prototype lookup resolves the invoked method to a non-callable data property or
+accessor result, instead of silently falling back to native formatting.

crates/perry-runtime/src/object/native_call_method.rs

@@ -398,11 +398,12 @@ unsafe fn builtin_proto_accessor_method(
 }
 
 /// A *user-installed* method on a builtin's prototype object (e.g.
-/// `Number.prototype.toLocaleString = function () { … }`). Returns the patched
-/// closure value, or `None` when the property is absent / not a real closure /
-/// the no-op-backed builtin placeholder — i.e. `None` means "the native
-/// builtin behavior is still in effect".
-unsafe fn builtin_proto_user_method(
+/// `Number.prototype.toLocaleString = function () { … }`). Returns the resolved
+/// value even when it is not callable: callers implementing `Invoke` must
+/// distinguish a present non-callable property (TypeError) from the
+/// no-op-backed builtin placeholder (`None`, meaning native behavior still
+/// applies).
+unsafe fn builtin_proto_user_value(
     builtin_name: &[u8],
     method_name: &str,
     receiver: f64,
@@ -431,17 +432,14 @@ unsafe fn builtin_proto_user_method(
             js_object_get_field_by_name(proto_ptr, key)
         }
     };
-    if (value.bits() & crate::value::TAG_MASK) != crate::value::POINTER_TAG {
-        return None;
-    }
-    let ptr = (value.bits() & crate::value::POINTER_MASK) as usize;
-    if !crate::closure::is_closure_ptr(ptr) {
-        return None;
-    }
-    if (*(ptr as *const crate::closure::ClosureHeader)).func_ptr
-        == super::global_this::global_this_builtin_noop_thunk as *const u8
-    {
-        return None;
+    if (value.bits() & crate::value::TAG_MASK) == crate::value::POINTER_TAG {
+        let ptr = (value.bits() & crate::value::POINTER_MASK) as usize;
+        if crate::closure::is_closure_ptr(ptr)
+            && (*(ptr as *const crate::closure::ClosureHeader)).func_ptr
+                == super::global_this::global_this_builtin_noop_thunk as *const u8
+        {
+            return None;
+        }
     }
     Some(value)
 }

crates/perry-runtime/src/object/native_call_method/object_proto.rs

@@ -139,13 +139,19 @@ pub(crate) unsafe fn js_object_default_to_locale_string(receiver: f64) -> f64 {
         };
         if !builtin_name.is_empty() {
             if let Some(patched) =
-                unsafe { super::builtin_proto_user_method(builtin_name, "toString", receiver) }
+                unsafe { super::builtin_proto_user_value(builtin_name, "toString", receiver) }
             {
                 if let Some(result) =
                     unsafe { call_primitive_closure_value(receiver, patched, std::ptr::null(), 0) }
                 {
                     return result;
                 }
+                // `Invoke(O, "toString")` must call the value returned by
+                // `GetV`. A present data property such as
+                // `String.prototype.toString = 42`, or an accessor returning
+                // a non-callable, throws rather than silently selecting the
+                // native fallback (#5901).
+                throw_object_to_string_not_function();
             }
         }
         return unsafe {

crates/perry-runtime/src/object/native_call_method/typed_array.rs

@@ -268,7 +268,7 @@ pub(crate) unsafe fn dispatch_typed_array_method(
             // formatted individually below, so pass `undefined`.
             let (patched, ta) =
                 ta_handle.across_mut::<crate::typedarray::TypedArrayHeader, _>(|| {
-                    builtin_proto_user_method(
+                    builtin_proto_user_value(
                         builtin,
                         "toLocaleString",
                         f64::from_bits(crate::value::TAG_UNDEFINED),
@@ -296,8 +296,23 @@ pub(crate) unsafe fn dispatch_typed_array_method(
                         let patched_now = JSValue::from_bits(patched_now.to_bits());
 
                         let (r, _) = patched_handle.across_nanbox(|| {
-                            call_primitive_closure_value(elem, patched_now, std::ptr::null(), 0)
-                                .unwrap_or(f64::from_bits(crate::value::TAG_UNDEFINED))
+                            match call_primitive_closure_value(
+                                elem,
+                                patched_now,
+                                std::ptr::null(),
+                                0,
+                            ) {
+                                Some(result) => result,
+                                None => {
+                                    let kind = if is_bigint { "bigint" } else { "number" };
+                                    crate::error::js_throw_type_error_not_a_function(
+                                        kind.as_ptr(),
+                                        kind.len(),
+                                        b"toLocaleString".as_ptr(),
+                                        "toLocaleString".len(),
+                                    )
+                                }
+                            }
                         });
 
                         let (s_hdr, ta_after) = ta_handle

test-files/test_gap_object_tolocalestring_primitive_receiver.ts

@@ -78,10 +78,42 @@ Object.defineProperty(Boolean.prototype, "toString", {
 
 console.log("data:", (true as any).toLocaleString());
 
-// NOT covered here: a `toString` that resolves to a NON-CALLABLE. The spec
-// throws (`Invoke` -> `Call` on a non-callable), while Perry falls through to
-// the native `toString`. That divergence is independent of the receiver rule —
-// it predates this file and reproduces identically through a plain DATA
-// property (`defineProperty(String.prototype, "toString", { value: 42 })`),
-// because the resolver collapses "absent" and "present but not callable" into
-// one "native behavior still applies" answer.
+// A present NON-CALLABLE must throw (`Invoke` -> `Call`), not collapse into the
+// "native behavior still applies" fallback.
+Object.defineProperty(String.prototype, "toString", {
+  configurable: true,
+  value: 42,
+});
+try {
+  ("abc" as any).toLocaleString();
+  console.log("data noncallable: missed");
+} catch (error) {
+  console.log("data noncallable:", error instanceof TypeError);
+}
+
+// The same distinction matters when an accessor resolves the value.
+Object.defineProperty(Boolean.prototype, "toString", {
+  configurable: true,
+  get: function () {
+    return null;
+  },
+});
+try {
+  (true as any).toLocaleString();
+  console.log("accessor noncallable: missed");
+} catch (error) {
+  console.log("accessor noncallable:", error instanceof TypeError);
+}
+
+// TypedArray.prototype.toLocaleString performs the same Invoke operation for
+// each numeric element, through the shared prototype resolver.
+Object.defineProperty(Number.prototype, "toLocaleString", {
+  configurable: true,
+  value: 42,
+});
+try {
+  new Int32Array([1]).toLocaleString();
+  console.log("typed array noncallable: missed");
+} catch (error) {
+  console.log("typed array noncallable:", error instanceof TypeError);
+}