Skip to content

Commit 7b4cd17

Browse files
committed
Start uplifting clippy::for_loops_over_fallibles
I refactored the code: - Removed handling of methods, as it felt entirely unnecessary - Removed clippy utils (obviously...) - Used some shiny compiler features (let-else is very handy for lints 👀) - I also renamed the lint to `for_loop_over_fallibles` (note: no `s`). I'm not sure what's the naming convention here, so maybe I'm wrong.
1 parent 2fbc08e commit 7b4cd17

File tree

2 files changed

+102
-0
lines changed

2 files changed

+102
-0
lines changed
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
use crate::{LateContext, LateLintPass, LintContext};
2+
3+
use hir::{Expr, Pat};
4+
use rustc_hir as hir;
5+
use rustc_middle::ty;
6+
use rustc_span::sym;
7+
8+
declare_lint! {
9+
/// ### What it does
10+
///
11+
/// Checks for `for` loops over `Option` or `Result` values.
12+
///
13+
/// ### Why is this bad?
14+
/// Readability. This is more clearly expressed as an `if
15+
/// let`.
16+
///
17+
/// ### Example
18+
///
19+
/// ```rust
20+
/// # let opt = Some(1);
21+
/// # let res: Result<i32, std::io::Error> = Ok(1);
22+
/// for x in opt {
23+
/// // ..
24+
/// }
25+
///
26+
/// for x in &res {
27+
/// // ..
28+
/// }
29+
///
30+
/// for x in res.iter() {
31+
/// // ..
32+
/// }
33+
/// ```
34+
///
35+
/// Use instead:
36+
/// ```rust
37+
/// # let opt = Some(1);
38+
/// # let res: Result<i32, std::io::Error> = Ok(1);
39+
/// if let Some(x) = opt {
40+
/// // ..
41+
/// }
42+
///
43+
/// if let Ok(x) = res {
44+
/// // ..
45+
/// }
46+
/// ```
47+
pub FOR_LOOP_OVER_FALLIBLES,
48+
Warn,
49+
"for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
50+
}
51+
52+
declare_lint_pass!(ForLoopOverFallibles => [FOR_LOOP_OVER_FALLIBLES]);
53+
54+
impl<'tcx> LateLintPass<'tcx> for ForLoopOverFallibles {
55+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
56+
let Some((pat, arg)) = extract_for_loop(expr) else { return };
57+
58+
let ty = cx.typeck_results().expr_ty(arg);
59+
60+
let ty::Adt(adt, _) = ty.kind() else { return };
61+
62+
let (article, ty, var) = match adt.did() {
63+
did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"),
64+
did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"),
65+
_ => return,
66+
};
67+
68+
let Ok(pat_snip) = cx.sess().source_map().span_to_snippet(pat.span) else { return };
69+
let Ok(arg_snip) = cx.sess().source_map().span_to_snippet(arg.span) else { return };
70+
71+
let help_string = format!(
72+
"consider replacing `for {pat_snip} in {arg_snip}` with `if let {var}({pat_snip}) = {arg_snip}`"
73+
);
74+
let msg = format!(
75+
"for loop over `{arg_snip}`, which is {article} `{ty}`. This is more readably written as an `if let` statement",
76+
);
77+
78+
cx.struct_span_lint(FOR_LOOP_OVER_FALLIBLES, arg.span, |diag| {
79+
diag.build(msg).help(help_string).emit()
80+
})
81+
}
82+
}
83+
84+
fn extract_for_loop<'tcx>(expr: &Expr<'tcx>) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>)> {
85+
if let hir::ExprKind::DropTemps(e) = expr.kind
86+
&& let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind
87+
&& let hir::ExprKind::Call(_, [arg]) = iterexpr.kind
88+
&& let hir::ExprKind::Loop(block, ..) = arm.body.kind
89+
&& let [stmt] = block.stmts
90+
&& let hir::StmtKind::Expr(e) = stmt.kind
91+
&& let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind
92+
&& let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind
93+
{
94+
Some((field.pat, arg))
95+
} else {
96+
None
97+
}
98+
99+
}

compiler/rustc_lint/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ mod context;
4848
mod early;
4949
mod enum_intrinsics_non_enums;
5050
mod expect;
51+
mod for_loop_over_fallibles;
5152
pub mod hidden_unicode_codepoints;
5253
mod internal;
5354
mod late;
@@ -80,6 +81,7 @@ use rustc_span::Span;
8081
use array_into_iter::ArrayIntoIter;
8182
use builtin::*;
8283
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
84+
use for_loop_over_fallibles::*;
8385
use hidden_unicode_codepoints::*;
8486
use internal::*;
8587
use methods::*;
@@ -178,6 +180,7 @@ macro_rules! late_lint_mod_passes {
178180
$macro!(
179181
$args,
180182
[
183+
ForLoopOverFallibles: ForLoopOverFallibles,
181184
HardwiredLints: HardwiredLints,
182185
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
183186
ImproperCTypesDefinitions: ImproperCTypesDefinitions,

0 commit comments

Comments
 (0)