From 96747c1a460a241c42a9aa262eeb37b910f06760 Mon Sep 17 00:00:00 2001
From: Bastian Kersting <bastian@cmbt.de>
Date: Sat, 5 Jun 2021 14:27:36 +0200
Subject: [PATCH] Enhance semicolon_if_nothing_returned according to #7324

---
 .../src/semicolon_if_nothing_returned.rs      | 20 ++++++++-
 clippy_utils/src/lib.rs                       |  5 +++
 tests/ui/semicolon_if_nothing_returned.rs     | 44 +++++++++++++++++++
 tests/ui/semicolon_if_nothing_returned.stderr | 14 +++++-
 4 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/clippy_lints/src/semicolon_if_nothing_returned.rs b/clippy_lints/src/semicolon_if_nothing_returned.rs
index 16e4d73851f..9e5d5b6e956 100644
--- a/clippy_lints/src/semicolon_if_nothing_returned.rs
+++ b/clippy_lints/src/semicolon_if_nothing_returned.rs
@@ -1,9 +1,9 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_macro_callsite;
-use clippy_utils::{in_macro, sugg};
+use clippy_utils::{get_parent_expr_for_hir, in_macro, spans_on_same_line, sugg};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{Block, ExprKind};
+use rustc_hir::{Block, BlockCheckMode, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
@@ -46,6 +46,22 @@ impl LateLintPass<'_> for SemicolonIfNothingReturned {
             if let snippet = snippet_with_macro_callsite(cx, expr.span, "}");
             if !snippet.ends_with('}');
             then {
+                // check if the block is inside a closure or an unsafe block and don't
+                // emit if the block is on the same line
+                if_chain! {
+                    if let Some(parent) = get_parent_expr_for_hir(cx, block.hir_id);
+
+                    if !matches!(block.rules, BlockCheckMode::DefaultBlock) ||
+                    matches!(parent.kind, ExprKind::Closure(..) | ExprKind::Block(..));
+
+                    if block.stmts.len() == 0;
+
+                    if spans_on_same_line(cx, parent.span, expr.span);
+                    then {
+                        return;
+                    }
+                }
+
                 // filter out the desugared `for` loop
                 if let ExprKind::DropTemps(..) = &expr.kind {
                     return;
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 769836aaf18..be625eb2655 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -820,6 +820,11 @@ fn line_span<T: LintContext>(cx: &T, span: Span) -> Span {
     Span::new(line_start, span.hi(), span.ctxt())
 }
 
+/// Checks if two spans begin on the same line.
+pub fn spans_on_same_line<T: LintContext>(cx: &T, left_span: Span, right_span: Span) -> bool {
+    line_span(cx, left_span).lo() == line_span(cx, right_span).lo()
+}
+
 /// Gets the parent node, if any.
 pub fn get_parent_node(tcx: TyCtxt<'_>, id: HirId) -> Option<Node<'_>> {
     tcx.hir().parent_iter(id).next().map(|(_, node)| node)
diff --git a/tests/ui/semicolon_if_nothing_returned.rs b/tests/ui/semicolon_if_nothing_returned.rs
index 0abe2cca267..79ba7402f1f 100644
--- a/tests/ui/semicolon_if_nothing_returned.rs
+++ b/tests/ui/semicolon_if_nothing_returned.rs
@@ -17,6 +17,24 @@ fn basic101(x: i32) {
     y = x + 1
 }
 
+#[rustfmt::skip]
+fn closure_error() {
+    let _d = || {
+        hello()
+    };
+}
+
+#[rustfmt::skip]
+fn unsafe_checks_error() {
+    use std::mem::MaybeUninit;
+    use std::ptr;
+
+    let mut s = MaybeUninit::<String>::uninit();
+    let _d = || unsafe { 
+        ptr::drop_in_place(s.as_mut_ptr()) 
+    };
+}
+
 // this is fine
 fn print_sum(a: i32, b: i32) {
     println!("{}", a + b);
@@ -53,3 +71,29 @@ fn loop_test(x: i32) {
         println!("{}", ext);
     }
 }
+
+fn closure() {
+    let _d = || hello();
+}
+
+#[rustfmt::skip]
+fn closure_block() {
+    let _d = || { hello() };
+}
+
+unsafe fn some_unsafe_op() {}
+unsafe fn some_other_unsafe_fn() {}
+
+fn do_something() {
+    unsafe { some_unsafe_op() };
+
+    unsafe { some_other_unsafe_fn() };
+}
+
+fn unsafe_checks() {
+    use std::mem::MaybeUninit;
+    use std::ptr;
+
+    let mut s = MaybeUninit::<String>::uninit();
+    let _d = || unsafe { ptr::drop_in_place(s.as_mut_ptr()) };
+}
diff --git a/tests/ui/semicolon_if_nothing_returned.stderr b/tests/ui/semicolon_if_nothing_returned.stderr
index b73f8967538..e88ebe2ad35 100644
--- a/tests/ui/semicolon_if_nothing_returned.stderr
+++ b/tests/ui/semicolon_if_nothing_returned.stderr
@@ -18,5 +18,17 @@ error: consider adding a `;` to the last statement for consistent formatting
 LL |     y = x + 1
    |     ^^^^^^^^^ help: add a `;` here: `y = x + 1;`
 
-error: aborting due to 3 previous errors
+error: consider adding a `;` to the last statement for consistent formatting
+  --> $DIR/semicolon_if_nothing_returned.rs:23:9
+   |
+LL |         hello()
+   |         ^^^^^^^ help: add a `;` here: `hello();`
+
+error: consider adding a `;` to the last statement for consistent formatting
+  --> $DIR/semicolon_if_nothing_returned.rs:34:9
+   |
+LL |         ptr::drop_in_place(s.as_mut_ptr()) 
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add a `;` here: `ptr::drop_in_place(s.as_mut_ptr());`
+
+error: aborting due to 5 previous errors