Skip to content

Commit 09edb05

Browse files
committed
Add new lint: manual_slice_match
Add a new pedantic lint that detects `if`/`else if` chains whose conditions only inspect the length/emptiness of one and the same slice or `Vec` (via `.is_empty()` or `.len()` compared against an integer literal) and rewrites them as a `match` on a slice pattern. The lint is conservative: it requires at least two conditions over the same receiver and an explicit final `else`, and rejects chains that mix in value guards. It generates concrete arms (`[]`, `[_]`, `[_, _, ..]`) where the length predicates are expressible as slice patterns, and falls back to a help-only diagnostic for comparisons that cannot be a single pattern set (`<`, `<=`, `!=`). Arrays are not linted, since their length is a compile-time constant. The suggestion is `MaybeIncorrect` (bodies are copied verbatim and matching a `Vec` borrows the receiver across the arms). It is gated on the slice-patterns MSRV (1.42).
1 parent 47c0125 commit 09edb05

6 files changed

Lines changed: 510 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7025,6 +7025,7 @@ Released 2018-09-13
70257025
[`manual_rotate`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rotate
70267026
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
70277027
[`manual_slice_fill`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_fill
7028+
[`manual_slice_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_match
70287029
[`manual_slice_size_calculation`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_size_calculation
70297030
[`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once
70307031
[`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
322322
crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
323323
crate::manual_retain::MANUAL_RETAIN_INFO,
324324
crate::manual_rotate::MANUAL_ROTATE_INFO,
325+
crate::manual_slice_match::MANUAL_SLICE_MATCH_INFO,
325326
crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,
326327
crate::manual_string_new::MANUAL_STRING_NEW_INFO,
327328
crate::manual_strip::MANUAL_STRIP_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ mod manual_range_patterns;
221221
mod manual_rem_euclid;
222222
mod manual_retain;
223223
mod manual_rotate;
224+
mod manual_slice_match;
224225
mod manual_slice_size_calculation;
225226
mod manual_string_new;
226227
mod manual_strip;
@@ -862,6 +863,7 @@ rustc_lint::late_lint_methods!(
862863
ByteCharSlice: byte_char_slices::ByteCharSlice = byte_char_slices::ByteCharSlice,
863864
ManualAssertEq: manual_assert_eq::ManualAssertEq = manual_assert_eq::ManualAssertEq,
864865
WithCapacityZero: with_capacity_zero::WithCapacityZero = with_capacity_zero::WithCapacityZero,
866+
ManualSliceMatch: manual_slice_match::ManualSliceMatch = manual_slice_match::ManualSliceMatch::new(conf),
865867
// add late passes here, used by `cargo dev new_lint`
866868
]]
867869
);
Lines changed: 310 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,310 @@
1+
use clippy_config::Conf;
2+
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
3+
use clippy_utils::msrvs::{self, Msrv};
4+
use clippy_utils::res::MaybeDef;
5+
use clippy_utils::source::{snippet_indent, snippet_opt};
6+
use clippy_utils::sugg::Sugg;
7+
use clippy_utils::{SpanlessEq, if_sequence, is_else_clause, is_in_const_context, sym};
8+
use rustc_ast::LitKind;
9+
use rustc_data_structures::packed::Pu128;
10+
use rustc_errors::Applicability;
11+
use rustc_hir::{BinOpKind, Block, Expr, ExprKind};
12+
use rustc_lint::{LateContext, LateLintPass};
13+
use rustc_middle::ty::{self, Ty};
14+
use rustc_session::impl_lint_pass;
15+
use rustc_span::SyntaxContext;
16+
use std::fmt::Write;
17+
18+
const MSG: &str = "`if` chain checking only the length can be rewritten with a `match` on a slice pattern";
19+
20+
declare_clippy_lint! {
21+
/// ### What it does
22+
/// Checks for `if`/`else if` chains whose conditions only inspect the
23+
/// length of one and the same slice, `Vec` or array (via `.is_empty()` or
24+
/// `.len()` compared against an integer literal) and which can be rewritten
25+
/// as a `match` on a slice pattern.
26+
///
27+
/// ### Why is this bad?
28+
/// `if` chains are not checked for exhaustiveness and the length checks tend
29+
/// to be repetitive. A `match` on a slice pattern is exhaustive and makes the
30+
/// handled shapes explicit, and the bound elements can be named instead of
31+
/// being indexed (which avoids potential out-of-bounds panics).
32+
///
33+
/// ### Example
34+
/// ```no_run
35+
/// # let v: Vec<u32> = vec![];
36+
/// if v.is_empty() {
37+
/// // ...
38+
/// } else if v.len() == 1 {
39+
/// println!("{}", v[0]);
40+
/// } else {
41+
/// // ...
42+
/// }
43+
/// ```
44+
/// Use instead:
45+
/// ```no_run
46+
/// # let v: Vec<u32> = vec![];
47+
/// match v.as_slice() {
48+
/// [] => { /* ... */ }
49+
/// [single] => println!("{single}"),
50+
/// _ => { /* ... */ }
51+
/// }
52+
/// ```
53+
#[clippy::version = "1.98.0"]
54+
pub MANUAL_SLICE_MATCH,
55+
pedantic,
56+
"`if` length-check chains that can be rewritten as a `match` on a slice pattern"
57+
}
58+
59+
impl_lint_pass!(ManualSliceMatch => [MANUAL_SLICE_MATCH]);
60+
61+
pub struct ManualSliceMatch {
62+
msrv: Msrv,
63+
}
64+
65+
impl ManualSliceMatch {
66+
pub fn new(conf: &'static Conf) -> Self {
67+
Self { msrv: conf.msrv }
68+
}
69+
}
70+
71+
impl<'tcx> LateLintPass<'tcx> for ManualSliceMatch {
72+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
73+
if expr.span.from_expansion() {
74+
return;
75+
}
76+
77+
// Only look at the top-most `if` in the chain.
78+
if is_else_clause(cx.tcx, expr) {
79+
return;
80+
}
81+
82+
if is_in_const_context(cx) {
83+
return;
84+
}
85+
86+
if !self.msrv.meets(cx, msrvs::SLICE_PATTERNS) {
87+
return;
88+
}
89+
90+
let (conds, blocks) = if_sequence(expr);
91+
92+
// Require at least two conditions and an explicit final `else`, so the
93+
// chain is a genuine partition over the collection's shape.
94+
if conds.len() < 2 || blocks.len() != conds.len() + 1 {
95+
return;
96+
}
97+
98+
// Every condition must be a length/emptiness predicate over the *same*
99+
// receiver. The first condition fixes the receiver, the rest must match.
100+
let Some((recv, _, _)) = len_predicate(conds[0]) else {
101+
return;
102+
};
103+
104+
let mut spanless_eq = SpanlessEq::new(cx);
105+
for cond in &conds[1..] {
106+
match len_predicate(cond) {
107+
Some((other, _, _)) if spanless_eq.eq_expr(SyntaxContext::root(), recv, other) => {},
108+
_ => return,
109+
}
110+
}
111+
112+
// The receiver has to be something we can match on as a slice.
113+
let ty = cx.typeck_results().expr_ty(recv).peel_refs();
114+
let Some(scrutinee) = slice_scrutinee(cx, ty, recv) else {
115+
return;
116+
};
117+
118+
// Try to build the full `match` with concrete slice-pattern arms. If the
119+
// chain uses comparisons we cannot express as a single set of patterns
120+
// (e.g. `len() < n` or `len() != n`), fall back to a help-only diagnostic.
121+
if let Some(sugg) = build_match(cx, &conds, &blocks, &scrutinee, expr.span) {
122+
span_lint_and_sugg(
123+
cx,
124+
MANUAL_SLICE_MATCH,
125+
expr.span,
126+
MSG,
127+
"consider rewriting the `if` chain with a `match`",
128+
sugg,
129+
// The bodies are copied verbatim (still using indexing), and matching a
130+
// `Vec` via `as_slice()` holds a borrow across the arms, so a body that
131+
// moves out of the receiver would not compile. Leave it to the user.
132+
Applicability::MaybeIncorrect,
133+
);
134+
} else {
135+
span_lint_and_help(
136+
cx,
137+
MANUAL_SLICE_MATCH,
138+
expr.span,
139+
MSG,
140+
None,
141+
format!("rewrite this as a `match` on a slice pattern, e.g. `match {scrutinee} {{ .. }}`"),
142+
);
143+
}
144+
}
145+
}
146+
147+
/// Builds the full `match` replacement text with one arm per `if`/`else if`
148+
/// branch plus a wildcard arm for the final `else`. Returns `None` if any
149+
/// condition cannot be turned into a slice pattern.
150+
fn build_match<'tcx>(
151+
cx: &LateContext<'tcx>,
152+
conds: &[&'tcx Expr<'tcx>],
153+
blocks: &[&'tcx Block<'tcx>],
154+
scrutinee: &str,
155+
span: rustc_span::Span,
156+
) -> Option<String> {
157+
let indent = snippet_indent(cx, span).unwrap_or_default();
158+
let arm_indent = format!("{indent} ");
159+
let last = conds.len() - 1;
160+
161+
let mut arms = String::new();
162+
let mut exact_lengths = Vec::new();
163+
for (i, cond) in conds.iter().enumerate() {
164+
let (_, op, n) = len_predicate(cond)?;
165+
let pat = arm_pattern(op, n, i == last)?;
166+
// Two arms matching the same fixed length would make the second one
167+
// unreachable; bail rather than emit a suggestion that warns.
168+
if let Some(len) = pat.exact_len {
169+
if exact_lengths.contains(&len) {
170+
return None;
171+
}
172+
exact_lengths.push(len);
173+
}
174+
let body = reindent_block(&snippet_opt(cx, blocks[i].span)?);
175+
let _ = writeln!(arms, "{arm_indent}{} => {body}", pat.text);
176+
}
177+
178+
let else_body = reindent_block(&snippet_opt(cx, blocks[last + 1].span)?);
179+
let _ = writeln!(arms, "{arm_indent}_ => {else_body}");
180+
181+
Some(format!("match {scrutinee} {{\n{arms}{indent}}}"))
182+
}
183+
184+
/// Indents every line of a block snippet except the first by one level, so a body
185+
/// lifted from an `if` branch nests correctly under its new `match` arm.
186+
fn reindent_block(snippet: &str) -> String {
187+
let mut out = String::new();
188+
for (i, line) in snippet.lines().enumerate() {
189+
if i != 0 {
190+
out.push('\n');
191+
if !line.is_empty() {
192+
out.push_str(" ");
193+
}
194+
}
195+
out.push_str(line);
196+
}
197+
out
198+
}
199+
200+
struct ArmPat {
201+
text: String,
202+
/// `Some(n)` when the pattern matches exactly `n` elements.
203+
exact_len: Option<u128>,
204+
}
205+
206+
/// Maps a normalized `len <op> n` predicate to a slice pattern. Open-ended
207+
/// patterns (`> n`, `>= n`) are only allowed in the final condition, where the
208+
/// following wildcard arm keeps the `match` exhaustive without overlap.
209+
fn arm_pattern(op: BinOpKind, n: u128, is_last: bool) -> Option<ArmPat> {
210+
let underscores = |k: u128| -> Option<String> { Some(vec!["_"; usize::try_from(k).ok()?].join(", ")) };
211+
match op {
212+
// `len() == n` -> exactly `n` elements.
213+
BinOpKind::Eq => Some(ArmPat {
214+
text: format!("[{}]", underscores(n)?),
215+
exact_len: Some(n),
216+
}),
217+
// `len() > n` -> at least `n + 1` elements.
218+
BinOpKind::Gt if is_last => Some(ArmPat {
219+
text: format!("[{}, ..]", underscores(n + 1)?),
220+
exact_len: None,
221+
}),
222+
// `len() >= n` -> at least `n` elements (`n == 0` would match everything).
223+
BinOpKind::Ge if is_last && n >= 1 => Some(ArmPat {
224+
text: format!("[{}, ..]", underscores(n)?),
225+
exact_len: None,
226+
}),
227+
_ => None,
228+
}
229+
}
230+
231+
/// If `cond` is a length/emptiness predicate, returns its receiver together with
232+
/// the comparison normalized to `len <op> n` form (the literal on the right) and
233+
/// the literal value. `recv.is_empty()` is treated as `len == 0`.
234+
fn len_predicate<'tcx>(cond: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, BinOpKind, u128)> {
235+
match cond.kind {
236+
// `recv.is_empty()`
237+
ExprKind::MethodCall(path, recv, [], _) if path.ident.name == sym::is_empty => Some((recv, BinOpKind::Eq, 0)),
238+
// `recv.len() <cmp> <int>` or `<int> <cmp> recv.len()`
239+
ExprKind::Binary(op, lhs, rhs) if is_len_cmp(op.node) => {
240+
if let Some(recv) = len_receiver(lhs)
241+
&& let Some(n) = int_lit_val(rhs)
242+
{
243+
Some((recv, op.node, n))
244+
} else if let Some(recv) = len_receiver(rhs)
245+
&& let Some(n) = int_lit_val(lhs)
246+
{
247+
// Flip the operator so the literal ends up on the right.
248+
Some((recv, flip_cmp(op.node), n))
249+
} else {
250+
None
251+
}
252+
},
253+
_ => None,
254+
}
255+
}
256+
257+
/// Returns the receiver of a `recv.len()` call.
258+
fn len_receiver<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
259+
if let ExprKind::MethodCall(path, recv, [], _) = expr.kind
260+
&& path.ident.name == sym::len
261+
{
262+
Some(recv)
263+
} else {
264+
None
265+
}
266+
}
267+
268+
fn int_lit_val(expr: &Expr<'_>) -> Option<u128> {
269+
if let ExprKind::Lit(lit) = expr.kind
270+
&& let LitKind::Int(Pu128(n), _) = lit.node
271+
{
272+
Some(n)
273+
} else {
274+
None
275+
}
276+
}
277+
278+
/// Swaps the operands of a comparison: `a <op> b` is equivalent to `b <flipped> a`.
279+
fn flip_cmp(kind: BinOpKind) -> BinOpKind {
280+
match kind {
281+
BinOpKind::Lt => BinOpKind::Gt,
282+
BinOpKind::Le => BinOpKind::Ge,
283+
BinOpKind::Gt => BinOpKind::Lt,
284+
BinOpKind::Ge => BinOpKind::Le,
285+
other => other,
286+
}
287+
}
288+
289+
fn is_len_cmp(kind: BinOpKind) -> bool {
290+
matches!(
291+
kind,
292+
BinOpKind::Eq | BinOpKind::Ne | BinOpKind::Lt | BinOpKind::Le | BinOpKind::Gt | BinOpKind::Ge
293+
)
294+
}
295+
296+
/// Builds the `match` scrutinee text for a sliceable receiver, or `None` if the
297+
/// receiver type cannot be matched on as a slice pattern.
298+
fn slice_scrutinee<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, recv: &'tcx Expr<'tcx>) -> Option<String> {
299+
let sugg = Sugg::hir(cx, recv, "..");
300+
match ty.kind() {
301+
// Slices can be matched directly. Arrays are deliberately excluded: their
302+
// length is a compile-time constant, so a length-check chain over an array
303+
// is degenerate and a slice pattern of a different length would not even
304+
// type-check.
305+
ty::Slice(_) => Some(sugg.to_string()),
306+
// `Vec` needs an explicit conversion to a slice.
307+
_ if ty.is_diag_item(cx, sym::Vec) => Some(format!("{}.as_slice()", sugg.maybe_paren())),
308+
_ => None,
309+
}
310+
}

0 commit comments

Comments
 (0)