fix missing_asserts_for_indexing ignores match for length

[wasd243]

Fixes #17398

missing_asserts_for_indexing previously ignored match expressions, so indexing inside a match on the slice's length was linted even when the match arms already guarantee the indices are in bounds. For example:

match supported.len() { // `match` without assert
    0 => {},
    1 => println!("{}", supported[0]),
    _ => println!("{} or {}", supported[0], supported[1]),
}

Here the 0 and 1 arms rule out all lengths below 2, so the indexing in the wildcard arm cannot fail and the bounds checks are already elided without an assert!.

This PR suppresses the lint for an index inside a match on slice.len() when the non-wildcard arms are integer literals contiguously covering 0..=max (so the wildcard arm implies len > max) and the index is at most max.

The check is deliberately conservative and falls back to linting as before when:

• the arm literals don't contiguously cover 0..=max (e.g. 5 => .., _ => .., where _ can still be 0)
• any arm has a guard
• an arm uses an or-pattern or any non-literal pattern (e.g. 0 | 1 => .. is safe in principle but not analyzed)

One behavioral note: suppression is per-index. If a match guarantees len > 1 but the wildcard arm indexes [0], [1] and [2], only [2] remains unsuppressed, which is equivalent to a single unchecked access and therefore doesn't lint (consistent with the lint never firing on single accesses).

changelog: [missing_asserts_for_indexing]: fix false positive when a match on the slice length guarantees the indices are in bounds

[rustbot]

Thanks for the pull request, and welcome!You should hear from one of our reviewers after this PR is reviewed by at least 2 reviewers from the community

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[github-actions]

Lintcheck changes for cd5f376

Lint	Added	Removed	Changed
clippy::missing_asserts_for_indexing	0	2	1

This comment will be updated if you push new changes

[Gri-ffin]

This assumes that if we have a wildcard anywhere, then the entire match is safe, even if we are accessing indexes we can't safely assume.

fn foo(supported: &[u8]) {
    match supported.len() {
        0 => {},
        1 => {},
        2 => println!("{} {}", supported[0], supported[2]),
        _ => println!("{} {}", supported[0], supported[2]),
    }
}

View changes since the review

[wasd243]

Fixed -- literal arms now require index < n (the arm pins len to exactly n), instead of the shared index <= max check.
Lints now:

match supported.len() {
    0 => {},
    1 => {},
    2 => println!("{} {}", supported[2], supported[3]), // both out of bounds for len == 2 → lints
    _ => {},
}

The example doesn't lint but for a pre-existing reason unrelated to the suppression:

match supported.len() {
    0 => {},
    1 => {},
    2 => println!("{} {}", supported[0], supported[2]), // [0] in bounds → suppressed; [2] NOT suppressed
    _ => println!("{} {}", supported[0], supported[2]), // both in bounds (len >= 3) → suppressed
}

[CommanderStorm]

I think this lint suggestion can be tightened by using the structural pattern matching on slices.

