only lint on definitions, not use

Author: ada4a

clippy_lints/src/mutex_atomic.rs

@@ -1,7 +1,8 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::res::MaybeDef;
+use clippy_utils::ty::ty_from_hir_ty;
 use rustc_errors::Diag;
-use rustc_hir::Expr;
+use rustc_hir::{Expr, Item, ItemKind, LetStmt};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{self, IntTy, Ty, UintTy};
 use rustc_session::declare_lint_pass;
@@ -89,26 +90,43 @@ declare_clippy_lint! {
 
 declare_lint_pass!(Mutex => [MUTEX_ATOMIC, MUTEX_INTEGER]);
 
+// NOTE: we don't use `check_expr` because that would make us lint every _use_ of such mutexes, not
+// just their definitions
 impl<'tcx> LateLintPass<'tcx> for Mutex {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        let ty = cx.typeck_results().expr_ty(expr);
-        if let ty::Adt(_, subst) = ty.kind()
-            && ty.is_diag_item(cx, sym::Mutex)
-            && let mutex_param = subst.type_at(0)
-            && let Some(atomic_name) = get_atomic_name(mutex_param)
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
+        if !item.span.from_expansion()
+            && let ItemKind::Static(_, _, ty, body_id) = item.kind
         {
-            let msg = "using a `Mutex` where an atomic would do";
-            let diag = |diag: &mut Diag<'_, _>| {
-                diag.help(format!("consider using an `{atomic_name}` instead"));
-                diag.help(
-                    "if you just want the locking behavior and not the internal type, consider using `Mutex<()>`",
-                );
-            };
-            match *mutex_param.kind() {
-                ty::Uint(t) if t != UintTy::Usize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag),
-                ty::Int(t) if t != IntTy::Isize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag),
-                _ => span_lint_and_then(cx, MUTEX_ATOMIC, expr.span, msg, diag),
-            }
+            let body = cx.tcx.hir_body(body_id);
+            let mid_ty = ty_from_hir_ty(cx, ty);
+            check_expr(cx, body.value.peel_blocks(), mid_ty);
+        }
+    }
+    fn check_local(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx LetStmt<'_>) {
+        if !stmt.span.from_expansion()
+            && let Some(init) = stmt.init
+        {
+            let mid_ty = cx.typeck_results().expr_ty(init);
+            check_expr(cx, init.peel_blocks(), mid_ty);
+        }
+    }
+}
+
+fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, ty: Ty<'tcx>) {
+    if let ty::Adt(_, subst) = ty.kind()
+        && ty.is_diag_item(cx, sym::Mutex)
+        && let mutex_param = subst.type_at(0)
+        && let Some(atomic_name) = get_atomic_name(mutex_param)
+    {
+        let msg = "using a `Mutex` where an atomic would do";
+        let diag = |diag: &mut Diag<'_, _>| {
+            diag.help(format!("consider using an `{atomic_name}` instead"));
+            diag.help("if you just want the locking behavior and not the internal type, consider using `Mutex<()>`");
+        };
+        match *mutex_param.kind() {
+            ty::Uint(t) if t != UintTy::Usize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag),
+            ty::Int(t) if t != IntTy::Isize => span_lint_and_then(cx, MUTEX_INTEGER, expr.span, msg, diag),
+            _ => span_lint_and_then(cx, MUTEX_ATOMIC, expr.span, msg, diag),
         }
     }
 }

tests/ui/mutex_atomic.rs

@@ -2,47 +2,64 @@
 #![warn(clippy::mutex_atomic)]
 #![allow(clippy::borrow_as_ptr)]
 
+use std::sync::Mutex;
+
 fn main() {
-    use std::sync::Mutex;
-    Mutex::new(true);
+    let _ = Mutex::new(true);
     //~^ mutex_atomic
 
-    Mutex::new(5usize);
+    let _ = Mutex::new(5usize);
     //~^ mutex_atomic
 
-    Mutex::new(9isize);
+    let _ = Mutex::new(9isize);
     //~^ mutex_atomic
 
     let mut x = 4u32;
-    Mutex::new(&x as *const u32);
+    let _ = Mutex::new(&x as *const u32);
     //~^ mutex_atomic
 
-    Mutex::new(&mut x as *mut u32);
+    let _ = Mutex::new(&mut x as *mut u32);
     //~^ mutex_atomic
 
-    Mutex::new(0u32);
+    let _ = Mutex::new(0u32);
     //~^ mutex_integer
 
-    Mutex::new(0i32);
+    let _ = Mutex::new(0i32);
     //~^ mutex_integer
 
-    Mutex::new(0f32); // there are no float atomics, so this should not lint
-    Mutex::new(0u8);
+    let _ = Mutex::new(0f32); // there are no float atomics, so this should not lint
+    let _ = Mutex::new(0u8);
     //~^ mutex_integer
 
-    Mutex::new(0i16);
+    let _ = Mutex::new(0i16);
     //~^ mutex_integer
 
     let _x: Mutex<i8> = Mutex::new(0);
     //~^ mutex_integer
 
     const X: i64 = 0;
-    Mutex::new(X);
+    let _ = Mutex::new(X);
     //~^ mutex_integer
 
     // there are no 128 atomics, so these two should not lint
     {
-        Mutex::new(0u128);
+        let _ = Mutex::new(0u128);
         let _x: Mutex<i128> = Mutex::new(0);
     }
 }
