iolivia / rust-sokoban

Rust Sokoban book and code samples
https://sokoban.iolivia.me
MIT License
155 stars 29 forks source link

Add clippy linting #104

Open bailey-coding opened 2 years ago

bailey-coding commented 2 years ago

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

iolivia commented 2 years ago

does clippy lint succeed on the current code? 😄

bailey-coding commented 2 years ago

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?

iolivia commented 2 years ago

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.

bailey-coding commented 2 years ago

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: @.***>