]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 | 1 | use clippy_utils::diagnostics::span_lint_and_then; |
04454e1e | 2 | use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context}; |
cdc7bbd5 XL |
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 { | |
f2b60f7d | 100 | ty.is_unit() || ty.is_never() |
f20569fa XL |
101 | } |
102 | ||
103 | fn is_unit_function(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { | |
104 | let ty = cx.typeck_results().expr_ty(expr); | |
105 | ||
106 | if let ty::FnDef(id, _) = *ty.kind() { | |
107 | if let Some(fn_type) = cx.tcx.fn_sig(id).no_bound_vars() { | |
108 | return is_unit_type(fn_type.output()); | |
109 | } | |
110 | } | |
111 | false | |
112 | } | |
113 | ||
114 | fn is_unit_expression(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { | |
115 | is_unit_type(cx.typeck_results().expr_ty(expr)) | |
116 | } | |
117 | ||
118 | /// The expression inside a closure may or may not have surrounding braces and | |
119 | /// semicolons, which causes problems when generating a suggestion. Given an | |
120 | /// expression that evaluates to '()' or '!', recursively remove useless braces | |
121 | /// and semi-colons until is suitable for including in the suggestion template | |
122 | fn reduce_unit_expression<'a>(cx: &LateContext<'_>, expr: &'a hir::Expr<'_>) -> Option<Span> { | |
123 | if !is_unit_expression(cx, expr) { | |
124 | return None; | |
125 | } | |
126 | ||
127 | match expr.kind { | |
5099ac24 | 128 | hir::ExprKind::Call(_, _) | hir::ExprKind::MethodCall(..) => { |
f20569fa XL |
129 | // Calls can't be reduced any more |
130 | Some(expr.span) | |
131 | }, | |
cdc7bbd5 | 132 | hir::ExprKind::Block(block, _) => { |
f20569fa | 133 | match (block.stmts, block.expr.as_ref()) { |
2b03887a | 134 | ([], Some(inner_expr)) => { |
f20569fa XL |
135 | // If block only contains an expression, |
136 | // reduce `{ X }` to `X` | |
137 | reduce_unit_expression(cx, inner_expr) | |
138 | }, | |
2b03887a | 139 | ([inner_stmt], None) => { |
f20569fa XL |
140 | // If block only contains statements, |
141 | // reduce `{ X; }` to `X` or `X;` | |
142 | match inner_stmt.kind { | |
cdc7bbd5 XL |
143 | hir::StmtKind::Local(local) => Some(local.span), |
144 | hir::StmtKind::Expr(e) => Some(e.span), | |
f20569fa XL |
145 | hir::StmtKind::Semi(..) => Some(inner_stmt.span), |
146 | hir::StmtKind::Item(..) => None, | |
147 | } | |
148 | }, | |
149 | _ => { | |
150 | // For closures that contain multiple statements | |
151 | // it's difficult to get a correct suggestion span | |
152 | // for all cases (multi-line closures specifically) | |
153 | // | |
154 | // We do not attempt to build a suggestion for those right now. | |
155 | None | |
156 | }, | |
157 | } | |
158 | }, | |
159 | _ => None, | |
160 | } | |
161 | } | |
162 | ||
163 | fn unit_closure<'tcx>( | |
164 | cx: &LateContext<'tcx>, | |
165 | expr: &hir::Expr<'_>, | |
166 | ) -> Option<(&'tcx hir::Param<'tcx>, &'tcx hir::Expr<'tcx>)> { | |
cdc7bbd5 | 167 | if_chain! { |
064997fb | 168 | if let hir::ExprKind::Closure(&hir::Closure { fn_decl, body, .. }) = expr.kind; |
923072b8 | 169 | let body = cx.tcx.hir().body(body); |
f20569fa | 170 | let body_expr = &body.value; |
923072b8 | 171 | if fn_decl.inputs.len() == 1; |
cdc7bbd5 | 172 | if is_unit_expression(cx, body_expr); |
923072b8 | 173 | if let Some(binding) = iter_input_pats(fn_decl, body).next(); |
cdc7bbd5 XL |
174 | then { |
175 | return Some((binding, body_expr)); | |
f20569fa XL |
176 | } |
177 | } | |
178 | None | |
179 | } | |
180 | ||
181 | /// Builds a name for the let binding variable (`var_arg`) | |
182 | /// | |
183 | /// `x.field` => `x_field` | |
184 | /// `y` => `_y` | |
185 | /// | |
186 | /// Anything else will return `a`. | |
187 | fn let_binding_name(cx: &LateContext<'_>, var_arg: &hir::Expr<'_>) -> String { | |
188 | match &var_arg.kind { | |
a2a8927a | 189 | hir::ExprKind::Field(_, _) => snippet(cx, var_arg.span, "_").replace('.', "_"), |
f20569fa XL |
190 | hir::ExprKind::Path(_) => format!("_{}", snippet(cx, var_arg.span, "")), |
191 | _ => "a".to_string(), | |
192 | } | |
193 | } | |
194 | ||
195 | #[must_use] | |
196 | fn suggestion_msg(function_type: &str, map_type: &str) -> String { | |
2b03887a | 197 | format!("called `map(f)` on an `{map_type}` value where `f` is a {function_type} that returns the unit type `()`") |
f20569fa XL |
198 | } |
199 | ||
f2b60f7d FG |
200 | fn lint_map_unit_fn( |
201 | cx: &LateContext<'_>, | |
202 | stmt: &hir::Stmt<'_>, | |
203 | expr: &hir::Expr<'_>, | |
204 | map_args: (&hir::Expr<'_>, &[hir::Expr<'_>]), | |
205 | ) { | |
206 | let var_arg = &map_args.0; | |
f20569fa | 207 | |
c295e0f8 XL |
208 | let (map_type, variant, lint) = if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(var_arg), sym::Option) { |
209 | ("Option", "Some", OPTION_MAP_UNIT_FN) | |
210 | } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(var_arg), sym::Result) { | |
211 | ("Result", "Ok", RESULT_MAP_UNIT_FN) | |
212 | } else { | |
213 | return; | |
214 | }; | |
f2b60f7d | 215 | let fn_arg = &map_args.1[0]; |
f20569fa XL |
216 | |
217 | if is_unit_function(cx, fn_arg) { | |
04454e1e | 218 | let mut applicability = Applicability::MachineApplicable; |
f20569fa XL |
219 | let msg = suggestion_msg("function", map_type); |
220 | let suggestion = format!( | |
221 | "if let {0}({binding}) = {1} {{ {2}({binding}) }}", | |
222 | variant, | |
04454e1e FG |
223 | snippet_with_applicability(cx, var_arg.span, "_", &mut applicability), |
224 | snippet_with_applicability(cx, fn_arg.span, "_", &mut applicability), | |
f20569fa XL |
225 | binding = let_binding_name(cx, var_arg) |
226 | ); | |
227 | ||
228 | span_lint_and_then(cx, lint, expr.span, &msg, |diag| { | |
04454e1e | 229 | diag.span_suggestion(stmt.span, "try this", suggestion, applicability); |
f20569fa XL |
230 | }); |
231 | } else if let Some((binding, closure_expr)) = unit_closure(cx, fn_arg) { | |
232 | let msg = suggestion_msg("closure", map_type); | |
233 | ||
234 | span_lint_and_then(cx, lint, expr.span, &msg, |diag| { | |
235 | if let Some(reduced_expr_span) = reduce_unit_expression(cx, closure_expr) { | |
04454e1e | 236 | let mut applicability = Applicability::MachineApplicable; |
f20569fa XL |
237 | let suggestion = format!( |
238 | "if let {0}({1}) = {2} {{ {3} }}", | |
239 | variant, | |
04454e1e FG |
240 | snippet_with_applicability(cx, binding.pat.span, "_", &mut applicability), |
241 | snippet_with_applicability(cx, var_arg.span, "_", &mut applicability), | |
242 | snippet_with_context(cx, reduced_expr_span, var_arg.span.ctxt(), "_", &mut applicability).0, | |
f20569fa | 243 | ); |
04454e1e | 244 | diag.span_suggestion(stmt.span, "try this", suggestion, applicability); |
f20569fa XL |
245 | } else { |
246 | let suggestion = format!( | |
247 | "if let {0}({1}) = {2} {{ ... }}", | |
248 | variant, | |
249 | snippet(cx, binding.pat.span, "_"), | |
250 | snippet(cx, var_arg.span, "_"), | |
251 | ); | |
252 | diag.span_suggestion(stmt.span, "try this", suggestion, Applicability::HasPlaceholders); | |
253 | } | |
254 | }); | |
255 | } | |
256 | } | |
257 | ||
258 | impl<'tcx> LateLintPass<'tcx> for MapUnit { | |
259 | fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &hir::Stmt<'_>) { | |
260 | if stmt.span.from_expansion() { | |
261 | return; | |
262 | } | |
263 | ||
cdc7bbd5 | 264 | if let hir::StmtKind::Semi(expr) = stmt.kind { |
f20569fa XL |
265 | if let Some(arglists) = method_chain_args(expr, &["map"]) { |
266 | lint_map_unit_fn(cx, stmt, expr, arglists[0]); | |
267 | } | |
268 | } | |
269 | } | |
270 | } |