diff --git a/crates/perry-hir/src/lower/expr_call/globals.rs b/crates/perry-hir/src/lower/expr_call/globals.rs index 5c9584236..262a7e017 100644 --- a/crates/perry-hir/src/lower/expr_call/globals.rs +++ b/crates/perry-hir/src/lower/expr_call/globals.rs @@ -135,19 +135,25 @@ pub(super) fn try_global_builtins( byte_offset: 0, })); } + // A missing argument to these is NOT an error in JS — the parameter is + // simply `undefined`, and each has well-defined behavior for it + // (`isNaN() === true`, `isFinite() === false`, `encodeURI() === "undefined"`, + // …, all verified against Node). Rejecting the call at COMPILE time made + // perry refuse to compile legal JS. Lower the omitted argument to + // `Expr::Undefined` and let the existing intrinsic produce Node's answer. + // + // NOT extended to `atob`/`btoa`/`structuredClone`: those are WebIDL + // required-argument throws where `f()` and `f(undefined)` genuinely + // DIFFER (`atob()` → TypeError but `atob(undefined)` → InvalidCharacterError; + // `btoa()` → TypeError but `btoa(undefined)` → "dW5kZWZpbmVk"), so they + // cannot be modelled by padding and need real arity plumbing. See #6366. "isNaN" => { - if !args.is_empty() { - return Ok(Ok(Expr::IsNaN(Box::new(args.remove(0))))); - } else { - return Err(anyhow!("isNaN requires one argument")); - } + let arg = arg_or_undefined(&mut args); + return Ok(Ok(Expr::IsNaN(Box::new(arg)))); } "isFinite" => { - if !args.is_empty() { - return Ok(Ok(Expr::IsFinite(Box::new(args.remove(0))))); - } else { - return Err(anyhow!("isFinite requires one argument")); - } + let arg = arg_or_undefined(&mut args); + return Ok(Ok(Expr::IsFinite(Box::new(arg)))); } "atob" => { if !args.is_empty() { @@ -164,32 +170,20 @@ pub(super) fn try_global_builtins( } } "encodeURI" => { - if !args.is_empty() { - return Ok(Ok(Expr::EncodeURI(Box::new(args.remove(0))))); - } else { - return Err(anyhow!("encodeURI requires one argument")); - } + let arg = arg_or_undefined(&mut args); + return Ok(Ok(Expr::EncodeURI(Box::new(arg)))); } "decodeURI" => { - if !args.is_empty() { - return Ok(Ok(Expr::DecodeURI(Box::new(args.remove(0))))); - } else { - return Err(anyhow!("decodeURI requires one argument")); - } + let arg = arg_or_undefined(&mut args); + return Ok(Ok(Expr::DecodeURI(Box::new(arg)))); } "encodeURIComponent" => { - if !args.is_empty() { - return Ok(Ok(Expr::EncodeURIComponent(Box::new(args.remove(0))))); - } else { - return Err(anyhow!("encodeURIComponent requires one argument")); - } + let arg = arg_or_undefined(&mut args); + return Ok(Ok(Expr::EncodeURIComponent(Box::new(arg)))); } "decodeURIComponent" => { - if !args.is_empty() { - return Ok(Ok(Expr::DecodeURIComponent(Box::new(args.remove(0))))); - } else { - return Err(anyhow!("decodeURIComponent requires one argument")); - } + let arg = arg_or_undefined(&mut args); + return Ok(Ok(Expr::DecodeURIComponent(Box::new(arg)))); } "structuredClone" => { if !args.is_empty() { @@ -957,3 +951,13 @@ pub(super) fn try_global_builtins( // exponential lowering walk. Ok(Err(args)) } + +/// An omitted argument to a JS global is `undefined`, not an error — mirrors the +/// `parseInt` / `parseFloat` / `BigInt` / `Object` padding above. +fn arg_or_undefined(args: &mut Vec) -> Expr { + if args.is_empty() { + Expr::Undefined + } else { + args.remove(0) + } +} diff --git a/crates/perry-hir/tests/global_builtin_shadowing.rs b/crates/perry-hir/tests/global_builtin_shadowing.rs index 22fec9749..c423f5614 100644 --- a/crates/perry-hir/tests/global_builtin_shadowing.rs +++ b/crates/perry-hir/tests/global_builtin_shadowing.rs @@ -49,12 +49,38 @@ fn local_isfinite_helper_zero_arg_call_does_not_use_global_builtin_arity() { ); } +/// The sibling of `local_isfinite_helper_zero_arg_call_does_not_use_global_builtin_arity`: +/// with NO local shadow, `isFinite()` must route to the global BUILTIN, not to a +/// user function. +/// +/// This used to assert a compile-time "isFinite requires one argument" error, using +/// that diagnostic as a proxy for "the builtin path was taken". But the diagnostic +/// itself was wrong: a missing argument is not an error in JS — the parameter is +/// simply `undefined`, and `isFinite()` evaluates to `false` in Node (it never +/// throws). Rejecting it made perry refuse to compile legal JS. Assert the routing +/// directly instead: the call lowers to the `IsFinite` intrinsic with the omitted +/// argument as `Expr::Undefined`. #[test] -fn unshadowed_global_isfinite_zero_arg_call_keeps_builtin_arity_error() { - let err = lower_src("const result = isFinite();") - .expect_err("unshadowed global isFinite() should keep the builtin arity diagnostic"); +fn unshadowed_global_isfinite_zero_arg_call_lowers_to_builtin_with_undefined() { + let module = lower_src("const result = isFinite();") + .expect("unshadowed global isFinite() is legal JS and must lower, not error"); + + let result_init = module + .init + .iter() + .find_map(|stmt| match stmt { + Stmt::Let { + name, + init: Some(init), + .. + } if name == "result" => Some(init), + _ => None, + }) + .expect("result binding should be lowered"); + assert!( - err.to_string().contains("isFinite requires one argument"), - "{err}" + matches!(result_init, Expr::IsFinite(arg) if matches!(arg.as_ref(), Expr::Undefined)), + "zero-arg isFinite() should reach the builtin intrinsic with an undefined \ + argument (Node evaluates it to false), got: {result_init:?}" ); } diff --git a/test-files/test_gap_6366_global_zero_arg_coercion.ts b/test-files/test_gap_6366_global_zero_arg_coercion.ts new file mode 100644 index 000000000..781677548 --- /dev/null +++ b/test-files/test_gap_6366_global_zero_arg_coercion.ts @@ -0,0 +1,29 @@ +// A missing argument to a JS global is NOT an error — the parameter is simply +// `undefined`, and each of these has well-defined behavior for it. Perry used to +// reject the zero-arg call at COMPILE time ("isNaN requires one argument"), which +// meant it refused to compile legal JS. +// +// `atob()` / `btoa()` / `structuredClone()` are deliberately NOT covered here: +// those are WebIDL required-argument throws where `f()` and `f(undefined)` really +// do differ, so they need arity plumbing rather than undefined-padding. +// +// Validated byte-for-byte against `node --experimental-strip-types`. + +console.log("isNaN():", isNaN()); +console.log("isFinite():", isFinite()); +console.log("encodeURI():", encodeURI()); +console.log("decodeURI():", decodeURI()); +console.log("encodeURIComponent():", encodeURIComponent()); +console.log("decodeURIComponent():", decodeURIComponent()); + +// explicit undefined behaves the same +console.log("isNaN(undefined):", isNaN(undefined)); +console.log("encodeURI(undefined):", encodeURI(undefined as unknown as string)); + +// the normal one-argument forms keep working +console.log("isNaN('x'):", isNaN("x" as unknown as number)); +console.log("isFinite(1):", isFinite(1)); +console.log("encodeURI('a b'):", encodeURI("a b")); +console.log("decodeURI('a%20b'):", decodeURI("a%20b")); +console.log("encodeURIComponent('a&b'):", encodeURIComponent("a&b")); +console.log("decodeURIComponent('a%26b'):", decodeURIComponent("a%26b"));