]>
Commit | Line | Data |
---|---|---|
2b03887a FG |
1 | use crate::{LateContext, LateLintPass, LintContext}; |
2 | ||
3 | use hir::{Expr, Pat}; | |
4 | use rustc_errors::{Applicability, DelayDm}; | |
5 | use rustc_hir as hir; | |
2b03887a FG |
6 | use rustc_infer::{infer::TyCtxtInferExt, traits::ObligationCause}; |
7 | use rustc_middle::ty::{self, List}; | |
8 | use rustc_span::{sym, Span}; | |
2b03887a FG |
9 | |
10 | declare_lint! { | |
11 | /// The `for_loops_over_fallibles` lint checks for `for` loops over `Option` or `Result` values. | |
12 | /// | |
13 | /// ### Example | |
14 | /// | |
15 | /// ```rust | |
16 | /// let opt = Some(1); | |
17 | /// for x in opt { /* ... */} | |
18 | /// ``` | |
19 | /// | |
20 | /// {{produces}} | |
21 | /// | |
22 | /// ### Explanation | |
23 | /// | |
24 | /// Both `Option` and `Result` implement `IntoIterator` trait, which allows using them in a `for` loop. | |
25 | /// `for` loop over `Option` or `Result` will iterate either 0 (if the value is `None`/`Err(_)`) | |
26 | /// or 1 time (if the value is `Some(_)`/`Ok(_)`). This is not very useful and is more clearly expressed | |
27 | /// via `if let`. | |
28 | /// | |
29 | /// `for` loop can also be accidentally written with the intention to call a function multiple times, | |
30 | /// while the function returns `Some(_)`, in these cases `while let` loop should be used instead. | |
31 | /// | |
32 | /// The "intended" use of `IntoIterator` implementations for `Option` and `Result` is passing them to | |
33 | /// generic code that expects something implementing `IntoIterator`. For example using `.chain(option)` | |
34 | /// to optionally add a value to an iterator. | |
35 | pub FOR_LOOPS_OVER_FALLIBLES, | |
36 | Warn, | |
37 | "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`" | |
38 | } | |
39 | ||
40 | declare_lint_pass!(ForLoopsOverFallibles => [FOR_LOOPS_OVER_FALLIBLES]); | |
41 | ||
42 | impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles { | |
43 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | |
44 | let Some((pat, arg)) = extract_for_loop(expr) else { return }; | |
45 | ||
46 | let ty = cx.typeck_results().expr_ty(arg); | |
47 | ||
48 | let &ty::Adt(adt, substs) = ty.kind() else { return }; | |
49 | ||
50 | let (article, ty, var) = match adt.did() { | |
51 | did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"), | |
52 | did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"), | |
53 | _ => return, | |
54 | }; | |
55 | ||
56 | let msg = DelayDm(|| { | |
57 | format!( | |
58 | "for loop over {article} `{ty}`. This is more readably written as an `if let` statement", | |
59 | ) | |
60 | }); | |
61 | ||
62 | cx.struct_span_lint(FOR_LOOPS_OVER_FALLIBLES, arg.span, msg, |lint| { | |
63 | if let Some(recv) = extract_iterator_next_call(cx, arg) | |
64 | && let Ok(recv_snip) = cx.sess().source_map().span_to_snippet(recv.span) | |
65 | { | |
66 | lint.span_suggestion( | |
67 | recv.span.between(arg.span.shrink_to_hi()), | |
68 | format!("to iterate over `{recv_snip}` remove the call to `next`"), | |
69 | ".by_ref()", | |
70 | Applicability::MaybeIncorrect | |
71 | ); | |
72 | } else { | |
73 | lint.multipart_suggestion_verbose( | |
74 | format!("to check pattern in a loop use `while let`"), | |
75 | vec![ | |
76 | // NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts | |
77 | (expr.span.with_hi(pat.span.lo()), format!("while let {var}(")), | |
78 | (pat.span.between(arg.span), format!(") = ")), | |
79 | ], | |
80 | Applicability::MaybeIncorrect | |
81 | ); | |
82 | } | |
83 | ||
84 | if suggest_question_mark(cx, adt, substs, expr.span) { | |
85 | lint.span_suggestion( | |
86 | arg.span.shrink_to_hi(), | |
87 | "consider unwrapping the `Result` with `?` to iterate over its contents", | |
88 | "?", | |
89 | Applicability::MaybeIncorrect, | |
90 | ); | |
91 | } | |
92 | ||
93 | lint.multipart_suggestion_verbose( | |
94 | "consider using `if let` to clear intent", | |
95 | vec![ | |
96 | // NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts | |
97 | (expr.span.with_hi(pat.span.lo()), format!("if let {var}(")), | |
98 | (pat.span.between(arg.span), format!(") = ")), | |
99 | ], | |
100 | Applicability::MaybeIncorrect, | |
101 | ) | |
102 | }) | |
103 | } | |
104 | } | |
105 | ||
106 | fn extract_for_loop<'tcx>(expr: &Expr<'tcx>) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>)> { | |
107 | if let hir::ExprKind::DropTemps(e) = expr.kind | |
108 | && let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind | |
109 | && let hir::ExprKind::Call(_, [arg]) = iterexpr.kind | |
110 | && let hir::ExprKind::Loop(block, ..) = arm.body.kind | |
111 | && let [stmt] = block.stmts | |
112 | && let hir::StmtKind::Expr(e) = stmt.kind | |
113 | && let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind | |
114 | && let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind | |
115 | { | |
116 | Some((field.pat, arg)) | |
117 | } else { | |
118 | None | |
119 | } | |
120 | } | |
121 | ||
122 | fn extract_iterator_next_call<'tcx>( | |
123 | cx: &LateContext<'_>, | |
124 | expr: &Expr<'tcx>, | |
125 | ) -> Option<&'tcx Expr<'tcx>> { | |
126 | // This won't work for `Iterator::next(iter)`, is this an issue? | |
127 | if let hir::ExprKind::MethodCall(_, recv, _, _) = expr.kind | |
128 | && cx.typeck_results().type_dependent_def_id(expr.hir_id) == cx.tcx.lang_items().next_fn() | |
129 | { | |
130 | Some(recv) | |
131 | } else { | |
132 | return None | |
133 | } | |
134 | } | |
135 | ||
136 | fn suggest_question_mark<'tcx>( | |
137 | cx: &LateContext<'tcx>, | |
138 | adt: ty::AdtDef<'tcx>, | |
139 | substs: &List<ty::GenericArg<'tcx>>, | |
140 | span: Span, | |
141 | ) -> bool { | |
142 | let Some(body_id) = cx.enclosing_body else { return false }; | |
143 | let Some(into_iterator_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { return false }; | |
144 | ||
145 | if !cx.tcx.is_diagnostic_item(sym::Result, adt.did()) { | |
146 | return false; | |
147 | } | |
148 | ||
149 | // Check that the function/closure/constant we are in has a `Result` type. | |
150 | // Otherwise suggesting using `?` may not be a good idea. | |
151 | { | |
152 | let ty = cx.typeck_results().expr_ty(&cx.tcx.hir().body(body_id).value); | |
153 | let ty::Adt(ret_adt, ..) = ty.kind() else { return false }; | |
154 | if !cx.tcx.is_diagnostic_item(sym::Result, ret_adt.did()) { | |
155 | return false; | |
156 | } | |
157 | } | |
158 | ||
159 | let ty = substs.type_at(0); | |
160 | let infcx = cx.tcx.infer_ctxt().build(); | |
2b03887a FG |
161 | let cause = ObligationCause::new( |
162 | span, | |
163 | body_id.hir_id, | |
164 | rustc_infer::traits::ObligationCauseCode::MiscObligation, | |
165 | ); | |
487cf647 | 166 | let errors = rustc_trait_selection::traits::fully_solve_bound( |
2b03887a | 167 | &infcx, |
487cf647 | 168 | cause, |
2b03887a FG |
169 | ty::ParamEnv::empty(), |
170 | // Erase any region vids from the type, which may not be resolved | |
171 | infcx.tcx.erase_regions(ty), | |
172 | into_iterator_did, | |
2b03887a FG |
173 | ); |
174 | ||
2b03887a FG |
175 | errors.is_empty() |
176 | } |