rust-lang / flate2-rs

DEFLATE, gzip, and zlib bindings for Rust
https://docs.rs/flate2
Apache License 2.0
891 stars 158 forks source link

Make clippy happy + a few more cleanups #285

Closed nyurik closed 1 year ago

nyurik commented 2 years ago

Following up from #283

expand original clippy messages > warning: this seems like a manual implementation of the non-exhaustive pattern > --> src/mem.rs:46:1 > | > 46 | pub enum FlushCompress { > | ^--------------------- > | | > | _help: add the attribute: `#[non_exhaustive] pub enum FlushCompress` > | | > 47 | | /// A typical parameter for passing to compression/decompression functions, > 48 | | /// this indicates that the underlying stream to decide how much data to > 49 | | /// accumulate before producing output in order to maximize compression. > ... | > 85 | | _Nonexhaustive, > 86 | | } > | |_^ > | > = note: `#[warn(clippy::manual_non_exhaustive)]` on by default > help: remove this variant > --> src/mem.rs:85:5 > | > 85 | _Nonexhaustive, > | ^^^^^^^^^^^^^^ > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive > > warning: this seems like a manual implementation of the non-exhaustive pattern > --> src/mem.rs:91:1 > | > 91 | pub enum FlushDecompress { > | ^----------------------- > | | > | _help: add the attribute: `#[non_exhaustive] pub enum FlushDecompress` > | | > 92 | | /// A typical parameter for passing to compression/decompression functions, > 93 | | /// this indicates that the underlying stream to decide how much data to > 94 | | /// accumulate before producing output in order to maximize compression. > ... | > 113 | | _Nonexhaustive, > 114 | | } > | |_^ > | > help: remove this variant > --> src/mem.rs:113:5 > | > 113 | _Nonexhaustive, > | ^^^^^^^^^^^^^^ > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive > > warning: the operation is ineffective. Consider reducing it to `(header[4] as u32)` > --> src/gz/bufread.rs:58:39 > | > 58 | r.part.header.mtime = ((header[4] as u32) << 0) > | ^^^^^^^^^^^^^^^^^^^^^^^^^ > | > = note: `#[warn(clippy::identity_op)]` on by default > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: `ref` on an entire `let` pattern is discouraged, take a reference with `&` instead > --> src/gz/bufread.rs:197:13 > | > 197 | let ref arr = [ > | ^^^^^^^ > | > = note: `#[warn(clippy::toplevel_ref_arg)]` on by default > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg > help: try > | > 197 ~ let arr = &[ > 198 + (crc.sum() >> 0) as u8, > 199 + (crc.sum() >> 8) as u8, > 200 + (crc.sum() >> 16) as u8, > 201 + (crc.sum() >> 24) as u8, > 202 + (crc.amount() >> 0) as u8, > ... > > warning: the operation is ineffective. Consider reducing it to `crc.sum()` > --> src/gz/bufread.rs:198:13 > | > 198 | (crc.sum() >> 0) as u8, > | ^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: the operation is ineffective. Consider reducing it to `crc.amount()` > --> src/gz/bufread.rs:202:13 > | > 202 | (crc.amount() >> 0) as u8, > | ^^^^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: the operation is ineffective. Consider reducing it to `(buf[0] as u32)` > --> src/gz/bufread.rs:233:15 > | > 233 | let crc = ((buf[0] as u32) << 0) > | ^^^^^^^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: the operation is ineffective. Consider reducing it to `(buf[4] as u32)` > --> src/gz/bufread.rs:237:15 > | > 237 | let amt = ((buf[4] as u32) << 0) > | ^^^^^^^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: the operation is ineffective. Consider reducing it to `sum` > --> src/gz/write.rs:102:17 > | > 102 | (sum >> 0) as u8, > | ^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: the operation is ineffective. Consider reducing it to `amt` > --> src/gz/write.rs:106:17 > | > 106 | (amt >> 0) as u8, > | ^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: the operation is ineffective. Consider reducing it to `(self.crc_bytes[0] as u32)` > --> src/gz/write.rs:307:19 > | > 307 | let crc = ((self.crc_bytes[0] as u32) << 0) > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: the operation is ineffective. Consider reducing it to `(self.crc_bytes[4] as u32)` > --> src/gz/write.rs:311:19 > | > 311 | let amt = ((self.crc_bytes[4] as u32) << 0) > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: the operation is ineffective. Consider reducing it to `v.len()` > --> src/gz/mod.rs:215:25 > | > 215 | header.push((v.len() >> 0) as u8); > | ^^^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: you are using an explicit closure for copying elements > --> src/gz/mod.rs:221:27 > | > 221 | header.extend(filename.as_bytes_with_nul().iter().map(|x| *x)); > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `filename.as_bytes_with_nul().iter().copied()` > | > = note: `#[warn(clippy::map_clone)]` on by default > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone > > warning: you are using an explicit closure for copying elements > --> src/gz/mod.rs:225:27 > | > 225 | header.extend(comment.as_bytes_with_nul().iter().map(|x| *x)); > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `comment.as_bytes_with_nul().iter().copied()` > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone > > warning: the operation is ineffective. Consider reducing it to `mtime` > --> src/gz/mod.rs:231:21 > | > 231 | header[4] = (mtime >> 0) as u8; > | ^^^^^^^^^^^^ > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op > > warning: use of `offset` with a `usize` casted to an `isize` > --> src/mem.rs:365:27 > | > 365 | let ptr = output.as_mut_ptr().offset(len as isize); > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `output.as_mut_ptr().add(len)` > | > = note: `#[warn(clippy::ptr_offset_with_cast)]` on by default > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast > > warning: use of `offset` with a `usize` casted to an `isize` > --> src/mem.rs:506:27 > | > 506 | let ptr = output.as_mut_ptr().offset(len as isize); > | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `output.as_mut_ptr().add(len)` > | > = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast > > warning: `flate2` (lib) generated 18 warnings > Finished dev [unoptimized + debuginfo] target(s) in 0.02s
JohnTitor commented 1 year ago

@nyurik Friendly-ping, could you check the above review comments?

nyurik commented 1 year ago

@JohnTitor thanks for the ping! I addressed the feedback, and also added a few minor ones

nyurik commented 1 year ago

Quick question: in write!(f, "deflate compression error: {}", msg) -- which Rust version does flate2-rs target? Can inline format args be used here?

JohnTitor commented 1 year ago

Quick question: in write!(f, "deflate compression error: {}", msg) -- which Rust version does flate2-rs target? Can inline format args be used here?

Let's leave them as-is for now. According to https://github.com/rust-lang/flate2-rs/issues/207#issuecomment-521771662, MSRV is N-1 stable. The maintainership has, however, been changed recently and I'd like to declare it explicitly first.

JohnTitor commented 1 year ago

Thanks!