Couple Clippy questions

code review
1
  1. Clippy gives me the warning:
Clipping rust/upload ...
warning: passing a unit value to a function
 --> rust/upload.rs:8:5
  |
8 |     Ok(Upload{}.show())
  |     ^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
  = note: `#[warn(clippy::unit_arg)]` on by default
help: move the expression in front of the call and replace it with the unit literal `()`
  |
8 ~     Upload{}.show();
9 +     Ok(())
  |

I read the clippy explanation and the risk of my code missed ';'. Indeed, I have no ';', but I do not need them. Any other reason why my code is bad?
2. I program in Rust using C style, so Clippy corrected me. Indeed, I tried the Clippy recommendation and it is 15% faster. But why? There is the code:

use std::error::Error;
use std::time::Instant;
trait CharLen {
    fn char_len(&self) -> usize ;
}

impl CharLen for String {
    fn char_len(&self) -> usize {
        self.chars().count()
    }
}

fn main() -> Result<(), Box<dyn Error>> {
    let s = "test ΑΒΓΔΕΖΗΘ".to_string();
    let size = 1001;
    let mut children = Vec::with_capacity(size);
    for _ in 0..size {
        children.push(s.clone())
    }
    let mut tot_len = 0;
    let start = Instant::now();
    for _ in 0..1000000 {
        for idx in 1..size {
            tot_len += children[idx].char_len()
        }
    }
    let duration = start.elapsed();

    println!("DR Time elapsed: {:?} - {tot_len}", duration);
    tot_len = 0;
    let start = Instant::now();
    for _ in 0..1000000 {
        for child in children.iter().take(size).skip(1) {
            tot_len += child.char_len()
        }
    }
    let duration = start.elapsed();

    println!("Clippy Time elapsed: {:?} - {tot_len}", duration);
    Ok(())
}

Is the C style code problem that it needs to check an index boundary from both sides, and for an iterator only upper boundary?

2
  1. This looks to a rust programmer like show returns an important value. In general, people assume that nesting in code has a purpose. But since that's not the case, it looks a little strange. You might've called the wrong function.
  2. Your code has to check the length of children on every iteration. In an ideal world this would be optimized to be the same as the other version, but here we are. Also, the corrected version isn't entirely equivalent. If size is past the end of children, it will add less elements instead of panicking. If that's what you want, then fine, but this would be a closer translation:
for child in &children[1..size] {
3 Likes
3

For what it's worth, some consider the first one a false positive (and some don't).

4

Another false positive I found recently. But actually Cipply not correctly provides a remedy:

warning: this `if` has identical blocks
    --> src/rust/terminal.rs:1510:73
     |
1510 |                       if file.is_file() && fs::remove_file(&file).is_ok() {
     |  _________________________________________________________________________^
1511 | |                            succ_count += 1 
1512 | |                     } else if file.is_dir() && fs::remove_dir_all(&file).is_ok() {
     | |_____________________^
     |
note: same as this
    --> src/rust/terminal.rs:1512:82
     |
1512 |                       } else if file.is_dir() && fs::remove_dir_all(&file).is_ok() {
     |  __________________________________________________________________________________^
1513 | |                         succ_count += 1
1514 | |                     }
     | |_____________________^
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
     = note: `#[warn(clippy::if_same_then_else)]` on by default

I resolved it by consolidating the if conditions by ||.

5

Related: