diff --git a/moli-webidl-derive/src/expand.rs b/moli-webidl-derive/src/expand.rs index 57161e8f0e..83201281c5 100644 --- a/moli-webidl-derive/src/expand.rs +++ b/moli-webidl-derive/src/expand.rs @@ -22,6 +22,31 @@ pub(crate) fn expand_webidl_args(input: DeriveInput) -> Result, Error>>()?; let bindings = fields .iter() .enumerate() @@ -38,6 +63,7 @@ pub(crate) fn expand_webidl_args(input: DeriveInput) -> Result, args: &v8::FunctionCallbackArguments<#scope_lifetime>, ) -> ::std::result::Result { + #(#required_checks)* #(#bindings)* ::std::result::Result::Ok(Self { #(#idents),* @@ -299,8 +325,8 @@ fn expand_args_field( }); } if let Some(with) = attrs.with.as_ref() { - // Custom parsers own the entire field binding. They receive the raw V8 - // argument list and resolved index so complex APIs can preserve + // Custom parsers own conversion after the required-arity preflight. + // They receive the raw V8 argument list and resolved index so APIs can preserve // browser-specific ordering or validation without fighting generated // scalar conversion. let arg_index = attrs.index.unwrap_or(index) as i32; @@ -316,13 +342,6 @@ fn expand_args_field( let unwrap_value = converter.unwrap_value(quote!(value)); let options = converter.options_expr(&attrs)?; let context = quote!(::moli_webidl::Context::argument(#prefix, #ordinal)); - let missing_error = if let Some(message) = attrs.missing_message.as_ref() { - quote!(::moli_webidl::WebIdlError::custom_message(#message)) - } else { - let message = default_required_arg_message(prefix, &ident, &attrs); - quote!(::moli_webidl::WebIdlError::custom_message(#message)) - }; - if attrs.nullable && !is_option_type(&field.ty) { return Err(Error::new( field.span(), @@ -337,9 +356,6 @@ fn expand_args_field( if attrs.required { quote! { let #ident = { - if args.length() <= #arg_index { - return ::std::result::Result::Err(#missing_error); - } let raw = args.get(#arg_index); if raw.is_null() || raw.is_undefined() { ::std::option::Option::None @@ -394,9 +410,6 @@ fn expand_args_field( } else if attrs.required { quote! { let #ident = { - if args.length() <= #arg_index { - return ::std::result::Result::Err(#missing_error); - } let value = ::moli_webidl::argument_with_options::<#converter_ty>( scope, args, diff --git a/moli-webidl/tests/arguments.rs b/moli-webidl/tests/arguments.rs new file mode 100644 index 0000000000..7f3ca2e61a --- /dev/null +++ b/moli-webidl/tests/arguments.rs @@ -0,0 +1,108 @@ +use moli_webidl::{Context, DomString, WebIdlArgs, WebIdlError}; + +#[derive(WebIdlArgs)] +#[webidl(prefix = "Test.arguments")] +struct Arguments { + #[webidl(required)] + first: String, + #[webidl(required)] + second: String, + #[webidl(required, nullable)] + nullable: Option, + #[webidl(default = "default")] + optional: String, + #[webidl(required, index = 5, with = custom_string, missing_message = "sixth argument missing")] + custom: String, + #[webidl(variadic, index = 6)] + rest: Vec, +} + +fn custom_string<'s>( + scope: &mut v8::PinScope<'s, '_>, + args: &v8::FunctionCallbackArguments<'s>, + index: i32, +) -> Result { + moli_webidl::argument::( + scope, + args, + index, + Context::argument("Test.arguments", index as usize + 1), + ) + .map(|value| value.0) +} + +fn callback<'s>( + scope: &mut v8::PinScope<'s, '_>, + args: v8::FunctionCallbackArguments<'s>, + mut rv: v8::ReturnValue<'s>, +) { + let Some(parsed) = moli_webidl::parse_args::(scope, &args) else { + return; + }; + let result = format!( + "{}|{}|{:?}|{}|{}|{:?}", + parsed.first, parsed.second, parsed.nullable, parsed.optional, parsed.custom, parsed.rest + ); + rv.set(v8::String::new(scope, &result).unwrap().into()); +} + +#[test] +fn required_arity_precedes_scalar_nullable_and_custom_argument_conversion() { + moli_v8_test_util::ensure_v8(); + let mut isolate = v8::Isolate::new(v8::CreateParams::default()); + let scope = std::pin::pin!(v8::HandleScope::new(&mut isolate)); + let scope = &mut scope.init(); + let callee_context = v8::Context::new(scope, Default::default()); + let (function, type_error) = { + let scope = &mut v8::ContextScope::new(scope, callee_context); + let function = v8::Function::new(scope, callback).unwrap(); + let key = v8::String::new(scope, "TypeError").unwrap(); + let type_error = callee_context.global(scope).get(scope, key.into()).unwrap(); + (function, type_error) + }; + let caller_context = v8::Context::new(scope, Default::default()); + let scope = &mut v8::ContextScope::new(scope, caller_context); + let global = caller_context.global(scope); + for (name, value) in [("parse", function.into()), ("CalleeTypeError", type_error)] { + let key = v8::String::new(scope, name).unwrap(); + global.set(scope, key.into(), value).unwrap(); + } + let source = v8::String::new( + scope, + r#" + (() => { + const assert = (ok, message) => { if (!ok) throw new Error(message); }; + let conversions = 0; + const sentinel = {}; + const poison = { toString() { conversions++; throw sentinel; } }; + for (let length = 0; length < 6; length++) { + let error; + try { parse(...Array(length).fill(poison)); } catch (caught) { error = caught; } + assert(error instanceof CalleeTypeError, 'missing argument: ' + length); + assert(!(error instanceof TypeError), 'callee error realm: ' + length); + if (length >= 3) assert(error.message === 'sixth argument missing', 'custom message'); + } + assert(conversions === 0, 'arity must precede all conversions'); + let error; + try { parse(poison, 'two', null, undefined, 'ignored', 'five'); } catch (caught) { error = caught; } + assert(error === sentinel && conversions === 1, 'preserve original conversion exception'); + const order = []; + const value = text => ({ toString() { order.push(text); return text; } }); + const result = parse(value('one'), value('two'), undefined, undefined, 'ignored', value('five'), value('six')); + assert(order.join() === 'one,two,five,six', 'successful conversion order: ' + order); + assert(result === 'one|two|None|default|five|["six"]', 'nullable/default/custom/rest: ' + result); + assert(parse('one', 'two', null, null, 'ignored', undefined) === 'one|two|None|null|undefined|[]', + 'present undefined custom argument must be converted'); + return true; + })() + "#, + ) + .unwrap(); + let script = v8::Script::compile(scope, source, None).unwrap(); + assert!( + script + .run(scope) + .expect("argument conversion checks") + .is_true() + ); +}