Amanieu / intrusive-rs

Intrusive collections for Rust
Apache License 2.0
423 stars 50 forks source link

Clippy cleanup #20

Closed bitemyapp closed 5 years ago

bitemyapp commented 6 years ago

Thanks for publishing this library!

This reduces the clippy warnings to these two:

warning: methods called `into_*` usually take self by value; consider choosing a less ambiguous name                                                                                                     
  --> src/unsafe_ref.rs:67:21                                                                                                                                                                            
   |                                                                                                                                                                                                     
67 |     pub fn into_raw(ptr: &Self) -> *mut T {                                                                                                                                                         
   |                     ^^^                                                                                                                                                                             
   |                                                                                                                                                                                                     
   = note: #[warn(clippy::wrong_self_convention)] on by default                                                                                                                                          
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#wrong_self_convention                                                                       

warning: methods called `into_*` usually take self by value; consider choosing a less ambiguous name                                                                                                     
  --> src/unsafe_ref.rs:89:28                                                                                                                                                                            
   |                                                                                                                                                                                                     
89 |     pub unsafe fn into_box(ptr: &Self) -> Box<T> {                                                                                                                                                  
   |                            ^^^                                                                                                                                                                      
   |                                                                                                                                                                                                     
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#wrong_self_convention

This is tough. Changing the interface would probably be best long term, but this patch seemed a lesser evil than unnecessary ownership of Self in the relevant methods. I cleaned up a couple other clippy warnings. Apologies if this is unwelcome or not a good use of your time. I lint libraries I am learning from and sometimes PR what I changed.

bitemyapp commented 6 years ago

Oh wow, I just noticed the build failure. I tested this with nightly or so I thought! I will try to figure out what has happened.

bitemyapp commented 6 years ago

From my local build:

[ callen@chalcis ~/work/github/intrusive-rs bitemyapp/clippy-cleanup O ]
$ cargo build
   Compiling intrusive-collections v0.7.4 (/home/callen/work/github/intrusive-rs)                                                                                                                        
    Finished dev [unoptimized + debuginfo] target(s) in 1.20s                                                                                                                                            
[ callen@chalcis ~/work/github/intrusive-rs bitemyapp/clippy-cleanup O ]
$ rustc --version       
rustc 1.30.0-nightly (bb0896af1 2018-09-29)

From TravisCI:

$ rustc --version
rustc 1.30.0-nightly (bb0896af1 2018-09-29)
Amanieu commented 6 years ago

To test the nightly features you need to add --features nightly to the build command-line.

bitemyapp commented 6 years ago

@Amanieu thank you, I have the repro now! Is there anywhere I could read about the cause of this type error with the nightly features turned on?

Amanieu commented 6 years ago

unsafe_ref.rs has has different code depending on whether the nightly feature is enabled. You just forgot to update the nightly implementation.

bitemyapp commented 6 years ago

@Amanieu right-o, serves me for coding in the wee hours. Thank you!

Amanieu commented 6 years ago

Please also fix my comments in the code. Some clippy warnings are too strict and should be explicitly disabled for certain functions.

Amanieu commented 6 years ago

You can ignore the test failure on nightly. This seems to be a rustc bug, I will look into it.

bitemyapp commented 5 years ago

Gonna kill this PR and restart from scratch if I reprise pursuing a clippy purge. Thank you @Amanieu!