Fix clippy warnings

[iceiix]

If all clippy lints were fixed, then #346 Azure Pipelines could enable build failures for new clippy warnings/errors

[iceiix]

619 of the warnings in steven_gl: brendanzab/gl-rs#523

Fixed many in steven_blocks, but this suggestion seems wrong:

warning: operator precedence can trip the unwary
   --> src/lib.rs:779:18
    |
779 |             Some(variant.offset() * (7 * 2) + ((distance as usize - 1) << 1) | (if decayable { 0 } else { 1 }))
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider parenthesizing your expression: `(variant.offset() * (7 * 2) + ((distance as usize - 1) << 1)) | (if decayable { 0 } else { 1 })`
    |
    = note: `#[warn(clippy::precedence)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#precedence

reduced test case:

fn f(a: usize, b: usize, c: bool) -> Option<usize> {
    Some(a + (b << 1) | (if c { 0 } else { 1 }))
}

fn main() {
    println!("{:?}", f(1, 2, true));
}

clippy suggests changing:

-Some(a + (b << 1) | (if c { 0 } else { 1 }))
+Some(a + (b << 1)) | (if c { 0 } else { 1 })

but this can't compile of course, error[E0369]: no implementation for std::option::Option<usize> | {integer}

update: it actually means parenthesize with Some, this fixes the warning:

Some((a + (b << 1)) | (if c { 0 } else { 1 }))

[iceiix]

Next up, there are 133 warnings in steven_blocks, all the same, but in macro-generated code:

warning: redundant field names in struct initialization
    --> src/lib.rs:381:39
     |
381  |                                       $($fname: $fname,)?
     |                                         ^^^^^^^^^^^^^^ help: replace it with: `drag`
...
509  | / define_blocks! {
510  | |     Air {
511  | |         props {},
512  | |         material material::Material {
...    |
5624 | |     }
5625 | | }
     | |_- in this macro invocation
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
     = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

$fname: $fname is used 4 times throughout src/lib.rs

[iceiix]

Working on steven_protocol, down to two warnings:

warning: defining a method called `from_str` on this type; consider implementing the `std::str::FromStr` trait or choosing a less ambiguous name
   --> src/protocol/mod.rs:423:5
    |
423 | /     pub fn from_str(s: &str) -> UUID {
424 | |         // TODO: Panics aren't the best idea here
425 | |         if s.len() != 36 {
426 | |             panic!("Invalid UUID format");
...   |
439 | |         UUID(high, low)
440 | |     }
    | |_____^
    |
    = note: `#[warn(clippy::should_implement_trait)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait

https://doc.rust-lang.org/std/str/trait.FromStr.html would be more appropriate here. It would solve the // TODO: Panics aren't the best idea here, since it returns a Result<Self, Self::Err>, allowing gracefully handling errors. Would need to update the callers.

warning: the loop variable `i` is only used to index `data`.
   --> src/protocol/mod.rs:488:18
    |
488 |         for i in 0..1024 {
    |                  ^^^^^^^
    |
    = note: `#[warn(clippy::needless_range_loop)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator
    |
488 |         for <item> in &mut data {
    |             ^^^^^^    ^^^^^^^^^

need to access the iterator mutably, to write data[i] = b;

[iceiix]

Zero clippy lints on steven_protocol (as well as steven_blocks) now. None already on steven_shared, or std_or_web, but steven_shared has a few.

• gl: 619
• error: aborting due to 59 previous errors; 160 warnings emitted

[iceiix]

Fixing more warnings, but not all lints may be appropriate for this project.

• clippy::too_many_arguments (>7): ok, the arguments can be replaced by a struct etc., but the gl functions have many args standard by default
• clippy::many_single_char_names: often single characters variable names are the most clear (x, y, z, ...)

[iceiix]

src/render/mod.rs TextureManager, this triggers a warning:

            version: { 
                let ver = res.read().unwrap().version();
                ver
            },

but is it really unnecessary?

warning: returning the result of a `let` binding from a block
   --> src/render/mod.rs:972:17
    |
971 |                 let ver = res.read().unwrap().version();
    |                 ---------------------------------------- unnecessary `let` binding
972 |                 ver
    |                 ^^^
    |
    = note: `#[warn(clippy::let_and_return)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly

it can't be returned directly, trips up the borrow checker:

error[E0505]: cannot move out of `res` because it is borrowed
   --> src/render/mod.rs:971:24
    |
970 |             version: res.read().unwrap().version(),
    |                      -------------------
    |                      |
    |                      borrow of `res` occurs here
    |                      a temporary with access to the borrow is created here ...
971 |             resources: res,
    |                        ^^^ move out of `res` occurs here
...
981 |         };
    |          - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `std::sync::RwLockReadGuard`

