From d1dbdcf332e1a91666999d25ee6a0900512dda82 Mon Sep 17 00:00:00 2001
From: kraktus <kraktus@users.noreply.github.com>
Date: Sun, 18 Sep 2022 20:41:41 +0200
Subject: [PATCH 1/7] refactor `needless_return`
---
clippy_lints/src/returns.rs | 113 +++++++++++++++---------------------
1 file changed, 48 insertions(+), 65 deletions(-)
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 91553240e3c..63c8811f62d 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -72,6 +72,28 @@ enum RetReplacement {
Unit,
}
+impl RetReplacement {
+ fn sugg_help(&self) -> &'static str {
+ match *self {
+ Self::Empty => "remove `return`",
+ Self::Block => "replace `return` with an empty block",
+ Self::Unit => "replace `return` with a unit value",
+
+ }
+ }
+}
+
+impl ToString for RetReplacement {
+ fn to_string(&self) -> String {
+ match *self {
+ Self::Empty => "",
+ Self::Block => "{}",
+ Self::Unit => "()",
+ }
+ .to_string()
+ }
+}
+
declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN]);
impl<'tcx> LateLintPass<'tcx> for Return {
@@ -221,74 +243,35 @@ fn emit_return_lint(
if ret_span.from_expansion() {
return;
}
- match inner_span {
- Some(inner_span) => {
- let mut applicability = Applicability::MachineApplicable;
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- let (snippet, _) = snippet_with_context(cx, inner_span, ret_span.ctxt(), "..", &mut applicability);
- diag.span_suggestion(ret_span, "remove `return`", snippet, applicability);
- },
- );
- },
- None => match replacement {
- RetReplacement::Empty => {
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
+ if let Some(inner_span) = inner_span {
+ let mut applicability = Applicability::MachineApplicable;
+ span_lint_hir_and_then(
+ cx,
+ NEEDLESS_RETURN,
+ emission_place,
+ ret_span,
+ "unneeded `return` statement",
+ |diag| {
+ let (snippet, _) = snippet_with_context(cx, inner_span, ret_span.ctxt(), "..", &mut applicability);
+ diag.span_suggestion(ret_span, "remove `return`", snippet, applicability);
+ },
+ );
+ } else {
+ span_lint_hir_and_then(
+ cx,
+ NEEDLESS_RETURN,
+ emission_place,
+ ret_span,
+ "unneeded `return` statement",
+ |diag| {
+ diag.span_suggestion(
ret_span,
- "unneeded `return` statement",
- |diag| {
- diag.span_suggestion(
- ret_span,
- "remove `return`",
- String::new(),
- Applicability::MachineApplicable,
- );
- },
+ replacement.sugg_help(),
+ replacement.to_string(),
+ Applicability::MachineApplicable,
);
},
- RetReplacement::Block => {
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- diag.span_suggestion(
- ret_span,
- "replace `return` with an empty block",
- "{}".to_string(),
- Applicability::MachineApplicable,
- );
- },
- );
- },
- RetReplacement::Unit => {
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- diag.span_suggestion(
- ret_span,
- "replace `return` with a unit value",
- "()".to_string(),
- Applicability::MachineApplicable,
- );
- },
- );
- },
- },
+ )
}
}
From bf8870eeadf4ce73c5e7dfed071fbc41231aa324 Mon Sep 17 00:00:00 2001
From: kraktus <kraktus@users.noreply.github.com>
Date: Sun, 18 Sep 2022 21:06:06 +0200
Subject: [PATCH 2/7] further refactor of `needless_return`
---
clippy_lints/src/returns.rs | 55 +++++++++++++++----------------------
1 file changed, 22 insertions(+), 33 deletions(-)
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 63c8811f62d..e4692a8d84b 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -78,7 +78,6 @@ impl RetReplacement {
Self::Empty => "remove `return`",
Self::Block => "replace `return` with an empty block",
Self::Unit => "replace `return` with a unit value",
-
}
}
}
@@ -161,37 +160,33 @@ impl<'tcx> LateLintPass<'tcx> for Return {
} else {
RetReplacement::Empty
};
- check_final_expr(cx, body.value, Some(body.value.span), replacement);
+ check_final_expr(cx, body.value, body.value.span, replacement);
},
FnKind::ItemFn(..) | FnKind::Method(..) => {
- if let ExprKind::Block(block, _) = body.value.kind {
- check_block_return(cx, block);
- }
+ check_block_return(cx, &body.value.kind);
},
}
}
}
-fn check_block_return<'tcx>(cx: &LateContext<'tcx>, block: &Block<'tcx>) {
- if let Some(expr) = block.expr {
- check_final_expr(cx, expr, Some(expr.span), RetReplacement::Empty);
- } else if let Some(stmt) = block.stmts.iter().last() {
- match stmt.kind {
- StmtKind::Expr(expr) | StmtKind::Semi(expr) => {
- check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
- },
- _ => (),
+// if `expr` is a block, check if there are needless returns in it
+fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>) {
+ if let ExprKind::Block(block, _) = expr_kind {
+ if let Some(block_expr) = block.expr {
+ check_final_expr(cx, block_expr, block_expr.span, RetReplacement::Empty);
+ } else if let Some(stmt) = block.stmts.iter().last() {
+ match stmt.kind {
+ StmtKind::Expr(expr) | StmtKind::Semi(expr) => {
+ check_final_expr(cx, expr, stmt.span, RetReplacement::Empty);
+ },
+ _ => (),
+ }
}
}
}
-fn check_final_expr<'tcx>(
- cx: &LateContext<'tcx>,
- expr: &'tcx Expr<'tcx>,
- span: Option<Span>,
- replacement: RetReplacement,
-) {
- match expr.kind {
+fn check_final_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, span: Span, replacement: RetReplacement) {
+ match &expr.peel_drop_temps().kind {
// simple return is always "bad"
ExprKind::Ret(ref inner) => {
if cx.tcx.hir().attrs(expr.hir_id).is_empty() {
@@ -200,23 +195,17 @@ fn check_final_expr<'tcx>(
emit_return_lint(
cx,
inner.map_or(expr.hir_id, |inner| inner.hir_id),
- span.expect("`else return` is not possible"),
+ span,
inner.as_ref().map(|i| i.span),
replacement,
);
}
}
},
- // a whole block? check it!
- ExprKind::Block(block, _) => {
- check_block_return(cx, block);
- },
ExprKind::If(_, then, else_clause_opt) => {
- if let ExprKind::Block(ifblock, _) = then.kind {
- check_block_return(cx, ifblock);
- }
+ check_block_return(cx, &then.kind);
if let Some(else_clause) = else_clause_opt {
- check_final_expr(cx, else_clause, None, RetReplacement::Empty);
+ check_block_return(cx, &else_clause.kind)
}
},
// a match expr, check all arms
@@ -225,11 +214,11 @@ 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::Unit);
+ check_final_expr(cx, arm.body, arm.body.span, RetReplacement::Unit);
}
},
- ExprKind::DropTemps(expr) => check_final_expr(cx, expr, None, RetReplacement::Empty),
- _ => (),
+ // if it's a whole block, check it
+ other_expr_kind => check_block_return(cx, &other_expr_kind),
}
}
From 15ec5d26084fe13c984b4e6c2933e048e070680c Mon Sep 17 00:00:00 2001
From: kraktus <kraktus@users.noreply.github.com>
Date: Sun, 18 Sep 2022 21:26:23 +0200
Subject: [PATCH 3/7] further refactor
---
clippy_lints/src/returns.rs | 51 ++++++++++++++++---------------------
1 file changed, 22 insertions(+), 29 deletions(-)
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index e4692a8d84b..26b6898e588 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -232,36 +232,29 @@ fn emit_return_lint(
if ret_span.from_expansion() {
return;
}
- if let Some(inner_span) = inner_span {
- let mut applicability = Applicability::MachineApplicable;
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- let (snippet, _) = snippet_with_context(cx, inner_span, ret_span.ctxt(), "..", &mut applicability);
- diag.span_suggestion(ret_span, "remove `return`", snippet, applicability);
- },
- );
+ let mut applicability = Applicability::MachineApplicable;
+ let return_replacement = inner_span.map_or_else(
+ || replacement.to_string(),
+ |inner_span| {
+ let (snippet, _) = snippet_with_context(cx, inner_span, ret_span.ctxt(), "..", &mut applicability);
+ snippet.to_string()
+ },
+ );
+ let sugg_help = if inner_span.is_some() {
+ "remove `return`"
} else {
- span_lint_hir_and_then(
- cx,
- NEEDLESS_RETURN,
- emission_place,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- diag.span_suggestion(
- ret_span,
- replacement.sugg_help(),
- replacement.to_string(),
- Applicability::MachineApplicable,
- );
- },
- )
- }
+ replacement.sugg_help()
+ };
+ span_lint_hir_and_then(
+ cx,
+ NEEDLESS_RETURN,
+ emission_place,
+ ret_span,
+ "unneeded `return` statement",
+ |diag| {
+ diag.span_suggestion(ret_span, sugg_help, return_replacement, applicability);
+ },
+ );
}
fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
From 23d1d07861457a42a401ea512b11614952339497 Mon Sep 17 00:00:00 2001
From: kraktus <kraktus@users.noreply.github.com>
Date: Mon, 19 Sep 2022 07:43:16 +0200
Subject: [PATCH 4/7] small refactor
---
clippy_lints/src/returns.rs | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 26b6898e588..721c5f64a90 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::span_lint_hir_and_then;
+use clippy_utils::diagnostics::{span_lint_hir_and_then, span_lint_and_then};
use clippy_utils::source::{snippet_opt, snippet_with_context};
use clippy_utils::{fn_def_id, path_to_local_id};
use if_chain::if_chain;
@@ -194,7 +194,6 @@ fn check_final_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, span:
if !borrows {
emit_return_lint(
cx,
- inner.map_or(expr.hir_id, |inner| inner.hir_id),
span,
inner.as_ref().map(|i| i.span),
replacement,
@@ -224,7 +223,6 @@ fn check_final_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, span:
fn emit_return_lint(
cx: &LateContext<'_>,
- emission_place: HirId,
ret_span: Span,
inner_span: Option<Span>,
replacement: RetReplacement,
@@ -245,10 +243,9 @@ fn emit_return_lint(
} else {
replacement.sugg_help()
};
- span_lint_hir_and_then(
+ span_lint_and_then(
cx,
NEEDLESS_RETURN,
- emission_place,
ret_span,
"unneeded `return` statement",
|diag| {
From 5c0cb0deaa116755bfb3f1fd05314c96f64959c9 Mon Sep 17 00:00:00 2001
From: kraktus <kraktus@users.noreply.github.com>
Date: Mon, 19 Sep 2022 12:07:53 +0200
Subject: [PATCH 5/7] [`needless_return`] Recursively remove unneeded
semicolons
---
clippy_lints/src/returns.rs | 65 ++++++-----
tests/ui/needless_return.fixed | 27 +++++
tests/ui/needless_return.rs | 27 +++++
tests/ui/needless_return.stderr | 189 +++++++++++++++++++++++++-------
4 files changed, 244 insertions(+), 64 deletions(-)
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 721c5f64a90..f7ceb415a79 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::{span_lint_hir_and_then, span_lint_and_then};
+use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::{snippet_opt, snippet_with_context};
use clippy_utils::{fn_def_id, path_to_local_id};
use if_chain::if_chain;
@@ -9,7 +9,7 @@ use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::source_map::Span;
+use rustc_span::source_map::{Span, DUMMY_SP};
declare_clippy_lint! {
/// ### What it does
@@ -73,8 +73,8 @@ enum RetReplacement {
}
impl RetReplacement {
- fn sugg_help(&self) -> &'static str {
- match *self {
+ fn sugg_help(self) -> &'static str {
+ match self {
Self::Empty => "remove `return`",
Self::Block => "replace `return` with an empty block",
Self::Unit => "replace `return` with a unit value",
@@ -160,24 +160,30 @@ impl<'tcx> LateLintPass<'tcx> for Return {
} else {
RetReplacement::Empty
};
- check_final_expr(cx, body.value, body.value.span, replacement);
+ check_final_expr(cx, body.value, vec![], replacement);
},
FnKind::ItemFn(..) | FnKind::Method(..) => {
- check_block_return(cx, &body.value.kind);
+ check_block_return(cx, &body.value.kind, vec![]);
},
}
}
}
// if `expr` is a block, check if there are needless returns in it
-fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>) {
+fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>, semi_spans: Vec<Span>) {
if let ExprKind::Block(block, _) = expr_kind {
if let Some(block_expr) = block.expr {
- check_final_expr(cx, block_expr, block_expr.span, RetReplacement::Empty);
+ check_final_expr(cx, block_expr, semi_spans, RetReplacement::Empty);
} else if let Some(stmt) = block.stmts.iter().last() {
match stmt.kind {
- StmtKind::Expr(expr) | StmtKind::Semi(expr) => {
- check_final_expr(cx, expr, stmt.span, RetReplacement::Empty);
+ StmtKind::Expr(expr) => {
+ check_final_expr(cx, expr, semi_spans, RetReplacement::Empty);
+ },
+ StmtKind::Semi(semi_expr) => {
+ let mut semi_spans_and_this_one = semi_spans;
+ // we only want the span containing the semicolon so we can remove it later. From `entry.rs:382`
+ semi_spans_and_this_one.push(stmt.span.trim_start(semi_expr.span).unwrap_or(DUMMY_SP));
+ check_final_expr(cx, semi_expr, semi_spans_and_this_one, RetReplacement::Empty);
},
_ => (),
}
@@ -185,8 +191,15 @@ fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>)
}
}
-fn check_final_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, span: Span, replacement: RetReplacement) {
- match &expr.peel_drop_temps().kind {
+fn check_final_expr<'tcx>(
+ cx: &LateContext<'tcx>,
+ expr: &'tcx Expr<'tcx>,
+ semi_spans: Vec<Span>, /* containing all the places where we would need to remove semicolons if finding an
+ * needless return */
+ replacement: RetReplacement,
+) {
+ let peeled_drop_expr = expr.peel_drop_temps();
+ match &peeled_drop_expr.kind {
// simple return is always "bad"
ExprKind::Ret(ref inner) => {
if cx.tcx.hir().attrs(expr.hir_id).is_empty() {
@@ -194,7 +207,8 @@ fn check_final_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, span:
if !borrows {
emit_return_lint(
cx,
- span,
+ peeled_drop_expr.span,
+ semi_spans,
inner.as_ref().map(|i| i.span),
replacement,
);
@@ -202,9 +216,9 @@ fn check_final_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, span:
}
},
ExprKind::If(_, then, else_clause_opt) => {
- check_block_return(cx, &then.kind);
+ check_block_return(cx, &then.kind, semi_spans.clone());
if let Some(else_clause) = else_clause_opt {
- check_block_return(cx, &else_clause.kind)
+ check_block_return(cx, &else_clause.kind, semi_spans);
}
},
// a match expr, check all arms
@@ -213,17 +227,18 @@ fn check_final_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, span:
// (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, arm.body.span, RetReplacement::Unit);
+ check_final_expr(cx, arm.body, semi_spans.clone(), RetReplacement::Unit);
}
},
// if it's a whole block, check it
- other_expr_kind => check_block_return(cx, &other_expr_kind),
+ other_expr_kind => check_block_return(cx, other_expr_kind, semi_spans),
}
}
fn emit_return_lint(
cx: &LateContext<'_>,
ret_span: Span,
+ semi_spans: Vec<Span>,
inner_span: Option<Span>,
replacement: RetReplacement,
) {
@@ -243,15 +258,13 @@ fn emit_return_lint(
} else {
replacement.sugg_help()
};
- span_lint_and_then(
- cx,
- NEEDLESS_RETURN,
- ret_span,
- "unneeded `return` statement",
- |diag| {
- diag.span_suggestion(ret_span, sugg_help, return_replacement, applicability);
- },
- );
+ span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
+ diag.span_suggestion_hidden(ret_span, sugg_help, return_replacement, applicability);
+ // for each parent statement, we need to remove the semicolon
+ for semi_stmt_span in semi_spans {
+ diag.tool_only_span_suggestion(semi_stmt_span, "remove this semicolon", "", applicability);
+ }
+ });
}
fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed
index 87c8fc03b3c..a5e28b9b1a4 100644
--- a/tests/ui/needless_return.fixed
+++ b/tests/ui/needless_return.fixed
@@ -233,4 +233,31 @@ fn issue_9361() -> i32 {
return 1 + 2;
}
+fn issue8336(x: i32) -> bool {
+ if x > 0 {
+ println!("something");
+ true
+ } else {
+ false
+ }
+}
+
+fn issue8156(x: u8) -> u64 {
+ match x {
+ 80 => {
+ 10
+ },
+ _ => {
+ 100
+ },
+ }
+}
+
+// Ideally the compiler should throw `unused_braces` in this case
+fn issue9192() -> i32 {
+ {
+ 0
+ }
+}
+
fn main() {}
diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs
index 5a86e656255..49fd49c7293 100644
--- a/tests/ui/needless_return.rs
+++ b/tests/ui/needless_return.rs
@@ -233,4 +233,31 @@ fn issue_9361() -> i32 {
return 1 + 2;
}
+fn issue8336(x: i32) -> bool {
+ if x > 0 {
+ println!("something");
+ return true;
+ } else {
+ return false;
+ };
+}
+
+fn issue8156(x: u8) -> u64 {
+ match x {
+ 80 => {
+ return 10;
+ },
+ _ => {
+ return 100;
+ },
+ };
+}
+
+// Ideally the compiler should throw `unused_braces` in this case
+fn issue9192() -> i32 {
+ {
+ return 0;
+ };
+}
+
fn main() {}
diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr
index 83ff0763869..d8dba766bf4 100644
--- a/tests/ui/needless_return.stderr
+++ b/tests/ui/needless_return.stderr
@@ -2,225 +2,338 @@ error: unneeded `return` statement
--> $DIR/needless_return.rs:27:5
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
|
= note: `-D clippy::needless-return` implied by `-D warnings`
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:31:5
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:36:9
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:38:9
|
LL | return false;
- | ^^^^^^^^^^^^^ help: remove `return`: `false`
+ | ^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:44:17
|
LL | true => return false,
- | ^^^^^^^^^^^^ help: remove `return`: `false`
+ | ^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:46:13
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:53:9
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:55:16
|
LL | let _ = || return true;
- | ^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:59:5
|
LL | return the_answer!();
- | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `the_answer!()`
+ | ^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:63:5
|
LL | return;
- | ^^^^^^^ help: remove `return`
+ | ^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:68:9
|
LL | return;
- | ^^^^^^^ help: remove `return`
+ | ^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:70:9
|
LL | return;
- | ^^^^^^^ help: remove `return`
+ | ^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:77:14
|
LL | _ => return,
- | ^^^^^^ help: replace `return` with a unit value: `()`
+ | ^^^^^^
+ |
+ = help: replace `return` with a unit value
error: unneeded `return` statement
--> $DIR/needless_return.rs:86:13
|
LL | return;
- | ^^^^^^^ help: remove `return`
+ | ^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:88:14
|
LL | _ => return,
- | ^^^^^^ help: replace `return` with a unit value: `()`
+ | ^^^^^^
+ |
+ = help: replace `return` with a unit value
error: unneeded `return` statement
--> $DIR/needless_return.rs:101:9
|
LL | return String::from("test");
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:103:9
|
LL | return String::new();
- | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
+ | ^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:125:32
|
LL | bar.unwrap_or_else(|_| return)
- | ^^^^^^ help: replace `return` with an empty block: `{}`
+ | ^^^^^^
+ |
+ = help: replace `return` with an empty block
error: unneeded `return` statement
--> $DIR/needless_return.rs:130:13
|
LL | return;
- | ^^^^^^^ help: remove `return`
+ | ^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:132:20
|
LL | let _ = || return;
- | ^^^^^^ help: replace `return` with an empty block: `{}`
+ | ^^^^^^
+ |
+ = help: replace `return` with an empty block
error: unneeded `return` statement
--> $DIR/needless_return.rs:138:32
|
LL | res.unwrap_or_else(|_| return Foo)
- | ^^^^^^^^^^ help: remove `return`: `Foo`
+ | ^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:147:5
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:151:5
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:156:9
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:158:9
|
LL | return false;
- | ^^^^^^^^^^^^^ help: remove `return`: `false`
+ | ^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:164:17
|
LL | true => return false,
- | ^^^^^^^^^^^^ help: remove `return`: `false`
+ | ^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:166:13
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:173:9
|
LL | return true;
- | ^^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:175:16
|
LL | let _ = || return true;
- | ^^^^^^^^^^^ help: remove `return`: `true`
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:179:5
|
LL | return the_answer!();
- | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `the_answer!()`
+ | ^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:183:5
|
LL | return;
- | ^^^^^^^ help: remove `return`
+ | ^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:188:9
|
LL | return;
- | ^^^^^^^ help: remove `return`
+ | ^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:190:9
|
LL | return;
- | ^^^^^^^ help: remove `return`
+ | ^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:197:14
|
LL | _ => return,
- | ^^^^^^ help: replace `return` with a unit value: `()`
+ | ^^^^^^
+ |
+ = help: replace `return` with a unit value
error: unneeded `return` statement
--> $DIR/needless_return.rs:210:9
|
LL | return String::from("test");
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::from("test")`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:212:9
|
LL | return String::new();
- | ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
+ | ^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:228:5
|
LL | return format!("Hello {}", "world!");
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `format!("Hello {}", "world!")`
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
-error: aborting due to 37 previous errors
+error: unneeded `return` statement
+ --> $DIR/needless_return.rs:239:9
+ |
+LL | return true;
+ | ^^^^^^^^^^^
+ |
+ = help: remove `return`
+
+error: unneeded `return` statement
+ --> $DIR/needless_return.rs:241:9
+ |
+LL | return false;
+ | ^^^^^^^^^^^^
+ |
+ = help: remove `return`
+
+error: unneeded `return` statement
+ --> $DIR/needless_return.rs:248:13
+ |
+LL | return 10;
+ | ^^^^^^^^^
+ |
+ = help: remove `return`
+
+error: unneeded `return` statement
+ --> $DIR/needless_return.rs:251:13
+ |
+LL | return 100;
+ | ^^^^^^^^^^
+ |
+ = help: remove `return`
+
+error: unneeded `return` statement
+ --> $DIR/needless_return.rs:259:9
+ |
+LL | return 0;
+ | ^^^^^^^^
+ |
+ = help: remove `return`
+
+error: aborting due to 42 previous errors
From cda754739471869a12d7055466180e4fec702dba Mon Sep 17 00:00:00 2001
From: kraktus <kraktus@users.noreply.github.com>
Date: Thu, 22 Sep 2022 16:33:14 +0200
Subject: [PATCH 6/7] Make `semicolon_span` code more refactor-tolerant
---
clippy_lints/src/returns.rs | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index f7ceb415a79..f758f4cff8b 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -9,7 +9,7 @@ use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::source_map::{Span, DUMMY_SP};
+use rustc_span::source_map::Span;
declare_clippy_lint! {
/// ### What it does
@@ -182,8 +182,10 @@ fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>,
StmtKind::Semi(semi_expr) => {
let mut semi_spans_and_this_one = semi_spans;
// we only want the span containing the semicolon so we can remove it later. From `entry.rs:382`
- semi_spans_and_this_one.push(stmt.span.trim_start(semi_expr.span).unwrap_or(DUMMY_SP));
- check_final_expr(cx, semi_expr, semi_spans_and_this_one, RetReplacement::Empty);
+ if let Some(semicolon_span) = stmt.span.trim_start(semi_expr.span) {
+ semi_spans_and_this_one.push(semicolon_span);
+ check_final_expr(cx, semi_expr, semi_spans_and_this_one, RetReplacement::Empty);
+ }
},
_ => (),
}
From b33364597105dc60555957a4ae59bb780eb15147 Mon Sep 17 00:00:00 2001
From: kraktus <kraktus@users.noreply.github.com>
Date: Thu, 22 Sep 2022 16:44:21 +0200
Subject: [PATCH 7/7] Add test with `unsafe` block to check #9503 is fixed too
s
---
tests/ui/needless_return.fixed | 10 ++++++++++
tests/ui/needless_return.rs | 10 ++++++++++
tests/ui/needless_return.stderr | 18 +++++++++++++++++-
3 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed
index a5e28b9b1a4..112a2f57b41 100644
--- a/tests/ui/needless_return.fixed
+++ b/tests/ui/needless_return.fixed
@@ -260,4 +260,14 @@ fn issue9192() -> i32 {
}
}
+fn issue9503(x: usize) -> isize {
+ unsafe {
+ if x > 12 {
+ *(x as *const isize)
+ } else {
+ !*(x as *const isize)
+ }
+ }
+}
+
fn main() {}
diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs
index 49fd49c7293..22aa2e11fad 100644
--- a/tests/ui/needless_return.rs
+++ b/tests/ui/needless_return.rs
@@ -260,4 +260,14 @@ fn issue9192() -> i32 {
};
}
+fn issue9503(x: usize) -> isize {
+ unsafe {
+ if x > 12 {
+ return *(x as *const isize);
+ } else {
+ return !*(x as *const isize);
+ };
+ };
+}
+
fn main() {}
diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr
index d8dba766bf4..45090dbe206 100644
--- a/tests/ui/needless_return.stderr
+++ b/tests/ui/needless_return.stderr
@@ -335,5 +335,21 @@ LL | return 0;
|
= help: remove `return`
-error: aborting due to 42 previous errors
+error: unneeded `return` statement
+ --> $DIR/needless_return.rs:266:13
+ |
+LL | return *(x as *const isize);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
+
+error: unneeded `return` statement
+ --> $DIR/needless_return.rs:268:13
+ |
+LL | return !*(x as *const isize);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: remove `return`
+
+error: aborting due to 44 previous errors