chore: Fix clippy lint

[tottoto]

Fixes clippy lint.

[tottoto]

There are some unresolved warnings (these quoted codes are at v0.11.4). Clippy warns them as derive_partial_eq_without_eq. I'm not sure that how they should be handled.

details

WireType in src/encodeng.rs

prost/src/encoding.rs

Lines 267 to 276 in d3ba4e8

	#[derive(Clone, Copy, Debug, PartialEq)]
	#[repr(u8)]
	pub enum WireType {
	Varint = 0,
	SixtyFourBit = 1,
	LengthDelimited = 2,
	StartGroup = 3,
	EndGroup = 4,
	ThirtyTwoBit = 5,
	}

DurationError in prost-types/src/lib.rs

prost/prost-types/src/lib.rs

Lines 145 to 165 in d3ba4e8

	#[derive(Debug, PartialEq)]
	#[non_exhaustive]
	pub enum DurationError {
	/// Indicates failure to parse a [`Duration`] from a string.
	///
	/// The [`Duration`] string format is specified in the [Protobuf JSON mapping specification][1].
	///
	/// [1]: https://developers.google.com/protocol-buffers/docs/proto3#json
	ParseFailure,
	/// Indicates failure to convert a `prost_types::Duration` to a `std::time::Duration` because
	/// the duration is negative. The included `std::time::Duration` matches the magnitude of the
	/// original negative `prost_types::Duration`.
	NegativeDuration(time::Duration),
	/// Indicates failure to convert a `std::time::Duration` to a `prost_types::Duration`.
	///
	/// Converting a `std::time::Duration` to a `prost_types::Duration` fails if the magnitude
	/// exceeds that representable by `prost_types::Duration`.
	OutOfRange,
	}

TimeStampError in prost-types/src/lib.rs

prost/prost-types/src/lib.rs

Lines 318 to 335 in d3ba4e8

	#[derive(Debug, PartialEq)]
	#[non_exhaustive]
	pub enum TimestampError {
	/// Indicates that a [`Timestamp`] could not be converted to
	/// [`SystemTime`][std::time::SystemTime] because it is out of range.
	///
	/// The range of times that can be represented by `SystemTime` depends on the platform. All
	/// `Timestamp`s are likely representable on 64-bit Unix-like platforms, but other platforms,
	/// such as Windows and 32-bit Linux, may not be able to represent the full range of
	/// `Timestamp`s.
	OutOfSystemRange(Timestamp),
	/// An error indicating failure to parse a timestamp in RFC-3339 format.
	ParseFailure,
	/// Indicates an error when constructing a timestamp due to invalid date or time data.
	InvalidDateTime,
	}

[LucioFranco]

@tottoto we should handle them the same way that we handle the other by having an allow. I think that makes the most sense, what do you think?

[tottoto]

I think WireType should implement Eq since it have to return true when all same variants ​​are compared (according to check_wire_type).

I also considered whether the PartialEq are actually needed or not on error enums. It seem that they are implemented only for tests. In particular the errors enum do not have to be implemented as PartialEq (like hyper's error) imho. If prost does not intend to provide the PartialEq functionality to its users, we might be able to remove PartialEq from them (and implement PartialEq and Eq, or implement PartialEq and allow derive_partial_eq_without_eq on them by #[cfg_attr(tests, ...)]).

[LucioFranco]

Yeah, I actually don't think I mind Eq on enums since that actually kinda makes sense. Im cautious to add Eq to anytime stamp stuff since generally you shouldn't compare them like that. What do you think?

[tottoto]

In my opinion, WireType should implement Eq because without Eq it can not assure that the same wire type check always return true.

On the other hand, DurationError and TimeStampError should not do. We should handle error enum by pattern matching so maybe PartialEq is not also unnecessary imho. However, since removing PartialEq is a big breaking change and discussing it is also a too big in order to fix lint warnings, I'd like to leave them unchanged and add allow rules.

[tottoto]

These are my proposal: e71d6f1, bf2696b.

[LucioFranco]

In my opinion, WireType should implement Eq because without Eq it can not assure that the same wire type check always return true.

On the other hand, DurationError and TimeStampError should not do. We should handle error enum by pattern matching so maybe PartialEq is not also unnecessary imho. However, since removing PartialEq is a big breaking change and discussing it is also a too big in order to fix lint warnings, I'd like to leave them unchanged and add allow rules.
LucioFranco reacted with thumbs up emoji

This makes 100% sense to me!

[LucioFranco]

Thanks for doing all these PRs!!! I really appreciate it!