]>
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; | |
5 | use clippy_utils::usage::contains_return_break_continue_macro; | |
17df50a5 | 6 | use clippy_utils::{eager_or_lazy, in_macro, is_else_clause, is_lang_ctor}; |
f20569fa | 7 | use if_chain::if_chain; |
f20569fa | 8 | use rustc_errors::Applicability; |
cdc7bbd5 | 9 | use rustc_hir::LangItem::OptionSome; |
94222f64 | 10 | use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Mutability, PatKind, UnOp}; |
f20569fa XL |
11 | use rustc_lint::{LateContext, LateLintPass}; |
12 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
13 | use rustc_span::sym; | |
14 | ||
15 | declare_clippy_lint! { | |
94222f64 | 16 | /// ### What it does |
f20569fa XL |
17 | /// Lints usage of `if let Some(v) = ... { y } else { x }` which is more |
18 | /// idiomatically done with `Option::map_or` (if the else bit is a pure | |
19 | /// expression) or `Option::map_or_else` (if the else bit is an impure | |
20 | /// expression). | |
21 | /// | |
94222f64 | 22 | /// ### Why is this bad? |
f20569fa XL |
23 | /// Using the dedicated functions of the Option type is clearer and |
24 | /// more concise than an `if let` expression. | |
25 | /// | |
94222f64 | 26 | /// ### Known problems |
f20569fa XL |
27 | /// This lint uses a deliberately conservative metric for checking |
28 | /// if the inside of either body contains breaks or continues which will | |
29 | /// cause it to not suggest a fix if either block contains a loop with | |
30 | /// continues or breaks contained within the loop. | |
31 | /// | |
94222f64 | 32 | /// ### Example |
f20569fa XL |
33 | /// ```rust |
34 | /// # let optional: Option<u32> = Some(0); | |
35 | /// # fn do_complicated_function() -> u32 { 5 }; | |
36 | /// let _ = if let Some(foo) = optional { | |
37 | /// foo | |
38 | /// } else { | |
39 | /// 5 | |
40 | /// }; | |
41 | /// let _ = if let Some(foo) = optional { | |
42 | /// foo | |
43 | /// } else { | |
44 | /// let y = do_complicated_function(); | |
45 | /// y*y | |
46 | /// }; | |
47 | /// ``` | |
48 | /// | |
49 | /// should be | |
50 | /// | |
51 | /// ```rust | |
52 | /// # let optional: Option<u32> = Some(0); | |
53 | /// # fn do_complicated_function() -> u32 { 5 }; | |
54 | /// let _ = optional.map_or(5, |foo| foo); | |
55 | /// let _ = optional.map_or_else(||{ | |
56 | /// let y = do_complicated_function(); | |
57 | /// y*y | |
58 | /// }, |foo| foo); | |
59 | /// ``` | |
60 | pub OPTION_IF_LET_ELSE, | |
61 | pedantic, | |
62 | "reimplementation of Option::map_or" | |
63 | } | |
64 | ||
65 | declare_lint_pass!(OptionIfLetElse => [OPTION_IF_LET_ELSE]); | |
66 | ||
67 | /// Returns true iff the given expression is the result of calling `Result::ok` | |
68 | fn is_result_ok(cx: &LateContext<'_>, expr: &'_ Expr<'_>) -> bool { | |
cdc7bbd5 | 69 | if let ExprKind::MethodCall(path, _, &[ref receiver], _) = &expr.kind { |
f20569fa | 70 | path.ident.name.as_str() == "ok" |
cdc7bbd5 | 71 | && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(receiver), sym::result_type) |
f20569fa XL |
72 | } else { |
73 | false | |
74 | } | |
75 | } | |
76 | ||
77 | /// A struct containing information about occurrences of the | |
78 | /// `if let Some(..) = .. else` construct that this lint detects. | |
79 | struct OptionIfLetElseOccurence { | |
80 | option: String, | |
81 | method_sugg: String, | |
82 | some_expr: String, | |
83 | none_expr: String, | |
f20569fa XL |
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 | |
94222f64 | 88 | fn extract_body_from_expr<'a>(expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> { |
f20569fa XL |
89 | if let ExprKind::Block( |
90 | Block { | |
94222f64 XL |
91 | stmts: block_stmts, |
92 | expr: Some(block_expr), | |
f20569fa XL |
93 | .. |
94 | }, | |
95 | _, | |
94222f64 | 96 | ) = expr.kind |
f20569fa | 97 | { |
94222f64 XL |
98 | if let [] = block_stmts { |
99 | Some(block_expr) | |
f20569fa | 100 | } else { |
94222f64 | 101 | Some(expr) |
f20569fa XL |
102 | } |
103 | } else { | |
104 | None | |
105 | } | |
106 | } | |
107 | ||
f20569fa XL |
108 | fn format_option_in_sugg(cx: &LateContext<'_>, cond_expr: &Expr<'_>, as_ref: bool, as_mut: bool) -> String { |
109 | format!( | |
110 | "{}{}", | |
111 | Sugg::hir(cx, cond_expr, "..").maybe_par(), | |
112 | if as_mut { | |
113 | ".as_mut()" | |
114 | } else if as_ref { | |
115 | ".as_ref()" | |
116 | } else { | |
117 | "" | |
118 | } | |
119 | ) | |
120 | } | |
121 | ||
122 | /// If this expression is the option if let/else construct we're detecting, then | |
123 | /// this function returns an `OptionIfLetElseOccurence` struct with details if | |
124 | /// this construct is found, or None if this construct is not found. | |
94222f64 | 125 | fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<OptionIfLetElseOccurence> { |
f20569fa | 126 | if_chain! { |
cdc7bbd5 | 127 | if !in_macro(expr.span); // Don't lint macros, because it behaves weirdly |
94222f64 | 128 | if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(cx, expr); |
17df50a5 | 129 | if !is_else_clause(cx.tcx, expr); |
94222f64 XL |
130 | if !is_result_ok(cx, let_expr); // Don't lint on Result::ok because a different lint does it already |
131 | if let PatKind::TupleStruct(struct_qpath, [inner_pat], _) = &let_pat.kind; | |
cdc7bbd5 | 132 | if is_lang_ctor(cx, struct_qpath, OptionSome); |
f20569fa | 133 | if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind; |
94222f64 XL |
134 | if !contains_return_break_continue_macro(if_then); |
135 | if !contains_return_break_continue_macro(if_else); | |
17df50a5 | 136 | |
f20569fa XL |
137 | then { |
138 | let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" }; | |
94222f64 XL |
139 | let some_body = extract_body_from_expr(if_then)?; |
140 | let none_body = extract_body_from_expr(if_else)?; | |
f20569fa XL |
141 | let method_sugg = if eager_or_lazy::is_eagerness_candidate(cx, none_body) { "map_or" } else { "map_or_else" }; |
142 | let capture_name = id.name.to_ident_string(); | |
94222f64 | 143 | let (as_ref, as_mut) = match &let_expr.kind { |
f20569fa XL |
144 | ExprKind::AddrOf(_, Mutability::Not, _) => (true, false), |
145 | ExprKind::AddrOf(_, Mutability::Mut, _) => (false, true), | |
146 | _ => (bind_annotation == &BindingAnnotation::Ref, bind_annotation == &BindingAnnotation::RefMut), | |
147 | }; | |
94222f64 | 148 | let cond_expr = match let_expr.kind { |
f20569fa XL |
149 | // Pointer dereferencing happens automatically, so we can omit it in the suggestion |
150 | ExprKind::Unary(UnOp::Deref, expr) | ExprKind::AddrOf(_, _, expr) => expr, | |
94222f64 | 151 | _ => let_expr, |
f20569fa XL |
152 | }; |
153 | Some(OptionIfLetElseOccurence { | |
154 | option: format_option_in_sugg(cx, cond_expr, as_ref, as_mut), | |
155 | method_sugg: method_sugg.to_string(), | |
156 | some_expr: format!("|{}{}| {}", capture_mut, capture_name, Sugg::hir(cx, some_body, "..")), | |
157 | none_expr: format!("{}{}", if method_sugg == "map_or" { "" } else { "|| " }, Sugg::hir(cx, none_body, "..")), | |
f20569fa XL |
158 | }) |
159 | } else { | |
160 | None | |
161 | } | |
162 | } | |
163 | } | |
164 | ||
165 | impl<'tcx> LateLintPass<'tcx> for OptionIfLetElse { | |
166 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { | |
167 | if let Some(detection) = detect_option_if_let_else(cx, expr) { | |
168 | span_lint_and_sugg( | |
169 | cx, | |
170 | OPTION_IF_LET_ELSE, | |
171 | expr.span, | |
172 | format!("use Option::{} instead of an if let/else", detection.method_sugg).as_str(), | |
173 | "try", | |
174 | format!( | |
17df50a5 XL |
175 | "{}.{}({}, {})", |
176 | detection.option, detection.method_sugg, detection.none_expr, detection.some_expr, | |
f20569fa XL |
177 | ), |
178 | Applicability::MaybeIncorrect, | |
179 | ); | |
180 | } | |
181 | } | |
182 | } |