Count unsafe operations only towards the innermost unsafe block

Also, macro calls are counted as at most one unsafe operation.

Author: samueltardieu

clippy_lints/src/multiple_unsafe_ops_per_block.rs

@@ -9,7 +9,7 @@ use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::hir::nested_filter;
 use rustc_middle::ty::{self, TyCtxt, TypeckResults};
 use rustc_session::declare_lint_pass;
-use rustc_span::{DesugaringKind, Span};
+use rustc_span::Span;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -56,12 +56,16 @@ declare_clippy_lint! {
     /// }
     /// ```
     ///
-    /// ### Note
+    /// ### Notes
     ///
-    /// Taking a raw pointer to a union field is always safe and will
-    /// not be considered unsafe by this lint, even when linting code written
-    /// with a specified Rust version of 1.91 or earlier (which required
-    /// using an `unsafe` block).
+    /// - Unsafe operations only count towards the total for the innermost
+    ///   enclosing `unsafe` block.
+    /// - Each call to a macro expanding to unsafe operations count for one
+    ///   unsafe operation.
+    /// - Taking a raw pointer to a union field is always safe and will
+    ///   not be considered unsafe by this lint, even when linting code written
+    ///   with a specified Rust version of 1.91 or earlier (which required
+    ///   using an `unsafe` block).
     #[clippy::version = "1.69.0"]
     pub MULTIPLE_UNSAFE_OPS_PER_BLOCK,
     restriction,
@@ -71,10 +75,7 @@ declare_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK])
 
 impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
     fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) {
-        if !matches!(block.rules, BlockCheckMode::UnsafeBlock(_))
-            || block.span.in_external_macro(cx.tcx.sess.source_map())
-            || block.span.is_desugaring(DesugaringKind::Await)
-        {
+        if !matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) || block.span.from_expansion() {
             return;
         }
         let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block);
@@ -126,6 +127,26 @@ impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
                 return self.visit_expr(e);
             },
 
+            // Do not recurse inside an inner `unsafe` block, it will be checked on its own
+            ExprKind::Block(block, _) if matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) => return,
+
+            // Macro calls containing at least one unsafe operations count
+            // as one unsafe operation.
+            _ if expr.span.from_expansion() => {
+                let mut inner_collector = Self {
+                    tcx: self.tcx,
+                    typeck_results: self.typeck_results,
+                    unsafe_ops: vec![],
+                };
+                walk_expr(&mut inner_collector, expr);
+                if !inner_collector.unsafe_ops.is_empty() {
+                    let span = expr.span.source_callsite();
+                    self.unsafe_ops
+                        .push(("macro call expanded to unsafe operation here", span));
+                }
+                return;
+            },
+
             ExprKind::InlineAsm(_) => self.unsafe_ops.push(("inline assembly used here", expr.span)),
 
             ExprKind::AddrOf(BorrowKind::Raw, _, mut inner) => {

tests/ui/multiple_unsafe_ops_per_block.rs

@@ -312,4 +312,60 @@ fn check_closures() {
     }
 }
 
+fn issue16116() {
+    unsafe fn foo() -> u32 {
+        0
+    }
+
+    unsafe {
+        // Do not lint even though `format!` expansion
+        // contains unsafe calls.
+        let _ = format!("{}", foo());
+    }
+
+    unsafe {
+        //~^ multiple_unsafe_ops_per_block
+        let _ = format!("{}", foo());
+        let _ = format!("{}", foo());
+    }
+
+    unsafe {
+        // Do not lint: only one `assert!()` argument is unsafe
+        assert_eq!(foo(), 0, "{}", 1 + 2);
+    }
+
+    unsafe {
+        // Do not lint, each macro call counts for at most
+        // one unsafe operation.
+        assert_eq!(foo(), 0, "{}", foo()); // One unsafe operation
+    }
+
+    unsafe {
+        // Do not lint, a macro that doesn't emit unsafe operations
+        // will not count.
+        assert_eq!(foo(), 0, "{}", foo()); // One unsafe operation
+        assert_eq!(1, 2); // Zero unsafe operations
+    }
+
+    unsafe {
+        //~^ multiple_unsafe_ops_per_block
+        assert_eq!(foo(), 0, "{}", 1 + 2);
+        assert_eq!(foo(), 0, "{}", 1 + 2);
+    }
+
+    unsafe {
+        // A macro whose expansion contains unsafe blocks will not
+        // check inside the blocks.
+        macro_rules! unsafe_twice {
+            ($e:expr) => {
+                unsafe {
+                    $e;
+                    $e;
+                }
+            };
+        };
+        unsafe_twice!(foo());
+    }
+}
+
 fn main() {}

tests/ui/multiple_unsafe_ops_per_block.stderr

@@ -359,5 +359,47 @@ note: unsafe function call occurs here
 LL |         apply(|| f(0));
    |                  ^^^^
 
-error: aborting due to 16 previous errors
+error: this `unsafe` block contains 2 unsafe operations, expected only one
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:326:5
+   |
+LL | /     unsafe {
+LL | |
+LL | |         let _ = format!("{}", foo());
+LL | |         let _ = format!("{}", foo());
+LL | |     }
+   | |_____^
+   |
+note: macro call expanded to unsafe operation here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:328:17
+   |
+LL |         let _ = format!("{}", foo());
+   |                 ^^^^^^^^^^^^^^^^^^^^
+note: macro call expanded to unsafe operation here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:329:17
+   |
+LL |         let _ = format!("{}", foo());
+   |                 ^^^^^^^^^^^^^^^^^^^^
+
+error: this `unsafe` block contains 2 unsafe operations, expected only one
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:350:5
+   |
+LL | /     unsafe {
+LL | |
+LL | |         assert_eq!(foo(), 0, "{}", 1 + 2);
+LL | |         assert_eq!(foo(), 0, "{}", 1 + 2);
+LL | |     }
+   | |_____^
+   |
+note: macro call expanded to unsafe operation here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:352:9
+   |
+LL |         assert_eq!(foo(), 0, "{}", 1 + 2);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: macro call expanded to unsafe operation here
+  --> tests/ui/multiple_unsafe_ops_per_block.rs:353:9
+   |
+LL |         assert_eq!(foo(), 0, "{}", 1 + 2);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 18 previous errors