diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs index 6863645a92db..f79c4e2dd580 100644 --- a/clippy_lints/src/implicit_return.rs +++ b/clippy_lints/src/implicit_return.rs @@ -1,13 +1,17 @@ -use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::match_panic_def_id; -use clippy_utils::source::snippet_opt; -use if_chain::if_chain; +use clippy_utils::{ + diagnostics::span_lint_and_sugg, + expr_sig, get_async_fn_body, is_async_fn, + source::{snippet_with_applicability, snippet_with_context, walk_span_to_context}, + visitors::visit_break_exprs, + ExprFnSig, +}; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, Expr, ExprKind, FnDecl, HirId, MatchSource, StmtKind}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, FnRetTy, HirId}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::Span; +use rustc_span::{Span, SyntaxContext}; declare_clippy_lint! { /// **What it does:** Checks for missing return statements at the end of a block. @@ -39,89 +43,152 @@ declare_clippy_lint! { declare_lint_pass!(ImplicitReturn => [IMPLICIT_RETURN]); -static LINT_BREAK: &str = "change `break` to `return` as shown"; -static LINT_RETURN: &str = "add `return` as shown"; - -fn lint(cx: &LateContext<'_>, outer_span: Span, inner_span: Span, msg: &str) { - let outer_span = outer_span.source_callsite(); - let inner_span = inner_span.source_callsite(); - - span_lint_and_then(cx, IMPLICIT_RETURN, outer_span, "missing `return` statement", |diag| { - if let Some(snippet) = snippet_opt(cx, inner_span) { - diag.span_suggestion( - outer_span, - msg, - format!("return {}", snippet), - Applicability::MachineApplicable, - ); - } - }); +fn lint_return(cx: &LateContext<'_>, span: Span) { + let mut app = Applicability::MachineApplicable; + let snip = snippet_with_applicability(cx, span, "..", &mut app); + span_lint_and_sugg( + cx, + IMPLICIT_RETURN, + span, + "missing `return` statement", + "add `return` as shown", + format!("return {}", snip), + app, + ); +} + +fn lint_break(cx: &LateContext<'_>, break_span: Span, expr_span: Span) { + let mut app = Applicability::MachineApplicable; + let snip = snippet_with_context(cx, expr_span, break_span.ctxt(), "..", &mut app).0; + span_lint_and_sugg( + cx, + IMPLICIT_RETURN, + break_span, + "missing `return` statement", + "change `break` to `return` as shown", + format!("return {}", snip), + app, + ) } -fn expr_match(cx: &LateContext<'_>, expr: &Expr<'_>) { +fn contains_implicit_return(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { match expr.kind { - // loops could be using `break` instead of `return` - ExprKind::Block(block, ..) | ExprKind::Loop(block, ..) => { - if let Some(expr) = &block.expr { - expr_match(cx, expr); - } - // only needed in the case of `break` with `;` at the end - else if let Some(stmt) = block.stmts.last() { - if_chain! { - if let StmtKind::Semi(expr, ..) = &stmt.kind; - // make sure it's a break, otherwise we want to skip - if let ExprKind::Break(.., Some(break_expr)) = &expr.kind; - then { - lint(cx, expr.span, break_expr.span, LINT_BREAK); - } + ExprKind::Block( + Block { + expr: Some(block_expr), .. + }, + _, + ) => contains_implicit_return(cx, block_expr), + + ExprKind::If(_, then_expr, Some(else_expr)) => { + contains_implicit_return(cx, then_expr) || contains_implicit_return(cx, else_expr) + }, + ExprKind::Match(_, arms, _) => arms.iter().any(|a| contains_implicit_return(cx, a.body)), + ExprKind::Loop(block, ..) => { + let mut break_found = false; + visit_break_exprs(block, |_, dest, _| { + if dest.target_id.ok() == Some(expr.hir_id) { + break_found = true; } - } + }); + break_found }, - // use `return` instead of `break` - ExprKind::Break(.., break_expr) => { - if let Some(break_expr) = break_expr { - lint(cx, expr.span, break_expr.span, LINT_BREAK); - } + + // Diverging function and method calls are fine. + ExprKind::MethodCall(..) + if cx + .tcx + .fn_sig(cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap()) + .skip_binder() + .output() + .is_never() => + { + false + }, + ExprKind::Call(func_expr, _) + if expr_sig(cx.tcx, cx.typeck_results(), func_expr) + .and_then(ExprFnSig::output) + .map_or(false, |ty| ty.skip_binder().is_never()) => + { + false }, - ExprKind::If(.., if_expr, else_expr) => { - expr_match(cx, if_expr); - if let Some(else_expr) = else_expr { - expr_match(cx, else_expr); + // If expressions without an else clause, and blocks without a final expression can only be the final expression + // if they are divergent, or return the unit type. + ExprKind::If(_, _, None) | ExprKind::Block(Block { expr: None, .. }, _) | ExprKind::Ret(_) => false, + + // everything else is missing `return` + _ => true, + } +} + +fn lint_implicit_returns(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, ctxt: SyntaxContext) { + match expr.kind { + ExprKind::Block( + Block { + expr: Some(block_expr), .. + }, + _, + ) => { + if ctxt == block_expr.span.ctxt() { + lint_implicit_returns(cx, block_expr, ctxt) + } else if contains_implicit_return(cx, block_expr) { + lint_return( + cx, + walk_span_to_context(block_expr.span, ctxt).unwrap_or(block_expr.span), + ); } }, - ExprKind::Match(.., arms, source) => { - let check_all_arms = match source { - MatchSource::IfLetDesugar { - contains_else_clause: has_else, - } => has_else, - _ => true, - }; - - if check_all_arms { - for arm in arms { - expr_match(cx, &arm.body); + + ExprKind::If(_, then_expr, Some(else_expr)) => { + lint_implicit_returns(cx, then_expr, ctxt); + lint_implicit_returns(cx, else_expr, ctxt); + }, + + ExprKind::Match(_, arms, _) => { + for arm in arms { + if ctxt == arm.body.span.ctxt() { + lint_implicit_returns(cx, arm.body, ctxt); + } else if contains_implicit_return(cx, arm.body) { + lint_return(cx, walk_span_to_context(expr.span, ctxt).unwrap_or(expr.span)); } - } else { - expr_match(cx, &arms.first().expect("`if let` doesn't have a single arm").body); } }, - // skip if it already has a return statement - ExprKind::Ret(..) => (), - // make sure it's not a call that panics - ExprKind::Call(expr, ..) => { - if_chain! { - if let ExprKind::Path(qpath) = &expr.kind; - if let Some(path_def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id(); - if match_panic_def_id(cx, path_def_id); - then { } - else { - lint(cx, expr.span, expr.span, LINT_RETURN) + ExprKind::Loop(block, ..) => { + let mut add_return = false; + visit_break_exprs(block, |break_expr, dest, sub_expr| { + if dest.target_id.ok() == Some(expr.hir_id) { + if break_expr.span.ctxt() == ctxt { + lint_break(cx, break_expr.span, sub_expr.unwrap().span); + } else { + add_return = true; + } } + }); + if add_return { + lint_return(cx, expr.span); } }, + + // Diverging function and method calls are fine. + ExprKind::MethodCall(..) + if cx + .tcx + .fn_sig(cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap()) + .skip_binder() + .output() + .is_never() => {}, + ExprKind::Call(func_expr, _) + if expr_sig(cx.tcx, cx.typeck_results(), func_expr) + .and_then(ExprFnSig::output) + .map_or(false, |ty| ty.skip_binder().is_never()) => {}, + + // If expressions without an else clause, and blocks without a final expression can only be the final expression + // if they are divergent, or return the unit type. + ExprKind::If(_, _, None) | ExprKind::Block(Block { expr: None, .. }, _) | ExprKind::Ret(_) => (), + // everything else is missing `return` - _ => lint(cx, expr.span, expr.span, LINT_RETURN), + _ => lint_return(cx, walk_span_to_context(expr.span, ctxt).unwrap_or(expr.span)), } } @@ -129,19 +196,26 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitReturn { fn check_fn( &mut self, cx: &LateContext<'tcx>, - _: FnKind<'tcx>, - _: &'tcx FnDecl<'_>, + kind: FnKind<'tcx>, + decl: &'tcx FnDecl<'_>, body: &'tcx Body<'_>, span: Span, _: HirId, ) { - if span.from_expansion() { + if (!matches!(kind, FnKind::Closure) && matches!(decl.output, FnRetTy::DefaultReturn(_))) + || span.ctxt() != body.value.span.ctxt() + || in_external_macro(cx.sess(), span) + || cx.typeck_results().expr_ty(&body.value).is_unit() + { return; } - let body = cx.tcx.hir().body(body.id()); - if cx.typeck_results().expr_ty(&body.value).is_unit() { - return; + + if is_async_fn(kind) { + if let Some(expr) = get_async_fn_body(cx.tcx, body) { + lint_implicit_returns(cx, expr, expr.span.ctxt()); + } + } else { + lint_implicit_returns(cx, &body.value, body.value.span.ctxt()); } - expr_match(cx, &body.value); } } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 9e4dcb600ed9..75240a4e45ac 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -51,6 +51,7 @@ pub use self::hir_utils::{both, eq_expr_value, over, SpanlessEq, SpanlessHash}; use std::collections::hash_map::Entry; use std::hash::BuildHasherDefault; +use std::slice; use if_chain::if_chain; use rustc_ast::ast::{self, Attribute, BorrowKind, LitKind}; @@ -58,10 +59,10 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; -use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; +use rustc_hir::intravisit::{self, FnKind, NestedVisitorMap, Visitor}; use rustc_hir::{ def, Arm, BindingAnnotation, Block, Body, Constness, CrateItem, Expr, ExprKind, FieldDef, FnDecl, ForeignItem, - GenericArgs, GenericParam, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind, LangItem, Lifetime, Local, + GenericArgs, GenericParam, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Lifetime, Local, MacroDef, MatchSource, Node, Param, Pat, PatKind, Path, PathSegment, QPath, Stmt, TraitItem, TraitItemKind, TraitRef, TyKind, Variant, Visibility, }; @@ -69,7 +70,10 @@ use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; use rustc_middle::hir::map::Map; use rustc_middle::ty as rustc_ty; -use rustc_middle::ty::{layout::IntegerExt, DefIdTree, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{ + layout::IntegerExt, subst::Subst, Binder, DefIdTree, FnSig, Predicate, PredicateKind, Ty, TyCtxt, TypeFoldable, + TypeckResults, +}; use rustc_semver::RustcVersion; use rustc_session::Session; use rustc_span::hygiene::{ExpnKind, MacroKind}; @@ -1222,6 +1226,190 @@ pub fn parent_node_is_if_expr(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool { ) } +/// Checks if the given function kind is an async function. +pub fn is_async_fn(kind: FnKind) -> bool { + matches!(kind, FnKind::ItemFn(_, _, header, _) if header.asyncness == IsAsync::Async) +} + +/// Peels away all the compiler generated code surrounding the body of an async function, +pub fn get_async_fn_body(tcx: TyCtxt<'tcx>, body: &Body<'_>) -> Option<&'tcx Expr<'tcx>> { + if let ExprKind::Call( + _, + &[Expr { + kind: ExprKind::Closure(_, _, body, _, _), + .. + }], + ) = body.value.kind + { + if let ExprKind::Block( + Block { + stmts: [], + expr: + Some(Expr { + kind: ExprKind::DropTemps(expr), + .. + }), + .. + }, + _, + ) = tcx.hir().body(body).value.kind + { + Some(expr) + } else { + None + } + } else { + None + } +} + +#[derive(Clone)] +pub struct PredicateIter<'tcx> { + tcx: TyCtxt<'tcx>, + iter: slice::Iter<'tcx, (Predicate<'tcx>, Span)>, + parent: Option, +} +impl Iterator for PredicateIter<'tcx> { + type Item = (Predicate<'tcx>, Span); + fn next(&mut self) -> Option { + loop { + if let Some(&x) = self.iter.next() { + break Some(x); + } else if let Some(parent) = self.parent { + let pred = self.tcx.predicates_of(parent); + self.iter = pred.predicates.iter(); + self.parent = pred.parent; + } else { + break None; + } + } + } +} + +/// Gets an iterator over all predicates which apply to the given item. +pub fn all_predicates_of(tcx: TyCtxt<'_>, id: DefId) -> PredicateIter<'_> { + let pred = tcx.predicates_of(id); + PredicateIter { + tcx, + iter: pred.predicates.iter(), + parent: pred.parent, + } +} + +/// If expr is a path, resolves it. Otherwise returns `None`. +pub fn expr_res(typeck: &TypeckResults<'_>, expr: &Expr<'_>) -> Option { + if let ExprKind::Path(p) = &expr.kind { + Some(typeck.qpath_res(p, expr.hir_id)) + } else { + None + } +} + +/// A signature for a function like type. +#[derive(Clone, Copy)] +pub enum ExprFnSig<'tcx> { + Sig(Binder>), + Closure(Binder>), + Trait(Binder>, Option>>), +} +impl ExprFnSig<'tcx> { + /// Gets the argument type at the given offset. + pub fn input(self, i: usize) -> Binder> { + match self { + Self::Sig(sig) => sig.input(i), + Self::Closure(sig) => sig.input(0).map_bound(|ty| ty.tuple_element_ty(i).unwrap()), + Self::Trait(inputs, _) => inputs.map_bound(|ty| ty.tuple_element_ty(i).unwrap()), + } + } + + /// Gets the result type, if one could be found. Note that the result type of a trait may not be + /// specified. + pub fn output(self) -> Option>> { + match self { + Self::Sig(sig) | Self::Closure(sig) => Some(sig.output()), + Self::Trait(_, output) => output, + } + } +} + +/// If expr is function like, get the signature for it. +pub fn expr_sig(tcx: TyCtxt<'tcx>, typeck: &TypeckResults<'tcx>, expr: &Expr<'_>) -> Option> { + if let Some(Res::Def(DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::AssocFn, id)) = expr_res(typeck, expr) + { + Some(ExprFnSig::Sig(tcx.fn_sig(id))) + } else { + let ty = typeck.expr_ty_adjusted(expr).peel_refs(); + match *ty.kind() { + rustc_ty::Closure(_, subs) => Some(ExprFnSig::Closure(subs.as_closure().sig())), + rustc_ty::FnDef(id, subs) => Some(ExprFnSig::Sig(tcx.fn_sig(id).subst(tcx, subs))), + rustc_ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig)), + rustc_ty::Dynamic(bounds, _) => { + let lang_items = tcx.lang_items(); + match bounds.principal() { + Some(bound) + if Some(bound.def_id()) == lang_items.fn_trait() + || Some(bound.def_id()) == lang_items.fn_once_trait() + || Some(bound.def_id()) == lang_items.fn_mut_trait() => + { + let output = bounds + .projection_bounds() + .find(|p| lang_items.fn_once_output().map_or(false, |id| id == p.item_def_id())) + .map(|p| p.map_bound(|p| p.ty)); + Some(ExprFnSig::Trait(bound.map_bound(|b| b.substs.type_at(0)), output)) + } + _ => None, + } + }, + rustc_ty::Param(_) | rustc_ty::Projection(..) => { + let mut inputs = None; + let mut output = None; + let lang_items = tcx.lang_items(); + + for (pred, _) in all_predicates_of(tcx, typeck.hir_owner.to_def_id()) { + let mut is_input = false; + if let Some(ty) = pred + .kind() + .map_bound(|pred| match pred { + PredicateKind::Trait(p, _) + if (lang_items.fn_trait() == Some(p.def_id()) + || lang_items.fn_mut_trait() == Some(p.def_id()) + || lang_items.fn_once_trait() == Some(p.def_id())) + && p.self_ty() == ty => + { + is_input = true; + Some(p.trait_ref.substs.type_at(1)) + } + PredicateKind::Projection(p) + if lang_items + .fn_once_output() + .map_or(false, |id| id == p.projection_ty.item_def_id) + && p.projection_ty.self_ty() == ty => + { + is_input = false; + Some(p.ty) + } + _ => None, + }) + .transpose() + { + if is_input && inputs.is_none() { + inputs = Some(ty); + } else if !is_input && output.is_none() { + output = Some(ty); + } else { + // Multiple different fn trait impls. Is this even allowed? + return None; + } + } + } + + inputs.map(|ty| ExprFnSig::Trait(ty, output)) + }, + _ => None, + } + } +} + // Finds the attribute with the given name, if any pub fn attr_by_name<'a>(attrs: &'a [Attribute], name: &'_ str) -> Option<&'a Attribute> { attrs diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 2d794d48dc5f..72a67609122f 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -271,17 +271,17 @@ pub fn snippet_with_context( default: &'a str, applicability: &mut Applicability, ) -> (Cow<'a, str>, bool) { - let outer_span = hygiene::walk_chain(span, outer); - let (span, is_macro_call) = if outer_span.ctxt() == outer { - (outer_span, span.ctxt() != outer) - } else { - // The span is from a macro argument, and the outer context is the macro using the argument - if *applicability != Applicability::Unspecified { - *applicability = Applicability::MaybeIncorrect; - } - // TODO: get the argument span. - (span, false) - }; + let (span, is_macro_call) = walk_span_to_context(span, outer).map_or_else( + || { + // The span is from a macro argument, and the outer context is the macro using the argument + if *applicability != Applicability::Unspecified { + *applicability = Applicability::MaybeIncorrect; + } + // TODO: get the argument span. + (span, false) + }, + |outer_span| (outer_span, span.ctxt() != outer), + ); ( snippet_with_applicability(cx, span, default, applicability), @@ -289,6 +289,15 @@ pub fn snippet_with_context( ) } +/// Walks the span up to the target context, thereby returning the macro call site if the span is +/// inside a macro expansion, or the original span if it is not. Note this will return `None` in the +/// case of the span being in a macro expansion, but the target context is from expanding a macro +/// argument. +pub fn walk_span_to_context(span: Span, outer: SyntaxContext) -> Option { + let outer_span = hygiene::walk_chain(span, outer); + (outer_span.ctxt() == outer).then(|| outer_span) +} + /// Removes block comments from the given `Vec` of lines. /// /// # Examples diff --git a/clippy_utils/src/visitors.rs b/clippy_utils/src/visitors.rs index 5a8c629e3338..d431bdf34eee 100644 --- a/clippy_utils/src/visitors.rs +++ b/clippy_utils/src/visitors.rs @@ -1,7 +1,7 @@ use crate::path_to_local_id; use rustc_hir as hir; -use rustc_hir::intravisit::{self, walk_expr, NestedVisitorMap, Visitor}; -use rustc_hir::{Arm, Body, Expr, HirId, Stmt}; +use rustc_hir::intravisit::{self, walk_expr, ErasedMap, NestedVisitorMap, Visitor}; +use rustc_hir::{Arm, Block, Body, Destination, Expr, ExprKind, HirId, Stmt}; use rustc_lint::LateContext; use rustc_middle::hir::map::Map; @@ -188,3 +188,54 @@ impl<'v> Visitor<'v> for LocalUsedVisitor<'v> { NestedVisitorMap::OnlyBodies(self.hir) } } + +pub trait Visitable<'tcx> { + fn visit>(self, v: &mut V); +} +impl Visitable<'tcx> for &'tcx Expr<'tcx> { + fn visit>(self, v: &mut V) { + v.visit_expr(self) + } +} +impl Visitable<'tcx> for &'tcx Block<'tcx> { + fn visit>(self, v: &mut V) { + v.visit_block(self) + } +} +impl<'tcx> Visitable<'tcx> for &'tcx Stmt<'tcx> { + fn visit>(self, v: &mut V) { + v.visit_stmt(self) + } +} +impl<'tcx> Visitable<'tcx> for &'tcx Body<'tcx> { + fn visit>(self, v: &mut V) { + v.visit_body(self) + } +} +impl<'tcx> Visitable<'tcx> for &'tcx Arm<'tcx> { + fn visit>(self, v: &mut V) { + v.visit_arm(self) + } +} + +pub fn visit_break_exprs<'tcx>( + node: impl Visitable<'tcx>, + f: impl FnMut(&'tcx Expr<'tcx>, Destination, Option<&'tcx Expr<'tcx>>), +) { + struct V(F); + impl<'tcx, F: FnMut(&'tcx Expr<'tcx>, Destination, Option<&'tcx Expr<'tcx>>)> Visitor<'tcx> for V { + type Map = ErasedMap<'tcx>; + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } + + fn visit_expr(&mut self, e: &'tcx Expr<'_>) { + if let ExprKind::Break(dest, sub_expr) = e.kind { + self.0(e, dest, sub_expr) + } + walk_expr(self, e); + } + } + + node.visit(&mut V(f)); +} diff --git a/tests/ui/implicit_return.fixed b/tests/ui/implicit_return.fixed index 59f7ad9c1062..dddce7174fac 100644 --- a/tests/ui/implicit_return.fixed +++ b/tests/ui/implicit_return.fixed @@ -1,7 +1,8 @@ +// edition:2018 // run-rustfix #![warn(clippy::implicit_return)] -#![allow(clippy::needless_return, unused)] +#![allow(clippy::needless_return, clippy::needless_bool, unused, clippy::never_loop)] fn test_end_of_fn() -> bool { if true { @@ -12,7 +13,6 @@ fn test_end_of_fn() -> bool { return true } -#[allow(clippy::needless_bool)] fn test_if_block() -> bool { if true { return true } else { return false } } @@ -25,7 +25,6 @@ fn test_match(x: bool) -> bool { } } -#[allow(clippy::needless_return)] fn test_match_with_unreachable(x: bool) -> bool { match x { true => return false, @@ -33,14 +32,12 @@ fn test_match_with_unreachable(x: bool) -> bool { } } -#[allow(clippy::never_loop)] fn test_loop() -> bool { loop { return true; } } -#[allow(clippy::never_loop)] fn test_loop_with_block() -> bool { loop { { @@ -49,7 +46,6 @@ fn test_loop_with_block() -> bool { } } -#[allow(clippy::never_loop)] fn test_loop_with_nests() -> bool { loop { if true { @@ -83,6 +79,37 @@ fn test_return_macro() -> String { return format!("test {}", "test") } +fn macro_branch_test() -> bool { + macro_rules! m { + ($t:expr, $f:expr) => { + if true { $t } else { $f } + }; + } + return m!(true, false) +} + +fn loop_test() -> bool { + 'outer: loop { + if true { + return true; + } + + let _ = loop { + if false { + return false; + } + if true { + break true; + } + }; + } +} + +// issue #6940 +async fn foo() -> bool { + return true +} + fn main() { let _ = test_end_of_fn(); let _ = test_if_block(); diff --git a/tests/ui/implicit_return.rs b/tests/ui/implicit_return.rs index 2c1bc0465150..99776d1257db 100644 --- a/tests/ui/implicit_return.rs +++ b/tests/ui/implicit_return.rs @@ -1,7 +1,8 @@ +// edition:2018 // run-rustfix #![warn(clippy::implicit_return)] -#![allow(clippy::needless_return, unused)] +#![allow(clippy::needless_return, clippy::needless_bool, unused, clippy::never_loop)] fn test_end_of_fn() -> bool { if true { @@ -12,7 +13,6 @@ fn test_end_of_fn() -> bool { true } -#[allow(clippy::needless_bool)] fn test_if_block() -> bool { if true { true } else { false } } @@ -25,7 +25,6 @@ fn test_match(x: bool) -> bool { } } -#[allow(clippy::needless_return)] fn test_match_with_unreachable(x: bool) -> bool { match x { true => return false, @@ -33,14 +32,12 @@ fn test_match_with_unreachable(x: bool) -> bool { } } -#[allow(clippy::never_loop)] fn test_loop() -> bool { loop { break true; } } -#[allow(clippy::never_loop)] fn test_loop_with_block() -> bool { loop { { @@ -49,7 +46,6 @@ fn test_loop_with_block() -> bool { } } -#[allow(clippy::never_loop)] fn test_loop_with_nests() -> bool { loop { if true { @@ -83,6 +79,37 @@ fn test_return_macro() -> String { format!("test {}", "test") } +fn macro_branch_test() -> bool { + macro_rules! m { + ($t:expr, $f:expr) => { + if true { $t } else { $f } + }; + } + m!(true, false) +} + +fn loop_test() -> bool { + 'outer: loop { + if true { + break true; + } + + let _ = loop { + if false { + break 'outer false; + } + if true { + break true; + } + }; + } +} + +// issue #6940 +async fn foo() -> bool { + true +} + fn main() { let _ = test_end_of_fn(); let _ = test_if_block(); diff --git a/tests/ui/implicit_return.stderr b/tests/ui/implicit_return.stderr index 3608319e5bd2..573c08616de6 100644 --- a/tests/ui/implicit_return.stderr +++ b/tests/ui/implicit_return.stderr @@ -1,5 +1,5 @@ error: missing `return` statement - --> $DIR/implicit_return.rs:12:5 + --> $DIR/implicit_return.rs:13:5 | LL | true | ^^^^ help: add `return` as shown: `return true` @@ -31,40 +31,64 @@ LL | false => { true }, | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:39:9 + --> $DIR/implicit_return.rs:37:9 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:47:13 + --> $DIR/implicit_return.rs:44:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:56:13 + --> $DIR/implicit_return.rs:52:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:74:18 + --> $DIR/implicit_return.rs:70:18 | LL | let _ = || { true }; | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:75:16 + --> $DIR/implicit_return.rs:71:16 | LL | let _ = || true; | ^^^^ help: add `return` as shown: `return true` error: missing `return` statement - --> $DIR/implicit_return.rs:83:5 + --> $DIR/implicit_return.rs:79:5 | LL | format!("test {}", "test") | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `return` as shown: `return format!("test {}", "test")` -error: aborting due to 11 previous errors +error: missing `return` statement + --> $DIR/implicit_return.rs:88:5 + | +LL | m!(true, false) + | ^^^^^^^^^^^^^^^ help: add `return` as shown: `return m!(true, false)` + +error: missing `return` statement + --> $DIR/implicit_return.rs:94:13 + | +LL | break true; + | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` + +error: missing `return` statement + --> $DIR/implicit_return.rs:99:17 + | +LL | break 'outer false; + | ^^^^^^^^^^^^^^^^^^ help: change `break` to `return` as shown: `return false` + +error: missing `return` statement + --> $DIR/implicit_return.rs:110:5 + | +LL | true + | ^^^^ help: add `return` as shown: `return true` + +error: aborting due to 15 previous errors