fix: functional-correctness batch 1 (semver.sort, immer hasOwnProperty, ctor return-override, jwt crypto keys)

crates/perry-codegen/src/lower_call/new.rs

@@ -172,20 +172,24 @@ fn local_constructor_symbol_exists(ctx: &FnCtx<'_>, class: &perry_hir::Class) ->
         .contains_key(&(class.name.clone(), ctor_method_name))
 }
 
+/// Emit a call to the shared standalone `<class>_constructor` symbol and
+/// return the raw value it produced. The standalone ctor function returns
+/// `undefined` for an ordinary constructor (implicit `return this`) or the
+/// explicitly-returned value for a `return <expr>` body — the caller applies
+/// `js_ctor_return_override` to that raw value to honor ECMAScript's
+/// constructor-return-override rule (a returned object/function replaces the
+/// freshly-allocated `this`). Returns `None` when no standalone symbol exists.
 fn call_local_constructor_symbol(
     ctx: &mut FnCtx<'_>,
     class: &perry_hir::Class,
     obj_box: &str,
     lowered_args: &[String],
-) {
+) -> Option<String> {
     let ctor_method_name = format!("{}_constructor", class.name);
-    let Some(ctor_name) = ctx
+    let ctor_name = ctx
         .methods
         .get(&(class.name.clone(), ctor_method_name))
-        .cloned()
-    else {
-        return;
-    };
+        .cloned()?;
     // The standalone `<class>_constructor` symbol's signature is the class's
     // OWN ctor params, OR — when the class has no own ctor — the closest
     // ancestor-with-a-ctor's params (codegen/artifacts.rs synthesizes the
@@ -243,7 +247,7 @@ fn call_local_constructor_symbol(
     for arg in &ctor_values {
         ctor_args.push((DOUBLE, arg.as_str()));
     }
-    let _ = ctx.block().call(DOUBLE, &ctor_name, &ctor_args);
+    Some(ctx.block().call(DOUBLE, &ctor_name, &ctor_args))
 }
 
 /// Lower `new ClassName(args…)` — Phase C.1.
@@ -871,7 +875,33 @@ pub(crate) fn lower_new(ctx: &mut FnCtx<'_>, class_name: &str, args: &[Expr]) ->
         || ctor_alias_collision
         || force_ctor_call
     {
-        call_local_constructor_symbol(ctx, class, &obj_box, &lowered_args);
+        // Apply ECMAScript constructor return-override semantics on the
+        // standalone-symbol path too. The shared `<class>_constructor` symbol
+        // returns `undefined` for an ordinary ctor (implicit `return this`) or
+        // the explicitly-returned value for a `return <expr>` body. Pre-fix this
+        // path discarded that value and always yielded `obj_box`, so a ctor like
+        // chalk's `class Chalk { constructor(o){ return chalkFactory(o); } }`
+        // produced the empty default instance instead of the returned factory
+        // function ("value is not a function" on `new Chalk(...).red(...)`).
+        // `js_ctor_return_override` returns `obj_box` for an `undefined`/
+        // primitive (base) return, so ordinary ctors are unaffected.
+        if let Some(ctor_ret) = call_local_constructor_symbol(ctx, class, &obj_box, &lowered_args) {
+            let is_derived = class.extends.is_some()
+                || class.extends_name.is_some()
+                || class.native_extends.is_some()
+                || class.extends_expr.is_some();
+            let is_derived_lit = if is_derived { "1" } else { "0" };
+            let final_box = ctx.block().call(
+                DOUBLE,
+                "js_ctor_return_override",
+                &[
+                    (DOUBLE, &obj_box),
+                    (DOUBLE, &ctor_ret),
+                    (crate::types::I32, is_derived_lit),
+                ],
+            );
+            return Ok(final_box);
+        }
         return Ok(obj_box);
     }
 

crates/perry-hir/src/lower/expr_call/array_only_methods.rs

@@ -303,6 +303,20 @@ pub(super) fn try_array_only_methods(
                         if ctx.namespace_import_locals.contains(&n) {
                             return Ok(Err(args));
                         }
+                        // A default/CJS-module import binding used as a method
+                        // receiver (`import semver from "semver"; semver.sort(x)`)
+                        // is a module-exports object, never a local array. It
+                        // lowers to an `ExternFuncRef` receiver, so the array-
+                        // method fold would mis-route `semver.sort(list)` into
+                        // `Expr::ArraySort { array: semver, comparator: list }`
+                        // (the single `list` arg landing in the comparator slot →
+                        // "comparison function must be either a function or
+                        // undefined"). Treat imported bindings like namespaces and
+                        // bail to the namespace/object method dispatch.
+                        if ctx.lookup_local(&n).is_none() && ctx.lookup_imported_func(&n).is_some()
+                        {
+                            return Ok(Err(args));
+                        }
                         let ty = ctx.lookup_local_type(&n);
                         let class_typed = ty
                             .as_ref()
@@ -722,7 +736,14 @@ pub(super) fn try_array_only_methods(
                         let array_expr = lower_expr(ctx, &member.obj)?;
                         return Ok(Ok(Expr::ArrayValues(Box::new(array_expr))));
                     }
-                    "sort" if !args.is_empty() => {
+                    // `!recv_is_class` gate: semver re-exports
+                    // `sort = (list) => list.sort(cmp)` and the driver calls
+                    // `semver.sort(list)`. Without the guard the namespace/class
+                    // receiver was folded to `Expr::ArraySort`, mis-routing the
+                    // single `list` argument into the comparator slot ("comparison
+                    // function must be either a function or undefined"). Mirrors
+                    // the sibling `map`/`filter`/`reduce` arms.
+                    "sort" if !args.is_empty() && !recv_is_class => {
                         let array_expr = lower_expr(ctx, &member.obj)?;
                         return Ok(Ok(Expr::ArraySort {
                             array: Box::new(array_expr),

crates/perry-hir/src/lower/expr_call/imported_array_methods.rs

@@ -119,7 +119,26 @@ pub(super) fn try_imported_array_methods(
                                     }
                                 }
                                 "sort" => {
-                                    if !args.is_empty() {
+                                    // Like `join` above: only fold when the
+                                    // imported binding is statically Array-typed.
+                                    // semver re-exports `sort = (list) =>
+                                    // list.sort(cmp)` and the driver calls
+                                    // `semver.sort(list)`; `semver` is an imported
+                                    // module-exports object (return_type Any), so
+                                    // folding to `Expr::ArraySort { array: semver,
+                                    // comparator: list }` mis-routed the single
+                                    // `list` arg into the comparator slot →
+                                    // "comparison function must be either a
+                                    // function or undefined". Fall through to the
+                                    // generic call path, which invokes the imported
+                                    // `sort` function correctly.
+                                    if !args.is_empty()
+                                        && matches!(
+                                            extern_ref,
+                                            Expr::ExternFuncRef { ref return_type, .. }
+                                                if matches!(return_type, Type::Array(_))
+                                        )
+                                    {
                                         return Ok(Ok(Expr::ArraySort {
                                             array: Box::new(extern_ref),
                                             comparator: Box::new(args.into_iter().next().unwrap()),

crates/perry-hir/src/lower/expr_call/local_array_methods.rs

@@ -15,6 +15,37 @@ use super::super::{
     resolve_typed_parse_ty, LoweringContext,
 };
 
+/// True when `recv_ty` is a statically-known class/namespace instance type
+/// (a `Named` or `Generic` type that is not itself an array). Used to gate
+/// array-method folds (`.sort`/`.map`/…) so a user/library method that merely
+/// shares a builtin Array name (e.g. semver's `semver.sort(list)`) is not
+/// rewritten into the corresponding `Expr::Array*` fast path. TypedArray
+/// `Named` types are deliberately treated as arrays (not class instances).
+fn receiver_is_class_instance(recv_ty: Option<&Type>) -> bool {
+    let is_typed_array = |n: &str| {
+        matches!(
+            n,
+            "Int8Array"
+                | "Int16Array"
+                | "Int32Array"
+                | "Uint8Array"
+                | "Uint8ClampedArray"
+                | "Uint16Array"
+                | "Uint32Array"
+                | "Float16Array"
+                | "Float32Array"
+                | "Float64Array"
+                | "BigInt64Array"
+                | "BigUint64Array"
+        )
+    };
+    match recv_ty {
+        Some(Type::Named(n)) => !is_typed_array(n),
+        Some(Type::Generic { base, .. }) => base != "Array",
+        _ => false,
+    }
+}
+
 pub(super) fn try_local_array_methods(
     ctx: &mut LoweringContext,
     call: &ast::CallExpr,
@@ -533,7 +564,19 @@ pub(super) fn try_local_array_methods(
                                 }
                             }
                             "sort" => {
-                                if !args.is_empty() {
+                                // semver `module.exports.sort = (list) => …` is
+                                // re-exported as a plain function and called as
+                                // `semver.sort(list)`. The receiver there is a
+                                // class/namespace instance, NOT an array, so
+                                // folding to `Expr::ArraySort` mis-routed the
+                                // single `list` argument into the comparator slot
+                                // → "comparison function must be either a function
+                                // or undefined". Only fold when the receiver is
+                                // not a statically-known class instance (mirrors
+                                // the `map`/`filter`/`with` guards).
+                                if !args.is_empty()
+                                    && !receiver_is_class_instance(ctx.lookup_local_type(&arr_name))
+                                {
                                     return Ok(Ok(Expr::ArraySort {
                                         array: Box::new(Expr::LocalGet(array_id)),
                                         comparator: Box::new(args.into_iter().next().unwrap()),

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

@@ -6387,6 +6387,57 @@ fn install_builtin_constructor_statics(name: &str, ctor: *mut crate::closure::Cl
                 true,
             );
             install_constructor_static(ctor, "hasOwn", object_hasown_thunk as *const u8, 2, false);
+            // `Object` is a function, so reading a non-static member resolves up
+            // its prototype chain (Function.prototype → Object.prototype). In
+            // particular `Object.hasOwnProperty` IS `Object.prototype.hasOwnProperty`
+            // — a callable. immer's `O.hasOwnProperty.call(proto, "constructor")`
+            // (with `const O = Object`) relied on this; without the inherited
+            // methods installed on the reified ctor value the read returned
+            // `undefined` and `.call` threw "Function.prototype.call on a value
+            // that is not a function". Install the Object.prototype methods that
+            // are reachable on the constructor by inheritance.
+            install_constructor_static(
+                ctor,
+                "hasOwnProperty",
+                object_prototype_has_own_property_thunk as *const u8,
+                1,
+                false,
+            );
+            install_constructor_static(
+                ctor,
+                "isPrototypeOf",
+                object_prototype_is_prototype_of_thunk as *const u8,
+                1,
+                false,
+            );
+            install_constructor_static(
+                ctor,
+                "propertyIsEnumerable",
+                object_prototype_property_is_enumerable_thunk as *const u8,
+                1,
+                false,
+            );
+            install_constructor_static(
+                ctor,
+                "toString",
+                object_prototype_to_string_thunk as *const u8,
+                0,
+                false,
+            );
+            install_constructor_static(
+                ctor,
+                "toLocaleString",
+                object_prototype_to_locale_string_thunk as *const u8,
+                0,
+                false,
+            );
+            install_constructor_static(
+                ctor,
+                "valueOf",
+                object_prototype_value_of_thunk as *const u8,
+                0,
+                false,
+            );
         }
         "Array" => {
             install_constructor_static(

crates/perry-stdlib/src/crypto/sign.rs

@@ -354,10 +354,30 @@ pub unsafe extern "C" fn js_crypto_create_public_key(
 pub unsafe extern "C" fn js_crypto_create_private_key_value(key_bits: f64) -> *mut StringHeader {
     let pem = match crypto_key_input_to_private_pem(key_bits.to_bits()) {
         Some(pem) => pem,
-        None => return std::ptr::null_mut(),
+        None => {
+            perry_runtime::fs::validate::throw_type_error_with_code(
+                "Invalid key object type, expected private key material",
+                "ERR_INVALID_ARG_TYPE",
+            );
+        }
     };
+    let asym_type = classify_private_key_surrogate(&pem);
+    // Node validates key material: `createPrivateKey("not a pem")` THROWS, it
+    // does NOT produce a key with `type === undefined`. jsonwebtoken's sign()
+    // relies on this — it `try { createPrivateKey(secret) } catch { createSecretKey(...) }`,
+    // so a string HMAC secret must make createPrivateKey throw to reach the
+    // createSecretKey fallback. Accept only inputs we can classify as a real
+    // private key, or that carry a PEM `-----BEGIN ... PRIVATE KEY-----` header
+    // (covers encrypted / DER-in-PEM forms the surrogate parsers may not cover);
+    // reject everything else.
+    if asym_type.is_none() && !pem.contains("PRIVATE KEY") {
+        perry_runtime::fs::validate::throw_type_error_with_code(
+            "Invalid PEM formatted message.",
+            "ERR_OSSL_PEM_NO_START_LINE",
+        );
+    }
     let ptr = js_string_from_bytes(pem.as_ptr(), pem.len() as u32);
-    if let Some(asym_type) = classify_private_key_surrogate(&pem) {
+    if let Some(asym_type) = asym_type {
         mark_keyobject_string(ptr, KeyKind::Private, asym_type);
     }
     ptr
@@ -367,10 +387,29 @@ pub unsafe extern "C" fn js_crypto_create_private_key_value(key_bits: f64) -> *m
 pub unsafe extern "C" fn js_crypto_create_public_key_value(key_bits: f64) -> *mut StringHeader {
     let pem = match crypto_key_input_to_public_pem(key_bits.to_bits()) {
         Some(pem) => pem,
-        None => return std::ptr::null_mut(),
+        None => {
+            perry_runtime::fs::validate::throw_type_error_with_code(
+                "Invalid key object type, expected public key material",
+                "ERR_INVALID_ARG_TYPE",
+            );
+        }
     };
+    let asym_type = classify_public_key_surrogate(&pem);
+    // Mirror `createPrivateKey`: Node throws on non-key material rather than
+    // producing a `type === undefined` key. jsonwebtoken's verify() does
+    // `try { createPublicKey(secret) } catch { createSecretKey(...) }`, so a
+    // string HMAC secret must make createPublicKey throw to reach the
+    // createSecretKey fallback (otherwise verify picks PUB_KEY_ALGS and rejects
+    // an HS256 token with "invalid algorithm"). Accept only classifiable public
+    // keys or PEM-headed input; reject everything else.
+    if asym_type.is_none() && !pem.contains("PUBLIC KEY") && !pem.contains("PRIVATE KEY") {
+        perry_runtime::fs::validate::throw_type_error_with_code(
+            "Invalid PEM formatted message.",
+            "ERR_OSSL_PEM_NO_START_LINE",
+        );
+    }
     let ptr = js_string_from_bytes(pem.as_ptr(), pem.len() as u32);
-    if let Some(asym_type) = classify_public_key_surrogate(&pem) {
+    if let Some(asym_type) = asym_type {
         mark_keyobject_string(ptr, KeyKind::Public, asym_type);
     }
     ptr

tests/test_crypto_create_key_invalid_throws.sh

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+# Node's `crypto.createPrivateKey` / `createPublicKey` THROW on input that is
+# not valid key material (e.g. a plain non-PEM string), rather than producing a
+# KeyObject with `type === undefined`. jsonwebtoken's sign()/verify() rely on
+# this: they `try { createPrivateKey(secret) } catch { createSecretKey(...) }`,
+# so a string HMAC secret must make createPrivateKey/createPublicKey throw to
+# reach the createSecretKey fallback. Pre-fix perry returned the string as a
+# bogus key (type undefined), so HS256 signing reported "secretOrPrivateKey
+# must be a symmetric key" and verify reported "invalid algorithm".
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
+PERRY="${PERRY_BIN:-${PERRY:-$REPO_ROOT/target/release/perry}}"
+if [[ ! -x "$PERRY" ]]; then PERRY="$REPO_ROOT/target/debug/perry"; fi
+if [[ ! -x "$PERRY" ]]; then
+    echo "SKIP: perry binary not found (build with cargo build -p perry)"
+    exit 0
+fi
+
+TMPDIR="$(mktemp -d)"
+trap 'rm -rf "$TMPDIR"' EXIT
+
+cat >"$TMPDIR/f.ts" <<'TS'
+import { createPrivateKey, createPublicKey, createSecretKey } from "crypto";
+
+let priv = false;
+try { createPrivateKey("test-secret-key"); } catch (_) { priv = true; }
+if (!priv) throw new Error("createPrivateKey should throw on a non-PEM string");
+
+let pub = false;
+try { createPublicKey("test-secret-key"); } catch (_) { pub = true; }
+if (!pub) throw new Error("createPublicKey should throw on a non-PEM string");
+
+// The legitimate symmetric-key fallback still works and is type 'secret'.
+const sk: any = createSecretKey(Buffer.from("test-secret-key"));
+if (sk.type !== "secret") throw new Error("createSecretKey type: " + sk.type);
+
+console.log("OK");
+TS
+
+OUT="$("$PERRY" run "$TMPDIR/f.ts" 2>&1)" || { echo "FAIL: perry run errored"; echo "$OUT"; exit 1; }
+if ! grep -q "^OK$" <<<"$OUT"; then echo "FAIL: expected OK, got:"; echo "$OUT"; exit 1; fi
+echo "PASS: createPrivateKey/createPublicKey throw on invalid key material"