Add lint to warn for calls to std::mem::drop with a reference argument

src/drop_ref.rs

@@ -0,0 +1,61 @@
+use rustc::lint::*;
+use rustc_front::hir::*;
+use rustc::middle::ty;
+use syntax::codemap::Span;
+
+use utils::DROP_PATH;
+use utils::{match_def_path, span_note_and_lint};
+
+/// **What it does:** This lint checks for calls to `std::mem::drop` with a reference instead of an owned value.
+///
+/// **Why is this bad?** Calling `drop` on a reference will only drop the reference itself, which is a no-op. It will not call the `drop` method (from the `Drop` trait implementation) on the underlying referenced value, which is likely what was intended.
+///
+/// **Known problems:** None
+///
+/// **Example:**
+/// ```rust
+/// let mut lock_guard = mutex.lock();
+/// std::mem::drop(&lock_guard) //Should have been drop(lock_guard), mutex still locked
+/// operation_that_requires_mutex_to_be_unlocked();
+/// ```
+declare_lint!(pub DROP_REF, Warn,
+    "call to `std::mem::drop` with a reference instead of an owned value, \
+    which will not not call the `Drop::drop` method on the underlying value");
+
+#[allow(missing_copy_implementations)]
+pub struct DropRefPass;
+
+impl LintPass for DropRefPass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(DROP_REF)
+    }
+}
+
+impl LateLintPass for DropRefPass {
+    fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
+        if let ExprCall(ref path, ref args) = expr.node {
+            if let ExprPath(None, _) = path.node {
+                let def_id = cx.tcx.def_map.borrow()[&path.id].def_id();
+                if match_def_path(cx, def_id, &DROP_PATH) {
+                    if args.len() != 1 {
+                        return;
+                    }
+                    check_drop_arg(cx, expr.span, &*args[0]);
+                }
+            }
+        }
+    }
+}
+
+fn check_drop_arg(cx: &LateContext, call_span: Span, arg: &Expr) {
+    let arg_ty = cx.tcx.expr_ty(arg);
+    if let ty::TyRef(..) = arg_ty.sty {
+        span_note_and_lint(cx,
+                           DROP_REF,
+                           call_span,
+                           "call to `std::mem::drop` with a reference argument. \
+                           Dropping a reference does nothing",
+                           arg.span,
+                           &format!("argument has type {}", arg_ty.sty));
+    }
+}

src/lib.rs

@@ -81,6 +81,7 @@ pub mod panic;
 pub mod derive;
 pub mod print;
 pub mod vec;
+pub mod drop_ref;
 
 mod reexport {
     pub use syntax::ast::{Name, NodeId};
@@ -148,6 +149,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box types::CharLitAsU8);
     reg.register_late_lint_pass(box print::PrintLint);
     reg.register_late_lint_pass(box vec::UselessVec);
+    reg.register_late_lint_pass(box drop_ref::DropRefPass);
 
 
     reg.register_lint_group("clippy_pedantic", vec![
@@ -184,6 +186,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
         derive::DERIVE_HASH_NOT_EQ,
         derive::EXPL_IMPL_CLONE_ON_COPY,
+        drop_ref::DROP_REF,
         entry::MAP_ENTRY,
         enum_variants::ENUM_VARIANT_NAMES,
         eq_op::EQ_OP,

src/utils.rs

@@ -27,6 +27,7 @@ pub const CLONE_PATH: [&'static str; 3] = ["clone", "Clone", "clone"];
 pub const CLONE_TRAIT_PATH: [&'static str; 2] = ["clone", "Clone"];
 pub const COW_PATH: [&'static str; 3] = ["collections", "borrow", "Cow"];
 pub const DEFAULT_TRAIT_PATH: [&'static str; 3] = ["core", "default", "Default"];
+pub const DROP_PATH: [&'static str; 3] = ["core", "mem", "drop"];
 pub const HASHMAP_ENTRY_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"];
 pub const HASHMAP_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"];
 pub const HASH_PATH: [&'static str; 2] = ["hash", "Hash"];