mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #4220 - d-dorazio:4182-needless-return-void-functions, r=flip1995

Browse Source 
make needless_return work with void functions

fixes https://github.com/rust-lang/rust-clippy/issues/4181.

changelog: make needless_return work with void functions.

I don't think the failure is related to my changes, but I'm not sure 🤔
This commit is contained in:
bors 2019-06-22 09:38:53 +00:00
commit c5d1ecd474
7 changed files with 143 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
clippy_lints/src/invalid_ref.rs
View File

@ -51,6 +51,5 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidRef {
span_help_and_lint(cx, INVALID_REF, expr.span, msg, HELP);
}
}
return;
}
}

11
clippy_lints/src/methods/mod.rs
View File

@ -1648,16 +1648,15 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args:
);
// Check if the first argument to .fold is a suitable literal
match fold_args[1].node {
hir::ExprKind::Lit(ref lit) => match lit.node {
if let hir::ExprKind::Lit(ref lit) = fold_args[1].node {
match lit.node {
ast::LitKind::Bool(false) => check_fold_with_op(cx, fold_args, hir::BinOpKind::Or, "any", true),
ast::LitKind::Bool(true) => check_fold_with_op(cx, fold_args, hir::BinOpKind::And, "all", true),
ast::LitKind::Int(0, _) => check_fold_with_op(cx, fold_args, hir::BinOpKind::Add, "sum", false),
ast::LitKind::Int(1, _) => check_fold_with_op(cx, fold_args, hir::BinOpKind::Mul, "product", false),
_ => return,
},
_ => return,
};
_ => (),
}
}
}
fn lint_iter_nth<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr, iter_args: &'tcx [hir::Expr], is_mut: bool) {

4
clippy_lints/src/non_expressive_names.rs
View File

@ -169,7 +169,9 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
.any(|id| id.name == ident.name)
{
return;
} else if let Some(scope) = &mut self.0.single_char_names.last_mut() {
}
if let Some(scope) = &mut self.0.single_char_names.last_mut() {
scope.push(ident);
}
}

4
clippy_lints/src/redundant_pattern_matching.rs
View File

@ -95,8 +95,6 @@ fn find_sugg_for_if_let<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr,
);
},
);
} else {
return;
}
}
@ -161,8 +159,6 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, o
},
);
}
} else {
return;
}
}

85
clippy_lints/src/returns.rs
View File

@ -83,6 +83,12 @@ declare_clippy_lint! {
"needless unit expression"
}
#[derive(PartialEq, Eq, Copy, Clone)]
enum RetReplacement {
Empty,
Unit,
}
declare_lint_pass!(Return => [NEEDLESS_RETURN, LET_AND_RETURN, UNUSED_UNIT]);
impl Return {
@ -91,7 +97,7 @@ impl Return {
if let Some(stmt) = block.stmts.last() {
match stmt.node {
ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => {
self.check_final_expr(cx, expr, Some(stmt.span));
self.check_final_expr(cx, expr, Some(stmt.span), RetReplacement::Empty);
},
_ => (),
}
@ -99,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<Span>) {
fn check_final_expr(
&mut self,
cx: &EarlyContext<'_>,
expr: &ast::Expr,
span: Option<Span>,
replacement: RetReplacement,
) {
match expr.node {
// simple return is always "bad"
ast::ExprKind::Ret(Some(ref inner)) => {
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.span);
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!
@ -117,32 +134,60 @@ impl Return {
// (except for unit type functions) so we don't match it
ast::ExprKind::If(_, ref ifblock, Some(ref elsexpr)) => {
self.check_block_return(cx, ifblock);
self.check_final_expr(cx, elsexpr, None);
self.check_final_expr(cx, elsexpr, None, RetReplacement::Empty);
},
// a match expr, check all arms
ast::ExprKind::Match(_, ref arms) => {
for arm in arms {
self.check_final_expr(cx, &arm.body, Some(arm.body.span));
self.check_final_expr(cx, &arm.body, Some(arm.body.span), RetReplacement::Unit);
}
},
_ => (),
}
}
fn emit_return_lint(&mut self, cx: &EarlyContext<'_>, ret_span: Span, inner_span: Span) {
if in_external_macro(cx.sess(), inner_span) || in_macro_or_desugar(inner_span) {
return;
fn emit_return_lint(
&mut self,
cx: &EarlyContext<'_>,
ret_span: Span,
inner_span: Option<Span>,
replacement: RetReplacement,
) {
match inner_span {
Some(inner_span) => {
if in_external_macro(cx.sess(), inner_span) || in_macro_or_desugar(inner_span) {
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`", 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,
);
});
},
},
}
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,
);
}
});
}
// Check for "let x = EXPR; x"
@ -195,7 +240,7 @@ impl EarlyLintPass for Return {
fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, decl: &ast::FnDecl, span: Span, _: ast::NodeId) {
match kind {
FnKind::ItemFn(.., block) | FnKind::Method(.., block) => self.check_block_return(cx, block),
FnKind::Closure(body) => self.check_final_expr(cx, body, Some(body.span)),
FnKind::Closure(body) => self.check_final_expr(cx, body, Some(body.span), RetReplacement::Empty),
}
if_chain! {
if let ast::FunctionRetTy::Ty(ref ty) = decl.output;

29
tests/ui/needless_return.rs
View File

@ -1,5 +1,11 @@
#![warn(clippy::needless_return)]
macro_rules! the_answer {
() => {
42
};
}
fn test_end_of_fn() -> bool {
if true {
// no error!
@ -36,6 +42,29 @@ fn test_closure() {
let _ = || return true;
}
fn test_macro_call() -> i32 {
return the_answer!();
}
fn test_void_fun() {
return;
}
fn test_void_if_fun(b: bool) {
if b {
return;
} else {
return;
}
}
fn test_void_match(x: u32) {
match x {
0 => (),
_ => return,
}
}
fn main() {
let _ = test_end_of_fn();
let _ = test_no_semicolon();

58
tests/ui/needless_return.stderr
View File

@ -1,52 +1,76 @@
error: unneeded return statement
--> $DIR/needless_return.rs:8:5
--> $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`
error: unneeded return statement
--> $DIR/needless_return.rs:12:5
--> $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:17:9
--> $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:19:9
--> $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:25:17
--> $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:27:13
--> $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:34:9
--> $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:36:16
--> $DIR/needless_return.rs:42:16
|
LL | let _ = || return true;
| ^^^^^^^^^^^ help: remove `return` as shown: `true`
| ^^^^^^^^^^^ help: remove `return`: `true`
error: aborting due to 8 previous errors
error: unneeded return statement
--> $DIR/needless_return.rs:50:5
|
LL | return;
| ^^^^^^^ help: remove `return`
error: unneeded return statement
--> $DIR/needless_return.rs:55:9
|
LL | return;
| ^^^^^^^ help: remove `return`
error: unneeded return statement
--> $DIR/needless_return.rs:57:9
|
LL | return;
| ^^^^^^^ help: remove `return`
error: unneeded return statement
--> $DIR/needless_return.rs:64:14
|
LL | _ => return,
| ^^^^^^ help: replace `return` with the unit type: `()`
error: aborting due to 12 previous errors