Skip to content

Implement PartialEq/Eq for InvalidUri? #849

Description

@joshka

I have a library with its own error type, and one variant wraps http::uri::InvalidUri. I would like that error type to derive PartialEq and Eq so unit tests can compare expected and actual errors directly, but InvalidUri currently prevents that.

For example:

#[derive(Debug, PartialEq, Eq)]
enum Error {
    InvalidUri(http::uri::InvalidUri),
}

Today I can work around this with a manual PartialEq implementation in my crate, but the only special case is this wrapped http error. The main thing I want is boring unit tests that compare errors normally.

Would it be reasonable for InvalidUri and InvalidUriParts to implement PartialEq and Eq?

The small patch would be:

#[derive(Debug, Eq, PartialEq)]
pub struct InvalidUri(ErrorKind);

#[derive(Debug, Eq, PartialEq)]
pub struct InvalidUriParts(InvalidUri);

ErrorKind already implements PartialEq and Eq, and this would keep it private. I am not trying to expose or match on the URI error kind.

I also realize this is still a public contract. I saw the previous discussions around error trait impls, especially #128 and #204 around Clone, and I understand the concern that trait impls can constrain future error internals. PartialEq/Eq seems like a smaller commitment than Clone, but it is still a commitment: users could compare two InvalidUri values and observe whether they are considered the same broad error.

My take is that this tradeoff is reasonable for these URI errors because the comparison can stay coarse, ErrorKind stays private, and the concrete downstream benefit is allowing library error types to derive equality for tests. I may be underestimating the compatibility cost, though, so I would appreciate maintainer feedback.

I am opening this as an issue even though the possible patch is small, because the useful part is the API decision. If the answer is "no, this is too much public contract for URI errors," that decision would be useful to have recorded for future searches.

One scope question: the same motivation may apply to other opaque conversion errors in this crate, such as InvalidMethod, InvalidStatusCode, InvalidHeaderName, InvalidHeaderValue, and ToStrError. I am starting with InvalidUri because that is the case I ran into, and InvalidUriParts is just a wrapper around it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions