Warn about arithmetic before type cast/conversion

[oherrala]

What it does

It is easy mistake to do arithmetics before type conversion (or cast) instead of the other way around as probably intended.

E.g. when parsing data where length field is u16 and internally in Rust code we want to use usize.

fn main() {
    let value: u16 = std::hint::black_box(0xFFFF); // black box is to fool compiler in this example

    // this line is wrong and can lead to integer overflow:
    let sum: usize = usize::from(value + 2); // or: (value + 2) as usize

    println!("{sum}");
}

Link to playground

This code can lead to unexpected results since value + 2 is arithmetics on u16 instead of usize. It's also quite difficult to spot in code review.

The correct code should be:

    let sum: usize = usize::from(value) + 2;

This issue can be found with arithmetic_side_effects if enabled, but that lint is really noisy. I think this is special case of arithmetic side effects.

Advantage

The recommended code (type conversion before arithmetics) produces expected results while incorrect code (arithmetics before type conversion) can hide a bug for long time and cause difficult to debug bug in release builds.

And if overflow is really wanted then there is .overflowing_add() function for that already and making it really obvious.

Drawbacks

No response

Example

let parsed_value: u16 = parse_something();
let length: usize = usize::from(parsed_value + 2);

or

let parsed_value: u16 = parse_something();
let length: usize = (parsed_value + 2) as usize;

Could be written as:

let parsed_value: u16 = parse_something();
let length: usize = usize::from(parsed_value) + 2;

[zacknewman]

usize::from only works for u8 and u16 since the only guarantee you have from Rust is that usize is at least as large as u16. There are some platforms where usize and u16 are the same. This means your suggestion doesn't do anything different for those platforms other than performing an unnecessary conversion from one type to another type that is actually the same. This means your suggestion would have to also account for the target platform to decide if usize is in fact larger than u16. Even then, to some overflow may seem likely for u32 but not u64; so does the lint trigger when using u32 when targeting 64-bit platforms? What if u64 is likely to cause overflow, then does it trigger for 128-bit platforms? If so, you have to use usize::try_from anyway which seems weird since you're saying overflow/underflow can't occur when doing arithmetic but type conversion can? Massive assumptions are being made on when overflow/undeflow is "likely" to occur (e.g., that overflow/underflow won't magically occur when using the platform-sized integer).

More importantly, if you care about overflow/underflow; then you shouldn't assume it won't occur for certain types (e.g., usize). It seems very fragile to just hope it won't occur for certain "blessed" types.