From 66dbc02b0594cc2bab953b8f821e3d1562436475 Mon Sep 17 00:00:00 2001 From: Daniele D'Orazio Date: Fri, 21 Jun 2019 14:46:34 +0200 Subject: [PATCH] more idiomatic code --- clippy_lints/src/returns.rs | 78 +++++++++++++++++++-------------- tests/ui/needless_return.stderr | 18 ++++---- 2 files changed, 53 insertions(+), 43 deletions(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index caeb1e8b9..2245b719c 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -86,7 +86,7 @@ declare_clippy_lint! { #[derive(PartialEq, Eq, Copy, Clone)] enum RetReplacement { Empty, - Unit + Unit, } declare_lint_pass!(Return => [NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT]); @@ -105,13 +105,24 @@ impl Return { } // Check a the final expression in a block if it's a return. - fn check_final_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr, span: Option, replacement: RetReplacement) { + fn check_final_expr( + &mut self, + cx: &EarlyContext<'_>, + expr: &ast::Expr, + span: Option, + replacement: RetReplacement, + ) { match expr.node { // simple return is always "bad" ast::ExprKind::Ret(ref inner) => { // allow `#[cfg(a)] return a; #[cfg(b)] return b;` if !expr.attrs.iter().any(attr_is_cfg) { - self.emit_return_lint(cx, span.expect("`else return` is not possible"), inner.as_ref().map(|i| i.span), replacement); + self.emit_return_lint( + cx, + span.expect("`else return` is not possible"), + inner.as_ref().map(|i| i.span), + replacement, + ); } }, // a whole block? check it! @@ -135,7 +146,13 @@ impl Return { } } - fn emit_return_lint(&mut self, cx: &EarlyContext<'_>, ret_span: Span, inner_span: Option, replacement: RetReplacement) { + fn emit_return_lint( + &mut self, + cx: &EarlyContext<'_>, + ret_span: Span, + inner_span: Option, + replacement: RetReplacement, + ) { match inner_span { Some(inner_span) => { if in_external_macro(cx.sess(), inner_span) || in_macro_or_desugar(inner_span) { @@ -144,39 +161,32 @@ impl Return { span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { if let Some(snippet) = snippet_opt(cx, inner_span) { - db.span_suggestion( - ret_span, - "remove `return` as shown", - snippet, - Applicability::MachineApplicable, - ); + db.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable); } }) }, - None => { - match replacement { - RetReplacement::Empty => { - span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { - db.span_suggestion( - ret_span, - "remove `return`", - String::new(), - Applicability::MachineApplicable, - ); - }); - } - RetReplacement::Unit => { - span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { - db.span_suggestion( - ret_span, - "replace `return` with the unit type `()`", - "()".to_string(), - Applicability::MachineApplicable, - ); - }); - } - } - } + None => match replacement { + RetReplacement::Empty => { + span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { + db.span_suggestion( + ret_span, + "remove `return`", + String::new(), + Applicability::MachineApplicable, + ); + }); + }, + RetReplacement::Unit => { + span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded return statement", |db| { + db.span_suggestion( + ret_span, + "replace `return` with the unit type", + "()".to_string(), + Applicability::MachineApplicable, + ); + }); + }, + }, } } diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index 88dc5252f..7858ecfba 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -2,7 +2,7 @@ error: unneeded return statement --> $DIR/needless_return.rs:14:5 | LL | return true; - | ^^^^^^^^^^^^ help: remove `return` as shown: `true` + | ^^^^^^^^^^^^ help: remove `return`: `true` | = note: `-D clippy::needless-return` implied by `-D warnings` @@ -10,43 +10,43 @@ error: unneeded return statement --> $DIR/needless_return.rs:18:5 | LL | return true; - | ^^^^^^^^^^^^ help: remove `return` as shown: `true` + | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement --> $DIR/needless_return.rs:23:9 | LL | return true; - | ^^^^^^^^^^^^ help: remove `return` as shown: `true` + | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement --> $DIR/needless_return.rs:25:9 | LL | return false; - | ^^^^^^^^^^^^^ help: remove `return` as shown: `false` + | ^^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded return statement --> $DIR/needless_return.rs:31:17 | LL | true => return false, - | ^^^^^^^^^^^^ help: remove `return` as shown: `false` + | ^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded return statement --> $DIR/needless_return.rs:33:13 | LL | return true; - | ^^^^^^^^^^^^ help: remove `return` as shown: `true` + | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement --> $DIR/needless_return.rs:40:9 | LL | return true; - | ^^^^^^^^^^^^ help: remove `return` as shown: `true` + | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement --> $DIR/needless_return.rs:42:16 | LL | let _ = || return true; - | ^^^^^^^^^^^ help: remove `return` as shown: `true` + | ^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement --> $DIR/needless_return.rs:50:5 @@ -70,7 +70,7 @@ error: unneeded return statement --> $DIR/needless_return.rs:64:14 | LL | _ => return, - | ^^^^^^ help: replace `return` with the unit type `()`: `()` + | ^^^^^^ help: replace `return` with the unit type: `()` error: aborting due to 12 previous errors