• Home
  • Line#
  • Scopes#
  • Navigate#
  • Raw
  • Download
1 use clippy_utils::diagnostics::span_lint_hir_and_then;
2 use clippy_utils::higher;
3 use clippy_utils::ty::is_type_diagnostic_item;
4 use clippy_utils::{path_to_local, usage::is_potentially_mutated};
5 use if_chain::if_chain;
6 use rustc_errors::Applicability;
7 use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, Visitor};
8 use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, PathSegment, UnOp};
9 use rustc_lint::{LateContext, LateLintPass};
10 use rustc_middle::hir::nested_filter;
11 use rustc_middle::lint::in_external_macro;
12 use rustc_middle::ty::Ty;
13 use rustc_session::{declare_lint_pass, declare_tool_lint};
14 use rustc_span::def_id::LocalDefId;
15 use rustc_span::source_map::Span;
16 use rustc_span::sym;
17 
18 declare_clippy_lint! {
19     /// ### What it does
20     /// Checks for calls of `unwrap[_err]()` that cannot fail.
21     ///
22     /// ### Why is this bad?
23     /// Using `if let` or `match` is more idiomatic.
24     ///
25     /// ### Example
26     /// ```rust
27     /// # let option = Some(0);
28     /// # fn do_something_with(_x: usize) {}
29     /// if option.is_some() {
30     ///     do_something_with(option.unwrap())
31     /// }
32     /// ```
33     ///
34     /// Could be written:
35     ///
36     /// ```rust
37     /// # let option = Some(0);
38     /// # fn do_something_with(_x: usize) {}
39     /// if let Some(value) = option {
40     ///     do_something_with(value)
41     /// }
42     /// ```
43     #[clippy::version = "pre 1.29.0"]
44     pub UNNECESSARY_UNWRAP,
45     complexity,
46     "checks for calls of `unwrap[_err]()` that cannot fail"
47 }
48 
49 declare_clippy_lint! {
50     /// ### What it does
51     /// Checks for calls of `unwrap[_err]()` that will always fail.
52     ///
53     /// ### Why is this bad?
54     /// If panicking is desired, an explicit `panic!()` should be used.
55     ///
56     /// ### Known problems
57     /// This lint only checks `if` conditions not assignments.
58     /// So something like `let x: Option<()> = None; x.unwrap();` will not be recognized.
59     ///
60     /// ### Example
61     /// ```rust
62     /// # let option = Some(0);
63     /// # fn do_something_with(_x: usize) {}
64     /// if option.is_none() {
65     ///     do_something_with(option.unwrap())
66     /// }
67     /// ```
68     ///
69     /// This code will always panic. The if condition should probably be inverted.
70     #[clippy::version = "pre 1.29.0"]
71     pub PANICKING_UNWRAP,
72     correctness,
73     "checks for calls of `unwrap[_err]()` that will always fail"
74 }
75 
76 /// Visitor that keeps track of which variables are unwrappable.
77 struct UnwrappableVariablesVisitor<'a, 'tcx> {
78     unwrappables: Vec<UnwrapInfo<'tcx>>,
79     cx: &'a LateContext<'tcx>,
80 }
81 
82 /// What kind of unwrappable this is.
83 #[derive(Copy, Clone, Debug)]
84 enum UnwrappableKind {
85     Option,
86     Result,
87 }
88 
89 impl UnwrappableKind {
success_variant_pattern(self) -> &'static str90     fn success_variant_pattern(self) -> &'static str {
91         match self {
92             UnwrappableKind::Option => "Some(..)",
93             UnwrappableKind::Result => "Ok(..)",
94         }
95     }
96 
error_variant_pattern(self) -> &'static str97     fn error_variant_pattern(self) -> &'static str {
98         match self {
99             UnwrappableKind::Option => "None",
100             UnwrappableKind::Result => "Err(..)",
101         }
102     }
103 }
104 
105 /// Contains information about whether a variable can be unwrapped.
106 #[derive(Copy, Clone, Debug)]
107 struct UnwrapInfo<'tcx> {
108     /// The variable that is checked
109     local_id: HirId,
110     /// The if itself
111     if_expr: &'tcx Expr<'tcx>,
112     /// The check, like `x.is_ok()`
113     check: &'tcx Expr<'tcx>,
114     /// The check's name, like `is_ok`
115     check_name: &'tcx PathSegment<'tcx>,
116     /// The branch where the check takes place, like `if x.is_ok() { .. }`
117     branch: &'tcx Expr<'tcx>,
118     /// Whether `is_some()` or `is_ok()` was called (as opposed to `is_err()` or `is_none()`).
119     safe_to_unwrap: bool,
120     /// What kind of unwrappable this is.
121     kind: UnwrappableKind,
122     /// If the check is the entire condition (`if x.is_ok()`) or only a part of it (`foo() &&
123     /// x.is_ok()`)
124     is_entire_condition: bool,
125 }
126 
127 /// Collects the information about unwrappable variables from an if condition
128 /// The `invert` argument tells us whether the condition is negated.
collect_unwrap_info<'tcx>( cx: &LateContext<'tcx>, if_expr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>, branch: &'tcx Expr<'_>, invert: bool, is_entire_condition: bool, ) -> Vec<UnwrapInfo<'tcx>>129 fn collect_unwrap_info<'tcx>(
130     cx: &LateContext<'tcx>,
131     if_expr: &'tcx Expr<'_>,
132     expr: &'tcx Expr<'_>,
133     branch: &'tcx Expr<'_>,
134     invert: bool,
135     is_entire_condition: bool,
136 ) -> Vec<UnwrapInfo<'tcx>> {
137     fn is_relevant_option_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: &str) -> bool {
138         is_type_diagnostic_item(cx, ty, sym::Option) && ["is_some", "is_none"].contains(&method_name)
139     }
140 
141     fn is_relevant_result_call(cx: &LateContext<'_>, ty: Ty<'_>, method_name: &str) -> bool {
142         is_type_diagnostic_item(cx, ty, sym::Result) && ["is_ok", "is_err"].contains(&method_name)
143     }
144 
145     if let ExprKind::Binary(op, left, right) = &expr.kind {
146         match (invert, op.node) {
147             (false, BinOpKind::And | BinOpKind::BitAnd) | (true, BinOpKind::Or | BinOpKind::BitOr) => {
148                 let mut unwrap_info = collect_unwrap_info(cx, if_expr, left, branch, invert, false);
149                 unwrap_info.append(&mut collect_unwrap_info(cx, if_expr, right, branch, invert, false));
150                 return unwrap_info;
151             },
152             _ => (),
153         }
154     } else if let ExprKind::Unary(UnOp::Not, expr) = &expr.kind {
155         return collect_unwrap_info(cx, if_expr, expr, branch, !invert, false);
156     } else {
157         if_chain! {
158             if let ExprKind::MethodCall(method_name, receiver, args, _) = &expr.kind;
159             if let Some(local_id) = path_to_local(receiver);
160             let ty = cx.typeck_results().expr_ty(receiver);
161             let name = method_name.ident.as_str();
162             if is_relevant_option_call(cx, ty, name) || is_relevant_result_call(cx, ty, name);
163             then {
164                 assert!(args.is_empty());
165                 let unwrappable = match name {
166                     "is_some" | "is_ok" => true,
167                     "is_err" | "is_none" => false,
168                     _ => unreachable!(),
169                 };
170                 let safe_to_unwrap = unwrappable != invert;
171                 let kind = if is_type_diagnostic_item(cx, ty, sym::Option) {
172                     UnwrappableKind::Option
173                 } else {
174                     UnwrappableKind::Result
175                 };
176 
177                 return vec![
178                     UnwrapInfo {
179                         local_id,
180                         if_expr,
181                         check: expr,
182                         check_name: method_name,
183                         branch,
184                         safe_to_unwrap,
185                         kind,
186                         is_entire_condition,
187                     }
188                 ]
189             }
190         }
191     }
192     Vec::new()
193 }
194 
195 impl<'a, 'tcx> UnwrappableVariablesVisitor<'a, 'tcx> {
visit_branch( &mut self, if_expr: &'tcx Expr<'_>, cond: &'tcx Expr<'_>, branch: &'tcx Expr<'_>, else_branch: bool, )196     fn visit_branch(
197         &mut self,
198         if_expr: &'tcx Expr<'_>,
199         cond: &'tcx Expr<'_>,
200         branch: &'tcx Expr<'_>,
201         else_branch: bool,
202     ) {
203         let prev_len = self.unwrappables.len();
204         for unwrap_info in collect_unwrap_info(self.cx, if_expr, cond, branch, else_branch, true) {
205             if is_potentially_mutated(unwrap_info.local_id, cond, self.cx)
206                 || is_potentially_mutated(unwrap_info.local_id, branch, self.cx)
207             {
208                 // if the variable is mutated, we don't know whether it can be unwrapped:
209                 continue;
210             }
211             self.unwrappables.push(unwrap_info);
212         }
213         walk_expr(self, branch);
214         self.unwrappables.truncate(prev_len);
215     }
216 }
217 
218 impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
219     type NestedFilter = nested_filter::OnlyBodies;
220 
visit_expr(&mut self, expr: &'tcx Expr<'_>)221     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
222         // Shouldn't lint when `expr` is in macro.
223         if in_external_macro(self.cx.tcx.sess, expr.span) {
224             return;
225         }
226         if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr) {
227             walk_expr(self, cond);
228             self.visit_branch(expr, cond, then, false);
229             if let Some(else_inner) = r#else {
230                 self.visit_branch(expr, cond, else_inner, true);
231             }
232         } else {
233             // find `unwrap[_err]()` calls:
234             if_chain! {
235                 if let ExprKind::MethodCall(method_name, self_arg, ..) = expr.kind;
236                 if let Some(id) = path_to_local(self_arg);
237                 if [sym::unwrap, sym::expect, sym!(unwrap_err)].contains(&method_name.ident.name);
238                 let call_to_unwrap = [sym::unwrap, sym::expect].contains(&method_name.ident.name);
239                 if let Some(unwrappable) = self.unwrappables.iter()
240                     .find(|u| u.local_id == id);
241                 // Span contexts should not differ with the conditional branch
242                 let span_ctxt = expr.span.ctxt();
243                 if unwrappable.branch.span.ctxt() == span_ctxt;
244                 if unwrappable.check.span.ctxt() == span_ctxt;
245                 then {
246                     if call_to_unwrap == unwrappable.safe_to_unwrap {
247                         let is_entire_condition = unwrappable.is_entire_condition;
248                         let unwrappable_variable_name = self.cx.tcx.hir().name(unwrappable.local_id);
249                         let suggested_pattern = if call_to_unwrap {
250                             unwrappable.kind.success_variant_pattern()
251                         } else {
252                             unwrappable.kind.error_variant_pattern()
253                         };
254 
255                         span_lint_hir_and_then(
256                             self.cx,
257                             UNNECESSARY_UNWRAP,
258                             expr.hir_id,
259                             expr.span,
260                             &format!(
261                                 "called `{}` on `{unwrappable_variable_name}` after checking its variant with `{}`",
262                                 method_name.ident.name,
263                                 unwrappable.check_name.ident.as_str(),
264                             ),
265                             |diag| {
266                                 if is_entire_condition {
267                                     diag.span_suggestion(
268                                         unwrappable.check.span.with_lo(unwrappable.if_expr.span.lo()),
269                                         "try",
270                                         format!(
271                                             "if let {suggested_pattern} = {unwrappable_variable_name}",
272                                         ),
273                                         // We don't track how the unwrapped value is used inside the
274                                         // block or suggest deleting the unwrap, so we can't offer a
275                                         // fixable solution.
276                                         Applicability::Unspecified,
277                                     );
278                                 } else {
279                                     diag.span_label(unwrappable.check.span, "the check is happening here");
280                                     diag.help("try using `if let` or `match`");
281                                 }
282                             },
283                         );
284                     } else {
285                         span_lint_hir_and_then(
286                             self.cx,
287                             PANICKING_UNWRAP,
288                             expr.hir_id,
289                             expr.span,
290                             &format!("this call to `{}()` will always panic",
291                             method_name.ident.name),
292                             |diag| { diag.span_label(unwrappable.check.span, "because of this check"); },
293                         );
294                     }
295                 }
296             }
297             walk_expr(self, expr);
298         }
299     }
300 
nested_visit_map(&mut self) -> Self::Map301     fn nested_visit_map(&mut self) -> Self::Map {
302         self.cx.tcx.hir()
303     }
304 }
305 
306 declare_lint_pass!(Unwrap => [PANICKING_UNWRAP, UNNECESSARY_UNWRAP]);
307 
308 impl<'tcx> LateLintPass<'tcx> for Unwrap {
check_fn( &mut self, cx: &LateContext<'tcx>, kind: FnKind<'tcx>, decl: &'tcx FnDecl<'_>, body: &'tcx Body<'_>, span: Span, fn_id: LocalDefId, )309     fn check_fn(
310         &mut self,
311         cx: &LateContext<'tcx>,
312         kind: FnKind<'tcx>,
313         decl: &'tcx FnDecl<'_>,
314         body: &'tcx Body<'_>,
315         span: Span,
316         fn_id: LocalDefId,
317     ) {
318         if span.from_expansion() {
319             return;
320         }
321 
322         let mut v = UnwrappableVariablesVisitor {
323             cx,
324             unwrappables: Vec::new(),
325         };
326 
327         walk_fn(&mut v, kind, decl, body.id(), fn_id);
328     }
329 }
330