From fc72e910fbd72b7b30a6cb0b6390e0b7b8d907d8 Mon Sep 17 00:00:00 2001 From: bors Date: Tue, 28 Dec 2021 11:15:53 +0000 Subject: [PATCH] `needless_return` suggest return unit type on void returns closes #8177 previously, `needless_return` suggests an empty block `{}` to replace void `return` on match arms, this PR improve the suggestion by suggesting a unit instead. changelog: `needless_return` suggests `()` instead of `{}` on match arms --- clippy_lints/src/returns.rs | 14 +- tests/ui/needless_return.fixed | 15 ++- tests/ui/needless_return.rs | 11 ++ tests/ui/needless_return.stderr | 218 +++++++++++++++++--------------- 4 files changed, 152 insertions(+), 106 deletions(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 494bc7dda18..112ccdcdd42 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -73,6 +73,7 @@ declare_clippy_lint! { enum RetReplacement { Empty, Block, + Unit, } declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN]); @@ -212,7 +213,7 @@ fn check_final_expr<'tcx>( // (except for unit type functions) so we don't match it ExprKind::Match(_, arms, MatchSource::Normal) => { for arm in arms.iter() { - check_final_expr(cx, arm.body, Some(arm.body.span), RetReplacement::Block); + check_final_expr(cx, arm.body, Some(arm.body.span), RetReplacement::Unit); } }, ExprKind::DropTemps(expr) => check_final_expr(cx, expr, None, RetReplacement::Empty), @@ -259,6 +260,17 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option { + span_lint_and_sugg( + cx, + NEEDLESS_RETURN, + ret_span, + "unneeded `return` statement", + "replace `return` with a unit value", + "()".to_string(), + Applicability::MachineApplicable, + ); + }, }, } } diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index 83f467c8400..603d438d558 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -71,7 +71,18 @@ fn test_void_if_fun(b: bool) { fn test_void_match(x: u32) { match x { 0 => (), - _ => {}, + _ => (), + } +} + +fn test_nested_match(x: u32) { + match x { + 0 => (), + 1 => { + let _ = 42; + + }, + _ => (), } } @@ -182,7 +193,7 @@ async fn async_test_void_if_fun(b: bool) { async fn async_test_void_match(x: u32) { match x { 0 => (), - _ => {}, + _ => (), } } diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index 341caf18bd6..c6c8cb9ec15 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -75,6 +75,17 @@ fn test_void_match(x: u32) { } } +fn test_nested_match(x: u32) { + match x { + 0 => (), + 1 => { + let _ = 42; + return; + }, + _ => return, + } +} + fn read_line() -> String { use std::io::BufRead; let stdin = ::std::io::stdin(); diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index c0abc2c63dd..5bc787c56a6 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -70,127 +70,139 @@ error: unneeded `return` statement --> $DIR/needless_return.rs:74:14 | LL | _ => return, - | ^^^^^^ help: replace `return` with an empty block: `{}` + | ^^^^^^ help: replace `return` with a unit value: `()` error: unneeded `return` statement - --> $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:91:9 - | -LL | return String::new(); - | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:113:32 - | -LL | bar.unwrap_or_else(|_| return) - | ^^^^^^ help: replace `return` with an empty block: `{}` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:118:13 + --> $DIR/needless_return.rs:83:13 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded `return` statement - --> $DIR/needless_return.rs:120:20 - | -LL | let _ = || return; - | ^^^^^^ help: replace `return` with an empty block: `{}` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:126:32 - | -LL | res.unwrap_or_else(|_| return Foo) - | ^^^^^^^^^^ help: remove `return`: `Foo` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:135:5 - | -LL | return true; - | ^^^^^^^^^^^^ help: remove `return`: `true` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:139:5 - | -LL | return true; - | ^^^^^^^^^^^^ help: remove `return`: `true` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:144:9 - | -LL | return true; - | ^^^^^^^^^^^^ help: remove `return`: `true` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:146:9 - | -LL | return false; - | ^^^^^^^^^^^^^ help: remove `return`: `false` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:152:17 - | -LL | true => return false, - | ^^^^^^^^^^^^ help: remove `return`: `false` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:154:13 - | -LL | return true; - | ^^^^^^^^^^^^ help: remove `return`: `true` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:161:9 - | -LL | return true; - | ^^^^^^^^^^^^ help: remove `return`: `true` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:163:16 - | -LL | let _ = || return true; - | ^^^^^^^^^^^ help: remove `return`: `true` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:171:5 - | -LL | return; - | ^^^^^^^ help: remove `return` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:176:9 - | -LL | return; - | ^^^^^^^ help: remove `return` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:178:9 - | -LL | return; - | ^^^^^^^ help: remove `return` - -error: unneeded `return` statement - --> $DIR/needless_return.rs:185:14 + --> $DIR/needless_return.rs:85:14 | LL | _ => return, - | ^^^^^^ help: replace `return` with an empty block: `{}` + | ^^^^^^ help: replace `return` with a unit value: `()` error: unneeded `return` statement - --> $DIR/needless_return.rs:200:9 + --> $DIR/needless_return.rs:100:9 | LL | return String::from("test"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` error: unneeded `return` statement - --> $DIR/needless_return.rs:202:9 + --> $DIR/needless_return.rs:102:9 | LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` -error: aborting due to 32 previous errors +error: unneeded `return` statement + --> $DIR/needless_return.rs:124:32 + | +LL | bar.unwrap_or_else(|_| return) + | ^^^^^^ help: replace `return` with an empty block: `{}` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:129:13 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:131:20 + | +LL | let _ = || return; + | ^^^^^^ help: replace `return` with an empty block: `{}` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:137:32 + | +LL | res.unwrap_or_else(|_| return Foo) + | ^^^^^^^^^^ help: remove `return`: `Foo` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:146:5 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:150:5 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:155:9 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:157:9 + | +LL | return false; + | ^^^^^^^^^^^^^ help: remove `return`: `false` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:163:17 + | +LL | true => return false, + | ^^^^^^^^^^^^ help: remove `return`: `false` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:165:13 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:172:9 + | +LL | return true; + | ^^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:174:16 + | +LL | let _ = || return true; + | ^^^^^^^^^^^ help: remove `return`: `true` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:182:5 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:187:9 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:189:9 + | +LL | return; + | ^^^^^^^ help: remove `return` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:196:14 + | +LL | _ => return, + | ^^^^^^ help: replace `return` with a unit value: `()` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:211:9 + | +LL | return String::from("test"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")` + +error: unneeded `return` statement + --> $DIR/needless_return.rs:213:9 + | +LL | return String::new(); + | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()` + +error: aborting due to 34 previous errors