Lint for warning on division of integer literals

[gsingh93]

It would be nice to warn the user if they wrote an expression like 1/2 and meant 0.5. There is also an open issue for this in Rust: rust-lang/rust#21737

[gsingh93]

Also, personally I'd like a lint for any integer division as well (not just literals). When I'm working on an application that specifically uses floating point numbers, every integer division I've done has been a bug.

[llogiq]

That's pretty easy – we already can get the (given or inferred) type of any expr using rustc::middle::ty. So we'd just search for ExprBinary(Spanned{node: BiDiv, ..}, ref left, ref right) where both left and right are any of {isize, i8, i16, i32, i64, usize, u8, u16, u32, u64} (for now – there well could be integral type.s with more bits in the future – perhaps we should ask rustc folks for an .is_integral() method?)

However, it's pretty easy to get around it, e.g. (1f32 / 2f32) as u32 would not warn, yet truncate the result anyway. As such, I'm not sure if warning is the right thing to do. Perhaps add it Allow-by-default and see how many matches pop up in actual code?

[Zarathustra30]

-1 on a lint for all integer division. I use it quite often, and would hate to add boilerplate just to get around a lint.

I support a lint for literals, but I could see something like 1000/7 being more understandable than 142.

[llogiq]

That's pretty much what I think – the compiler cannot detect your intent, and there are many algorithms where integer division is intentional.

[gsingh93]

I would like the lint to be off by default. There are only certain times I
want it.

On Sat, Jul 11, 2015, 12:59 PM llogiq notifications@github.com wrote:

That's pretty much what I think – the compiler cannot detect your intent,
and there are many algorithms where integer division is intentional.

—
Reply to this email directly or view it on GitHub
https://github.com/Manishearth/rust-clippy/issues/109#issuecomment-120644640
.

[gsingh93]

Also, I'd prefer 1000./7. as u8 over 142

On Sat, Jul 11, 2015, 2:06 PM Gulshan Singh gsingh_2011@yahoo.com wrote:

I would like the lint to be off by default. There are only certain times I
want it.

On Sat, Jul 11, 2015, 12:59 PM llogiq notifications@github.com wrote:

That's pretty much what I think – the compiler cannot detect your intent,
and there are many algorithms where integer division is intentional.

—
Reply to this email directly or view it on GitHub
https://github.com/Manishearth/rust-clippy/issues/109#issuecomment-120644640
.

[birkenfeld]

I also think this should not lint. Integer division used to be pretty confusing e.g. in Python because of dynamic typing, but in Rust it should be pretty much unneeded.

[gsingh93]

I really don't see the problem with making this a lint that is off by default. I don't want this everywhere, just like everyone else. However, I've written multiple applications that have been very heavy on floating point divisions where I would have like to enable this feature. In these applications, not a single integer division was valid, but I accidentally made this mistake multiple times.

[llogiq]

In what language did you write those applications?

[Manishearth]

FWIW I don't think there should be a high bar for inclusion of Allow lints in clippy 😄

[llogiq]

Full ack.

[gsingh93]

@llogiq Yes. Here's a small example of an integer division bug:

fn main() {
    let (width, height) = (1080, 720);
    let aspect_ratio = width / height;

    let (x, y) = (500, 500);
    let (scaled_x, scaled_y) = (aspect_ratio * x, aspect_ratio * y);

    println!("{} {}", x, y);
    println!("{} {}", scaled_x, scaled_y);
}

Obviously, not every application that needs to calculate the aspect ratio would enable this lint, but there are certain applications where I'd know ahead of time this lint would be useful. In the rare cases where I'd be wrong about the lint being useful, I'd disable it, possibly just for a particular function.

[gsingh93]

Oops, I only meant to scale the x coordinate in the last example. Irrelevant to the example, but just in case someone calls me out on that later.

[thiagoarrais]

Is this still relevant? I'd like to work on it if so.

[oli-obk]

yes, we still want that, albeit as allow by default