Warn on indexing in general?

[shnewto]

👋
I'm interested in contributing a lint I've put together but would love some feedback before starting the PR process. The general idea is that I'd like to dissuade behavior that could cause a runtime panic in my project, in this case, indexing.

Some preliminary work behaves something like the following:

fn main() {
    // Vector containing a single element.
    let x = vec![0;1];
    let _a = x[10];
    //~^ WARNING indexing can result in runtime panics
    let _b = &x[10..]; 
    //~^ WARNING ranged indexing with start/end values can result in runtime panics
    let _c = &x[..10]; 
    //~^ WARNING ranged indexing with start/end values can result in runtime panics
    let _d = &x[10..100]; 
    //~^ WARNING ranged indexing with start/end values can result in runtime panics
    let _e = &x[..]; // OK
}

I guess to start, I'm interested in whether this lint seems like could fit in here. If it does we can go from there, thanks!

[oli-obk]

Well.. indexing is fine if you know the value cannot be out of bounds. We should write such a lint as a MIR processing lint that triggers on all reachable panics. This is nontrivial to do, but we can start with a simple one and keep improving it. Note that this has a lot of overlap with the const propagator in rustc, so we should probably keep improving that one instead and create an RFC for a no-panic lint

[shnewto]

@oli-obk thanks for the feedback, I love the idea of a more general no-panic lint (especially if it won't warn on a value that cannot be out of bounds). I'm unclear on what would be a productive next step for me though. I'm certainly interested in the idea of working toward some familiarity with MIR, whether that means starting simple like you mentioned or digging into the rustc const propagator.

[oli-obk]

I think for now the lint as you originally suggested sounds like a good medium term solution. The full analysis version is still far on the horizon. Feel free to open a PR to add it to the pedantic group.

It would be great if indexing with compile-time indices into arrays would not report the lint as long as the index is in bounds.

[shnewto]

@oli-obk I believe that I can avoid reporting in-bounds array indexes by using some of the functionality that already exists in the array_indexing lint here https://github.com/shnewto/rust-clippy/blob/master/clippy_lints/src/array_indexing.rs

I'd like to avoid as much code duplication as possible though and am interested in any thoughts around it. One strategy would be incorporate the out-of-bounds check from array_indexing into the lint I'm proposing and then move array_indexing into the deprecated lints (if I'm checking if the index is in bounds, I'll know if it isn't...). That would mean some of the" indexing in general" lint would be registered as clippy_pedantic lints and some as clippy_restriction lints. Is that acceptable? I'm open to any suggestions / other strategies if it isn't.

[oli-obk]

You need to create different lints for different groups, you can't have parts of a lint in different groups. But that's absolutely no problem. Create as many fine grained lints as you think makes sense.