]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 XL |
1 | use clippy_utils::diagnostics::span_lint_and_sugg; |
2 | use clippy_utils::sugg::Sugg; | |
c295e0f8 | 3 | use clippy_utils::{ |
2b03887a | 4 | can_move_expr_to_closure, eager_or_lazy, higher, in_constant, is_else_clause, is_res_lang_ctor, peel_blocks, |
a2a8927a | 5 | peel_hir_expr_while, CaptureKind, |
c295e0f8 | 6 | }; |
f20569fa | 7 | use if_chain::if_chain; |
f20569fa | 8 | use rustc_errors::Applicability; |
f2b60f7d FG |
9 | use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; |
10 | use rustc_hir::{ | |
11 | def::Res, Arm, BindingAnnotation, Expr, ExprKind, MatchSource, Mutability, Pat, PatKind, Path, QPath, UnOp, | |
12 | }; | |
f20569fa XL |
13 | use rustc_lint::{LateContext, LateLintPass}; |
14 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
f20569fa XL |
15 | |
16 | declare_clippy_lint! { | |
94222f64 | 17 | /// ### What it does |
f2b60f7d FG |
18 | /// Lints usage of `if let Some(v) = ... { y } else { x }` and |
19 | /// `match .. { Some(v) => y, None/_ => x }` which are more | |
f20569fa XL |
20 | /// idiomatically done with `Option::map_or` (if the else bit is a pure |
21 | /// expression) or `Option::map_or_else` (if the else bit is an impure | |
22 | /// expression). | |
23 | /// | |
94222f64 | 24 | /// ### Why is this bad? |
3c0e092e | 25 | /// Using the dedicated functions of the `Option` type is clearer and |
f20569fa XL |
26 | /// more concise than an `if let` expression. |
27 | /// | |
94222f64 | 28 | /// ### Known problems |
f20569fa XL |
29 | /// This lint uses a deliberately conservative metric for checking |
30 | /// if the inside of either body contains breaks or continues which will | |
31 | /// cause it to not suggest a fix if either block contains a loop with | |
32 | /// continues or breaks contained within the loop. | |
33 | /// | |
94222f64 | 34 | /// ### Example |
f20569fa XL |
35 | /// ```rust |
36 | /// # let optional: Option<u32> = Some(0); | |
37 | /// # fn do_complicated_function() -> u32 { 5 }; | |
38 | /// let _ = if let Some(foo) = optional { | |
39 | /// foo | |
40 | /// } else { | |
41 | /// 5 | |
42 | /// }; | |
f2b60f7d FG |
43 | /// let _ = match optional { |
44 | /// Some(val) => val + 1, | |
45 | /// None => 5 | |
46 | /// }; | |
f20569fa XL |
47 | /// let _ = if let Some(foo) = optional { |
48 | /// foo | |
49 | /// } else { | |
50 | /// let y = do_complicated_function(); | |
51 | /// y*y | |
52 | /// }; | |
53 | /// ``` | |
54 | /// | |
55 | /// should be | |
56 | /// | |
57 | /// ```rust | |
58 | /// # let optional: Option<u32> = Some(0); | |
59 | /// # fn do_complicated_function() -> u32 { 5 }; | |
60 | /// let _ = optional.map_or(5, |foo| foo); | |
f2b60f7d | 61 | /// let _ = optional.map_or(5, |val| val + 1); |
f20569fa XL |
62 | /// let _ = optional.map_or_else(||{ |
63 | /// let y = do_complicated_function(); | |
64 | /// y*y | |
65 | /// }, |foo| foo); | |
66 | /// ``` | |
f2b60f7d FG |
67 | // FIXME: Before moving this lint out of nursery, the lint name needs to be updated. It now also |
68 | // covers matches and `Result`. | |
a2a8927a | 69 | #[clippy::version = "1.47.0"] |
f20569fa | 70 | pub OPTION_IF_LET_ELSE, |
c295e0f8 | 71 | nursery, |
f20569fa XL |
72 | "reimplementation of Option::map_or" |
73 | } | |
74 | ||
75 | declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]); | |
76 | ||
f2b60f7d FG |
77 | /// A struct containing information about occurrences of construct that this lint detects |
78 | /// | |
79 | /// Such as: | |
80 | /// | |
81 | /// ```ignore | |
82 | /// if let Some(..) = {..} else {..} | |
83 | /// ``` | |
84 | /// or | |
85 | /// ```ignore | |
86 | /// match x { | |
87 | /// Some(..) => {..}, | |
88 | /// None/_ => {..} | |
89 | /// } | |
90 | /// ``` | |
2b03887a | 91 | struct OptionOccurrence { |
f20569fa XL |
92 | option: String, |
93 | method_sugg: String, | |
94 | some_expr: String, | |
95 | none_expr: String, | |
f20569fa XL |
96 | } |
97 | ||
f20569fa XL |
98 | fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String { |
99 | format!( | |
100 | "{}{}", | |
a2a8927a | 101 | Sugg::hir_with_macro_callsite(cx, cond_expr, "..").maybe_par(), |
f20569fa XL |
102 | if as_mut { |
103 | ".as_mut()" | |
104 | } else if as_ref { | |
105 | ".as_ref()" | |
106 | } else { | |
107 | "" | |
108 | } | |
109 | ) | |
110 | } | |
111 | ||
2b03887a | 112 | fn try_get_option_occurrence<'tcx>( |
f2b60f7d FG |
113 | cx: &LateContext<'tcx>, |
114 | pat: &Pat<'tcx>, | |
115 | expr: &Expr<'_>, | |
116 | if_then: &'tcx Expr<'_>, | |
117 | if_else: &'tcx Expr<'_>, | |
2b03887a | 118 | ) -> Option<OptionOccurrence> { |
f2b60f7d FG |
119 | let cond_expr = match expr.kind { |
120 | ExprKind::Unary(UnOp::Deref, inner_expr) | ExprKind::AddrOf(_, _, inner_expr) => inner_expr, | |
121 | _ => expr, | |
122 | }; | |
123 | let inner_pat = try_get_inner_pat(cx, pat)?; | |
f20569fa | 124 | if_chain! { |
f2b60f7d | 125 | if let PatKind::Binding(bind_annotation, _, id, None) = inner_pat.kind; |
c295e0f8 XL |
126 | if let Some(some_captures) = can_move_expr_to_closure(cx, if_then); |
127 | if let Some(none_captures) = can_move_expr_to_closure(cx, if_else); | |
128 | if some_captures | |
129 | .iter() | |
130 | .filter_map(|(id, &c)| none_captures.get(id).map(|&c2| (c, c2))) | |
131 | .all(|(x, y)| x.is_imm_ref() && y.is_imm_ref()); | |
f20569fa | 132 | then { |
f2b60f7d | 133 | let capture_mut = if bind_annotation == BindingAnnotation::MUT { "mut " } else { "" }; |
a2a8927a XL |
134 | let some_body = peel_blocks(if_then); |
135 | let none_body = peel_blocks(if_else); | |
136 | let method_sugg = if eager_or_lazy::switch_to_eager_eval(cx, none_body) { "map_or" } else { "map_or_else" }; | |
f20569fa | 137 | let capture_name = id.name.to_ident_string(); |
f2b60f7d | 138 | let (as_ref, as_mut) = match &expr.kind { |
f20569fa XL |
139 | ExprKind::AddrOf(_, Mutability::Not, _) => (true, false), |
140 | ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true), | |
f2b60f7d | 141 | _ => (bind_annotation == BindingAnnotation::REF, bind_annotation == BindingAnnotation::REF_MUT), |
f20569fa | 142 | }; |
f2b60f7d | 143 | |
c295e0f8 XL |
144 | // Check if captures the closure will need conflict with borrows made in the scrutinee. |
145 | // TODO: check all the references made in the scrutinee expression. This will require interacting | |
146 | // with the borrow checker. Currently only `<local>[.<field>]*` is checked for. | |
147 | if as_ref || as_mut { | |
148 | let e = peel_hir_expr_while(cond_expr, |e| match e.kind { | |
149 | ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => Some(e), | |
150 | _ => None, | |
151 | }); | |
152 | if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(local_id), .. })) = e.kind { | |
153 | match some_captures.get(local_id) | |
064997fb | 154 | .or_else(|| (method_sugg == "map_or_else").then_some(()).and_then(|_| none_captures.get(local_id))) |
c295e0f8 XL |
155 | { |
156 | Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return None, | |
157 | Some(CaptureKind::Ref(Mutability::Not)) if as_mut => return None, | |
158 | Some(CaptureKind::Ref(Mutability::Not)) | None => (), | |
159 | } | |
160 | } | |
161 | } | |
f2b60f7d | 162 | |
2b03887a | 163 | return Some(OptionOccurrence { |
f20569fa XL |
164 | option: format_option_in_sugg(cx, cond_expr, as_ref, as_mut), |
165 | method_sugg: method_sugg.to_string(), | |
2b03887a | 166 | some_expr: format!("|{capture_mut}{capture_name}| {}", Sugg::hir_with_macro_callsite(cx, some_body, "..")), |
a2a8927a | 167 | none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir_with_macro_callsite(cx, none_body, "..")), |
f2b60f7d FG |
168 | }); |
169 | } | |
170 | } | |
171 | ||
172 | None | |
173 | } | |
174 | ||
175 | fn try_get_inner_pat<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<&'tcx Pat<'tcx>> { | |
176 | if let PatKind::TupleStruct(ref qpath, [inner_pat], ..) = pat.kind { | |
2b03887a FG |
177 | let res = cx.qpath_res(qpath, pat.hir_id); |
178 | if is_res_lang_ctor(cx, res, OptionSome) || is_res_lang_ctor(cx, res, ResultOk) { | |
f2b60f7d FG |
179 | return Some(inner_pat); |
180 | } | |
181 | } | |
182 | None | |
183 | } | |
184 | ||
185 | /// If this expression is the option if let/else construct we're detecting, then | |
2b03887a | 186 | /// this function returns an `OptionOccurrence` struct with details if |
f2b60f7d | 187 | /// this construct is found, or None if this construct is not found. |
2b03887a | 188 | fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<OptionOccurrence> { |
f2b60f7d FG |
189 | if let Some(higher::IfLet { |
190 | let_pat, | |
191 | let_expr, | |
192 | if_then, | |
193 | if_else: Some(if_else), | |
194 | }) = higher::IfLet::hir(cx, expr) | |
195 | { | |
196 | if !is_else_clause(cx.tcx, expr) { | |
2b03887a | 197 | return try_get_option_occurrence(cx, let_pat, let_expr, if_then, if_else); |
f2b60f7d FG |
198 | } |
199 | } | |
200 | None | |
201 | } | |
202 | ||
2b03887a | 203 | fn detect_option_match<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<OptionOccurrence> { |
f2b60f7d FG |
204 | if let ExprKind::Match(ex, arms, MatchSource::Normal) = expr.kind { |
205 | if let Some((let_pat, if_then, if_else)) = try_convert_match(cx, arms) { | |
2b03887a | 206 | return try_get_option_occurrence(cx, let_pat, ex, if_then, if_else); |
f2b60f7d FG |
207 | } |
208 | } | |
209 | None | |
210 | } | |
211 | ||
212 | fn try_convert_match<'tcx>( | |
213 | cx: &LateContext<'tcx>, | |
214 | arms: &[Arm<'tcx>], | |
215 | ) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> { | |
6522a427 EL |
216 | if let [first_arm, second_arm] = arms |
217 | && first_arm.guard.is_none() | |
218 | && second_arm.guard.is_none() | |
219 | { | |
220 | return if is_none_or_err_arm(cx, second_arm) { | |
221 | Some((first_arm.pat, first_arm.body, second_arm.body)) | |
222 | } else if is_none_or_err_arm(cx, first_arm) { | |
223 | Some((second_arm.pat, second_arm.body, first_arm.body)) | |
f20569fa XL |
224 | } else { |
225 | None | |
f2b60f7d FG |
226 | }; |
227 | } | |
228 | None | |
229 | } | |
230 | ||
231 | fn is_none_or_err_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { | |
232 | match arm.pat.kind { | |
2b03887a | 233 | PatKind::Path(ref qpath) => is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), OptionNone), |
f2b60f7d | 234 | PatKind::TupleStruct(ref qpath, [first_pat], _) => { |
2b03887a FG |
235 | is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), ResultErr) |
236 | && matches!(first_pat.kind, PatKind::Wild) | |
f2b60f7d FG |
237 | }, |
238 | PatKind::Wild => true, | |
239 | _ => false, | |
f20569fa XL |
240 | } |
241 | } | |
242 | ||
243 | impl<'tcx> LateLintPass<'tcx> for OptionIfLetElse { | |
244 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { | |
f2b60f7d FG |
245 | // Don't lint macros and constants |
246 | if expr.span.from_expansion() || in_constant(cx, expr.hir_id) { | |
247 | return; | |
248 | } | |
249 | ||
250 | let detection = detect_option_if_let_else(cx, expr).or_else(|| detect_option_match(cx, expr)); | |
251 | if let Some(det) = detection { | |
f20569fa XL |
252 | span_lint_and_sugg( |
253 | cx, | |
254 | OPTION_IF_LET_ELSE, | |
255 | expr.span, | |
f2b60f7d | 256 | format!("use Option::{} instead of an if let/else", det.method_sugg).as_str(), |
f20569fa XL |
257 | "try", |
258 | format!( | |
17df50a5 | 259 | "{}.{}({}, {})", |
f2b60f7d | 260 | det.option, det.method_sugg, det.none_expr, det.some_expr |
f20569fa XL |
261 | ), |
262 | Applicability::MaybeIncorrect, | |
263 | ); | |
264 | } | |
265 | } | |
266 | } |