fix missing_asserts_for_indexing ignores match for length

clippy_lints/src/missing_asserts_for_indexing.rs

@@ -7,12 +7,12 @@ use clippy_utils::higher::{If, Range};
 use clippy_utils::macros::{find_assert_eq_args, first_node_macro_backtrace, root_macro_call};
 use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::visitors::for_each_expr_without_closures;
-use clippy_utils::{eq_expr_value, hash_expr};
+use clippy_utils::{eq_expr_value, hash_expr, is_wild};
 use rustc_ast::{BinOpKind, LitKind, RangeLimits};
 use rustc_data_structures::packed::Pu128;
 use rustc_data_structures::unhash::UnindexMap;
 use rustc_errors::{Applicability, Diag};
-use rustc_hir::{Block, Body, Expr, ExprKind, UnOp};
+use rustc_hir::{Arm, Block, Body, Expr, ExprKind, MatchSource, Node, PatExprKind, PatKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::{Span, Spanned, Symbol, sym};
@@ -123,6 +123,101 @@ fn len_comparison<'hir>(
     }
 }
 
+/// Checks if the index expression is inside a `match` on the slice's length
+/// whose arms make the indexing infallible.
+/// This only applies when the non-wildcard arms are integer literals that
+/// contiguously cover `0..=max` otherwise the wildcard
+/// arm gives no lower bound on the length and the lint fires as usual.
+fn is_index_covered_by_match(cx: &LateContext<'_>, index_expr: &Expr<'_>, slice: &Expr<'_>) -> bool {
+    let mut containing_arm = None;
+    for (_, node) in cx.tcx.hir_parent_iter(index_expr.hir_id) {
+        match node {
+            Node::Expr(expr) => {
+                if let ExprKind::Match(scrutinee, arms, MatchSource::Normal) = expr.kind
+                    && is_slice_len_expr(cx, scrutinee, slice)
+                    && match_arms_bound_len(cx, index_expr, arms, containing_arm)
+                {
+                    return arms.iter().any(|arm| is_wild(arm.pat));
+                }
+            },
+            Node::Block(_) | Node::Stmt(_) | Node::LetStmt(_) => {},
+            Node::Arm(arm) => containing_arm = Some(arm),
+            _ => return false,
+        }
+    }
+    false
+}
+
+/// Checks if the non-wildcard arms are integer literals contiguously covering
+/// `0..=max`, so that the wildcard arm implies `len > max`. Returns `true`
+/// only if the index used is at most `max`, i.e. provably in bounds.
+fn match_arms_bound_len(
+    cx: &LateContext<'_>,
+    index_expr: &Expr<'_>,
+    arms: &[Arm<'_>],
+    containing_arm: Option<&Arm<'_>>,
+) -> bool {
+    let ExprKind::Index(_, index_lit, _) = index_expr.kind else {
+        return false;
+    };
+    let Some(index) = upper_index_expr(cx, index_lit) else {
+        return false;
+    };
+
+    // index inside a literal arm `n => ...`: len is exactly `n` there,
+    // so anything at or past `n` is out of bounds
+    if let Some(arm) = containing_arm
+        && let Some(n) = arm_len_literal(arm)
+        && index >= n
+    {
+        return false;
+    }
+
+    let mut matched_lens = Vec::new();
+    for arm in arms {
+        if arm.guard.is_some() {
+            return false;
+        }
+        if is_wild(arm.pat) {
+            continue;
+        }
+        let Some(n) = arm_len_literal(arm) else {
+            return false;
+        };
+        matched_lens.push(n);
+    }
+
+    matched_lens.sort_unstable();
+    matched_lens.dedup();
+
+    // literals must be exactly 0..=max, so `_` implies len > max
+    let Some(&max) = matched_lens.last() else {
+        return false;
+    };
+    matched_lens.len() == max + 1 && index <= max
+}
+
+fn arm_len_literal(arm: &Arm<'_>) -> Option<usize> {
+    if let PatKind::Expr(pat_expr) = arm.pat.kind
+        && let PatExprKind::Lit { lit, .. } = pat_expr.kind
+        && let LitKind::Int(Pu128(n), _) = lit.node
+    {
+        Some(n as usize)
+    } else {
+        None
+    }
+}
+
+fn is_slice_len_expr(cx: &LateContext<'_>, expr: &Expr<'_>, slice: &Expr<'_>) -> bool {
+    if let ExprKind::MethodCall(method, recv, [], _) = expr.kind
+        && method.ident.name == sym::len
+    {
+        eq_expr_value(cx, expr.span.ctxt(), recv, slice)
+    } else {
+        false
+    }
+}
+
 /// Attempts to extract parts out of an `assert!`-like expression
 /// in the form `assert!(some_slice.len() > 5)`.
 ///
@@ -239,6 +334,7 @@ fn check_index<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>, map: &mut Uni
     if let ExprKind::Index(slice, index_lit, _) = expr.kind
         && cx.typeck_results().expr_ty_adjusted(slice).peel_refs().is_slice()
         && let Some(index) = upper_index_expr(cx, index_lit)
+        && !is_index_covered_by_match(cx, expr, slice)
     {
         let hash = hash_expr(cx, slice);
 

tests/ui/missing_asserts_for_indexing.fixed

@@ -180,4 +180,35 @@ mod issue15988 {
     }
 }
 
+mod issue17398 {
+    // ok
+    fn fix_match_case(supported: &[u8]) {
+        match supported.len() {
+            0 => {},
+            1 => println!("{}", supported[0]),
+            _ => println!("{} or {}", supported[0], supported[1]),
+        }
+    }
+
+    // ok
+    fn one_index_too_high(supported: &[u8]) {
+        match supported.len() {
+            0 => {},
+            1 => {},
+            _ => println!("{} {} {}", supported[0], supported[1], supported[2]),
+        }
+    }
+
+    // The `2 =>` arm's `supported[2]` is NOT suppressed (len == 2 there),
+    // but single remaining access never lints, same as `single_access`.
+    fn literal_arm_single_unsuppressed(supported: &[u8]) {
+        match supported.len() {
+            0 => {},
+            1 => {},
+            2 => println!("{} {}", supported[0], supported[2]),
+            _ => println!("{} {}", supported[0], supported[2]),
+        }
+    }
+}
+
 fn main() {}

tests/ui/missing_asserts_for_indexing.rs

@@ -180,4 +180,35 @@ mod issue15988 {
     }
 }
 