+
+static MTX: Mutex<u32> = Mutex::new(0);
+//~^ mutex_integer
+
+// don't lint on _use_, only declaration
+fn issue13378() {
+    let mut guard = MTX.lock().unwrap();
+    *guard += 1;
+
+    let mtx = Mutex::new(0);
+    //~^ mutex_integer
+    // This will still lint, since we're reassigning the mutex to a variable -- oh well.
+    // But realistically something like this won't really come up.
+    let reassigned = mtx;
+    //~^ mutex_integer
+}

tests/ui/mutex_atomic.stderr

@@ -1,90 +1,90 @@
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:7:5
+  --> tests/ui/mutex_atomic.rs:8:13
    |
-LL |     Mutex::new(true);
-   |     ^^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(true);
+   |             ^^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicBool` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
    = note: `-D clippy::mutex-atomic` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::mutex_atomic)]`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:10:5
+  --> tests/ui/mutex_atomic.rs:11:13
    |
-LL |     Mutex::new(5usize);
-   |     ^^^^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(5usize);
+   |             ^^^^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicUsize` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:13:5
+  --> tests/ui/mutex_atomic.rs:14:13
    |
-LL |     Mutex::new(9isize);
-   |     ^^^^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(9isize);
+   |             ^^^^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicIsize` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:17:5
+  --> tests/ui/mutex_atomic.rs:18:13
    |
-LL |     Mutex::new(&x as *const u32);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(&x as *const u32);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicPtr` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:20:5
+  --> tests/ui/mutex_atomic.rs:21:13
    |
-LL |     Mutex::new(&mut x as *mut u32);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(&mut x as *mut u32);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicPtr` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:23:5
+  --> tests/ui/mutex_atomic.rs:24:13
    |
-LL |     Mutex::new(0u32);
-   |     ^^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(0u32);
+   |             ^^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicU32` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
    = note: `-D clippy::mutex-integer` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::mutex_integer)]`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:26:5
+  --> tests/ui/mutex_atomic.rs:27:13
    |
-LL |     Mutex::new(0i32);
-   |     ^^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(0i32);
+   |             ^^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicI32` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:30:5
+  --> tests/ui/mutex_atomic.rs:31:13
    |
-LL |     Mutex::new(0u8);
-   |     ^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(0u8);
+   |             ^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicU8` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:33:5
+  --> tests/ui/mutex_atomic.rs:34:13
    |
-LL |     Mutex::new(0i16);
-   |     ^^^^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(0i16);
+   |             ^^^^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicI16` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:36:25
+  --> tests/ui/mutex_atomic.rs:37:25
    |
 LL |     let _x: Mutex<i8> = Mutex::new(0);
    |                         ^^^^^^^^^^^^^
@@ -93,13 +93,40 @@ LL |     let _x: Mutex<i8> = Mutex::new(0);
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
 error: using a `Mutex` where an atomic would do
-  --> tests/ui/mutex_atomic.rs:40:5
+  --> tests/ui/mutex_atomic.rs:41:13
    |
-LL |     Mutex::new(X);
-   |     ^^^^^^^^^^^^^
+LL |     let _ = Mutex::new(X);
+   |             ^^^^^^^^^^^^^
    |
    = help: consider using an `AtomicI64` instead
    = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
 
-error: aborting due to 11 previous errors
+error: using a `Mutex` where an atomic would do
+  --> tests/ui/mutex_atomic.rs:51:26
+   |
+LL | static MTX: Mutex<u32> = Mutex::new(0);
+   |                          ^^^^^^^^^^^^^
+   |
+   = help: consider using an `AtomicU32` instead
+   = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
+
+error: using a `Mutex` where an atomic would do
+  --> tests/ui/mutex_atomic.rs:59:15
+   |
+LL |     let mtx = Mutex::new(0);
+   |               ^^^^^^^^^^^^^
+   |
+   = help: consider using an `AtomicI32` instead
+   = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
+
+error: using a `Mutex` where an atomic would do
+  --> tests/ui/mutex_atomic.rs:63:22
+   |
+LL |     let reassigned = mtx;
+   |                      ^^^
+   |
+   = help: consider using an `AtomicI32` instead
+   = help: if you just want the locking behavior and not the internal type, consider using `Mutex<()>`
+
+error: aborting due to 14 previous errors