Merge pull request #841 from zeenix/interface-proxy-fixes

zeenix · web-flow · commit eca4cf649578 · 2024-06-13T18:11:23.000+02:00
A few fixes &amp; enhancements to proxy generation of interface macro
diff --git a/zbus/src/fdo.rs b/zbus/src/fdo.rs
@@ -827,7 +827,7 @@ assert_impl_all!(DBusProxy<'_>: Send, Sync, Unpin);
 #[zbus(prefix = "org.freedesktop.DBus.Error", impl_display = true)]
 #[allow(clippy::upper_case_acronyms)]
 pub enum Error {
-    /// Unknown or fall-through ZBus error.
+    /// Unknown or fall-through zbus error.
     #[zbus(error)]
     ZBus(zbus::Error),
 
diff --git a/zbus_macros/src/iface.rs b/zbus_macros/src/iface.rs
@@ -169,6 +169,8 @@ struct MethodInfo {
     output: ReturnType,
     /// The cfg attributes of the method.
     cfg_attrs: Vec<Attribute>,
+    /// The doc attributes of the method.
+    doc_attrs: Vec<Attribute>,
 }
 
 impl MethodInfo {
@@ -177,6 +179,7 @@ impl MethodInfo {
         method: &ImplItemFn,
         attrs: &MethodAttrs,
         cfg_attrs: &[&Attribute],
+        doc_attrs: &[&Attribute],
     ) -> syn::Result<MethodInfo> {
         let is_async = method.sig.asyncness.is_some();
         let Signature {
@@ -319,6 +322,7 @@ impl MethodInfo {
             proxy_attrs,
             output: output.clone(),
             cfg_attrs: cfg_attrs.iter().cloned().cloned().collect(),
+            doc_attrs: doc_attrs.iter().cloned().cloned().collect(),
         })
     }
 }
@@ -354,12 +358,7 @@ pub fn expand<T: AttrParse + Into<ImplAttrs>, M: AttrParse + Into<MethodAttrs>>(
 
     let (iface_name, with_spawn, mut proxy) = {
         let (name, interface, spawn, proxy) = match T::parse_nested_metas(args)?.into() {
-            ImplAttrs::New(new) => (
-                new.name,
-                new.interface,
-                new.spawn,
-                new.proxy.map(|p| Proxy::new(ty, p)),
-            ),
+            ImplAttrs::New(new) => (new.name, new.interface, new.spawn, new.proxy),
             // New proxy attributes are not supported for old `dbus_interface`.
             ImplAttrs::Old(old) => (old.name, old.interface, old.spawn, None),
         };
@@ -373,6 +372,7 @@ pub fn expand<T: AttrParse + Into<ImplAttrs>, M: AttrParse + Into<MethodAttrs>>(
                     "`name` and `interface` attributes should not be specified at the same time",
                 )),
             };
+        let proxy = proxy.map(|p| Proxy::new(ty, &name, p, &zbus));
 
         (name, !spawn.unwrap_or(false), proxy)
     };
@@ -424,8 +424,13 @@ pub fn expand<T: AttrParse + Into<ImplAttrs>, M: AttrParse + Into<MethodAttrs>>(
             .iter()
             .filter(|a| a.path().is_ident("cfg"))
             .collect();
+        let doc_attrs: Vec<_> = method
+            .attrs
+            .iter()
+            .filter(|a| a.path().is_ident("doc"))
+            .collect();
 
-        let method_info = MethodInfo::new(&zbus, method, &attrs, &cfg_attrs)?;
+        let method_info = MethodInfo::new(&zbus, method, &attrs, &cfg_attrs, &doc_attrs)?;
         let attr_property = match attrs {
             MethodAttrs::Old(o) => o.property.map(|op| PropertyAttributes {
                 emits_changed_signal: op.emits_changed_signal,
@@ -773,7 +778,7 @@ pub fn expand<T: AttrParse + Into<ImplAttrs>, M: AttrParse + Into<MethodAttrs>>(
         }
 
         if let Some(proxy) = &mut proxy {
-            proxy.add_method(info, &properties, &zbus);
+            proxy.add_method(info, &properties);
         }
     }
 
@@ -794,7 +799,7 @@ pub fn expand<T: AttrParse + Into<ImplAttrs>, M: AttrParse + Into<MethodAttrs>>(
         }
     };
 
-    let proxy = proxy.map(|proxy| proxy.gen(&iface_name, &zbus));
+    let proxy = proxy.map(|proxy| proxy.gen());
 
     Ok(quote! {
         #input
@@ -1115,7 +1120,7 @@ fn introspect_output_arg(
     )
 }
 
-fn get_result_type(p: &TypePath) -> syn::Result<&Type> {
+fn get_result_inner_type(p: &TypePath) -> syn::Result<&Type> {
     if let PathArguments::AngleBracketed(AngleBracketedGenericArguments { args, .. }) = &p
         .path
         .segments
@@ -1151,7 +1156,7 @@ fn introspect_add_output_args(
                 .ident
                 == "Result";
             if is_result_output {
-                ty = get_result_type(p)?;
+                ty = get_result_inner_type(p)?;
             }
         }
 
@@ -1187,7 +1192,7 @@ fn get_return_type(output: &ReturnType) -> syn::Result<&Type> {
                 .ident
                 == "Result";
             if is_result_output {
-                return get_result_type(p);
+                return get_result_inner_type(p);
             }
         }
 
@@ -1307,6 +1312,10 @@ impl Parse for ImplItemSignal {
 struct Proxy {
     // The type name
     ty: Ident,
+    // The interface name
+    iface_name: String,
+    // The zbus crate
+    zbus: TokenStream,
 
     // Input
     attrs: ProxyAttributes,
@@ -1316,34 +1325,25 @@ struct Proxy {
 }
 
 impl Proxy {
-    fn new(ty: &Ident, attrs: ProxyAttributes) -> Self {
+    fn new(ty: &Ident, iface_name: &str, attrs: ProxyAttributes, zbus: &TokenStream) -> Self {
         Self {
+            iface_name: iface_name.to_string(),
             ty: ty.clone(),
+            zbus: zbus.clone(),
             attrs,
             methods: quote!(),
         }
     }
 
-    fn add_method(
-        &mut self,
-        method_info: MethodInfo,
-        properties: &BTreeMap<String, Property<'_>>,
-        zbus: &TokenStream,
-    ) {
+    fn add_method(&mut self, method_info: MethodInfo, properties: &BTreeMap<String, Property<'_>>) {
         let inputs: Punctuated<PatType, Comma> = method_info
             .typed_inputs
             .iter()
-            .enumerate()
-            .filter(|(idx, input)| {
-                if method_info.method_type == MethodType::Signal {
-                    // Skip the `SignalContext` argument.
-                    return *idx != 0;
-                }
-
+            .filter(|input| {
                 let a = ArgAttributes::parse(&input.attrs).unwrap();
                 !a.object_server && !a.connection && !a.header && !a.signal_context
             })
-            .map(|(_, p)| p.clone())
+            .cloned()
             .collect();
         let ret = get_return_type(&method_info.output)
             .map(|r| quote!(#r))
@@ -1386,14 +1386,17 @@ impl Proxy {
             }
         }
         let cfg_attrs = method_info.cfg_attrs;
+        let zbus = &self.zbus;
+        let doc_attrs = method_info.doc_attrs;
         self.methods.extend(quote! {
             #(#cfg_attrs)*
+            #(#doc_attrs)*
             #[zbus(#proxy_method_attrs)]
             fn #ident(&self, #inputs) -> #zbus::Result<#ret>;
         });
     }
 
-    fn gen(&self, iface_name: &str, zbus: &TokenStream) -> TokenStream {
+    fn gen(&self) -> TokenStream {
         let attrs = &self.attrs;
         let (
             assume_defaults,
@@ -1432,7 +1435,11 @@ impl Proxy {
             &self.ty,
             &self.methods,
         );
+        let iface_name = &self.iface_name;
+        let zbus = &self.zbus;
+        let proxy_doc = format!("Proxy for the `{iface_name}` interface.");
         quote! {
+            #[doc = #proxy_doc]
             #[#zbus::proxy(
                 name = #iface_name,
                 #assume_defaults
diff --git a/zvariant/src/lib.rs b/zvariant/src/lib.rs
@@ -820,13 +820,13 @@ mod tests {
         // Signature: "a(yu(xbxas)s)");
         let ar = vec![(
             // top-most simple fields
-            u8::max_value(),
-            u32::max_value(),
+            u8::MAX,
+            u32::MAX,
             (
                 // 2nd level simple fields
-                i64::max_value(),
+                i64::MAX,
                 true,
-                i64::max_value(),
+                i64::MAX,
                 // 2nd level array field
                 &["Hello", "World"][..],
             ),
@@ -841,12 +841,12 @@ mod tests {
             encoded.deserialize().unwrap().0;
         assert_eq!(decoded.len(), 1);
         let r = &decoded[0];
-        assert_eq!(r.0, u8::max_value());
-        assert_eq!(r.1, u32::max_value());
+        assert_eq!(r.0, u8::MAX);
+        assert_eq!(r.1, u32::MAX);
         let inner_r = &r.2;
-        assert_eq!(inner_r.0, i64::max_value());
+        assert_eq!(inner_r.0, i64::MAX);
         assert!(inner_r.1);
-        assert_eq!(inner_r.2, i64::max_value());
+        assert_eq!(inner_r.2, i64::MAX);
         let as_ = &inner_r.3;
         assert_eq!(as_.len(), 2);
         assert_eq!(as_[0], "Hello");
@@ -863,12 +863,12 @@ mod tests {
                 gv_encoded.deserialize().unwrap().0;
             assert_eq!(decoded.len(), 1);
             let r = &decoded[0];
-            assert_eq!(r.0, u8::max_value());
-            assert_eq!(r.1, u32::max_value());
+            assert_eq!(r.0, u8::MAX);
+            assert_eq!(r.1, u32::MAX);
             let inner_r = &r.2;
-            assert_eq!(inner_r.0, i64::max_value());
+            assert_eq!(inner_r.0, i64::MAX);
             assert!(inner_r.1);
-            assert_eq!(inner_r.2, i64::max_value());
+            assert_eq!(inner_r.2, i64::MAX);
             let as_ = &inner_r.3;
             assert_eq!(as_.len(), 2);
             assert_eq!(as_[0], "Hello");
@@ -883,8 +883,8 @@ mod tests {
             assert_eq!(variant.n_children(), 1);
             let r: (u8, u32, (i64, bool, i64, Vec<String>), String) =
                 variant.child_value(0).get().unwrap();
-            assert_eq!(r.0, u8::max_value());
-            assert_eq!(r.1, u32::max_value());
+            assert_eq!(r.0, u8::MAX);
+            assert_eq!(r.1, u32::MAX);
         }
         let ctxt = Context::new_dbus(LE, 0);
 
@@ -900,13 +900,13 @@ mod tests {
             let r = &array[0];
             if let Value::Structure(r) = r {
                 let fields = r.fields();
-                assert_eq!(fields[0], Value::U8(u8::max_value()));
-                assert_eq!(fields[1], Value::U32(u32::max_value()));
+                assert_eq!(fields[0], Value::U8(u8::MAX));
+                assert_eq!(fields[1], Value::U32(u32::MAX));
                 if let Value::Structure(r) = &fields[2] {
                     let fields = r.fields();
-                    assert_eq!(fields[0], Value::I64(i64::max_value()));
+                    assert_eq!(fields[0], Value::I64(i64::MAX));
                     assert_eq!(fields[1], Value::Bool(true));
-                    assert_eq!(fields[2], Value::I64(i64::max_value()));
+                    assert_eq!(fields[2], Value::I64(i64::MAX));
                     if let Value::Array(as_) = &fields[3] {
                         assert_eq!(as_.len(), 2);
                         assert_eq!(as_[0], Value::new("Hello"));
@@ -940,13 +940,13 @@ mod tests {
                 let r = &array.get(0).unwrap().unwrap();
                 if let Value::Structure(r) = r {
                     let fields = r.fields();
-                    assert_eq!(fields[0], Value::U8(u8::max_value()));
-                    assert_eq!(fields[1], Value::U32(u32::max_value()));
+                    assert_eq!(fields[0], Value::U8(u8::MAX));
+                    assert_eq!(fields[1], Value::U32(u32::MAX));
                     if let Value::Structure(r) = &fields[2] {
                         let fields = r.fields();
-                        assert_eq!(fields[0], Value::I64(i64::max_value()));
+                        assert_eq!(fields[0], Value::I64(i64::MAX));
                         assert_eq!(fields[1], Value::Bool(true));
-                        assert_eq!(fields[2], Value::I64(i64::max_value()));
+                        assert_eq!(fields[2], Value::I64(i64::MAX));
                         if let Value::Array(as_) = &fields[3] {
                             assert_eq!(as_.len(), 2);
                             assert_eq!(as_.get(0).unwrap(), Some("Hello"));
@@ -972,8 +972,8 @@ mod tests {
             let child: Variant = variant.child_value(0);
             let r: (u8, u32, (i64, bool, i64, Vec<String>), String) =
                 child.child_value(0).get().unwrap();
-            assert_eq!(r.0, u8::max_value());
-            assert_eq!(r.1, u32::max_value());
+            assert_eq!(r.0, u8::MAX);
+            assert_eq!(r.1, u32::MAX);
 
             let mut rng = thread_rng();
             // Let's test GVariant ser/de of a 254 byte array with variable-width elements as to
@@ -1165,9 +1165,9 @@ mod tests {
         let expect_iter = expect.iter().map(|(k, v)| (k, v)).collect::<Vec<_>>();
         let actual = dict.iter().collect::<Vec<_>>();
         assert_eq!(actual, expect_iter);
-        let actual = (&dict).iter().collect::<Vec<_>>();
+        let actual = dict.iter().collect::<Vec<_>>();
         assert_eq!(actual, expect_iter);
-        let actual = (&mut dict).iter().collect::<Vec<_>>();
+        let actual = dict.iter().collect::<Vec<_>>();
         assert_eq!(actual, expect_iter);
         for (_, v) in dict.iter_mut() {
             if let Value::Str(vv) = v {
diff --git a/zvariant/src/ser.rs b/zvariant/src/ser.rs
@@ -29,7 +29,7 @@ impl Write for NullWriteSeek {
 
 impl Seek for NullWriteSeek {
     fn seek(&mut self, _pos: std::io::SeekFrom) -> std::io::Result<u64> {
-        Ok(std::u64::MAX) // should never read the return value!
+        Ok(u64::MAX) // should never read the return value!
     }
 }
 
diff --git a/zvariant/src/utils.rs b/zvariant/src/utils.rs
@@ -58,7 +58,7 @@ pub(crate) fn padding_for_n_bytes(value: usize, align: usize) -> usize {
 
 pub(crate) fn usize_to_u32(value: usize) -> u32 {
     assert!(
-        value <= (std::u32::MAX as usize),
+        value <= (u32::MAX as usize),
         "{} too large for `u32`",
         value,
     );
@@ -67,21 +67,13 @@ pub(crate) fn usize_to_u32(value: usize) -> u32 {
 }
 
 pub(crate) fn usize_to_u8(value: usize) -> u8 {
-    assert!(
-        value <= (std::u8::MAX as usize),
-        "{} too large for `u8`",
-        value,
-    );
+    assert!(value <= (u8::MAX as usize), "{} too large for `u8`", value,);
 
     value as u8
 }
 
 pub(crate) fn f64_to_f32(value: f64) -> f32 {
-    assert!(
-        value <= (std::f32::MAX as f64),
-        "{} too large for `f32`",
-        value,
-    );
+    assert!(value <= (f32::MAX as f64), "{} too large for `f32`", value,);
 
     value as f32
 }

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ impl Write for NullWriteSeek {`
`29`	`29`
`30`	`30`	`impl Seek for NullWriteSeek {`
`31`	`31`	`fn seek(&mut self, _pos: std::io::SeekFrom) -> std::io::Result<u64> {`
`32`		`- Ok(std::u64::MAX) // should never read the return value!`
	`32`	`+ Ok(u64::MAX) // should never read the return value!`
`33`	`33`	`}`
`34`	`34`	`}`
`35`	`35`