+mod issue17398 {
+    // ok
+    fn fix_match_case(supported: &[u8]) {
+        match supported.len() {
+            0 => {},
+            1 => println!("{}", supported[0]),
+            _ => println!("{} or {}", supported[0], supported[1]),
+        }
+    }
+
+    // ok
+    fn one_index_too_high(supported: &[u8]) {
+        match supported.len() {
+            0 => {},
+            1 => {},
+            _ => println!("{} {} {}", supported[0], supported[1], supported[2]),
+        }
+    }
+
+    // The `2 =>` arm's `supported[2]` is NOT suppressed (len == 2 there),
+    // but single remaining access never lints, same as `single_access`.
+    fn literal_arm_single_unsuppressed(supported: &[u8]) {
+        match supported.len() {
+            0 => {},
+            1 => {},
+            2 => println!("{} {}", supported[0], supported[2]),
+            _ => println!("{} {}", supported[0], supported[2]),
+        }
+    }
+}
+
 fn main() {}

tests/ui/missing_asserts_for_indexing_unfixable.rs

@@ -85,4 +85,53 @@ fn issue14255(v1: &[u8]) {
     //~^ missing_asserts_for_indexing
 }
 
+// not contiguous from 0: `_` can still be 0
+fn non_contiguous(supported: &[u8]) {
+    match supported.len() {
+        5 => {},
+        _ => println!("{} or {}", supported[0], supported[1]),
+        //~^ missing_asserts_for_indexing
+    }
+}
+
+// wildcard only: no length information
+#[allow(clippy::match_single_binding)]
+fn wildcard_only(supported: &[u8]) {
+    match supported.len() {
+        _ => println!("{} or {}", supported[0], supported[1]),
+        //~^ missing_asserts_for_indexing
+    }
+}
+
+// guard defeats the exclusion
+fn with_guard(supported: &[u8], flag: bool) {
+    match supported.len() {
+        0 if flag => {},
+        1 => {},
+        _ => println!("{} or {}", supported[0], supported[1]),
+        //~^ missing_asserts_for_indexing
+    }
+}
+
+// arms only guarantee len > 1, but 2 and 3 are indexed
+fn index_too_high(supported: &[u8]) {
+    match supported.len() {
+        0 => {},
+        1 => {},
+        _ => println!("{} {}", supported[2], supported[3]),
+        //~^ missing_asserts_for_indexing
+    }
+}
+
+// literal arm pins len to exactly 2, so indexing at 2 and 3 must lint
+fn literal_arm_out_of_bounds(supported: &[u8]) {
+    match supported.len() {
+        0 => {},
+        1 => {},
+        2 => println!("{} {}", supported[2], supported[3]),
+        //~^ missing_asserts_for_indexing
+        _ => {},
+    }
+}
+
 fn main() {}

tests/ui/missing_asserts_for_indexing_unfixable.stderr

@@ -89,5 +89,45 @@ LL |     let _ = v1[0] + v1[1] + v1[2];
    |
    = help: consider asserting the length before indexing: `assert!(v1.len() > 2);`
 
-error: aborting due to 10 previous errors
+error: indexing into a slice multiple times without an `assert`
+  --> tests/ui/missing_asserts_for_indexing_unfixable.rs:92:35
+   |
+LL |         _ => println!("{} or {}", supported[0], supported[1]),
+   |                                   ^^^^^^^^^^^^  ^^^^^^^^^^^^
+   |
+   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`
+
+error: indexing into a slice multiple times without an `assert`
+  --> tests/ui/missing_asserts_for_indexing_unfixable.rs:101:35
+   |
+LL |         _ => println!("{} or {}", supported[0], supported[1]),
+   |                                   ^^^^^^^^^^^^  ^^^^^^^^^^^^
+   |
+   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`
+
+error: indexing into a slice multiple times without an `assert`
+  --> tests/ui/missing_asserts_for_indexing_unfixable.rs:111:35
+   |
+LL |         _ => println!("{} or {}", supported[0], supported[1]),
+   |                                   ^^^^^^^^^^^^  ^^^^^^^^^^^^
+   |
+   = help: consider asserting the length before indexing: `assert!(supported.len() > 1);`
+
+error: indexing into a slice multiple times without an `assert`
+  --> tests/ui/missing_asserts_for_indexing_unfixable.rs:121:32
+   |
+LL |         _ => println!("{} {}", supported[2], supported[3]),
+   |                                ^^^^^^^^^^^^  ^^^^^^^^^^^^
+   |
+   = help: consider asserting the length before indexing: `assert!(supported.len() > 3);`
+
+error: indexing into a slice multiple times without an `assert`
+  --> tests/ui/missing_asserts_for_indexing_unfixable.rs:131:32
+   |
+LL |         2 => println!("{} {}", supported[2], supported[3]),
+   |                                ^^^^^^^^^^^^  ^^^^^^^^^^^^
+   |
+   = help: consider asserting the length before indexing: `assert!(supported.len() > 3);`
+
+error: aborting due to 15 previous errors