Name collisions in generated Proxy code when propery getter/setter exists as a method #241

Open zeenix opened 2 years ago

In GitLab by @hansihe on Dec 24, 2021, 19:44

There are several different variants of this, all of which I found in interfaces by NetworkManager.

Case 1: Method collision with property

See the org.freedesktop.NetworkManager interface for an example.

This interface has both a signal and a method named State:

    /// state method
    fn state(&self) -> zbus::Result<u32>;

    /// State property
    #[dbus_proxy(property)]
    fn state(&self) -> zbus::Result<u32>;

This will generate the following error when compiled:

error[E0201]: duplicate definitions with name `state`:
  --> service/src/dbus/network_manager/gen/org_freedesktop_NetworkManager.rs:15:1
   |
15 | #[dbus_proxy(interface = "org.freedesktop.NetworkManager")]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   | |
   | previous definition of `state` here
   | duplicate definition
   |
   = note: this error originates in the attribute macro `dbus_proxy` (in Nightly builds, run with -Z macro-backtrace for more info)

Case 2: Method/property/signal collision with [signal]_changed function

See the org.freedesktop.NetworkManager interface for an example.

The code generation will generate a function named receive_state_changed for both of these:

    /// StateChanged signal
    #[dbus_proxy(signal)]
    fn state_changed(&self, state: u32) -> zbus::Result<()>;

    /// State property
    #[dbus_proxy(property)]
    fn state(&self) -> zbus::Result<u32>;

This will generate the following error when compiled:

error[E0201]: duplicate definitions with name `receive_state_changed`:
  --> service/src/dbus/network_manager/gen/org_freedesktop_NetworkManager.rs:15:1
   |
15 | #[dbus_proxy(interface = "org.freedesktop.NetworkManager")]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   | |
   | previous definition of `receive_state_changed` here
   | duplicate definition
   |
   = note: this error originates in the attribute macro `dbus_proxy` (in Nightly builds, run with -Z macro-backtrace for more info)

Thoughts

I imagine almost all of these can be solved by adding an argument to the dbus_proxy attribute like #[dbus_proxy(property, rename = "state_property"].

While the XML code generator would still fail, the code would be fixable very easily.

As is today, unless I have missed something, cases like these are unrepresentable with zbus_macros.

Thanks for reporting this.

I imagine almost all of these can be solved by adding an argument to the dbus_proxy attribute like #[dbus_proxy(property, rename = "state_property"]

We already do have name attribute for that actually, does that not work for properties?

While the XML code generator would still fail, the code would be fixable very easily.

Yeah but we could also make xmlgen smarter. :)

I imagine almost all of these can be solved by adding an argument to the dbus_proxy attribute like #[dbus_proxy(property, rename = "state_property"]

We already do have name attribute for that actually, does that not work for properties?

Note that it's called name and not rename because you're providing the name of the property/method on D-Bus, which is in PascalCase.

In GitLab by @hansihe on Dec 25, 2021, 14:37

Ah! I must have missed that attribute somehow. Thanks for pointing me in the right direction!

I guess this is now mainly an issue for making xmlgen smarter :)

I'll update the issue text in a bit.

Ah! I must have missed that attribute somehow. Thanks for pointing me in the right direction!

Np. :)

I guess this is now mainly an issue for making xmlgen smarter

Yeah, it's not super smart at the moment, unfortunately.

I'll update the issue text in a bit.

No hurry. Happy holidays. :)

dbus2 / zbus-old