Lint for Unnecessary Manual Bounds Checking

[mmstick]

I ran across a section of translated C code that was written as such to manually avoid accessing a value out of bounds and returning a default value if it is out of bounds:

let hash_value = if pos + ZOPFLI_MIN_MATCH <= array.len() {
    array[pos + ZOPFLI_MIN_MATCH - 1]
} else {
    0
};

However, it would be better to detect this behaviour and recommend the following:

let hash_value = array.iter().nth(pos + ZOPFLI_MIN_MATCH - 1).cloned().unwrap_or(0);

Or even the following for more complicated mapping.

let hash_value = array.iter().nth(pos + ZOPFLI_MIN_MATCH - 1).map_or(0, |value| *value);

[mcarton]

Decide between cloned and map_or might be hard (cc #443, #498), but otherwise 👍

[mmstick]

I would think that cloned would be ideal for simply copying the value in the case of |value| *value whereas map_or would be ideal for more complicated actions where you want to do more than just copying the value, like so:

let result_is_even = array.iter().nth(index).map_or(false, |value| *value % 2 == 0);

[mcarton]

Yes but it's hard to check whether cloned can be called because it has a weird bound.

[mcarton]

Oh actually this is Option::cloned which only needs T: Clone. It shouldn't be a problem to check then.

[Manishearth]

Its better to use .get() over iterators for arrays, no? That iterator might get optimized, but not in debug, and get is cleaner anyway.

[mcarton]

I guess we also need a lint for s/iter().nth/get/ then 😄