rust-sokoban icon indicating copy to clipboard operation
rust-sokoban copied to clipboard

Add clippy linting

Open bailey-coding opened this issue 3 years ago • 4 comments

Relates to https://github.com/iolivia/rust-sokoban/issues/13

bailey-coding avatar Feb 13 '22 13:02 bailey-coding

does clippy lint succeed on the current code? 😄

iolivia avatar Feb 13 '22 13:02 iolivia

Thanks for the great project, by the way! I just followed it and enjoyed it a lot!

There are just a few warnings:

warning: writing `&String` instead of `&str` involves a new object where a slice will do
  --> src/audio.rs:12:64
   |
12 |     pub fn play_sound(&mut self, context: &mut Context, sound: &String) {
   |                                                                ^^^^^^^ help: change this to: `&str`
   |
   = note: `#[warn(clippy::ptr_arg)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg

warning: very complex type used. Consider factoring parts into `type` definitions
  --> src/systems/event_system.rs:17:23
   |
17 |       type SystemData = (
   |  _______________________^
18 | |         Write<'a, EventQueue>,
19 | |         Write<'a, AudioStore>,
20 | |         Entities<'a>,
...  |
23 | |         ReadStorage<'a, Position>,
24 | |     );
   | |_____^
   |
   = note: `#[warn(clippy::type_complexity)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity

warning: very complex type used. Consider factoring parts into `type` definitions
  --> src/systems/input_system.rs:15:23
   |
15 |       type SystemData = (
   |  _______________________^
16 | |         Write<'a, EventQueue>,
17 | |         Write<'a, InputQueue>,
18 | |         Write<'a, Gameplay>,
...  |
23 | |         ReadStorage<'a, Immovable>,
24 | |     );
   | |_____^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity

warning: using `clone` on type `u32` which implements the `Copy` trait
  --> src/systems/input_system.rs:80:56
   |
80 |                         Some(id) => to_move.push((key, id.clone())),
   |                                                        ^^^^^^^^^^ help: try dereferencing it: `*id`
   |
   = note: `#[warn(clippy::clone_on_copy)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

warning: length comparison to zero
  --> src/systems/input_system.rs:99:12
   |
99 |         if to_move.len() > 0 {
   |            ^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!to_move.is_empty()`
   |
   = note: `#[warn(clippy::len_zero)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero

warning: `rust-sokoban` (bin "rust-sokoban") generated 5 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 2m 36s

I can make an MR for the !is_empty, but not sure about the complex type ones?

bailey-coding avatar Feb 13 '22 15:02 bailey-coding

hey, super glad to hear, thanks for all the fixes, really appreciate it! 🙏

is it possible to ignore the type complexity rule when we run clippy? maybe there is some clippy config where we can add an ignore for this specific rule without changing the code.

the rest should be fine to fix, the main thing is to make sure all the code changes stays on the same line, this is very unfortunate but cause we are using line numbers in the book if the code changes significantly it's a pain to change.

iolivia avatar Feb 13 '22 16:02 iolivia

I think there is some clippy config that we can put in Cargo.toml or somewhere that might avoid adding ignore lines to all the specific files.

I'll see if I can get that working.

On Sun, 13 Feb 2022 at 17:47, Olivia Ifrim @.***> wrote:

hey, super glad to hear, thanks for all the fixes, really appreciate it! 🙏

is it possible to ignore the type complexity rule when we run clippy? maybe there is some clippy config where we can add an ignore for this specific rule without changing the code.

the rest should be fine to fix, the main thing is to make sure all the code changes stays on the same line, this is very unfortunate but cause we are using line numbers in the book if the code changes significantly it's a pain to change.

— Reply to this email directly, view it on GitHub https://github.com/iolivia/rust-sokoban/pull/104#issuecomment-1038253865, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHBGLRRLQX3JZAWE6UTFAO3U27OCVANCNFSM5OI7437Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

bailey-coding avatar Feb 13 '22 18:02 bailey-coding