]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 | 1 | use clippy_utils::diagnostics::span_lint_and_sugg; |
94222f64 | 2 | use clippy_utils::higher; |
cdc7bbd5 XL |
3 | use clippy_utils::sugg::Sugg; |
4 | use clippy_utils::ty::is_type_diagnostic_item; | |
c295e0f8 XL |
5 | use clippy_utils::{ |
6 | can_move_expr_to_closure, eager_or_lazy, in_constant, in_macro, is_else_clause, is_lang_ctor, peel_hir_expr_while, | |
7 | CaptureKind, | |
8 | }; | |
f20569fa | 9 | use if_chain::if_chain; |
f20569fa | 10 | use rustc_errors::Applicability; |
cdc7bbd5 | 11 | use rustc_hir::LangItem::OptionSome; |
c295e0f8 | 12 | use rustc_hir::{def::Res, BindingAnnotation, Block, Expr, ExprKind, Mutability, PatKind, Path, QPath, UnOp}; |
f20569fa XL |
13 | use rustc_lint::{LateContext, LateLintPass}; |
14 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
15 | use rustc_span::sym; | |
16 | ||
17 | declare_clippy_lint! { | |
94222f64 | 18 | /// ### What it does |
f20569fa XL |
19 | /// Lints usage of `if let Some(v) = ... { y } else { x }` which is more |
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? |
f20569fa XL |
25 | /// Using the dedicated functions of the Option type is clearer and |
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 | /// }; | |
43 | /// let _ = if let Some(foo) = optional { | |
44 | /// foo | |
45 | /// } else { | |
46 | /// let y = do_complicated_function(); | |
47 | /// y*y | |
48 | /// }; | |
49 | /// ``` | |
50 | /// | |
51 | /// should be | |
52 | /// | |
53 | /// ```rust | |
54 | /// # let optional: Option<u32> = Some(0); | |
55 | /// # fn do_complicated_function() -> u32 { 5 }; | |
56 | /// let _ = optional.map_or(5, |foo| foo); | |
57 | /// let _ = optional.map_or_else(||{ | |
58 | /// let y = do_complicated_function(); | |
59 | /// y*y | |
60 | /// }, |foo| foo); | |
61 | /// ``` | |
62 | pub OPTION_IF_LET_ELSE, | |
c295e0f8 | 63 | nursery, |
f20569fa XL |
64 | "reimplementation of Option::map_or" |
65 | } | |
66 | ||
67 | declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]); | |
68 | ||
69 | /// Returns true iff the given expression is the result of calling `Result::ok` | |
70 | fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool { | |
cdc7bbd5 | 71 | if let ExprKind::MethodCall(path, _, &[ref receiver], _) = &expr.kind { |
f20569fa | 72 | path.ident.name.as_str() == "ok" |
c295e0f8 | 73 | && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(receiver), sym::Result) |
f20569fa XL |
74 | } else { |
75 | false | |
76 | } | |
77 | } | |
78 | ||
79 | /// A struct containing information about occurrences of the | |
80 | /// `if let Some(..) = .. else` construct that this lint detects. | |
81 | struct OptionIfLetElseOccurence { | |
82 | option: String, | |
83 | method_sugg: String, | |
84 | some_expr: String, | |
85 | none_expr: String, | |
f20569fa XL |
86 | } |
87 | ||
88 | /// Extracts the body of a given arm. If the arm contains only an expression, | |
89 | /// then it returns the expression. Otherwise, it returns the entire block | |
94222f64 | 90 | fn extract_body_from_expr<'a>(expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> { |
f20569fa XL |
91 | if let ExprKind::Block( |
92 | Block { | |
94222f64 XL |
93 | stmts: block_stmts, |
94 | expr: Some(block_expr), | |
f20569fa XL |
95 | .. |
96 | }, | |
97 | _, | |
94222f64 | 98 | ) = expr.kind |
f20569fa | 99 | { |
94222f64 XL |
100 | if let [] = block_stmts { |
101 | Some(block_expr) | |
f20569fa | 102 | } else { |
94222f64 | 103 | Some(expr) |
f20569fa XL |
104 | } |
105 | } else { | |
106 | None | |
107 | } | |
108 | } | |
109 | ||
f20569fa XL |
110 | fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String { |
111 | format!( | |
112 | "{}{}", | |
113 | Sugg::hir(cx, cond_expr, "..").maybe_par(), | |
114 | if as_mut { | |
115 | ".as_mut()" | |
116 | } else if as_ref { | |
117 | ".as_ref()" | |
118 | } else { | |
119 | "" | |
120 | } | |
121 | ) | |
122 | } | |
123 | ||
124 | /// If this expression is the option if let/else construct we're detecting, then | |
125 | /// this function returns an `OptionIfLetElseOccurence` struct with details if | |
126 | /// this construct is found, or None if this construct is not found. | |
94222f64 | 127 | fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<OptionIfLetElseOccurence> { |
f20569fa | 128 | if_chain! { |
cdc7bbd5 | 129 | if !in_macro(expr.span); // Don't lint macros, because it behaves weirdly |
c295e0f8 XL |
130 | if !in_constant(cx, expr.hir_id); |
131 | if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) | |
132 | = higher::IfLet::hir(cx, expr); | |
17df50a5 | 133 | if !is_else_clause(cx.tcx, expr); |
94222f64 XL |
134 | if !is_result_ok(cx, let_expr); // Don't lint on Result::ok because a different lint does it already |
135 | if let PatKind::TupleStruct(struct_qpath, [inner_pat], _) = &let_pat.kind; | |
cdc7bbd5 | 136 | if is_lang_ctor(cx, struct_qpath, OptionSome); |
f20569fa | 137 | if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind; |
c295e0f8 XL |
138 | if let Some(some_captures) = can_move_expr_to_closure(cx, if_then); |
139 | if let Some(none_captures) = can_move_expr_to_closure(cx, if_else); | |
140 | if some_captures | |
141 | .iter() | |
142 | .filter_map(|(id, &c)| none_captures.get(id).map(|&c2| (c, c2))) | |
143 | .all(|(x, y)| x.is_imm_ref() && y.is_imm_ref()); | |
17df50a5 | 144 | |
f20569fa XL |
145 | then { |
146 | let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" }; | |
94222f64 XL |
147 | let some_body = extract_body_from_expr(if_then)?; |
148 | let none_body = extract_body_from_expr(if_else)?; | |
c295e0f8 XL |
149 | let method_sugg = if eager_or_lazy::is_eagerness_candidate(cx, none_body) { |
150 | "map_or" | |
151 | } else { | |
152 | "map_or_else" | |
153 | }; | |
f20569fa | 154 | let capture_name = id.name.to_ident_string(); |
94222f64 | 155 | let (as_ref, as_mut) = match &let_expr.kind { |
f20569fa XL |
156 | ExprKind::AddrOf(_, Mutability::Not, _) => (true, false), |
157 | ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true), | |
158 | _ => (bind_annotation == &BindingAnnotation::Ref, bind_annotation == &BindingAnnotation::RefMut), | |
159 | }; | |
94222f64 | 160 | let cond_expr = match let_expr.kind { |
f20569fa XL |
161 | // Pointer dereferencing happens automatically, so we can omit it in the suggestion |
162 | ExprKind::Unary(UnOp::Deref, expr) | ExprKind::AddrOf(_, _, expr) => expr, | |
94222f64 | 163 | _ => let_expr, |
f20569fa | 164 | }; |
c295e0f8 XL |
165 | // Check if captures the closure will need conflict with borrows made in the scrutinee. |
166 | // TODO: check all the references made in the scrutinee expression. This will require interacting | |
167 | // with the borrow checker. Currently only `<local>[.<field>]*` is checked for. | |
168 | if as_ref || as_mut { | |
169 | let e = peel_hir_expr_while(cond_expr, |e| match e.kind { | |
170 | ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => Some(e), | |
171 | _ => None, | |
172 | }); | |
173 | if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(local_id), .. })) = e.kind { | |
174 | match some_captures.get(local_id) | |
175 | .or_else(|| (method_sugg == "map_or_else").then(|| ()).and_then(|_| none_captures.get(local_id))) | |
176 | { | |
177 | Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return None, | |
178 | Some(CaptureKind::Ref(Mutability::Not)) if as_mut => return None, | |
179 | Some(CaptureKind::Ref(Mutability::Not)) | None => (), | |
180 | } | |
181 | } | |
182 | } | |
f20569fa XL |
183 | Some(OptionIfLetElseOccurence { |
184 | option: format_option_in_sugg(cx, cond_expr, as_ref, as_mut), | |
185 | method_sugg: method_sugg.to_string(), | |
186 | some_expr: format!("|{}{}| {}", capture_mut, capture_name, Sugg::hir(cx, some_body, "..")), | |
187 | none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")), | |
f20569fa XL |
188 | }) |
189 | } else { | |
190 | None | |
191 | } | |
192 | } | |
193 | } | |
194 | ||
195 | impl<'tcx> LateLintPass<'tcx> for OptionIfLetElse { | |
196 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { | |
197 | if let Some(detection) = detect_option_if_let_else(cx, expr) { | |
198 | span_lint_and_sugg( | |
199 | cx, | |
200 | OPTION_IF_LET_ELSE, | |
201 | expr.span, | |
202 | format!("use Option::{} instead of an if let/else", detection.method_sugg).as_str(), | |
203 | "try", | |
204 | format!( | |
17df50a5 XL |
205 | "{}.{}({}, {})", |
206 | detection.option, detection.method_sugg, detection.none_expr, detection.some_expr, | |
f20569fa XL |
207 | ), |
208 | Applicability::MaybeIncorrect, | |
209 | ); | |
210 | } | |
211 | } | |
212 | } |