From c34bc4277f331bece8319bf807f8d97fde86276b Mon Sep 17 00:00:00 2001 From: ldm0 Date: Fri, 11 Sep 2026 21:40:48 +0800 Subject: [PATCH] refactor(webapi): share accessor preparation and validation --- moli-webapi-declare-derive/src/attrs.rs | 81 +++++---- moli-webapi-declare-derive/src/expand.rs | 214 +++++++++-------------- moli-webapi-declare/tests/receivers.rs | 2 + 3 files changed, 134 insertions(+), 163 deletions(-) diff --git a/moli-webapi-declare-derive/src/attrs.rs b/moli-webapi-declare-derive/src/attrs.rs index 1a34c4afa2..4985902927 100644 --- a/moli-webapi-declare-derive/src/attrs.rs +++ b/moli-webapi-declare-derive/src/attrs.rs @@ -738,33 +738,34 @@ pub(crate) fn parse_field_attrs_with_defaults( "field setter can only be specified for #[webapi(accessor_property)] or #[webapi(native_data_property)] fields", )); } - if matches!(parsed.kind, FieldKind::AccessorProperty) && parsed.callback.is_some() { - return Err(Error::new( - field.span(), - "`accessor_property` fields use #[webapi(getter = path)] and optional #[webapi(setter = path)] instead of callback", - )); - } - if matches!(parsed.kind, FieldKind::AccessorProperty) - && (parsed.length.is_some() || parsed.value.is_some() || parsed.init.is_some()) - { - return Err(Error::new( - field.span(), - "`accessor_property` fields cannot use length, value, or init attributes", - )); - } - if matches!(parsed.kind, FieldKind::NativeDataProperty) && parsed.callback.is_some() { - return Err(Error::new( - field.span(), - "`native_data_property` fields use #[webapi(getter = path)] and optional #[webapi(setter = path)] instead of callback", - )); - } - if matches!(parsed.kind, FieldKind::NativeDataProperty) - && (parsed.length.is_some() || parsed.value.is_some() || parsed.init.is_some()) - { - return Err(Error::new( - field.span(), - "`native_data_property` fields cannot use length, value, or init attributes", - )); + let accessor_kind = match parsed.kind { + FieldKind::AccessorProperty => Some("accessor_property"), + FieldKind::NativeDataProperty => Some("native_data_property"), + _ => None, + }; + if let Some(kind) = accessor_kind { + if parsed.readonly { + return Err(Error::new( + field.span(), + format!( + "`{kind}` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly" + ), + )); + } + if parsed.callback.is_some() { + return Err(Error::new( + field.span(), + format!( + "`{kind}` fields use #[webapi(getter = path)] and optional #[webapi(setter = path)] instead of callback" + ), + )); + } + if parsed.length.is_some() || parsed.value.is_some() || parsed.init.is_some() { + return Err(Error::new( + field.span(), + format!("`{kind}` fields cannot use length, value, or init attributes"), + )); + } } if matches!(parsed.kind, FieldKind::IntrinsicDataProperty(_)) && (parsed.function_name.is_some() @@ -992,6 +993,7 @@ mod tests { parse_field_attrs_with_defaults, parse_function_template_attrs, parse_object_attrs, }; use syn::Field; + use syn::parse::Parser; #[test] fn object_roles_are_exclusive() { @@ -1126,14 +1128,23 @@ mod tests { } #[test] - fn readonly_accessor_property_attribute_is_parsed() { - let field = syn::parse_quote! { - #[webapi(accessor_property, readonly, getter = sample_getter)] - value: () - }; - let attrs = parse_field_attrs(&field).expect("readonly accessor property should parse"); - assert!(matches!(attrs.kind, FieldKind::AccessorProperty)); - assert!(attrs.readonly); + fn readonly_accessor_properties_are_rejected() { + for kind in ["accessor_property", "native_data_property"] { + let field: Field = syn::Field::parse_named + .parse_str(&format!( + "#[webapi({kind}, readonly, getter = sample_getter)] value: ()" + )) + .expect("parse field"); + let error = parse_field_attrs(&field) + .err() + .expect("invalid writable attribute"); + assert_eq!( + error.to_string(), + format!( + "`{kind}` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly" + ) + ); + } } #[test] diff --git a/moli-webapi-declare-derive/src/expand.rs b/moli-webapi-declare-derive/src/expand.rs index 8e82b954c6..0f8e26222a 100644 --- a/moli-webapi-declare-derive/src/expand.rs +++ b/moli-webapi-declare-derive/src/expand.rs @@ -672,12 +672,6 @@ fn expand_function_template_accessor_property_field( attrs: &crate::attrs::FieldAttrs, template_name: &proc_macro2::TokenStream, ) -> Result { - if attrs.readonly { - return Err(Error::new( - field.span(), - "function-template `accessor_property` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly", - )); - } let key = webapi_field_key(field, attrs)?; let getter_class_name = expand_template_accessor_class_name( "get", @@ -691,35 +685,27 @@ fn expand_function_template_accessor_property_field( ); let name = key.display_name; let property_key = key.property_key; - let Some(getter) = attrs.getter.as_ref() else { + let (getter_member, setter) = expand_accessor_callbacks(attrs, |callback, is_setter, data| { + let class_name = if is_setter { + &setter_class_name + } else { + &getter_class_name + }; + expand_template_function_member( + callback, + i32::from(is_setter), + data, + template_name, + &name, + quote!(#class_name), + ) + }); + let Some(getter_member) = getter_member else { return Err(Error::new( field.span(), "`accessor_property` field requires #[webapi(getter = path)]", )); }; - let getter = expand_callback(getter, attrs, false); - let getter_member = expand_template_function_member( - &getter, - 0, - attrs.data.as_ref(), - template_name, - &name, - quote!(#getter_class_name), - ); - let setter = attrs.setter.as_ref().map(|setter| { - let setter_data = attrs.setter_data.as_ref().or(attrs.data.as_ref()); - let setter = expand_callback(setter, attrs, true); - let setter_member = expand_template_function_member( - &setter, - 1, - setter_data, - template_name, - &name, - quote!(#setter_class_name), - ); - quote!(::std::option::Option::Some(#setter_member)) - }); - let setter = setter.unwrap_or_else(|| quote!(::std::option::Option::None)); let attributes = property_attributes(attrs); let field_read = field .ident @@ -773,49 +759,17 @@ fn expand_function_template_native_data_property_field( field: &Field, attrs: &crate::attrs::FieldAttrs, ) -> Result { - if attrs.readonly { - return Err(Error::new( - field.span(), - "function-template `native_data_property` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly", - )); - } - if attrs.setter_data.is_some() { - return Err(Error::new( - field.span(), - "function-template `native_data_property` fields share one callback data value and cannot use #[webapi(setter_data = ...)]", - )); - } let key = webapi_field_key(field, attrs)?; let name = key.display_name; let property_key = key.property_key; - let Some(getter) = attrs.getter.as_ref() else { - return Err(Error::new( - field.span(), - "`native_data_property` field requires #[webapi(getter = path)]", - )); - }; - let setter = attrs.setter.as_ref().map(|setter| { - quote! { - .setter(#setter) - } - }); - let data = attrs.data.as_ref().map(|data| { - quote! { - .data((#data).into()) - } - }); - let attributes = property_attributes(attrs); + let configuration = expand_native_data_property_configuration(field, attrs)?; let field_read = field .ident .as_ref() .map(|ident| quote!(let _ = ::std::stringify!(#ident);)); Ok(quote! { #field_read - let __webapi_native_data_property_configuration = - ::moli_webapi_declare::v8::NativeDataPropertyConfiguration::new(#getter) - #setter - #data - .property_attribute(#attributes); + let __webapi_native_data_property_configuration = #configuration; prototype.set_native_data_property_with_configuration( #property_key, __webapi_native_data_property_configuration, @@ -1093,12 +1047,6 @@ fn expand_accessor_property_field( attrs: &crate::attrs::FieldAttrs, object: proc_macro2::TokenStream, ) -> Result { - if attrs.readonly { - return Err(Error::new( - field.span(), - "runtime-object `accessor_property` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly", - )); - } let key = webapi_field_key(field, attrs)?; let name = key.display_name; let property_key = key.property_key; @@ -1108,41 +1056,22 @@ fn expand_accessor_property_field( "`accessor_property` field cannot declare both #[webapi(getter = ...)] and #[webapi(getter_value = ...)]", )); } - let getter = if let Some(getter) = attrs.getter.as_ref() { - let getter = expand_callback(getter, attrs, false); - let getter = expand_function_builder(&getter, 0, attrs.data.as_ref(), &name); + let (getter, setter) = expand_accessor_callbacks(attrs, |callback, is_setter, data| { + let accessor_name = if is_setter { "setter" } else { "getter" }; + let function = expand_function_builder(callback, i32::from(is_setter), data, &name); quote! { - #getter.ok_or_else(|| { + #function.ok_or_else(|| { ::moli_webapi_declare::BindError::new( - ::std::format!("failed to build declared `{}` getter", #name) + ::std::format!("failed to build declared `{}` {}", #name, #accessor_name) ) })? } - } else if let Some(getter_value) = attrs.getter_value.as_ref() { - quote!(#getter_value) - } else { - return Err(Error::new( + }); + let getter = getter.or_else(|| attrs.getter_value.as_ref().map(|value| quote!(#value))) + .ok_or_else(|| Error::new( field.span(), "`accessor_property` field requires #[webapi(getter = path)] or #[webapi(getter_value = expr)]", - )); - }; - let setter = attrs.setter.as_ref().map(|setter| { - let setter_data = attrs.setter_data.as_ref().or(attrs.data.as_ref()); - let setter = expand_callback(setter, attrs, true); - expand_function_builder(&setter, 1, setter_data, &name) - }); - let setter = match setter { - Some(setter) => quote! { - ::std::option::Option::Some( - #setter.ok_or_else(|| { - ::moli_webapi_declare::BindError::new( - ::std::format!("failed to build declared `{}` setter", #name) - ) - })? - ) - }, - None => quote!(::std::option::Option::None), - }; + ))?; let attributes = property_attributes(attrs); let field_read = field .ident @@ -1168,43 +1097,17 @@ fn expand_native_data_property_field( field: &Field, attrs: &crate::attrs::FieldAttrs, ) -> Result { - if attrs.readonly { - return Err(Error::new( - field.span(), - "`native_data_property` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly", - )); - } let key = webapi_field_key(field, attrs)?; let name = key.display_name; let property_key = key.property_key; - let Some(getter) = attrs.getter.as_ref() else { - return Err(Error::new( - field.span(), - "`native_data_property` field requires #[webapi(getter = path)]", - )); - }; - let setter = attrs.setter.as_ref().map(|setter| { - quote! { - .setter(#setter) - } - }); - let data = attrs.data.as_ref().map(|data| { - quote! { - .data((#data).into()) - } - }); - let attributes = property_attributes(attrs); + let configuration = expand_native_data_property_configuration(field, attrs)?; let field_read = field .ident .as_ref() .map(|ident| quote!(let _ = &self.#ident;)); Ok(quote! { #field_read - let __webapi_native_data_property_configuration = - ::moli_webapi_declare::v8::NativeDataPropertyConfiguration::new(#getter) - #setter - #data - .property_attribute(#attributes); + let __webapi_native_data_property_configuration = #configuration; object .set_native_data_property_with_configuration( scope, @@ -1376,6 +1279,61 @@ fn expand_alias_field( }) } +fn expand_native_data_property_configuration( + field: &Field, + attrs: &FieldAttrs, +) -> Result { + let getter = attrs.getter.as_ref().ok_or_else(|| { + Error::new( + field.span(), + "`native_data_property` field requires #[webapi(getter = path)]", + ) + })?; + let setter = attrs.setter.as_ref().map(|setter| quote!(.setter(#setter))); + let data = attrs + .data + .as_ref() + .map(|data| quote!(.data((#data).into()))); + let attributes = property_attributes(attrs); + Ok(quote! { + ::moli_webapi_declare::v8::NativeDataPropertyConfiguration::new(#getter) + #setter + #data + .property_attribute(#attributes) + }) +} + +/// Resolve callback policies and setter-data fallback once. Each backend owns +/// function construction and its installation-time failure handling. +fn expand_accessor_callbacks( + attrs: &FieldAttrs, + mut build: impl FnMut( + &proc_macro2::TokenStream, + bool, + Option<&syn::Expr>, + ) -> proc_macro2::TokenStream, +) -> (Option, proc_macro2::TokenStream) { + let mut expand = |callback: &Option, is_setter| { + callback.as_ref().map(|callback| { + let data = if is_setter { + attrs.setter_data.as_ref().or(attrs.data.as_ref()) + } else { + attrs.data.as_ref() + }; + build( + &expand_callback(callback, attrs, is_setter), + is_setter, + data, + ) + }) + }; + let getter = expand(&attrs.getter, false); + let setter = expand(&attrs.setter, true) + .map(|setter| quote!(::std::option::Option::Some(#setter))) + .unwrap_or_else(|| quote!(::std::option::Option::None)); + (getter, setter) +} + /// Generate a native callback adapter, without changing callback data or /// introducing a JavaScript wrapper. Receiver checks precede argument conversion. fn expand_callback( @@ -2108,7 +2066,7 @@ mod tests { }; assert_eq!( error.to_string(), - "function-template `accessor_property` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly" + "`accessor_property` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly" ); } @@ -2127,7 +2085,7 @@ mod tests { }; assert_eq!( error.to_string(), - "runtime-object `accessor_property` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly" + "`accessor_property` fields have no writable attribute; omit #[webapi(setter)] instead of using readonly" ); } diff --git a/moli-webapi-declare/tests/receivers.rs b/moli-webapi-declare/tests/receivers.rs index 09b0b0fab7..263acf5062 100644 --- a/moli-webapi-declare/tests/receivers.rs +++ b/moli-webapi-declare/tests/receivers.rs @@ -272,6 +272,8 @@ fn object_and_template_declarations_use_the_same_receiver_policy() { assert(throwsTypeError(() => surface.method.call({})), 'invalid receiver'); const value = Object.getOwnPropertyDescriptor(surface, 'value'); assert(value.get.call(object) === 9, 'getter data'); + assert(value.set.call(object, 'ok') === 9, 'setter falls back to getter data'); + assert(value.get.length === 0 && value.set.length === 1, 'accessor lengths'); assert(throwsTypeError(() => value.set.call({})), 'invalid setter receiver'); const promise = Object.getOwnPropertyDescriptor(surface, 'ready').get.call({}); assert(promise instanceof Promise, 'invalid Promise getter receiver');