Adding #[derive(Eq)] to type_attribute conflicts with the generated code for Enumeration

[bliednov]

The following code stated in the documentation won't work, because enums already derive from Eq, so it produces an error.

# let mut config = prost_build::Config::new();
// Nothing around uses floats, so we can derive real `Eq` in addition to `PartialEq`.
config.type_attribute(".", "#[derive(Eq)]");

Here is an error:

   Compiling test-proto v0.1.0 (/Users/test/pro/co/test/tools/nexttest/test)
error[E0119]: conflicting implementations of trait `std::cmp::Eq` for type `test::Test`:
  --> /Users/test/pro/co/test/tools/nexttest/target/debug/build/test-proto-822a20cdddd51ab3/out/test.rs:37:41
   |
37 | #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)]
   |                                         ^^ conflicting implementation for `test::Test`
38 | #[repr(i32)]
39 | #[derive(Eq)]
   |          -- first implementation here

How to add #[derive(Eq)] only to all Messages?

[bliednov]

IMHO adding it to each message specifically is impractical because if you have even 20 messages, it's already a lot to do by hand, but what if one has 100 different messages? I think there should be a way to add type_attribute to all messages or to all enums.

[danburkert]

See also #371

[robo-corg]

Hitting this also. Ideally would be great of prost-build could just merge derives. Only workaround I could find is to manually list out every non-enum to add Eq and Hash which is pretty painful.

Having some visibility into what prost-build is doing via visitors (or just being able to add a pass that runs over all types) would be a really nice escape hatch for this kind of stuff.

[lilyball]

I just ran into this because Rust 1.63.0 now has a new Clippy lint for deriving PartialEq without Eq, which means all of my prost-generated structs are now throwing this warning.

Like @robo-corg said, being able to merge derives would help. This would presumably work not by parsing out the #[derive()] attributes but by having a separate method that takes an Iterator<Item=&str> or something, so it can just dedupe them.

Alternatively there could be some solution specific to Eq. Or, heck, prost could just derive Eq automatically for any struct that doesn't contain a float.

[LucioFranco]

Imho prost shouldn't be deriving Eq over encoded messages and esp protobuf which have a lot of backwards/forward compat featuers. C++ provides this https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.util.message_differencer which we could try to implement but requires features we are missing. We could add that derive iterator feature but in addition, there are many things that we can add to decorate the codegen and I am not really happy with the current approach. It doesn't feel sustainable to use type_attributes etc or keep adding methods like that. Generally, I would recommend to implement these sort of things on your end via fn/traits etc.

[robo-corg]

@LucioFranco That makes sense. The use case I was using it for was using protobufs as keys in a HashMap and a key value store which is probably definitely a bad idea. The code in question should probably be projecting these onto types managed in the crate or something like that, it is just difficult to do this ergonomically but probably worth doing for the reasons you state.

The right call here seems like allowing Eq to be added as a derive but not automatically adding it, if only to keep confusing errors from occurring. Interestingly it really seems like the argument here could be extended to all traits that are not Clone or Copy. Maybe some warnings or documentation explaining why deriving these traits are bad?

[upbqdn]

I solved this issue by calling message_attribute instead of type_attribute.