fix(webidl): check required arity before argument conversion

moli-webidl-derive/src/expand.rs

@@ -22,6 +22,31 @@ pub(crate) fn expand_webidl_args(input: DeriveInput) -> Result<proc_macro2::Toke
         impl_parts_for_scope(&generics, attrs.scope_lifetime.as_ref(), struct_name.span())?;
     let fields = named_fields(&input.data)?;
 
+    // Arity is checked before any conversion, including custom parsers. A
+    // missing later argument must not invoke an earlier argument's toString.
+    let required_checks = fields
+        .iter()
+        .enumerate()
+        .map(|(index, field)| {
+            let attrs = parse_field_attrs(field)?;
+            if !attrs.required {
+                return Ok(quote! {});
+            }
+            let ident = field_ident(field)?;
+            let index = attrs.index.unwrap_or(index) as i32;
+            let message = attrs
+                .missing_message
+                .clone()
+                .unwrap_or_else(|| default_required_arg_message(&prefix, &ident, &attrs));
+            Ok(quote! {
+                if args.length() <= #index {
+                    return ::std::result::Result::Err(
+                        ::moli_webidl::WebIdlError::custom_message(#message),
+                    );
+                }
+            })
+        })
+        .collect::<Result<Vec<_>, Error>>()?;
     let bindings = fields
         .iter()
         .enumerate()
@@ -38,6 +63,7 @@ pub(crate) fn expand_webidl_args(input: DeriveInput) -> Result<proc_macro2::Toke
                 scope: &mut v8::PinScope<#scope_lifetime, '_>,
                 args: &v8::FunctionCallbackArguments<#scope_lifetime>,
             ) -> ::std::result::Result<Self, ::moli_webidl::WebIdlError> {
+                #(#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,

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<String>,
+    #[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<String>,
+}
+
+fn custom_string<'s>(
+    scope: &mut v8::PinScope<'s, '_>,
+    args: &v8::FunctionCallbackArguments<'s>,
+    index: i32,
+) -> Result<String, WebIdlError> {
+    moli_webidl::argument::<DomString>(
+        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::<Arguments>(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()
+    );
+}