about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-06-09 08:39:56 +0000
committerbors <bors@rust-lang.org>2021-06-09 08:39:56 +0000
commitdd0246813f3f42a6cadc2109cdbda76300495981 (patch)
tree453f713bffc8c1c6c63fca271ca0658cf0a4e4e7
parentda0538eaca6be7f89d1734bd4a89fb3af97b0049 (diff)
parent5ec80f32c75eae1fd27389ce0556ec4011144498 (diff)
downloadrust-dd0246813f3f42a6cadc2109cdbda76300495981.tar.gz
rust-dd0246813f3f42a6cadc2109cdbda76300495981.zip
Auto merge of #7326 - 1c3t3a:1c3t3a-issue-7324, r=flip1995
Fix false positive on `semicolon_if_nothing_returned`

Currently the [`semicolon_if_nothing_returned`](https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned) lint fires in unwanted situations where a block only spans one line. An example of this was given in #7324. This code:

```rust
use std::mem::MaybeUninit;
use std::ptr;

fn main() {
    let mut s = MaybeUninit::<String>::uninit();
    let _d = || unsafe { ptr::drop_in_place(s.as_mut_ptr()) };
}
```

yields the following clippy error:
```
error: consider adding a `;` to the last statement for consistent formatting
 --> src/main.rs:6:26
  |
6 |     let _d = || unsafe { ptr::drop_in_place(s.as_mut_ptr()) };
  |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add a `;` here: `ptr::drop_in_place(s.as_mut_ptr());`
  |
  = note: `-D clippy::semicolon-if-nothing-returned` implied by `-D clippy::pedantic`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
```

I updated the lint to check if the statement is inside an `unsafe` block, a closure or a normal block and if the block only spans one line, in that case the lint is not emitted.

This closes #7324.

changelog: enhanced semicolon if nothing returned according to  #7324.
-rw-r--r--clippy_lints/src/semicolon_if_nothing_returned.rs2
-rw-r--r--tests/ui/semicolon_if_nothing_returned.rs44
-rw-r--r--tests/ui/semicolon_if_nothing_returned.stderr14
3 files changed, 59 insertions, 1 deletions
diff --git a/clippy_lints/src/semicolon_if_nothing_returned.rs b/clippy_lints/src/semicolon_if_nothing_returned.rs
index 16e4d73851f..da3e30af35c 100644
--- a/clippy_lints/src/semicolon_if_nothing_returned.rs
+++ b/clippy_lints/src/semicolon_if_nothing_returned.rs
@@ -1,3 +1,4 @@
+use crate::rustc_lint::LintContext;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_macro_callsite;
 use clippy_utils::{in_macro, sugg};
@@ -45,6 +46,7 @@ impl LateLintPass<'_> for SemicolonIfNothingReturned {
             if t_expr.is_unit();
             if let snippet = snippet_with_macro_callsite(cx, expr.span, "}");
             if !snippet.ends_with('}');
+            if cx.sess().source_map().is_multiline(block.span);
             then {
                 // filter out the desugared `for` loop
                 if let ExprKind::DropTemps(..) = &expr.kind {
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