]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 XL |
1 | use clippy_utils::diagnostics::span_lint_and_then; |
2 | use clippy_utils::source::snippet; | |
3 | use clippy_utils::ty::is_type_diagnostic_item; | |
4 | use clippy_utils::{iter_input_pats, method_chain_args}; | |
f20569fa XL |
5 | use if_chain::if_chain; |
6 | use rustc_errors::Applicability; | |
7 | use rustc_hir as hir; | |
8 | use rustc_lint::{LateContext, LateLintPass}; | |
9 | use rustc_middle::ty::{self, Ty}; | |
10 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
11 | use rustc_span::source_map::Span; | |
12 | use rustc_span::sym; | |
13 | ||
14 | declare_clippy_lint! { | |
94222f64 XL |
15 | /// ### What it does |
16 | /// Checks for usage of `option.map(f)` where f is a function | |
f20569fa XL |
17 | /// or closure that returns the unit type `()`. |
18 | /// | |
94222f64 XL |
19 | /// ### Why is this bad? |
20 | /// Readability, this can be written more clearly with | |
f20569fa XL |
21 | /// an if let statement |
22 | /// | |
94222f64 | 23 | /// ### Example |
f20569fa XL |
24 | /// ```rust |
25 | /// # fn do_stuff() -> Option<String> { Some(String::new()) } | |
26 | /// # fn log_err_msg(foo: String) -> Option<String> { Some(foo) } | |
27 | /// # fn format_msg(foo: String) -> String { String::new() } | |
28 | /// let x: Option<String> = do_stuff(); | |
29 | /// x.map(log_err_msg); | |
30 | /// # let x: Option<String> = do_stuff(); | |
31 | /// x.map(|msg| log_err_msg(format_msg(msg))); | |
32 | /// ``` | |
33 | /// | |
34 | /// The correct use would be: | |
35 | /// | |
36 | /// ```rust | |
37 | /// # fn do_stuff() -> Option<String> { Some(String::new()) } | |
38 | /// # fn log_err_msg(foo: String) -> Option<String> { Some(foo) } | |
39 | /// # fn format_msg(foo: String) -> String { String::new() } | |
40 | /// let x: Option<String> = do_stuff(); | |
41 | /// if let Some(msg) = x { | |
42 | /// log_err_msg(msg); | |
43 | /// } | |
44 | /// | |
45 | /// # let x: Option<String> = do_stuff(); | |
46 | /// if let Some(msg) = x { | |
47 | /// log_err_msg(format_msg(msg)); | |
48 | /// } | |
49 | /// ``` | |
a2a8927a | 50 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
51 | pub OPTION_MAP_UNIT_FN, |
52 | complexity, | |
53 | "using `option.map(f)`, where `f` is a function or closure that returns `()`" | |
54 | } | |
55 | ||
56 | declare_clippy_lint! { | |
94222f64 XL |
57 | /// ### What it does |
58 | /// Checks for usage of `result.map(f)` where f is a function | |
f20569fa XL |
59 | /// or closure that returns the unit type `()`. |
60 | /// | |
94222f64 XL |
61 | /// ### Why is this bad? |
62 | /// Readability, this can be written more clearly with | |
f20569fa XL |
63 | /// an if let statement |
64 | /// | |
94222f64 | 65 | /// ### Example |
f20569fa XL |
66 | /// ```rust |
67 | /// # fn do_stuff() -> Result<String, String> { Ok(String::new()) } | |
68 | /// # fn log_err_msg(foo: String) -> Result<String, String> { Ok(foo) } | |
69 | /// # fn format_msg(foo: String) -> String { String::new() } | |
70 | /// let x: Result<String, String> = do_stuff(); | |
71 | /// x.map(log_err_msg); | |
72 | /// # let x: Result<String, String> = do_stuff(); | |
73 | /// x.map(|msg| log_err_msg(format_msg(msg))); | |
74 | /// ``` | |
75 | /// | |
76 | /// The correct use would be: | |
77 | /// | |
78 | /// ```rust | |
79 | /// # fn do_stuff() -> Result<String, String> { Ok(String::new()) } | |
80 | /// # fn log_err_msg(foo: String) -> Result<String, String> { Ok(foo) } | |
81 | /// # fn format_msg(foo: String) -> String { String::new() } | |
82 | /// let x: Result<String, String> = do_stuff(); | |
83 | /// if let Ok(msg) = x { | |
84 | /// log_err_msg(msg); | |
85 | /// }; | |
86 | /// # let x: Result<String, String> = do_stuff(); | |
87 | /// if let Ok(msg) = x { | |
88 | /// log_err_msg(format_msg(msg)); | |
89 | /// }; | |
90 | /// ``` | |
a2a8927a | 91 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
92 | pub RESULT_MAP_UNIT_FN, |
93 | complexity, | |
94 | "using `result.map(f)`, where `f` is a function or closure that returns `()`" | |
95 | } | |
96 | ||
97 | declare_lint_pass!(MapUnit => [OPTION_MAP_UNIT_FN, RESULT_MAP_UNIT_FN]); | |
98 | ||
99 | fn is_unit_type(ty: Ty<'_>) -> bool { | |
100 | match ty.kind() { | |
101 | ty::Tuple(slice) => slice.is_empty(), | |
102 | ty::Never => true, | |
103 | _ => false, | |
104 | } | |
105 | } | |
106 | ||
107 | fn is_unit_function(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { | |
108 | let ty = cx.typeck_results().expr_ty(expr); | |
109 | ||
110 | if let ty::FnDef(id, _) = *ty.kind() { | |
111 | if let Some(fn_type) = cx.tcx.fn_sig(id).no_bound_vars() { | |
112 | return is_unit_type(fn_type.output()); | |
113 | } | |
114 | } | |
115 | false | |
116 | } | |
117 | ||
118 | fn is_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { | |
119 | is_unit_type(cx.typeck_results().expr_ty(expr)) | |
120 | } | |
121 | ||
122 | /// The expression inside a closure may or may not have surrounding braces and | |
123 | /// semicolons, which causes problems when generating a suggestion. Given an | |
124 | /// expression that evaluates to '()' or '!', recursively remove useless braces | |
125 | /// and semi-colons until is suitable for including in the suggestion template | |
126 | fn reduce_unit_expression<'a>(cx: &LateContext<'_>, expr: &'a hir::Expr<'_>) -> Option<Span> { | |
127 | if !is_unit_expression(cx, expr) { | |
128 | return None; | |
129 | } | |
130 | ||
131 | match expr.kind { | |
5099ac24 | 132 | hir::ExprKind::Call(_, _) | hir::ExprKind::MethodCall(..) => { |
f20569fa XL |
133 | // Calls can't be reduced any more |
134 | Some(expr.span) | |
135 | }, | |
cdc7bbd5 | 136 | hir::ExprKind::Block(block, _) => { |
f20569fa XL |
137 | match (block.stmts, block.expr.as_ref()) { |
138 | (&[], Some(inner_expr)) => { | |
139 | // If block only contains an expression, | |
140 | // reduce `{ X }` to `X` | |
141 | reduce_unit_expression(cx, inner_expr) | |
142 | }, | |
143 | (&[ref inner_stmt], None) => { | |
144 | // If block only contains statements, | |
145 | // reduce `{ X; }` to `X` or `X;` | |
146 | match inner_stmt.kind { | |
cdc7bbd5 XL |
147 | hir::StmtKind::Local(local) => Some(local.span), |
148 | hir::StmtKind::Expr(e) => Some(e.span), | |
f20569fa XL |
149 | hir::StmtKind::Semi(..) => Some(inner_stmt.span), |
150 | hir::StmtKind::Item(..) => None, | |
151 | } | |
152 | }, | |
153 | _ => { | |
154 | // For closures that contain multiple statements | |
155 | // it's difficult to get a correct suggestion span | |
156 | // for all cases (multi-line closures specifically) | |
157 | // | |
158 | // We do not attempt to build a suggestion for those right now. | |
159 | None | |
160 | }, | |
161 | } | |
162 | }, | |
163 | _ => None, | |
164 | } | |
165 | } | |
166 | ||
167 | fn unit_closure<'tcx>( | |
168 | cx: &LateContext<'tcx>, | |
169 | expr: &hir::Expr<'_>, | |
170 | ) -> Option<(&'tcx hir::Param<'tcx>, &'tcx hir::Expr<'tcx>)> { | |
cdc7bbd5 XL |
171 | if_chain! { |
172 | if let hir::ExprKind::Closure(_, decl, inner_expr_id, _, _) = expr.kind; | |
f20569fa XL |
173 | let body = cx.tcx.hir().body(inner_expr_id); |
174 | let body_expr = &body.value; | |
cdc7bbd5 XL |
175 | if decl.inputs.len() == 1; |
176 | if is_unit_expression(cx, body_expr); | |
177 | if let Some(binding) = iter_input_pats(decl, body).next(); | |
178 | then { | |
179 | return Some((binding, body_expr)); | |
f20569fa XL |
180 | } |
181 | } | |
182 | None | |
183 | } | |
184 | ||
185 | /// Builds a name for the let binding variable (`var_arg`) | |
186 | /// | |
187 | /// `x.field` => `x_field` | |
188 | /// `y` => `_y` | |
189 | /// | |
190 | /// Anything else will return `a`. | |
191 | fn let_binding_name(cx: &LateContext<'_>, var_arg: &hir::Expr<'_>) -> String { | |
192 | match &var_arg.kind { | |
a2a8927a | 193 | hir::ExprKind::Field(_, _) => snippet(cx, var_arg.span, "_").replace('.', "_"), |
f20569fa XL |
194 | hir::ExprKind::Path(_) => format!("_{}", snippet(cx, var_arg.span, "")), |
195 | _ => "a".to_string(), | |
196 | } | |
197 | } | |
198 | ||
199 | #[must_use] | |
200 | fn suggestion_msg(function_type: &str, map_type: &str) -> String { | |
201 | format!( | |
202 | "called `map(f)` on an `{0}` value where `f` is a {1} that returns the unit type `()`", | |
203 | map_type, function_type | |
204 | ) | |
205 | } | |
206 | ||
207 | fn lint_map_unit_fn(cx: &LateContext<'_>, stmt: &hir::Stmt<'_>, expr: &hir::Expr<'_>, map_args: &[hir::Expr<'_>]) { | |
208 | let var_arg = &map_args[0]; | |
209 | ||
c295e0f8 XL |
210 | let (map_type, variant, lint) = if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(var_arg), sym::Option) { |
211 | ("Option", "Some", OPTION_MAP_UNIT_FN) | |
212 | } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(var_arg), sym::Result) { | |
213 | ("Result", "Ok", RESULT_MAP_UNIT_FN) | |
214 | } else { | |
215 | return; | |
216 | }; | |
f20569fa XL |
217 | let fn_arg = &map_args[1]; |
218 | ||
219 | if is_unit_function(cx, fn_arg) { | |
220 | let msg = suggestion_msg("function", map_type); | |
221 | let suggestion = format!( | |
222 | "if let {0}({binding}) = {1} {{ {2}({binding}) }}", | |
223 | variant, | |
224 | snippet(cx, var_arg.span, "_"), | |
225 | snippet(cx, fn_arg.span, "_"), | |
226 | binding = let_binding_name(cx, var_arg) | |
227 | ); | |
228 | ||
229 | span_lint_and_then(cx, lint, expr.span, &msg, |diag| { | |
230 | diag.span_suggestion(stmt.span, "try this", suggestion, Applicability::MachineApplicable); | |
231 | }); | |
232 | } else if let Some((binding, closure_expr)) = unit_closure(cx, fn_arg) { | |
233 | let msg = suggestion_msg("closure", map_type); | |
234 | ||
235 | span_lint_and_then(cx, lint, expr.span, &msg, |diag| { | |
236 | if let Some(reduced_expr_span) = reduce_unit_expression(cx, closure_expr) { | |
237 | let suggestion = format!( | |
238 | "if let {0}({1}) = {2} {{ {3} }}", | |
239 | variant, | |
240 | snippet(cx, binding.pat.span, "_"), | |
241 | snippet(cx, var_arg.span, "_"), | |
242 | snippet(cx, reduced_expr_span, "_") | |
243 | ); | |
244 | diag.span_suggestion( | |
245 | stmt.span, | |
246 | "try this", | |
247 | suggestion, | |
248 | Applicability::MachineApplicable, // snippet | |
249 | ); | |
250 | } else { | |
251 | let suggestion = format!( | |
252 | "if let {0}({1}) = {2} {{ ... }}", | |
253 | variant, | |
254 | snippet(cx, binding.pat.span, "_"), | |
255 | snippet(cx, var_arg.span, "_"), | |
256 | ); | |
257 | diag.span_suggestion(stmt.span, "try this", suggestion, Applicability::HasPlaceholders); | |
258 | } | |
259 | }); | |
260 | } | |
261 | } | |
262 | ||
263 | impl<'tcx> LateLintPass<'tcx> for MapUnit { | |
264 | fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &hir::Stmt<'_>) { | |
265 | if stmt.span.from_expansion() { | |
266 | return; | |
267 | } | |
268 | ||
cdc7bbd5 | 269 | if let hir::StmtKind::Semi(expr) = stmt.kind { |
f20569fa XL |
270 | if let Some(arglists) = method_chain_args(expr, &["map"]) { |
271 | lint_map_unit_fn(cx, stmt, expr, arglists[0]); | |
272 | } | |
273 | } | |
274 | } | |
275 | } |