scullionw / dirstat-rs

(fastest?) disk usage cli, similar to windirstat.
MIT License
157 stars 11 forks source link

Fix clippy warnings and add Cargo-clippy check to CI #12

Open Mart-Bogdan opened 1 year ago

Mart-Bogdan commented 1 year ago

When cargo clippy is run against our source we are getting following warnings:

[winnie@bm-linux dirstat-rs]$ cargo clippy
    Checking dirstat-rs v0.3.8 (/home/winnie/work/dirstat-rs)
warning: this expression creates a reference which is immediately dereferenced by the compiler
  --> src/lib.rs:28:24
   |
28 |             .unwrap_or(&OsStr::new("."))
   |                        ^^^^^^^^^^^^^^^^ help: change this to: `OsStr::new(".")`
   |
   = note: `#[warn(clippy::needless_borrow)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow

warning: `dirstat-rs` (lib) generated 1 warning
warning: this expression creates a reference which is immediately dereferenced by the compiler
  --> src/bin/main.rs:27:41
   |
27 |     let file_info = FileInfo::from_path(&target_dir, config.apparent)?;
   |                                         ^^^^^^^^^^^ help: change this to: `target_dir`
   |
   = note: `#[warn(clippy::needless_borrow)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow

warning: this expression creates a reference which is immediately dereferenced by the compiler
  --> src/bin/main.rs:44:36
   |
44 |             DiskItem::from_analyze(&target_dir, config.apparent, volume_id)?
   |                                    ^^^^^^^^^^^ help: change this to: `target_dir`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow

warning: this expression creates a reference which is immediately dereferenced by the compiler
  --> src/bin/main.rs:62:21
   |
62 |     show_item(item, &info, buffer)?;
   |                     ^^^^^ help: change this to: `info`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow

warning: `format!` in `write!` args
  --> src/bin/main.rs:90:5
   |
90 |     write!(buffer, " {} ", format!("{:.2}%", info.fraction))?;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(clippy::format_in_format_args)]` on by default
   = help: combine the `format!(..)` arguments with the outer `write!(..)` call
   = help: or consider changing `format!` to `format_args!`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#format_in_format_args

warning: manual `RangeInclusive::contains` implementation
   --> src/bin/main.rs:199:8
    |
199 |     if num >= 0.0 && num <= 100.0 {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `(0.0..=100.0).contains(&num)`
    |
    = note: `#[warn(clippy::manual_range_contains)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains

warning: `dirstat-rs` (bin "ds") generated 5 warnings

Those warnings can be easily fixed.

Also CI check should be added.

as actions-rs/clippy don't work at all anymore, and actions-rs/clippy-check is unable to process externall PRs for now we can just run clippy as exec/run step in workflow. And fail if we have non zero exit code. And just have errors in console, not as annotations in PR.

Unresolved question is: Should we threat them as errors, preventing PR, or just as warning, so just run check, but don't fail CI because of it?

_Originally posted by @scullionw in https://github.com/scullionw/dirstat-rs/pull/10#discussion_r1051650162_