error: aborting due to previous error

[iceiix]

Chipping away at the lints, down to 58 excluding steven_gl:

warning: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
warning: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
warning: `if` chain can be rewritten with `match`
warning: bit mask could be simplified with a call to `trailing_zeros`
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type
warning: calling `as_bytes()` on a string literal
warning: large size difference between variants
warning: length comparison to zero
warning: length comparison to zero
warning: manual implementation of an assign operation
warning: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
warning: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
warning: redundant pattern matching, consider using `is_err()`
warning: redundant pattern matching, consider using `is_ok()`
warning: redundant pattern matching, consider using `is_ok()`
warning: redundant pattern matching, consider using `is_some()`
warning: returning the result of a `let` binding from a block
warning: the loop variable `i` is used to index `block_add`
warning: the loop variable `i` is used to index `block_meta`
warning: the loop variable `i` is used to index `block_types`
warning: this block may be rewritten with the `?` operator
warning: this boolean expression can be simplified
warning: this boolean expression can be simplified
warning: use of `offset` with a `usize` casted to an `isize`
warning: use of `offset` with a `usize` casted to an `isize`
warning: use of `offset` with a `usize` casted to an `isize`
warning: use of `offset` with a `usize` casted to an `isize`
warning: use of `offset` with a `usize` casted to an `isize`
warning: use of `unwrap_or` followed by a function call
warning: use of `unwrap_or` followed by a function call
warning: use of `unwrap_or` followed by a function call
warning: use of a blacklisted/placeholder name `bar`
warning: use of a blacklisted/placeholder name `bar`
warning: very complex type used. Consider factoring parts into `type` definitions
warning: very complex type used. Consider factoring parts into `type` definitions
warning: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
warning: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
warning: you seem to want to iterate on a map's values
warning: you seem to want to iterate on a map's values
warning: you seem to want to iterate on a map's values
warning: you seem to want to iterate on a map's values

[iceiix]

Continuing to make progress, but have a few left to resolve. Down to 25 warnings total:

8	warning: you seem to want to iterate on a map's values
4	warning: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
4	warning: very complex type used. Consider factoring parts into `type` definitions
4	warning: use of a blacklisted/placeholder name `bar`
4	warning: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
4	warning: length comparison to zero
4	warning: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
2	warning: this public function dereferences a raw pointer but is not marked `unsafe`
2	warning: this block may be rewritten with the `?` operator
2	warning: the loop variable `i` is used to index `block_types`
2	warning: the loop variable `i` is used to index `block_meta`
2	warning: the loop variable `i` is used to index `block_add`
2	warning: large size difference between variants
2	warning: calling `as_bytes()` on a string literal
2	warning: bit mask could be simplified with a call to `trailing_zeros`
2	warning: `if` chain can be rewritten with `match`
2	warning: 25 warnings emitted
1	warning: lint name `all` is deprecated and does not have an effect anymore. Use: clippy::all
1	warning: file found to be present in multiple build targets: /Users/admin/games/rust/steven/src/main.rs
1	warning: 1 warning emitted