]>
Commit | Line | Data |
---|---|---|
f20569fa XL |
1 | use crate::utils::{ |
2 | contains_return, in_macro, match_qpath, paths, return_ty, snippet, span_lint_and_then, | |
3 | visitors::find_all_ret_expressions, | |
4 | }; | |
5 | use if_chain::if_chain; | |
6 | use rustc_errors::Applicability; | |
7 | use rustc_hir::intravisit::FnKind; | |
8 | use rustc_hir::{Body, ExprKind, FnDecl, HirId, Impl, ItemKind, Node}; | |
9 | use rustc_lint::{LateContext, LateLintPass}; | |
10 | use rustc_middle::ty; | |
11 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
12 | use rustc_span::symbol::sym; | |
13 | use rustc_span::Span; | |
14 | ||
15 | declare_clippy_lint! { | |
16 | /// **What it does:** Checks for private functions that only return `Ok` or `Some`. | |
17 | /// | |
18 | /// **Why is this bad?** It is not meaningful to wrap values when no `None` or `Err` is returned. | |
19 | /// | |
20 | /// **Known problems:** There can be false positives if the function signature is designed to | |
21 | /// fit some external requirement. | |
22 | /// | |
23 | /// **Example:** | |
24 | /// | |
25 | /// ```rust | |
26 | /// fn get_cool_number(a: bool, b: bool) -> Option<i32> { | |
27 | /// if a && b { | |
28 | /// return Some(50); | |
29 | /// } | |
30 | /// if a { | |
31 | /// Some(0) | |
32 | /// } else { | |
33 | /// Some(10) | |
34 | /// } | |
35 | /// } | |
36 | /// ``` | |
37 | /// Use instead: | |
38 | /// ```rust | |
39 | /// fn get_cool_number(a: bool, b: bool) -> i32 { | |
40 | /// if a && b { | |
41 | /// return 50; | |
42 | /// } | |
43 | /// if a { | |
44 | /// 0 | |
45 | /// } else { | |
46 | /// 10 | |
47 | /// } | |
48 | /// } | |
49 | /// ``` | |
50 | pub UNNECESSARY_WRAPS, | |
51 | pedantic, | |
52 | "functions that only return `Ok` or `Some`" | |
53 | } | |
54 | ||
55 | declare_lint_pass!(UnnecessaryWraps => [UNNECESSARY_WRAPS]); | |
56 | ||
57 | impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps { | |
58 | fn check_fn( | |
59 | &mut self, | |
60 | cx: &LateContext<'tcx>, | |
61 | fn_kind: FnKind<'tcx>, | |
62 | fn_decl: &FnDecl<'tcx>, | |
63 | body: &Body<'tcx>, | |
64 | span: Span, | |
65 | hir_id: HirId, | |
66 | ) { | |
67 | // Abort if public function/method or closure. | |
68 | match fn_kind { | |
69 | FnKind::ItemFn(.., visibility) | FnKind::Method(.., Some(visibility)) => { | |
70 | if visibility.node.is_pub() { | |
71 | return; | |
72 | } | |
73 | }, | |
74 | FnKind::Closure => return, | |
75 | _ => (), | |
76 | } | |
77 | ||
78 | // Abort if the method is implementing a trait or of it a trait method. | |
79 | if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { | |
80 | if matches!( | |
81 | item.kind, | |
82 | ItemKind::Impl(Impl { of_trait: Some(_), .. }) | ItemKind::Trait(..) | |
83 | ) { | |
84 | return; | |
85 | } | |
86 | } | |
87 | ||
88 | // Get the wrapper and inner types, if can't, abort. | |
89 | let (return_type_label, path, inner_type) = if let ty::Adt(adt_def, subst) = return_ty(cx, hir_id).kind() { | |
90 | if cx.tcx.is_diagnostic_item(sym::option_type, adt_def.did) { | |
91 | ("Option", &paths::OPTION_SOME, subst.type_at(0)) | |
92 | } else if cx.tcx.is_diagnostic_item(sym::result_type, adt_def.did) { | |
93 | ("Result", &paths::RESULT_OK, subst.type_at(0)) | |
94 | } else { | |
95 | return; | |
96 | } | |
97 | } else { | |
98 | return; | |
99 | }; | |
100 | ||
101 | // Check if all return expression respect the following condition and collect them. | |
102 | let mut suggs = Vec::new(); | |
103 | let can_sugg = find_all_ret_expressions(cx, &body.value, |ret_expr| { | |
104 | if_chain! { | |
105 | if !in_macro(ret_expr.span); | |
106 | // Check if a function call. | |
107 | if let ExprKind::Call(ref func, ref args) = ret_expr.kind; | |
108 | // Get the Path of the function call. | |
109 | if let ExprKind::Path(ref qpath) = func.kind; | |
110 | // Check if OPTION_SOME or RESULT_OK, depending on return type. | |
111 | if match_qpath(qpath, path); | |
112 | if args.len() == 1; | |
113 | // Make sure the function argument does not contain a return expression. | |
114 | if !contains_return(&args[0]); | |
115 | then { | |
116 | suggs.push( | |
117 | ( | |
118 | ret_expr.span, | |
119 | if inner_type.is_unit() { | |
120 | "".to_string() | |
121 | } else { | |
122 | snippet(cx, args[0].span.source_callsite(), "..").to_string() | |
123 | } | |
124 | ) | |
125 | ); | |
126 | true | |
127 | } else { | |
128 | false | |
129 | } | |
130 | } | |
131 | }); | |
132 | ||
133 | if can_sugg && !suggs.is_empty() { | |
134 | let (lint_msg, return_type_sugg_msg, return_type_sugg, body_sugg_msg) = if inner_type.is_unit() { | |
135 | ( | |
136 | "this function's return value is unnecessary".to_string(), | |
137 | "remove the return type...".to_string(), | |
138 | snippet(cx, fn_decl.output.span(), "..").to_string(), | |
139 | "...and then remove returned values", | |
140 | ) | |
141 | } else { | |
142 | ( | |
143 | format!( | |
144 | "this function's return value is unnecessarily wrapped by `{}`", | |
145 | return_type_label | |
146 | ), | |
147 | format!("remove `{}` from the return type...", return_type_label), | |
148 | inner_type.to_string(), | |
149 | "...and then change returning expressions", | |
150 | ) | |
151 | }; | |
152 | ||
153 | span_lint_and_then(cx, UNNECESSARY_WRAPS, span, lint_msg.as_str(), |diag| { | |
154 | diag.span_suggestion( | |
155 | fn_decl.output.span(), | |
156 | return_type_sugg_msg.as_str(), | |
157 | return_type_sugg, | |
158 | Applicability::MaybeIncorrect, | |
159 | ); | |
160 | diag.multipart_suggestion(body_sugg_msg, suggs, Applicability::MaybeIncorrect); | |
161 | }); | |
162 | } | |
163 | } | |
164 | } |