Skip to content

feat(uri): derive equality for URI errors#850

Closed
joshka wants to merge 1 commit into
hyperium:masterfrom
joshka:joshka/invalid-uri-eq
Closed

feat(uri): derive equality for URI errors#850
joshka wants to merge 1 commit into
hyperium:masterfrom
joshka:joshka/invalid-uri-eq

Conversation

@joshka

@joshka joshka commented Jul 4, 2026

Copy link
Copy Markdown

Motivation

I opened #849 as the durable place for the API tradeoff. This PR is a small concrete version of the change so maintainers can evaluate the actual patch shape.

I have a library error type that wraps http::uri::InvalidUri. I would like that error type to derive PartialEq and Eq for straightforward unit tests, but InvalidUri currently prevents that.

Change

This derives PartialEq and Eq for InvalidUri and InvalidUriParts. ErrorKind remains private.

The test checks that both public error types implement Eq and that repeated InvalidUri / InvalidUriParts failures can be compared directly.

Compatibility

I realize this is still a public contract. Prior discussions such as #128 and #204 were cautious about trait impls on error types because they can constrain future internals.

My take is that PartialEq/Eq is a smaller commitment than Clone: it does not expose ErrorKind, and equality can remain a coarse comparison over the existing broad URI error category. That said, users could observe whether two InvalidUri values compare equal, so I am opening this as a draft PR to get maintainer feedback on that tradeoff.

Refs #849.

Validation

cargo fmt --check
cargo test

Allow InvalidUri and InvalidUriParts to implement PartialEq and Eq so
library error types that wrap them can derive equality for tests. Keep
ErrorKind private and leave broader error-type equality as a separate API
question.
@joshka joshka marked this pull request as ready for review July 12, 2026 00:31
@joshka

joshka commented Jul 14, 2026

Copy link
Copy Markdown
Author

Thanks for considering this. I looked further into the concern that implementing equality could constrain future internal changes.

One relevant standard-library precedent is TryReserveError. It originally derived PartialEq and Eq, and Rust later deliberately changed it into an opaque struct because the private error details were still unsettled (rust-lang/rust#87408). The stabilization proposal explicitly left open both adding support for future allocator details and reducing the error to a unit struct, while the public type was stabilized with its equality implementations intact (rust-lang/rust#87993).

I did not find an explicit discussion of equality in those reviews, so I would not claim that this settles the question for InvalidUri. It does show that std has stabilized equality for an opaque error whose private representation was deliberately left open to substantial change. Together with Utf8Error implementing equality despite carrying positional information, that makes me less concerned that these possible future details are necessarily blocking.

My practical motivation remains that InvalidUri is a leaf error from a foundational crate. Including it in a downstream error enum prevents that enum from deriving PartialEq, moving the implementation work into each downstream crate. My take is that this compositional benefit outweighs the limited compatibility constraint here, particularly if equality is defined without exposing the private kind or making future diagnostic details significant.

That said, the downstream workaround is reasonable, and I understand the preference not to add the contract. I’m happy to leave this undone and close the PR.

@joshka joshka closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant