hyperium / http

Rust HTTP types
Apache License 2.0
1.12k stars 283 forks source link

Clippy error which propose breaking change #646

Closed tottoto closed 7 months ago

tottoto commented 7 months ago

Clippy lint to the current head (c51f40f8da5abf725ab545bff842028be20a09f3) reports some warnings and errors. Although most of them seems to be able to be handled as internal implementations, the followings errors propose API breaking changes. These should be considered before stabilizing.

error: methods with the following characteristics: (`to_*` and `self` type is not `Copy`) usually take `self` by reference
    --> src/header/map.rs:3228:15
     |
3228 |     fn to_red(&mut self) {
     |               ^^^^^^^^^
     |
     = help: consider choosing a less ambiguous name
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention
     = note: `#[deny(clippy::wrong_self_convention)]` implied by `#[deny(warnings)]`
error: methods with the following characteristics: (`to_*` and `self` type is not `Copy`) usually take `self` by reference
    --> src/header/map.rs:3240:18
     |
3240 |     fn to_yellow(&mut self) {
     |                  ^^^^^^^^^
     |
     = help: consider choosing a less ambiguous name
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention
error: methods with the following characteristics: (`to_*` and `self` type is not `Copy`) usually take `self` by reference
    --> src/header/map.rs:3249:17
     |
3249 |     fn to_green(&mut self) {
     |                 ^^^^^^^^^
     |
     = help: consider choosing a less ambiguous name
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention
error: method `from_str` can be confused for the standard trait method `std::str::FromStr::from_str`
   --> src/header/value.rs:125:5
    |
125 | /     pub fn from_str(src: &str) -> Result<HeaderValue, InvalidHeaderValue> {
126 | |         HeaderValue::try_from_generic(src, |s| Bytes::copy_from_slice(s.as_bytes()))
127 | |     }
    | |_____^
    |
    = help: consider implementing the trait `std::str::FromStr` or choosing a less ambiguous method name
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait
    = note: `#[deny(clippy::should_implement_trait)]` implied by `#[deny(warnings)]`
seanmonstar commented 7 months ago

The color methods are private. The from_str constructor does the same as the trait, I don't see a problem with it.

tottoto commented 7 months ago

Thanks for the review. Overlooked that color methods are private APIs. Regarding from_str constructor, I understand that it is not a problem by your feedback.