http
http copied to clipboard
StatusCode TryFrom and PartialEq with integer types
As suggested in #230, I added TryFrom
<usize>
, <u32>
and <u64>
and From<StatusCode>
for the same integer types, and re-implemented <u16>
with the same macro. Oddly enough, if I stop with unsigned types, then previously working doctests for response::{Response, Builder} start failing with the following:
error[E0277]: the trait bound `StatusCode: From<i32>` is not satisfied
--> src/response.rs:537:6
|
7 | .status(200)
| ^^^^^^ the trait `From<i32>` is not implemented for `StatusCode`
|
= help: the following implementations were found:
<StatusCode as From<&'a StatusCode>>
= note: required because of the requirements on the impl of `Into<StatusCode>` for `i32`
= note: required because of the requirements on the impl of `TryFrom<i32>` for `StatusCode`
...~~which seems almost more like an obscure rustc or stdlib bug? (Fails on MSRV 1.39.0 as well as a recent nightly)~~ (edit: see comment below). As this was done with macro_rules!
, its easy to extend to the signed types i16
, i32
, isize
, and i64
types to avoid the above issue. Since its implemented via u16::try_from
there is nothing particularly more unusual about including support for the signed types: negative values are just another case of InvalidStatusCode
.
Also added bi-directional PartialEq
for all of these integer types. This makes it so u16
doesn't have any particular favor in the public interface other then the existing StatusCode::from_u16
and as_u16
. If a user sticks to TryInto
, From
and PartialEq
then u16
(and NonZeroU16
) is just a private implementation detail.
The PR includes some misc related test and doc improvements.
Given the warning of the about mentioned compile error, I would not suggest merging this for release in a patch release like 0.2.2. It warrants at least a MINOR bump 0.3.0 or 1.0.0.
Fixes #230
Asked about the error mentioned above and got a helpful, quick answer here:
https://users.rust-lang.org/t/whats-so-special-about-the-i32-type-here/52613/3
So its known limitation of the compiler and i32 conversions need to be implemented to handle the i32 fallback for non-inferred integer literals. Inference no longer happens with the multiple TryFrom impls of this PR.
@kaj how would this play out with #442, #454? Could you cross review?
@kaj how would this play out with #442, #454? Could you cross review?
I think there is no relevant conflict. #442 has Builder2::status(code)
which takes a StatusCode
rather than a TryInto<StatusCode>
, so it doesn't really matter to that which types implement TryInto<StatusCode>
. I might amend it to also proivde a Builder2::try_status(...)
method that taks a TryInto<StatusCode>
, but I don't think that is a conflict either. Of course, if this i merged first and there is a conflict, I'll be happy to resolve it.
Also, I think this change looks good in itself (but I'm not a maintainer of this crate).
Okay, thanks. That was the kind of sanity check on this PR I was hoping for.