]>
Commit | Line | Data |
---|---|---|
f20569fa XL |
1 | use crate::utils; |
2 | use crate::utils::eager_or_lazy; | |
3 | use crate::utils::sugg::Sugg; | |
4 | use crate::utils::{is_type_diagnostic_item, paths, span_lint_and_sugg}; | |
5 | use if_chain::if_chain; | |
6 | ||
7 | use rustc_errors::Applicability; | |
8 | use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, MatchSource, Mutability, PatKind, UnOp}; | |
9 | use rustc_lint::{LateContext, LateLintPass}; | |
10 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
11 | use rustc_span::sym; | |
12 | ||
13 | declare_clippy_lint! { | |
14 | /// **What it does:** | |
15 | /// Lints usage of `if let Some(v) = ... { y } else { x }` which is more | |
16 | /// idiomatically done with `Option::map_or` (if the else bit is a pure | |
17 | /// expression) or `Option::map_or_else` (if the else bit is an impure | |
18 | /// expression). | |
19 | /// | |
20 | /// **Why is this bad?** | |
21 | /// Using the dedicated functions of the Option type is clearer and | |
22 | /// more concise than an `if let` expression. | |
23 | /// | |
24 | /// **Known problems:** | |
25 | /// This lint uses a deliberately conservative metric for checking | |
26 | /// if the inside of either body contains breaks or continues which will | |
27 | /// cause it to not suggest a fix if either block contains a loop with | |
28 | /// continues or breaks contained within the loop. | |
29 | /// | |
30 | /// **Example:** | |
31 | /// | |
32 | /// ```rust | |
33 | /// # let optional: Option<u32> = Some(0); | |
34 | /// # fn do_complicated_function() -> u32 { 5 }; | |
35 | /// let _ = if let Some(foo) = optional { | |
36 | /// foo | |
37 | /// } else { | |
38 | /// 5 | |
39 | /// }; | |
40 | /// let _ = if let Some(foo) = optional { | |
41 | /// foo | |
42 | /// } else { | |
43 | /// let y = do_complicated_function(); | |
44 | /// y*y | |
45 | /// }; | |
46 | /// ``` | |
47 | /// | |
48 | /// should be | |
49 | /// | |
50 | /// ```rust | |
51 | /// # let optional: Option<u32> = Some(0); | |
52 | /// # fn do_complicated_function() -> u32 { 5 }; | |
53 | /// let _ = optional.map_or(5, |foo| foo); | |
54 | /// let _ = optional.map_or_else(||{ | |
55 | /// let y = do_complicated_function(); | |
56 | /// y*y | |
57 | /// }, |foo| foo); | |
58 | /// ``` | |
59 | pub OPTION_IF_LET_ELSE, | |
60 | pedantic, | |
61 | "reimplementation of Option::map_or" | |
62 | } | |
63 | ||
64 | declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]); | |
65 | ||
66 | /// Returns true iff the given expression is the result of calling `Result::ok` | |
67 | fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool { | |
68 | if let ExprKind::MethodCall(ref path, _, &[ref receiver], _) = &expr.kind { | |
69 | path.ident.name.as_str() == "ok" | |
70 | && is_type_diagnostic_item(cx, &cx.typeck_results().expr_ty(&receiver), sym::result_type) | |
71 | } else { | |
72 | false | |
73 | } | |
74 | } | |
75 | ||
76 | /// A struct containing information about occurrences of the | |
77 | /// `if let Some(..) = .. else` construct that this lint detects. | |
78 | struct OptionIfLetElseOccurence { | |
79 | option: String, | |
80 | method_sugg: String, | |
81 | some_expr: String, | |
82 | none_expr: String, | |
83 | wrap_braces: bool, | |
84 | } | |
85 | ||
86 | /// Extracts the body of a given arm. If the arm contains only an expression, | |
87 | /// then it returns the expression. Otherwise, it returns the entire block | |
88 | fn extract_body_from_arm<'a>(arm: &'a Arm<'a>) -> Option<&'a Expr<'a>> { | |
89 | if let ExprKind::Block( | |
90 | Block { | |
91 | stmts: statements, | |
92 | expr: Some(expr), | |
93 | .. | |
94 | }, | |
95 | _, | |
96 | ) = &arm.body.kind | |
97 | { | |
98 | if let [] = statements { | |
99 | Some(&expr) | |
100 | } else { | |
101 | Some(&arm.body) | |
102 | } | |
103 | } else { | |
104 | None | |
105 | } | |
106 | } | |
107 | ||
108 | /// If this is the else body of an if/else expression, then we need to wrap | |
109 | /// it in curly braces. Otherwise, we don't. | |
110 | fn should_wrap_in_braces(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { | |
111 | utils::get_enclosing_block(cx, expr.hir_id).map_or(false, |parent| { | |
112 | let mut should_wrap = false; | |
113 | ||
114 | if let Some(Expr { | |
115 | kind: | |
116 | ExprKind::Match( | |
117 | _, | |
118 | arms, | |
119 | MatchSource::IfLetDesugar { | |
120 | contains_else_clause: true, | |
121 | }, | |
122 | ), | |
123 | .. | |
124 | }) = parent.expr | |
125 | { | |
126 | should_wrap = expr.hir_id == arms[1].body.hir_id; | |
127 | } else if let Some(Expr { | |
128 | kind: ExprKind::If(_, _, Some(else_clause)), | |
129 | .. | |
130 | }) = parent.expr | |
131 | { | |
132 | should_wrap = expr.hir_id == else_clause.hir_id; | |
133 | } | |
134 | ||
135 | should_wrap | |
136 | }) | |
137 | } | |
138 | ||
139 | fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String { | |
140 | format!( | |
141 | "{}{}", | |
142 | Sugg::hir(cx, cond_expr, "..").maybe_par(), | |
143 | if as_mut { | |
144 | ".as_mut()" | |
145 | } else if as_ref { | |
146 | ".as_ref()" | |
147 | } else { | |
148 | "" | |
149 | } | |
150 | ) | |
151 | } | |
152 | ||
153 | /// If this expression is the option if let/else construct we're detecting, then | |
154 | /// this function returns an `OptionIfLetElseOccurence` struct with details if | |
155 | /// this construct is found, or None if this construct is not found. | |
156 | fn detect_option_if_let_else<'tcx>( | |
157 | cx: &'_ LateContext<'tcx>, | |
158 | expr: &'_ Expr<'tcx>, | |
159 | ) -> Option<OptionIfLetElseOccurence> { | |
160 | if_chain! { | |
161 | if !utils::in_macro(expr.span); // Don't lint macros, because it behaves weirdly | |
162 | if let ExprKind::Match(cond_expr, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind; | |
163 | if arms.len() == 2; | |
164 | if !is_result_ok(cx, cond_expr); // Don't lint on Result::ok because a different lint does it already | |
165 | if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind; | |
166 | if utils::match_qpath(struct_qpath, &paths::OPTION_SOME); | |
167 | if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind; | |
168 | if !utils::usage::contains_return_break_continue_macro(arms[0].body); | |
169 | if !utils::usage::contains_return_break_continue_macro(arms[1].body); | |
170 | then { | |
171 | let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" }; | |
172 | let some_body = extract_body_from_arm(&arms[0])?; | |
173 | let none_body = extract_body_from_arm(&arms[1])?; | |
174 | let method_sugg = if eager_or_lazy::is_eagerness_candidate(cx, none_body) { "map_or" } else { "map_or_else" }; | |
175 | let capture_name = id.name.to_ident_string(); | |
176 | let wrap_braces = should_wrap_in_braces(cx, expr); | |
177 | let (as_ref, as_mut) = match &cond_expr.kind { | |
178 | ExprKind::AddrOf(_, Mutability::Not, _) => (true, false), | |
179 | ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true), | |
180 | _ => (bind_annotation == &BindingAnnotation::Ref, bind_annotation == &BindingAnnotation::RefMut), | |
181 | }; | |
182 | let cond_expr = match &cond_expr.kind { | |
183 | // Pointer dereferencing happens automatically, so we can omit it in the suggestion | |
184 | ExprKind::Unary(UnOp::Deref, expr) | ExprKind::AddrOf(_, _, expr) => expr, | |
185 | _ => cond_expr, | |
186 | }; | |
187 | Some(OptionIfLetElseOccurence { | |
188 | option: format_option_in_sugg(cx, cond_expr, as_ref, as_mut), | |
189 | method_sugg: method_sugg.to_string(), | |
190 | some_expr: format!("|{}{}| {}", capture_mut, capture_name, Sugg::hir(cx, some_body, "..")), | |
191 | none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")), | |
192 | wrap_braces, | |
193 | }) | |
194 | } else { | |
195 | None | |
196 | } | |
197 | } | |
198 | } | |
199 | ||
200 | impl<'tcx> LateLintPass<'tcx> for OptionIfLetElse { | |
201 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { | |
202 | if let Some(detection) = detect_option_if_let_else(cx, expr) { | |
203 | span_lint_and_sugg( | |
204 | cx, | |
205 | OPTION_IF_LET_ELSE, | |
206 | expr.span, | |
207 | format!("use Option::{} instead of an if let/else", detection.method_sugg).as_str(), | |
208 | "try", | |
209 | format!( | |
210 | "{}{}.{}({}, {}){}", | |
211 | if detection.wrap_braces { "{ " } else { "" }, | |
212 | detection.option, | |
213 | detection.method_sugg, | |
214 | detection.none_expr, | |
215 | detection.some_expr, | |
216 | if detection.wrap_braces { " }" } else { "" }, | |
217 | ), | |
218 | Applicability::MaybeIncorrect, | |
219 | ); | |
220 | } | |
221 | } | |
222 | } |