|
| 1 | +use rustc_hir::{Arm, Expr, ExprKind, Node}; |
| 2 | +use rustc_span::sym; |
| 3 | + |
| 4 | +use crate::{ |
| 5 | + lints::{DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag}, |
| 6 | + LateContext, LateLintPass, LintContext, |
| 7 | +}; |
| 8 | + |
| 9 | +declare_lint! { |
| 10 | + /// The `drop_ref` lint checks for calls to `std::mem::drop` with a reference |
| 11 | + /// instead of an owned value. |
| 12 | + /// |
| 13 | + /// ### Example |
| 14 | + /// |
| 15 | + /// ```rust |
| 16 | + /// # fn operation_that_requires_mutex_to_be_unlocked() {} // just to make it compile |
| 17 | + /// # let mutex = std::sync::Mutex::new(1); // just to make it compile |
| 18 | + /// let mut lock_guard = mutex.lock(); |
| 19 | + /// std::mem::drop(&lock_guard); // Should have been drop(lock_guard), mutex |
| 20 | + /// // still locked |
| 21 | + /// operation_that_requires_mutex_to_be_unlocked(); |
| 22 | + /// ``` |
| 23 | + /// |
| 24 | + /// {{produces}} |
| 25 | + /// |
| 26 | + /// ### Explanation |
| 27 | + /// |
| 28 | + /// Calling `drop` on a reference will only drop the |
| 29 | + /// reference itself, which is a no-op. It will not call the `drop` method (from |
| 30 | + /// the `Drop` trait implementation) on the underlying referenced value, which |
| 31 | + /// is likely what was intended. |
| 32 | + pub DROP_REF, |
| 33 | + Warn, |
| 34 | + "calls to `std::mem::drop` with a reference instead of an owned value" |
| 35 | +} |
| 36 | + |
| 37 | +declare_lint! { |
| 38 | + /// The `forget_ref` lint checks for calls to `std::mem::forget` with a reference |
| 39 | + /// instead of an owned value. |
| 40 | + /// |
| 41 | + /// ### Example |
| 42 | + /// |
| 43 | + /// ```rust |
| 44 | + /// let x = Box::new(1); |
| 45 | + /// std::mem::forget(&x); // Should have been forget(x), x will still be dropped |
| 46 | + /// ``` |
| 47 | + /// |
| 48 | + /// {{produces}} |
| 49 | + /// |
| 50 | + /// ### Explanation |
| 51 | + /// |
| 52 | + /// Calling `forget` on a reference will only forget the |
| 53 | + /// reference itself, which is a no-op. It will not forget the underlying |
| 54 | + /// referenced value, which is likely what was intended. |
| 55 | + pub FORGET_REF, |
| 56 | + Warn, |
| 57 | + "calls to `std::mem::forget` with a reference instead of an owned value" |
| 58 | +} |
| 59 | + |
| 60 | +declare_lint! { |
| 61 | + /// The `drop_copy` lint checks for calls to `std::mem::drop` with a value |
| 62 | + /// that derives the Copy trait. |
| 63 | + /// |
| 64 | + /// ### Example |
| 65 | + /// |
| 66 | + /// ```rust |
| 67 | + /// let x: i32 = 42; // i32 implements Copy |
| 68 | + /// std::mem::drop(x); // A copy of x is passed to the function, leaving the |
| 69 | + /// // original unaffected |
| 70 | + /// ``` |
| 71 | + /// |
| 72 | + /// {{produces}} |
| 73 | + /// |
| 74 | + /// ### Explanation |
| 75 | + /// |
| 76 | + /// Calling `std::mem::drop` [does nothing for types that |
| 77 | + /// implement Copy](https://doc.rust-lang.org/std/mem/fn.drop.html), since the |
| 78 | + /// value will be copied and moved into the function on invocation. |
| 79 | + pub DROP_COPY, |
| 80 | + Warn, |
| 81 | + "calls to `std::mem::drop` with a value that implements Copy" |
| 82 | +} |
| 83 | + |
| 84 | +declare_lint! { |
| 85 | + /// The `forget_copy` lint checks for calls to `std::mem::forget` with a value |
| 86 | + /// that derives the Copy trait. |
| 87 | + /// |
| 88 | + /// ### Example |
| 89 | + /// |
| 90 | + /// ```rust |
| 91 | + /// let x: i32 = 42; // i32 implements Copy |
| 92 | + /// std::mem::forget(x); // A copy of x is passed to the function, leaving the |
| 93 | + /// // original unaffected |
| 94 | + /// ``` |
| 95 | + /// |
| 96 | + /// {{produces}} |
| 97 | + /// |
| 98 | + /// ### Explanation |
| 99 | + /// |
| 100 | + /// Calling `std::mem::forget` [does nothing for types that |
| 101 | + /// implement Copy](https://doc.rust-lang.org/std/mem/fn.drop.html) since the |
| 102 | + /// value will be copied and moved into the function on invocation. |
| 103 | + /// |
| 104 | + /// An alternative, but also valid, explanation is that Copy types do not |
| 105 | + /// implement the Drop trait, which means they have no destructors. Without a |
| 106 | + /// destructor, there is nothing for `std::mem::forget` to ignore. |
| 107 | + pub FORGET_COPY, |
| 108 | + Warn, |
| 109 | + "calls to `std::mem::forget` with a value that implements Copy" |
| 110 | +} |
| 111 | + |
| 112 | +declare_lint_pass!(DropForgetUseless => [DROP_REF, FORGET_REF, DROP_COPY, FORGET_COPY]); |
| 113 | + |
| 114 | +impl<'tcx> LateLintPass<'tcx> for DropForgetUseless { |
| 115 | + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { |
| 116 | + if let ExprKind::Call(path, [arg]) = expr.kind |
| 117 | + && let ExprKind::Path(ref qpath) = path.kind |
| 118 | + && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() |
| 119 | + && let Some(fn_name) = cx.tcx.get_diagnostic_name(def_id) |
| 120 | + { |
| 121 | + let arg_ty = cx.typeck_results().expr_ty(arg); |
| 122 | + let is_copy = arg_ty.is_copy_modulo_regions(cx.tcx, cx.param_env); |
| 123 | + let drop_is_single_call_in_arm = is_single_call_in_arm(cx, arg, expr); |
| 124 | + match fn_name { |
| 125 | + sym::mem_drop if arg_ty.is_ref() && !drop_is_single_call_in_arm => { |
| 126 | + cx.emit_spanned_lint(DROP_REF, expr.span, DropRefDiag { arg_ty, label: arg.span }); |
| 127 | + }, |
| 128 | + sym::mem_forget if arg_ty.is_ref() => { |
| 129 | + cx.emit_spanned_lint(FORGET_REF, expr.span, ForgetRefDiag { arg_ty, label: arg.span }); |
| 130 | + }, |
| 131 | + sym::mem_drop if is_copy && !drop_is_single_call_in_arm => { |
| 132 | + cx.emit_spanned_lint(DROP_COPY, expr.span, DropCopyDiag { arg_ty, label: arg.span }); |
| 133 | + } |
| 134 | + sym::mem_forget if is_copy => { |
| 135 | + cx.emit_spanned_lint(FORGET_COPY, expr.span, ForgetCopyDiag { arg_ty, label: arg.span }); |
| 136 | + } |
| 137 | + _ => return, |
| 138 | + }; |
| 139 | + } |
| 140 | + } |
| 141 | +} |
| 142 | + |
| 143 | +// Dropping returned value of a function, as in the following snippet is considered idiomatic, see |
| 144 | +// rust-lang/rust-clippy#9482 for examples. |
| 145 | +// |
| 146 | +// ``` |
| 147 | +// match <var> { |
| 148 | +// <pat> => drop(fn_with_side_effect_and_returning_some_value()), |
| 149 | +// .. |
| 150 | +// } |
| 151 | +// ``` |
| 152 | +fn is_single_call_in_arm<'tcx>( |
| 153 | + cx: &LateContext<'tcx>, |
| 154 | + arg: &'tcx Expr<'_>, |
| 155 | + drop_expr: &'tcx Expr<'_>, |
| 156 | +) -> bool { |
| 157 | + if matches!(arg.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)) { |
| 158 | + let parent_node = cx.tcx.hir().find_parent(drop_expr.hir_id); |
| 159 | + if let Some(Node::Arm(Arm { body, .. })) = &parent_node { |
| 160 | + return body.hir_id == drop_expr.hir_id; |
| 161 | + } |
| 162 | + } |
| 163 | + false |
| 164 | +} |
0 commit comments