clippy::derive_partial_eq_without_eq violated on nightly

[nstinus]

Hi,

Given a simple message, prost violates derive_partial_eq_without_eq on nightly as shown in https://github.com/nstinus/prost-nightly.

Output is:

  |
1 | #[derive(Clone, PartialEq, ::prost::Message)]
  |                 ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
  |
  = note: `-D clippy::derive-partial-eq-without-eq` implied by `-D clippy::all`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq

error: could not compile `prost-nightly` due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `prost-nightly` due to previous error

[LucioFranco]

Since this is just a clippy lint and not an error from rustc and in addition, to support this working will require additional logic to prost-build. I don't think its worth adding this support now and the correct solution will be to ignore the warning.

[nstinus]

In my case, since clippy::all is routinely enabled by default, that means disabling the lint wherever this crate is used. Not the end of the world, but it would be much better if this lib could avoid triggering it altogether. I don't feel like it is particularly exotic. Now, it may be difficult to fix but that is a different problem.

Can you confirm that your recommendation is for client code to do something like that?

#[allow(clippy::derive_partial_eq_without_eq)]

pub mod pb {
    tonic::include_proto!("dummy");
}

[LucioFranco]

I think for things like codegen is quite normal to add allow's. Tonic does this for some built in rust warnings https://github.com/hyperium/tonic/blob/master/tonic-build/src/server.rs#L62

I would actually apply the allow just to the mod like so.

pub mod pb {
    #![allow(clippy::derive_partial_eq_without_eq)]

    tonic::include_proto!("dummy");
}

[jialez0]

Hi all, I tried to add #![allow(clippy::derive_partial_eq_without_eq)] to my code in this way:

pub mod pb {
    #![allow(clippy::derive_partial_eq_without_eq)]
    tonic::include_proto!("dummy");
}

It can pass the clippy lint check before rustc 1.63.0 (4b91a6ea7 2022-08-08), but will still failed of rustc 1.63.0 (4b91a6ea7 2022-08-08)...

[LucioFranco]

@jialez0 not totally sure why (or what is actually failing) but clippy for sure allows you to allow specific ones, so maybe you're not adding it in all places?

[Sherlock-Holo]

I think for things like codegen is quite normal to add allow's. Tonic does this for some built in rust warnings https://github.com/hyperium/tonic/blob/master/tonic-build/src/server.rs#L62

I would actually apply the allow just to the mod like so.

pub mod pb {
    #![allow(clippy::derive_partial_eq_without_eq)]

    tonic::include_proto!("dummy");
}

in the past users just include the generated files without any additional operations, and we believe the generated files are correct, no warn no error

so I think we should keep status quo, let users still include the generated files without any additional operations, and prost-build fix this warn

there are 2 ways:

1. the tonic::include_proto not just include the files, and add the #![allow(clippy::derive_partial_eq_without_eq)] too
2. when prost-build generated codes, also add the #![allow(clippy::derive_partial_eq_without_eq)]

[LucioFranco]

imho clippy isn't a "required" tool but a tool that users choose to use in their project. I believe prost should only allow warnings that come from the compiler and not other tools. Users can config clippy for their project to their liking but you can't do this with the compiler.

[de-vri-es]

imho clippy isn't a "required" tool but a tool that users choose to use in their project. I believe prost should only allow warnings that come from the compiler and not other tools. Users can config clippy for their project to their liking but you can't do this with the compiler.

Clippy is a very standard tool to use though. It even comes with the toolchain by default when you use rustup. It does make sense in my opinion to consider it here.

[teohhanhui]

The problem with the workaround is that the #![allow(clippy::derive_partial_eq_without_eq)] then leaks to the entire module, if the user doesn't create a submodule specifically for the generated items (why should they?)...

EDIT: Uhh, but I guess they could do:

pub use pb::*;

mod pb {
    #![allow(clippy::derive_partial_eq_without_eq)]

    tonic::include_proto!("dummy");
}