From 53260df2fa499f1e1908fb2a6b5ce3442f9a023e Mon Sep 17 00:00:00 2001 From: Takayuki Date: Mon, 12 Apr 2021 21:36:49 +0900 Subject: [PATCH 1/3] fix a false negative on `needless_return` --- clippy_lints/src/returns.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index af772cf4a145..3a6151dce710 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -223,6 +223,7 @@ fn check_final_expr<'tcx>( }, _ => (), }, + ExprKind::DropTemps(expr) => check_final_expr(cx, expr, None, RetReplacement::Empty), _ => (), } } From 0218a3b12f3aca97fc018bdb1e2bf9710c75fa2b Mon Sep 17 00:00:00 2001 From: Takayuki Date: Mon, 12 Apr 2021 21:37:19 +0900 Subject: [PATCH 2/3] add tests for a false negative on `needless_return` --- tests/ui/needless_return.fixed | 103 ++++++++++++++++++++++ tests/ui/needless_return.rs | 103 ++++++++++++++++++++++ tests/ui/needless_return.stderr | 146 +++++++++++++++++++++++++++----- 3 files changed, 333 insertions(+), 19 deletions(-) diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index 82d95cc041fb..70ee6e9b31a1 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -1,4 +1,5 @@ // run-rustfix +// edition:2018 #![allow(unused)] #![allow( @@ -125,6 +126,108 @@ mod issue6501 { } } +async fn async_test_end_of_fn() -> bool { + if true { + // no error! + return true; + } + true +} + +async fn async_test_no_semicolon() -> bool { + true +} + +async fn async_test_if_block() -> bool { + if true { + true + } else { + false + } +} + +async fn async_test_match(x: bool) -> bool { + match x { + true => false, + false => { + true + }, + } +} + +async fn async_test_closure() { + let _ = || { + true + }; + let _ = || true; +} + +async fn async_test_macro_call() -> i32 { + return the_answer!(); +} + +async fn async_test_void_fun() { + +} + +async fn async_test_void_if_fun(b: bool) { + if b { + + } else { + + } +} + +async fn async_test_void_match(x: u32) { + match x { + 0 => (), + _ => {}, + } +} + +async fn async_read_line() -> String { + use std::io::BufRead; + let stdin = ::std::io::stdin(); + return stdin.lock().lines().next().unwrap().unwrap(); +} + +async fn async_borrows_but_not_last(value: bool) -> String { + if value { + use std::io::BufRead; + let stdin = ::std::io::stdin(); + let _a = stdin.lock().lines().next().unwrap().unwrap(); + String::from("test") + } else { + String::new() + } +} + +async fn async_test_return_in_macro() { + // This will return and the macro below won't be executed. Removing the `return` from the macro + // will change semantics. + needed_return!(10); + needed_return!(0); +} + +mod async_issue6501 { + async fn foo(bar: Result<(), ()>) { + bar.unwrap_or_else(|_| {}) + } + + async fn async_test_closure() { + let _ = || { + + }; + let _ = || {}; + } + + struct Foo; + #[allow(clippy::unnecessary_lazy_evaluations)] + async fn bar(res: Result) -> Foo { + res.unwrap_or_else(|_| Foo) + } +} + fn main() { let _ = test_end_of_fn(); let _ = test_no_semicolon(); diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index 8a471f802e11..d0da17cf8624 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -1,4 +1,5 @@ // run-rustfix +// edition:2018 #![allow(unused)] #![allow( @@ -125,6 +126,108 @@ mod issue6501 { } } +async fn async_test_end_of_fn() -> bool { + if true { + // no error! + return true; + } + return true; +} + +async fn async_test_no_semicolon() -> bool { + return true; +} + +async fn async_test_if_block() -> bool { + if true { + return true; + } else { + return false; + } +} + +async fn async_test_match(x: bool) -> bool { + match x { + true => return false, + false => { + return true; + }, + } +} + +async fn async_test_closure() { + let _ = || { + return true; + }; + let _ = || return true; +} + +async fn async_test_macro_call() -> i32 { + return the_answer!(); +} + +async fn async_test_void_fun() { + return; +} + +async fn async_test_void_if_fun(b: bool) { + if b { + return; + } else { + return; + } +} + +async fn async_test_void_match(x: u32) { + match x { + 0 => (), + _ => return, + } +} + +async fn async_read_line() -> String { + use std::io::BufRead; + let stdin = ::std::io::stdin(); + return stdin.lock().lines().next().unwrap().unwrap(); +} + +async fn async_borrows_but_not_last(value: bool) -> String { + if value { + use std::io::BufRead; + let stdin = ::std::io::stdin(); + let _a = stdin.lock().lines().next().unwrap().unwrap(); + return String::from("test"); + } else { + return String::new(); + } +} + +async fn async_test_return_in_macro() { + // This will return and the macro below won't be executed. Removing the `return` from the macro + // will change semantics. + needed_return!(10); + needed_return!(0); +} + +mod async_issue6501 { + async fn foo(bar: Result<(), ()>) { + bar.unwrap_or_else(|_| return) + } + + async fn async_test_closure() { + let _ = || { + return; + }; + let _ = || return; + } + + struct Foo; + #[allow(clippy::unnecessary_lazy_evaluations)] + async fn bar(res: Result) -> Foo { + res.unwrap_or_else(|_| return Foo) + } +} + fn main() { let _ = test_end_of_fn(); let _ = test_no_semicolon(); diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index 075db22456f7..44b293618023 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -1,5 +1,5 @@ error: unneeded `return` statement - --> $DIR/needless_return.rs:23:5 + --> $DIR/needless_return.rs:24:5 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` @@ -7,106 +7,214 @@ LL | return true; = note: `-D clippy::needless-return` implied by `-D warnings` error: unneeded `return` statement - --> $DIR/needless_return.rs:27:5 + --> $DIR/needless_return.rs:28:5 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:32:9 + --> $DIR/needless_return.rs:33:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:34:9 + --> $DIR/needless_return.rs:35:9 | LL | return false; | ^^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded `return` statement - --> $DIR/needless_return.rs:40:17 + --> $DIR/needless_return.rs:41:17 | LL | true => return false, | ^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded `return` statement - --> $DIR/needless_return.rs:42:13 + --> $DIR/needless_return.rs:43:13 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:49:9 + --> $DIR/needless_return.rs:50:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:51:16 + --> $DIR/needless_return.rs:52:16 | LL | let _ = || return true; | ^^^^^^^^^^^ help: remove `return`: `true` error: unneeded `return` statement - --> $DIR/needless_return.rs:59:5 + --> $DIR/needless_return.rs:60:5 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:64:9 + --> $DIR/needless_return.rs:65:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:66:9 + --> $DIR/needless_return.rs:67:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:73:14 + --> $DIR/needless_return.rs:74:14 | LL | _ => return, | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:88:9 + --> $DIR/needless_return.rs:89:9 | LL | return String::from("test"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` error: unneeded `return` statement - --> $DIR/needless_return.rs:90:9 + --> $DIR/needless_return.rs:91:9 | LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` error: unneeded `return` statement - --> $DIR/needless_return.rs:111:32 + --> $DIR/needless_return.rs:112:32 | LL | bar.unwrap_or_else(|_| return) | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:116:13 + --> $DIR/needless_return.rs:117:13 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:118:20 + --> $DIR/needless_return.rs:119:20 | LL | let _ = || return; | ^^^^^^ help: replace `return` with an empty block: `{}` error: unneeded `return` statement - --> $DIR/needless_return.rs:124:32 + --> $DIR/needless_return.rs:125:32 | LL | res.unwrap_or_else(|_| return Foo) | ^^^^^^^^^^ help: remove `return`: `Foo` -error: aborting due to 18 previous errors +error: unneeded `return` statement + --> $DIR/needless_return.rs:134:5 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:138:5 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:143:9 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:145:9 + | +LL | return false; + | ^^^^^^^^^^^^^ help: remove `return`: `false` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:151:17 + | +LL | true => return false, + | ^^^^^^^^^^^^ help: remove `return`: `false` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:153:13 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:160:9 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:162:16 + | +LL | let _ = || return true; + | ^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:170:5 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:175:9 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:177:9 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:184:14 + | +LL | _ => return, + | ^^^^^^ help: replace `return` with an empty block: `{}` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:199:9 + | +LL | return String::from("test"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:201:9 + | +LL | return String::new(); + | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:214:32 + | +LL | bar.unwrap_or_else(|_| return) + | ^^^^^^ help: replace `return` with an empty block: `{}` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:219:13 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:221:20 + | +LL | let _ = || return; + | ^^^^^^ help: replace `return` with an empty block: `{}` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:227:32 + | +LL | res.unwrap_or_else(|_| return Foo) + | ^^^^^^^^^^ help: remove `return`: `Foo` + +error: aborting due to 36 previous errors From e6c67ad2bfd7bb9b4777bb95aea7c4ce70e2dd8a Mon Sep 17 00:00:00 2001 From: Takayuki Date: Mon, 12 Apr 2021 21:58:34 +0900 Subject: [PATCH 3/3] fix limit_stderr_length error --- tests/ui/needless_return.fixed | 29 +---------------------------- tests/ui/needless_return.rs | 29 +---------------------------- tests/ui/needless_return.stderr | 26 +------------------------- 3 files changed, 3 insertions(+), 81 deletions(-) diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index 70ee6e9b31a1..5c4fd466c041 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -203,35 +203,8 @@ async fn async_borrows_but_not_last(value: bool) -> String { } async fn async_test_return_in_macro() { - // This will return and the macro below won't be executed. Removing the `return` from the macro - // will change semantics. needed_return!(10); needed_return!(0); } -mod async_issue6501 { - async fn foo(bar: Result<(), ()>) { - bar.unwrap_or_else(|_| {}) - } - - async fn async_test_closure() { - let _ = || { - - }; - let _ = || {}; - } - - struct Foo; - #[allow(clippy::unnecessary_lazy_evaluations)] - async fn bar(res: Result) -> Foo { - res.unwrap_or_else(|_| Foo) - } -} - -fn main() { - let _ = test_end_of_fn(); - let _ = test_no_semicolon(); - let _ = test_if_block(); - let _ = test_match(true); - test_closure(); -} +fn main() {} diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index d0da17cf8624..34811db7413a 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -203,35 +203,8 @@ async fn async_borrows_but_not_last(value: bool) -> String { } async fn async_test_return_in_macro() { - // This will return and the macro below won't be executed. Removing the `return` from the macro - // will change semantics. needed_return!(10); needed_return!(0); } -mod async_issue6501 { - async fn foo(bar: Result<(), ()>) { - bar.unwrap_or_else(|_| return) - } - - async fn async_test_closure() { - let _ = || { - return; - }; - let _ = || return; - } - - struct Foo; - #[allow(clippy::unnecessary_lazy_evaluations)] - async fn bar(res: Result) -> Foo { - res.unwrap_or_else(|_| return Foo) - } -} - -fn main() { - let _ = test_end_of_fn(); - let _ = test_no_semicolon(); - let _ = test_if_block(); - let _ = test_match(true); - test_closure(); -} +fn main() {} diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index 44b293618023..74dda971fdab 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -192,29 +192,5 @@ error: unneeded `return` statement LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` -error: unneeded `return` statement - --> $DIR/needless_return.rs:214:32 - | -LL | bar.unwrap_or_else(|_| return) - | ^^^^^^ help: replace `return` with an empty block: `{}` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:219:13 - | -LL | return; - | ^^^^^^^ help: remove `return` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:221:20 - | -LL | let _ = || return; - | ^^^^^^ help: replace `return` with an empty block: `{}` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:227:32 - | -LL | res.unwrap_or_else(|_| return Foo) - | ^^^^^^^^^^ help: remove `return`: `Foo` - -error: aborting due to 36 previous errors +error: aborting due to 32 previous errors