Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-27 15:11:30 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #10187 - dswij:issue-10182-semicolon, r=Jarcho

Browse source 
[needless_return]: Remove all semicolons on suggestion

Closes #10182

Multiple semicolons currently breaks autofix for `needless_return` suggestions. Any semicolons left after removing return means that the return type will always be `()`, and thus fail to compile.

This PR allows `needless_return` to remove multiple semicolons.

The change won't cover the case where there is multiple line yet.

i.e.

```rust
fn needless_return() -> bool {
    return true;
;;
}
```

---

changelog: Sugg: [`needless_return`]: Now removes all semicolons on the same line
[#10187](https://github.com/rust-lang/rust-clippy/pull/10187)
<!-- changelog_checked -->
This commit is contained in:
bors 2023-01-15 15:49:40 +00:00
commit 07a7603994
5 changed files with 101 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
clippy_lints/src/returns.rs
View file

@ -1,7 +1,7 @@
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::visitors::{for_each_expr, Descend};
use clippy_utils::{fn_def_id, path_to_local_id};
use clippy_utils::{fn_def_id, path_to_local_id, span_find_starting_semi};
use core::ops::ControlFlow;
use if_chain::if_chain;
use rustc_errors::Applicability;
@ -151,7 +151,7 @@ impl<'tcx> LateLintPass<'tcx> for Return {
kind: FnKind<'tcx>,
_: &'tcx FnDecl<'tcx>,
body: &'tcx Body<'tcx>,
_: Span,
sp: Span,
_: HirId,
) {
match kind {
@ -166,14 +166,14 @@ impl<'tcx> LateLintPass<'tcx> for Return {
check_final_expr(cx, body.value, vec![], replacement);
},
FnKind::ItemFn(..) | FnKind::Method(..) => {
check_block_return(cx, &body.value.kind, vec![]);
check_block_return(cx, &body.value.kind, sp, 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>, semi_spans: Vec<Span>) {
fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>, sp: Span, mut semi_spans: Vec<Span>) {
if let ExprKind::Block(block, _) = expr_kind {
if let Some(block_expr) = block.expr {
check_final_expr(cx, block_expr, semi_spans, RetReplacement::Empty);
@ -183,12 +183,14 @@ fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>,
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`
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);
// Remove ending semicolons and any whitespace ' ' in between.
// Without `return`, the suggestion might not compile if the semicolon is retained
if let Some(semi_span) = stmt.span.trim_start(semi_expr.span) {
let semi_span_to_remove =
span_find_starting_semi(cx.sess().source_map(), semi_span.with_hi(sp.hi()));
semi_spans.push(semi_span_to_remove);
}
check_final_expr(cx, semi_expr, semi_spans, RetReplacement::Empty);
},
_ => (),
}
@ -231,9 +233,9 @@ fn check_final_expr<'tcx>(
emit_return_lint(cx, ret_span, semi_spans, inner.as_ref().map(|i| i.span), replacement);
},
ExprKind::If(_, then, else_clause_opt) => {
check_block_return(cx, &then.kind, semi_spans.clone());
check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone());
if let Some(else_clause) = else_clause_opt {
check_block_return(cx, &else_clause.kind, semi_spans);
check_block_return(cx, &else_clause.kind, peeled_drop_expr.span, semi_spans);
}
},
// a match expr, check all arms
@ -246,7 +248,7 @@ fn check_final_expr<'tcx>(
}
},
// if it's a whole block, check it
other_expr_kind => check_block_return(cx, other_expr_kind, semi_spans),
other_expr_kind => check_block_return(cx, other_expr_kind, peeled_drop_expr.span, semi_spans),
}
}

4
clippy_utils/src/lib.rs
View file

@ -2491,6 +2491,10 @@ pub fn span_extract_comment(sm: &SourceMap, span: Span) -> String {
comments_buf.join("\n")
}
pub fn span_find_starting_semi(sm: &SourceMap, span: Span) -> Span {
sm.span_take_while(span, |&ch| ch == ' ' || ch == ';')
}
macro_rules! op_utils {
($($name:ident $assign:ident)*) => {
/// Binary operation traits like `LangItem::Add`

10
tests/ui/needless_return.fixed
View file

@ -31,6 +31,16 @@ fn test_no_semicolon() -> bool {
true
}
#[rustfmt::skip]
fn test_multiple_semicolon() -> bool {
true
}
#[rustfmt::skip]
fn test_multiple_semicolon_with_spaces() -> bool {
true
}
fn test_if_block() -> bool {
if true {
true

10
tests/ui/needless_return.rs
View file

@ -31,6 +31,16 @@ fn test_no_semicolon() -> bool {
return true;
}
#[rustfmt::skip]
fn test_multiple_semicolon() -> bool {
return true;;;
}
#[rustfmt::skip]
fn test_multiple_semicolon_with_spaces() -> bool {
return true;; ; ;
}
fn test_if_block() -> bool {
if true {
return true;

110
tests/ui/needless_return.stderr
View file

@ -16,7 +16,23 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:36:9
--> $DIR/needless_return.rs:36:5
|
LL | return true;;;
| ^^^^^^^^^^^
|
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:41:5
|
LL | return true;; ; ;
| ^^^^^^^^^^^
|
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:46:9
|
LL | return true;
| ^^^^^^^^^^^
@ -24,7 +40,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:38:9
--> $DIR/needless_return.rs:48:9
|
LL | return false;
| ^^^^^^^^^^^^
@ -32,7 +48,7 @@ LL | return false;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:44:17
--> $DIR/needless_return.rs:54:17
|
LL | true => return false,
| ^^^^^^^^^^^^
@ -40,7 +56,7 @@ LL | true => return false,
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:46:13
--> $DIR/needless_return.rs:56:13
|
LL | return true;
| ^^^^^^^^^^^
@ -48,7 +64,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:53:9
--> $DIR/needless_return.rs:63:9
|
LL | return true;
| ^^^^^^^^^^^
@ -56,7 +72,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:55:16
--> $DIR/needless_return.rs:65:16
|
LL | let _ = || return true;
| ^^^^^^^^^^^
@ -64,7 +80,7 @@ LL | let _ = || return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:59:5
--> $DIR/needless_return.rs:69:5
|
LL | return the_answer!();
| ^^^^^^^^^^^^^^^^^^^^
@ -72,7 +88,7 @@ LL | return the_answer!();
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:62:21
--> $DIR/needless_return.rs:72:21
|
LL | fn test_void_fun() {
| _____________________^
@ -82,7 +98,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:67:11
--> $DIR/needless_return.rs:77:11
|
LL | if b {
| ___________^
@ -92,7 +108,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:69:13
--> $DIR/needless_return.rs:79:13
|
LL | } else {
| _____________^
@ -102,7 +118,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:77:14
--> $DIR/needless_return.rs:87:14
|
LL | _ => return,
| ^^^^^^
@ -110,7 +126,7 @@ LL | _ => return,
= help: replace `return` with a unit value
error: unneeded `return` statement
--> $DIR/needless_return.rs:85:24
--> $DIR/needless_return.rs:95:24
|
LL | let _ = 42;
| ________________________^
@ -120,7 +136,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:88:14
--> $DIR/needless_return.rs:98:14
|
LL | _ => return,
| ^^^^^^
@ -128,7 +144,7 @@ LL | _ => return,
= help: replace `return` with a unit value
error: unneeded `return` statement
--> $DIR/needless_return.rs:101:9
--> $DIR/needless_return.rs:111:9
|
LL | return String::from("test");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -136,7 +152,7 @@ LL | return String::from("test");
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:103:9
--> $DIR/needless_return.rs:113:9
|
LL | return String::new();
| ^^^^^^^^^^^^^^^^^^^^
@ -144,7 +160,7 @@ LL | return String::new();
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:125:32
--> $DIR/needless_return.rs:135:32
|
LL | bar.unwrap_or_else(|_| return)
| ^^^^^^
@ -152,7 +168,7 @@ LL | bar.unwrap_or_else(|_| return)
= help: replace `return` with an empty block
error: unneeded `return` statement
--> $DIR/needless_return.rs:129:21
--> $DIR/needless_return.rs:139:21
|
LL | let _ = || {
| _____________________^
@ -162,7 +178,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:132:20
--> $DIR/needless_return.rs:142:20
|
LL | let _ = || return;
| ^^^^^^
@ -170,7 +186,7 @@ LL | let _ = || return;
= help: replace `return` with an empty block
error: unneeded `return` statement
--> $DIR/needless_return.rs:138:32
--> $DIR/needless_return.rs:148:32
|
LL | res.unwrap_or_else(|_| return Foo)
| ^^^^^^^^^^
@ -178,7 +194,7 @@ LL | res.unwrap_or_else(|_| return Foo)
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:147:5
--> $DIR/needless_return.rs:157:5
|
LL | return true;
| ^^^^^^^^^^^
@ -186,7 +202,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:151:5
--> $DIR/needless_return.rs:161:5
|
LL | return true;
| ^^^^^^^^^^^
@ -194,7 +210,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:156:9
--> $DIR/needless_return.rs:166:9
|
LL | return true;
| ^^^^^^^^^^^
@ -202,7 +218,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:158:9
--> $DIR/needless_return.rs:168:9
|
LL | return false;
| ^^^^^^^^^^^^
@ -210,7 +226,7 @@ LL | return false;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:164:17
--> $DIR/needless_return.rs:174:17
|
LL | true => return false,
| ^^^^^^^^^^^^
@ -218,7 +234,7 @@ LL | true => return false,
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:166:13
--> $DIR/needless_return.rs:176:13
|
LL | return true;
| ^^^^^^^^^^^
@ -226,7 +242,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:173:9
--> $DIR/needless_return.rs:183:9
|
LL | return true;
| ^^^^^^^^^^^
@ -234,7 +250,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:175:16
--> $DIR/needless_return.rs:185:16
|
LL | let _ = || return true;
| ^^^^^^^^^^^
@ -242,7 +258,7 @@ LL | let _ = || return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:179:5
--> $DIR/needless_return.rs:189:5
|
LL | return the_answer!();
| ^^^^^^^^^^^^^^^^^^^^
@ -250,7 +266,7 @@ LL | return the_answer!();
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:182:33
--> $DIR/needless_return.rs:192:33
|
LL | async fn async_test_void_fun() {
| _________________________________^
@ -260,7 +276,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:187:11
--> $DIR/needless_return.rs:197:11
|
LL | if b {
| ___________^
@ -270,7 +286,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:189:13
--> $DIR/needless_return.rs:199:13
|
LL | } else {
| _____________^
@ -280,7 +296,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:197:14
--> $DIR/needless_return.rs:207:14
|
LL | _ => return,
| ^^^^^^
@ -288,7 +304,7 @@ LL | _ => return,
= help: replace `return` with a unit value
error: unneeded `return` statement
--> $DIR/needless_return.rs:210:9
--> $DIR/needless_return.rs:220:9
|
LL | return String::from("test");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -296,7 +312,7 @@ LL | return String::from("test");
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:212:9
--> $DIR/needless_return.rs:222:9
|
LL | return String::new();
| ^^^^^^^^^^^^^^^^^^^^
@ -304,7 +320,7 @@ LL | return String::new();
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:228:5
--> $DIR/needless_return.rs:238:5
|
LL | return format!("Hello {}", "world!");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -312,7 +328,7 @@ LL | return format!("Hello {}", "world!");
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:239:9
--> $DIR/needless_return.rs:249:9
|
LL | return true;
| ^^^^^^^^^^^
@ -320,7 +336,7 @@ LL | return true;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:241:9
--> $DIR/needless_return.rs:251:9
|
LL | return false;
| ^^^^^^^^^^^^
@ -328,7 +344,7 @@ LL | return false;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:248:13
--> $DIR/needless_return.rs:258:13
|
LL | return 10;
| ^^^^^^^^^
@ -336,7 +352,7 @@ LL | return 10;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:251:13
--> $DIR/needless_return.rs:261:13
|
LL | return 100;
| ^^^^^^^^^^
@ -344,7 +360,7 @@ LL | return 100;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:259:9
--> $DIR/needless_return.rs:269:9
|
LL | return 0;
| ^^^^^^^^
@ -352,7 +368,7 @@ LL | return 0;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:266:13
--> $DIR/needless_return.rs:276:13
|
LL | return *(x as *const isize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -360,7 +376,7 @@ LL | return *(x as *const isize);
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:268:13
--> $DIR/needless_return.rs:278:13
|
LL | return !*(x as *const isize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -368,7 +384,7 @@ LL | return !*(x as *const isize);
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:275:20
--> $DIR/needless_return.rs:285:20
|
LL | let _ = 42;
| ____________________^
@ -379,7 +395,7 @@ LL | | return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:282:20
--> $DIR/needless_return.rs:292:20
|
LL | let _ = 42; return;
| ^^^^^^^
@ -387,7 +403,7 @@ LL | let _ = 42; return;
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:294:9
--> $DIR/needless_return.rs:304:9
|
LL | return Ok(format!("ok!"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^
@ -395,12 +411,12 @@ LL | return Ok(format!("ok!"));
= help: remove `return`
error: unneeded `return` statement
--> $DIR/needless_return.rs:296:9
--> $DIR/needless_return.rs:306:9
|
LL | return Err(format!("err!"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: remove `return`
error: aborting due to 48 previous errors
error: aborting due to 50 previous errors