]>
Commit | Line | Data |
---|---|---|
f20569fa XL |
1 | use crate::utils::{ |
2 | differing_macro_contexts, is_type_diagnostic_item, span_lint_and_then, usage::is_potentially_mutated, | |
3 | }; | |
4 | use if_chain::if_chain; | |
5 | use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, NestedVisitorMap, Visitor}; | |
6 | use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Path, QPath, UnOp}; | |
7 | use rustc_lint::{LateContext, LateLintPass}; | |
8 | use rustc_middle::hir::map::Map; | |
9 | use rustc_middle::lint::in_external_macro; | |
10 | use rustc_middle::ty::Ty; | |
11 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
12 | use rustc_span::source_map::Span; | |
13 | use rustc_span::sym; | |
14 | ||
15 | declare_clippy_lint! { | |
16 | /// **What it does:** Checks for calls of `unwrap[_err]()` that cannot fail. | |
17 | /// | |
18 | /// **Why is this bad?** Using `if let` or `match` is more idiomatic. | |
19 | /// | |
20 | /// **Known problems:** None | |
21 | /// | |
22 | /// **Example:** | |
23 | /// ```rust | |
24 | /// # let option = Some(0); | |
25 | /// # fn do_something_with(_x: usize) {} | |
26 | /// if option.is_some() { | |
27 | /// do_something_with(option.unwrap()) | |
28 | /// } | |
29 | /// ``` | |
30 | /// | |
31 | /// Could be written: | |
32 | /// | |
33 | /// ```rust | |
34 | /// # let option = Some(0); | |
35 | /// # fn do_something_with(_x: usize) {} | |
36 | /// if let Some(value) = option { | |
37 | /// do_something_with(value) | |
38 | /// } | |
39 | /// ``` | |
40 | pub UNNECESSARY_UNWRAP, | |
41 | complexity, | |
42 | "checks for calls of `unwrap[_err]()` that cannot fail" | |
43 | } | |
44 | ||
45 | declare_clippy_lint! { | |
46 | /// **What it does:** Checks for calls of `unwrap[_err]()` that will always fail. | |
47 | /// | |
48 | /// **Why is this bad?** If panicking is desired, an explicit `panic!()` should be used. | |
49 | /// | |
50 | /// **Known problems:** This lint only checks `if` conditions not assignments. | |
51 | /// So something like `let x: Option<()> = None; x.unwrap();` will not be recognized. | |
52 | /// | |
53 | /// **Example:** | |
54 | /// ```rust | |
55 | /// # let option = Some(0); | |
56 | /// # fn do_something_with(_x: usize) {} | |
57 | /// if option.is_none() { | |
58 | /// do_something_with(option.unwrap()) | |
59 | /// } | |
60 | /// ``` | |
61 | /// | |
62 | /// This code will always panic. The if condition should probably be inverted. | |
63 | pub PANICKING_UNWRAP, | |
64 | correctness, | |
65 | "checks for calls of `unwrap[_err]()` that will always fail" | |
66 | } | |
67 | ||
68 | /// Visitor that keeps track of which variables are unwrappable. | |
69 | struct UnwrappableVariablesVisitor<'a, 'tcx> { | |
70 | unwrappables: Vec<UnwrapInfo<'tcx>>, | |
71 | cx: &'a LateContext<'tcx>, | |
72 | } | |
73 | /// Contains information about whether a variable can be unwrapped. | |
74 | #[derive(Copy, Clone, Debug)] | |
75 | struct UnwrapInfo<'tcx> { | |
76 | /// The variable that is checked | |
77 | ident: &'tcx Path<'tcx>, | |
78 | /// The check, like `x.is_ok()` | |
79 | check: &'tcx Expr<'tcx>, | |
80 | /// The branch where the check takes place, like `if x.is_ok() { .. }` | |
81 | branch: &'tcx Expr<'tcx>, | |
82 | /// Whether `is_some()` or `is_ok()` was called (as opposed to `is_err()` or `is_none()`). | |
83 | safe_to_unwrap: bool, | |
84 | } | |
85 | ||
86 | /// Collects the information about unwrappable variables from an if condition | |
87 | /// The `invert` argument tells us whether the condition is negated. | |
88 | fn collect_unwrap_info<'tcx>( | |
89 | cx: &LateContext<'tcx>, | |
90 | expr: &'tcx Expr<'_>, | |
91 | branch: &'tcx Expr<'_>, | |
92 | invert: bool, | |
93 | ) -> Vec<UnwrapInfo<'tcx>> { | |
94 | fn is_relevant_option_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: &str) -> bool { | |
95 | is_type_diagnostic_item(cx, ty, sym::option_type) && ["is_some", "is_none"].contains(&method_name) | |
96 | } | |
97 | ||
98 | fn is_relevant_result_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: &str) -> bool { | |
99 | is_type_diagnostic_item(cx, ty, sym::result_type) && ["is_ok", "is_err"].contains(&method_name) | |
100 | } | |
101 | ||
102 | if let ExprKind::Binary(op, left, right) = &expr.kind { | |
103 | match (invert, op.node) { | |
104 | (false, BinOpKind::And | BinOpKind::BitAnd) | (true, BinOpKind::Or | BinOpKind::BitOr) => { | |
105 | let mut unwrap_info = collect_unwrap_info(cx, left, branch, invert); | |
106 | unwrap_info.append(&mut collect_unwrap_info(cx, right, branch, invert)); | |
107 | return unwrap_info; | |
108 | }, | |
109 | _ => (), | |
110 | } | |
111 | } else if let ExprKind::Unary(UnOp::Not, expr) = &expr.kind { | |
112 | return collect_unwrap_info(cx, expr, branch, !invert); | |
113 | } else { | |
114 | if_chain! { | |
115 | if let ExprKind::MethodCall(method_name, _, args, _) = &expr.kind; | |
116 | if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].kind; | |
117 | let ty = cx.typeck_results().expr_ty(&args[0]); | |
118 | let name = method_name.ident.as_str(); | |
119 | if is_relevant_option_call(cx, ty, &name) || is_relevant_result_call(cx, ty, &name); | |
120 | then { | |
121 | assert!(args.len() == 1); | |
122 | let unwrappable = match name.as_ref() { | |
123 | "is_some" | "is_ok" => true, | |
124 | "is_err" | "is_none" => false, | |
125 | _ => unreachable!(), | |
126 | }; | |
127 | let safe_to_unwrap = unwrappable != invert; | |
128 | return vec![UnwrapInfo { ident: path, check: expr, branch, safe_to_unwrap }]; | |
129 | } | |
130 | } | |
131 | } | |
132 | Vec::new() | |
133 | } | |
134 | ||
135 | impl<'a, 'tcx> UnwrappableVariablesVisitor<'a, 'tcx> { | |
136 | fn visit_branch(&mut self, cond: &'tcx Expr<'_>, branch: &'tcx Expr<'_>, else_branch: bool) { | |
137 | let prev_len = self.unwrappables.len(); | |
138 | for unwrap_info in collect_unwrap_info(self.cx, cond, branch, else_branch) { | |
139 | if is_potentially_mutated(unwrap_info.ident, cond, self.cx) | |
140 | || is_potentially_mutated(unwrap_info.ident, branch, self.cx) | |
141 | { | |
142 | // if the variable is mutated, we don't know whether it can be unwrapped: | |
143 | continue; | |
144 | } | |
145 | self.unwrappables.push(unwrap_info); | |
146 | } | |
147 | walk_expr(self, branch); | |
148 | self.unwrappables.truncate(prev_len); | |
149 | } | |
150 | } | |
151 | ||
152 | impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> { | |
153 | type Map = Map<'tcx>; | |
154 | ||
155 | fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { | |
156 | // Shouldn't lint when `expr` is in macro. | |
157 | if in_external_macro(self.cx.tcx.sess, expr.span) { | |
158 | return; | |
159 | } | |
160 | if let ExprKind::If(cond, then, els) = &expr.kind { | |
161 | walk_expr(self, cond); | |
162 | self.visit_branch(cond, then, false); | |
163 | if let Some(els) = els { | |
164 | self.visit_branch(cond, els, true); | |
165 | } | |
166 | } else { | |
167 | // find `unwrap[_err]()` calls: | |
168 | if_chain! { | |
169 | if let ExprKind::MethodCall(ref method_name, _, ref args, _) = expr.kind; | |
170 | if let ExprKind::Path(QPath::Resolved(None, ref path)) = args[0].kind; | |
171 | if [sym::unwrap, sym!(unwrap_err)].contains(&method_name.ident.name); | |
172 | let call_to_unwrap = method_name.ident.name == sym::unwrap; | |
173 | if let Some(unwrappable) = self.unwrappables.iter() | |
174 | .find(|u| u.ident.res == path.res); | |
175 | // Span contexts should not differ with the conditional branch | |
176 | if !differing_macro_contexts(unwrappable.branch.span, expr.span); | |
177 | if !differing_macro_contexts(unwrappable.branch.span, unwrappable.check.span); | |
178 | then { | |
179 | if call_to_unwrap == unwrappable.safe_to_unwrap { | |
180 | span_lint_and_then( | |
181 | self.cx, | |
182 | UNNECESSARY_UNWRAP, | |
183 | expr.span, | |
184 | &format!("you checked before that `{}()` cannot fail, \ | |
185 | instead of checking and unwrapping, it's better to use `if let` or `match`", | |
186 | method_name.ident.name), | |
187 | |diag| { diag.span_label(unwrappable.check.span, "the check is happening here"); }, | |
188 | ); | |
189 | } else { | |
190 | span_lint_and_then( | |
191 | self.cx, | |
192 | PANICKING_UNWRAP, | |
193 | expr.span, | |
194 | &format!("this call to `{}()` will always panic", | |
195 | method_name.ident.name), | |
196 | |diag| { diag.span_label(unwrappable.check.span, "because of this check"); }, | |
197 | ); | |
198 | } | |
199 | } | |
200 | } | |
201 | walk_expr(self, expr); | |
202 | } | |
203 | } | |
204 | ||
205 | fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { | |
206 | NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) | |
207 | } | |
208 | } | |
209 | ||
210 | declare_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]); | |
211 | ||
212 | impl<'tcx> LateLintPass<'tcx> for Unwrap { | |
213 | fn check_fn( | |
214 | &mut self, | |
215 | cx: &LateContext<'tcx>, | |
216 | kind: FnKind<'tcx>, | |
217 | decl: &'tcx FnDecl<'_>, | |
218 | body: &'tcx Body<'_>, | |
219 | span: Span, | |
220 | fn_id: HirId, | |
221 | ) { | |
222 | if span.from_expansion() { | |
223 | return; | |
224 | } | |
225 | ||
226 | let mut v = UnwrappableVariablesVisitor { | |
227 | cx, | |
228 | unwrappables: Vec::new(), | |
229 | }; | |
230 | ||
231 | walk_fn(&mut v, kind, decl, body.id(), span, fn_id); | |
232 | } | |
233 | } |