Suggested change

	fn fix_match_case(supported: &[u8]) {
	match supported.len() {
	0 => {},
	1 => println!("{}", supported[0]),
	_ => println!("{} or {}", supported[0], supported[1]),
	}
	fn fix_match_case(supported: &[u8]) {
	match supported {
	[] => {},
	[first] => println!("{}", first),
	[first, second, ...] => println!("{} or {}", first, second),
	}

Much more work though and might be better in another lint.

View changes since the review

[eirnym]

@CommanderStorm pattern matching is not always available: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=0afb61b6183cf5bd030edea69c8a52e9

[CommanderStorm]

Yes, you neee to typecheck that it is an slice, if the alternative suggestion makes sense.

[rustbot]

☔ The latest upstream changes (possibly #17607) made this pull request unmergeable. Please resolve the merge conflicts.

[wasd243]

This PR may be out of scope.

It's difficult to reliably reason about potential panics in match/if let expressions.

Although the issue is a valid false positive, addressing it more generally would conflict with the current scope and behavior of the lint, and some of the related work would likely be better suited for a separate lint.

I think it would be better to close this PR temporarily.

[eirnym]

While I agree on generalization issue, is there a possibility to cover at least some cases. I don't even ask about suggestions to replace with, which can land in a documentation rather than be automatic replacements.

[CommanderStorm]

It's difficult to reliably reason about potential panics in match/if let expressions.

Can we boil this down to very simple patterns where the lint is save?
There is also Applicability::MaybeIncorrect as a lint if we are not sure.
Though we obviously only try to add lints where the false positive/negative rate is aceeptable/near zero.

Is there something that we can learn from your effort for this different lint?

• indexing_slicing ignores match for length #17399

[wasd243]

Can we boil this down to very simple patterns where the lint is save?

Maybe we could, but if we only use HIR's patterns and hardcode each specific cases, it would be complex to review the code and fix other cases FP/FN. Even if we only treat this as a false-positive case, the amount of code would be difficult to keep under control. I tried to handle this as a narrow false positive in this PR, but it's 100-200 lines added only for a match, and too many edge case appears such as

fn foo(supported: &[u8]) {
    match supported.len() {
        0 => {},
        1 => {},
        2 => println!("{} {}", supported[0], supported[2]),
        _ => println!("{} {}", supported[0], supported[2]),
    }
}

and y21 already suggested on #17399 an even better solution:

To be fair, maybe a general solution is possible using a dataflow pass on the MIR or something like that, and handling specific patterns is not so unreasonable for a lint that would be warn-by-default

In declare_clippy_lint!, the docs say:

Drawbacks
False positives. It is, in general, very difficult to predict how well
the optimizer will be able to elide bounds checks and it very much depends on
the surrounding code. For example, indexing into the slice yielded by the
slice::chunks_exact
iterator will likely have all of the bounds checks elided even without an assert
if the chunk_size is a constant.

This kind of false positive is already recorded in the lint's docs as a known limitation, and the docs explicitly note that these cases can be difficult to reason about.

For this, my personal answer is no; it may be out of scope.

Is the suggestion maybe incorrect?

Yes.

I tried this code, the same reproducer in issue #17398

fn issue17398(supported: &[u8]) {  
    match supported.len() { // `match` without assert  
        0 => {},  
        1 => println!("{}", supported[0]),  
        _ => println!("{} or {}", supported[0], supported[1]),  
        //~^^ missing_asserts_for_indexing  
    }  
}

error: test got exit code: 1, but expected 0
   --> tests/ui/missing_asserts_for_indexing.fixed:187:33
    |
187 |             1 => println!("{}", supported[0]),
    |                                 ^^^^^^^^^^^^ after rustfix is applied, all errors should be gone, but weren't
    |

full stderr:
error: indexing into a slice multiple times without an `assert`
  --> tests\ui\missing_asserts_for_indexing.fixed:187:33
   |
LL |             1 => println!("{}", supported[0]),
   |                                 ^^^^^^^^^^^^
LL |             _ => println!("{} or {}", supported[0], supported[1]),
   |                                       ^^^^^^^^^^^^  ^^^^^^^^^^^^
   |
   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`
   = note: asserting the length before indexing will elide bounds checks
   = note: `-D clippy::missing-asserts-for-indexing` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::missing_asserts_for_indexing)]`

error: aborting due to 1 previous error

In this false-positive case, rustfix does not apply the suggestion to the .fixed file, and the UI test fails because the lint is still emitted.

Is the current value being Applicability::MachineApplicable? I'm not sure for it, is there's already some issue/PR related on it?

If not, it would be better to change it into Applicability::MaybeIncorrect because this lint's false-positive is an issue that's already recorded in docs.

[eirnym]

It's worth a while to at least list all false positives you've met and struggled with, so there would be an extensive test scenarios for the future. This also would provide an overview, how problematic would be an implementation for general cases.

As for the fix, without knowledge o an exact type (e.g must be a slice for pattern matching for elements), it's impossible to suggest any fix, but it's worth a while to mention some in documentation, otherwise, this assert could become an annoyance.

[wasd243]

FPs edge cases found or mentioned in #17400

lint: [missing_asserts_for_indexing]

In this PR, we found these cases that cause false-positives.

I summarized them into this markdown file to record these cases which could be used as a checklist for future implementations.

I run the cargo uibless for these stderr outputs. Maybe the line number and spans look wired because I retests them separately, but if you put these in tests/ you'll get similar outputs, if you think I was wrong and some cases are NOT false-positives, please reply to me and I'll remove it.

FP-1:

fn match_len(supported: &[u8]) {  
    match supported.len() {  
        0 => {}  
        1 => println!("{}", supported[0]),  
        _ => println!("{} or {}", supported[0], supported[1]),  
    }  
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> tests/ui/missing_asserts_for_indexing.rs:187:33  
   |  
LL |             1 => println!("{}", supported[0]),  
   |                                 ^^^^^^^^^^^^  
LL |             _ => println!("{} or {}", supported[0], supported[1]),  
   |                                       ^^^^^^^^^^^^  ^^^^^^^^^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`  
  
error: aborting due to 16 previous errors

Reason: usize is not negative and value 0 was explicitly checked, lint should never fire.

---

FP-2:

fn if_let_len(supported: &[u8]) {  
    if let 0 | 1 = supported.len() {  
        println!("checks for supported len previously");  
    } else {  
        println!("{} {}", supported[0], supported[1]);  
    }  
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> tests/ui/missing_asserts_for_indexing.rs:188:31  
   |  
LL |             println!("{} {}", supported[0], supported[1]);  
   |                               ^^^^^^^^^^^^  ^^^^^^^^^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`  
  
error: aborting due to 16 previous errors

Reason: The if let 0 | 1 already checked so when the code executing to else {} the supported must be >= 2 (u8 has no negative number).

---

FP-3

fn let_else_len(supported: &[u8]) {  
    let (0 | 1) = supported.len() else {  
        println!("{} {}", supported[0], supported[1]);  
        return;  
    };  
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> tests/ui/missing_asserts_for_indexing.rs:186:31  
   |  
LL |             println!("{} {}", supported[0], supported[1]);  
   |                               ^^^^^^^^^^^^  ^^^^^^^^^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`  
  
error: aborting due to 16 previous errors

Reason: The lint requires adding an assert!(supported.len() > 1); before it, but we've already checked it by let .. else ... It's impossible that the supported.len() < 1.

---

FP-4 serde-1.0.204/src/de/mod.rs:2285

A minor reproduction for serde-1.0.204, this reproducer is NOT stays the same as the original one, click here to visit CI record.

#![warn(clippy::missing_asserts_for_indexing, reason="show-off")]  
use std::fmt::{self, Display};  
  
struct OneOf {  
    names: &'static [&'static str],  
}  
  
impl Display for OneOf {  
    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {  
        match self.names.len() {  
            0 => panic!(),  
            1 => write!(formatter, "`{}`", self.names[0]),  
            2 => write!(formatter, "`{}` or `{}`", self.names[0], self.names[1]),  
            _ => {  
                formatter.write_str("one of ")?;  
                for (i, alt) in self.names.iter().enumerate() {  
                    if i > 0 {  
                        formatter.write_str(", ")?;  
                    }  
                    write!(formatter, "`{}`", alt)?;  
                }  
                Ok(())  
            }  
        }  
    }  
}  
  
fn main() {  
    let one_of = OneOf { names: &["a", "b", "c"] };  
    println!("{}", one_of);  
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> src/main.rs:12:44  
   |  
12 |             1 => write!(formatter, "`{}`", self.names[0]),  
   |                                            ^^^^^^^^^^^^^  
13 |             2 => write!(formatter, "`{}` or `{}`", self.names[0], self.names[1]),  
   |                                                    ^^^^^^^^^^^^^  ^^^^^^^^^^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(self.names.len() > 1);`  
   = note: asserting the length before indexing will elide bounds checks  
   = note: `-D clippy::missing-asserts-for-indexing` implied by `-D warnings`  
   = help: to override `-D warnings` add `#[allow(clippy::missing_asserts_for_indexing)]`  
  
error: could not compile `missing_asserts_for_indexing_false_positive` (bin "missing_asserts_for_indexing_false_positive") due to 1 previous error

Reason: self.names.len() is matched against 0, 1, and 2 before falling into _. In the 1 arm, len() == 1 is guaranteed, so self.names[0] is in bounds. In the 2 arm, len() == 2 is guaranteed, so both self.names[0] and self.names[1] are in bounds. The _ arm (len >= 3) performs no indexing at all (only iterates via .iter()), so no assert is ever needed here.

---

FP-5

fn guard_len(supported: &[u8]) {  
    match supported.len() {  
        n if n < 2 => {}  
        _ => println!("{} {}", supported[0], supported[1]),  
    }  
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> tests/ui/missing_asserts_for_indexing.rs:186:32  
   |  
LL |         _ => println!("{} {}", supported[0], supported[1]),  
   |                                ^^^^^^^^^^^^  ^^^^^^^^^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`

Reason: The guard already ensured supported.len() >= 2, but linted.

---

FP-6

fn range_arm_len(supported: &[u8]) {  
    match supported.len() {  
        0..=1 => {}  
        _ => println!("{} {}", supported[0], supported[1]),  
    }  
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> tests/ui/missing_asserts_for_indexing.rs:193:32  
   |  
LL |         _ => println!("{} {}", supported[0], supported[1]),  
   |                                ^^^^^^^^^^^^  ^^^^^^^^^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`

Reason: The range already ensured the supported.len() >= 2, but linted.

---

FP-7 prettypretty/crates/prettytty/src/util.rs

A minor reproduction based on render_hexdump's inner loop

fn render_pairs(chunk: &[u8]) {  
    for pair in chunk.chunks(2) {  
        // Allow for uneven number of bytes in final chunk.  
        print!("{:02x}", pair[0]);  
        if pair.len() == 1 {  
            print!("   ");  
        } else {  
            print!("{:02x} ", pair[1]);  
        }  
    }  
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> tests/ui/missing_asserts_for_indexing.rs:186:26  
   |  
LL |         print!("{:02x}", pair[0]);  
   |                          ^^^^^^^  
...  
LL |             print!("{:02x} ", pair[1]);  
   |                               ^^^^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(pair.len() > 1);`  
  
error: aborting due to 16 previous errors

Reason: the if expr already checked pair.len() >=2, but linted.

---

FP-8

fn some_value_inside_and_do_some_calc() {  
    let mut buf = [0u8; 13];  
    let word = &mut buf[6..=10];  
    word[0] = b'e';  
    word[1] = b'a';  
    word[2] = b'r';  
    word[3] = b't';  
    word[4] = b'h';  
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> tests/ui/missing_asserts_for_indexing.rs:186:5  
   |  
LL |     word[0] = b'e';  
   |     ^^^^^^^  
LL |     word[1] = b'a';  
   |     ^^^^^^^  
LL |     word[2] = b'r';  
   |     ^^^^^^^  
LL |     word[3] = b't';  
   |     ^^^^^^^  
LL |     word[4] = b'h';  
   |     ^^^^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(word.len() > 4);`  
  
error: aborting due to 16 previous errors

Reason: 6..=10 is a closed range, so its length is fixed at 10-6+1=5. It's impossible that word.len() != 5.

---

FP-9

fn is_sorted_strict(slice: &[u8]) -> bool { 
	slice.windows(2).all(|w| w[0] < w[1]) 
}

stderr output

error: indexing into a slice multiple times without an `assert`  
  --> tests/ui/missing_asserts_for_indexing.rs:184:30  
   |  
LL |     slice.windows(2).all(|w| w[0] < w[1])  
   |                              ^^^^   ^^^^  
   |  
   = help: consider asserting the length before indexing: `assert!(w.len() > 1);`  
  
error: aborting due to 16 previous errors

Reason: w.len() = 2 .

---

If you are reading this and trying to fix this lint's FP, I definitely suggest you try to use MIR find a general solution rather than hardcode some specific cases, there are lots of #[allow(clippy::missing_asserts_for_indexing)] on GitHub. And I summarized these cases as some examples which have representativity.

#17398 and #17399 are similar issue and in my opinion both of them maybe could be fixed with MIR but not HIR.

I left this as a type of checklist, though we have lint check in CI, if you successfully fix this, you probably should see a lots of FPs have been removed (or not, because almost everybody use #[allow], so I'm really not sure about this PR and close it).

Notes

This record ONLY for missing_asserts_for_indexing lint, if you come from #17399 you'd better treat this as an example and DO NOT put these in indexing_slicing tests (These are only some representatives FP cases for missing_asserts_for_indexing, not indexing_slicing).

All above only represent my PERSONAL suggestion, if you want to fix it in other ways I appreciated that and please only treat this as an normal examples.

[eirnym]

Thank you for the comprehensive list of your findings.

My comments:

FP-7: There's no guarantee, that pair has at least 1 element as per slice.chunks documentation. Documentation advises to use as_chunks, which has this guarantee for first part of the result and the same absence of guarantee of the tail chunk length.

FP-9: I agree here, as documentation for slice::windows tells explicitly Returns an iterator over all contiguous windows of length size. The windows overlap. If the slice is shorter than size, the iterator returns no values..

side note: I'm curious why there's no const fn as_windows<const N: usize>()-> &[[T;N]] similar to as_chunks method.

FP-8: we have all lengths checked by construction.

FP-5: complex match, where MIR is required.

and at least some of the rest examples were covered by this PR.