diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs index 75c66d22647..96022db56aa 100644 --- a/clippy_lints/src/implicit_return.rs +++ b/clippy_lints/src/implicit_return.rs @@ -45,6 +45,19 @@ declare_clippy_lint! { pub struct Pass; impl Pass { + fn lint(cx: &LateContext<'_, '_>, outer_span: syntax_pos::Span, inner_span: syntax_pos::Span, msg: &str) { + span_lint_and_then(cx, IMPLICIT_RETURN, outer_span, "missing return statement", |db| { + if let Some(snippet) = snippet_opt(cx, inner_span) { + db.span_suggestion_with_applicability( + outer_span, + msg, + format!("return {}", snippet), + Applicability::MachineApplicable, + ); + } + }); + } + fn expr_match(cx: &LateContext<'_, '_>, expr: &rustc::hir::Expr) { match &expr.node { // loops could be using `break` instead of `return` @@ -55,23 +68,19 @@ impl Pass { // only needed in the case of `break` with `;` at the end else if let Some(stmt) = block.stmts.last() { if let rustc::hir::StmtKind::Semi(expr, ..) = &stmt.node { - Self::expr_match(cx, expr); + // make sure it's a break, otherwise we want to skip + if let ExprKind::Break(.., break_expr) = &expr.node { + if let Some(break_expr) = break_expr { + Self::lint(cx, expr.span, break_expr.span, "change `break` to `return` as shown"); + } + } } } }, // use `return` instead of `break` ExprKind::Break(.., break_expr) => { if let Some(break_expr) = break_expr { - span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| { - if let Some(snippet) = snippet_opt(cx, break_expr.span) { - db.span_suggestion_with_applicability( - expr.span, - "change `break` to `return` as shown", - format!("return {}", snippet), - Applicability::MachineApplicable, - ); - } - }); + Self::lint(cx, expr.span, break_expr.span, "change `break` to `return` as shown"); } }, ExprKind::If(.., if_expr, else_expr) => { @@ -89,16 +98,7 @@ impl Pass { // skip if it already has a return statement ExprKind::Ret(..) => (), // everything else is missing `return` - _ => span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| { - if let Some(snippet) = snippet_opt(cx, expr.span) { - db.span_suggestion_with_applicability( - expr.span, - "add `return` as shown", - format!("return {}", snippet), - Applicability::MachineApplicable, - ); - } - }), + _ => Self::lint(cx, expr.span, expr.span, "add `return` as shown"), } } } diff --git a/tests/ui/implicit_return.rs b/tests/ui/implicit_return.rs index 6188835e555..61cb35e1209 100644 --- a/tests/ui/implicit_return.rs +++ b/tests/ui/implicit_return.rs @@ -42,6 +42,27 @@ fn test_loop() -> bool { } } +#[allow(clippy::never_loop)] +fn test_loop_with_block() -> bool { + loop { + { + break true; + } + } +} + +#[allow(clippy::never_loop)] +fn test_loop_with_nests() -> bool { + loop { + if true { + let _ = true; + } + else { + break true; + } + } +} + fn test_closure() { #[rustfmt::skip] let _ = || { true }; @@ -53,5 +74,7 @@ fn main() { let _ = test_if_block(); let _ = test_match(true); let _ = test_loop(); + let _ = test_loop_with_block(); + let _ = test_loop_with_nests(); test_closure(); } diff --git a/tests/ui/implicit_return.stderr b/tests/ui/implicit_return.stderr index b2feec3f57a..6d0761554cd 100644 --- a/tests/ui/implicit_return.stderr +++ b/tests/ui/implicit_return.stderr @@ -37,16 +37,28 @@ error: missing return statement | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:47:18 + --> $DIR/implicit_return.rs:49:13 | -47 | let _ = || { true }; +49 | break true; + | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` + +error: missing return statement + --> $DIR/implicit_return.rs:61:13 + | +61 | break true; + | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` + +error: missing return statement + --> $DIR/implicit_return.rs:68:18 + | +68 | let _ = || { true }; | ^^^^ help: add `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:48:16 + --> $DIR/implicit_return.rs:69:16 | -48 | let _ = || true; +69 | let _ = || true; | ^^^^ help: add `return` as shown: `return true` -error: aborting due to 8 previous errors +error: aborting due to 10 previous errors