Merge pull request #1250 from zeenix/dict-as-prop

✨ zv: *Value derive now supports optional fields in dict structs
dbus2 · Feb 6, 2025 · 1757689 · 1757689
2 parents d60b895 + e470733
commit 1757689
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 21 deletions.
diff --git a/zvariant_derive/src/value.rs b/zvariant_derive/src/value.rs
@@ -4,6 +4,7 @@ use syn::{
     spanned::Spanned, Attribute, Data, DataEnum, DeriveInput, Error, Fields, Generics, Ident,
     Lifetime, LifetimeParam,
 };
+use zvariant_utils::macros;
 
 use crate::utils::*;
 
@@ -123,32 +124,70 @@ fn impl_struct(
                 .map(|field| field.ident.to_token_stream())
                 .collect();
             let (from_value_impl, into_value_impl) = match signature {
-                Some(signature) if signature == "a{sv}" => (
+                Some(signature) if signature == "a{sv}" => {
                     // User wants the type to be encoded as a dict.
                     // FIXME: Not the most efficient implementation.
-                    quote! {
-                        let mut fields = <::std::collections::HashMap::<::std::string::String, #zv::Value>>::try_from(value)?;
+                    let (fields_init, entries_init): (TokenStream, TokenStream) = fields
+                        .iter()
+                        .map(|field| {
+                            let field_name = field.ident.to_token_stream();
+                            let convert = if macros::ty_is_option(&field.ty) {
+                                quote! {
+                                    .map(#zv::Value::downcast)
+                                    .transpose()?
+                                }
+                            } else {
+                                quote! {
+                                    .ok_or_else(|| #zv::Error::IncorrectType)?
+                                    .downcast()?
+                                }
+                            };
 
-                        ::std::result::Result::Ok(Self {
-                            #(
-                                #field_names:
-                                    fields
-                                        .remove(stringify!(#field_names))
-                                        .ok_or_else(|| #zv::Error::IncorrectType)?
-                                        .downcast()?
-                            ),*
+                            let fields_init = quote! {
+                                #field_name: fields
+                                    .remove(stringify!(#field_name))
+                                    #convert,
+                            };
+                            let entries_init = if macros::ty_is_option(&field.ty) {
+                                quote! {
+                                    if let Some(v) = s.#field_name {
+                                        fields.insert(
+                                            stringify!(#field_name),
+                                            #zv::Value::from(v),
+                                        );
+                                    }
+                                }
+                            } else {
+                                quote! {
+                                    fields.insert(
+                                        stringify!(#field_name),
+                                        #zv::Value::from(s.#field_name),
+                                    );
+                                }
+                            };
+
+                            (fields_init, entries_init)
                         })
-                    },
-                    quote! {
-                        let mut fields = ::std::collections::HashMap::new();
-                        #(
-                            fields.insert(stringify!(#field_names), #zv::Value::from(s.#field_names));
-                        )*
+                        .unzip();
 
-                        <#value_type>::#into_value_method(#zv::Value::from(fields))
-                            #into_value_error_transform
-                    },
-                ),
+                    (
+                        quote! {
+                            let mut fields = <::std::collections::HashMap::<
+                                ::std::string::String,
+                                #zv::Value,
+                            >>::try_from(value)?;
+
+                            ::std::result::Result::Ok(Self { #fields_init })
+                        },
+                        quote! {
+                            let mut fields = ::std::collections::HashMap::new();
+                            #entries_init
+
+                            <#value_type>::#into_value_method(#zv::Value::from(fields))
+                                #into_value_error_transform
+                        },
+                    )
+                }
                 Some(_) | None => (
                     quote! {
                         let mut fields = #zv::Structure::try_from(value)?.into_fields();

diff --git a/zvariant_derive/tests/tests.rs b/zvariant_derive/tests/tests.rs
@@ -82,3 +82,20 @@ fn derive_dict() {
 
     assert_eq!(Test::SIGNATURE, "a{sv}")
 }
+
+#[test]
+#[ignore]
+fn issues_311() {
+    // Issue 311: Value macro not able to handle Option in Dict.
+    //
+    // org.freedesktop.ModemManager1.Modem.Signal props are a dict with optional values depending on
+    // the property you read.
+    #[derive(Debug, Type, DeserializeDict, OwnedValue, Value)]
+    #[zbus(signature = "dict")]
+    pub struct SignalInfo {
+        pub rssi: Option<i32>,
+        pub ecio: Option<i32>,
+        pub io: Option<i32>,
+        pub sinr: Option<i32>,
+    }
+}