diff --git a/CHANGELOG.md b/CHANGELOG.md index 161fa630ed47..0ba53e40bb35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5800,6 +5800,7 @@ Released 2018-09-13 [`neg_multiply`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_multiply [`negative_feature_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#negative_feature_names [`never_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#never_loop +[`never_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#never_returns [`new_ret_no_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_ret_no_self [`new_without_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default [`new_without_default_derive`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default_derive diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 670b5cbef823..fcd1ebad31cc 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -349,6 +349,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat * [`large_types_passed_by_value`](https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value) * [`linkedlist`](https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist) * [`needless_pass_by_ref_mut`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut) +* [`never_returns`](https://rust-lang.github.io/rust-clippy/master/index.html#never_returns) * [`option_option`](https://rust-lang.github.io/rust-clippy/master/index.html#option_option) * [`rc_buffer`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer) * [`rc_mutex`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 600d5b6e2c8d..9607429c29ea 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -428,6 +428,7 @@ define_Conf! { large_types_passed_by_value, linkedlist, needless_pass_by_ref_mut, + never_returns, option_option, rc_buffer, rc_mutex, diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index edb52851e0cb..6d59807ae2a7 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -552,6 +552,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::needless_update::NEEDLESS_UPDATE_INFO, crate::neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD_INFO, crate::neg_multiply::NEG_MULTIPLY_INFO, + crate::never_returns::NEVER_RETURNS_INFO, crate::new_without_default::NEW_WITHOUT_DEFAULT_INFO, crate::no_effect::NO_EFFECT_INFO, crate::no_effect::NO_EFFECT_UNDERSCORE_BINDING_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b92da0e62cf4..a8f3e1d5c4d1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -266,6 +266,7 @@ mod needless_question_mark; mod needless_update; mod neg_cmp_op_on_partial_ord; mod neg_multiply; +mod never_returns; mod new_without_default; mod no_effect; mod no_mangle_with_rust_abi; @@ -951,5 +952,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp)); store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound)); store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf))); + store.register_late_pass(|_| Box::new(never_returns::NeverReturns::new(conf))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/never_returns.rs b/clippy_lints/src/never_returns.rs new file mode 100644 index 000000000000..f785948c2a0a --- /dev/null +++ b/clippy_lints/src/never_returns.rs @@ -0,0 +1,190 @@ +use clippy_config::Conf; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_opt; +use clippy_utils::{ReturnType, ReturnVisitor, is_entrypoint_fn, visit_returns}; +use rustc_errors::Applicability; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::{BodyId, FnRetTy, FnSig, ImplItemKind, Item, ItemKind, TyKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::TypeckResults; +use rustc_middle::ty::adjustment::{Adjust, Adjustment}; +use rustc_session::impl_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// + /// Detects functions that do not return, but do not have `!` as their return type. + /// + /// ### Why is this bad? + /// + /// Returning `!` is a more accurate API for your callers, and allows for optimisations/further linting. + /// + /// ### Example + /// ```no_run + /// # fn do_thing() {} + /// fn run() { + /// loop { + /// do_thing(); + /// } + /// } + /// ``` + /// Use instead: + /// ```no_run + /// # fn do_thing() {} + /// fn run() -> ! { + /// loop { + /// do_thing(); + /// } + /// } + /// ``` + #[clippy::version = "1.83.0"] + pub NEVER_RETURNS, + pedantic, + "functions that never return, but are typed to" +} + +#[derive(Clone, Copy)] +pub(crate) struct NeverReturns { + avoid_breaking_exported_api: bool, +} + +impl_lint_pass!(NeverReturns => [NEVER_RETURNS]); + +impl NeverReturns { + pub fn new(conf: &Conf) -> Self { + Self { + avoid_breaking_exported_api: conf.avoid_breaking_exported_api, + } + } + + fn check_item_fn(self, cx: &LateContext<'_>, sig: FnSig<'_>, def_id: LocalDefId, body_id: BodyId) { + let returns_unit = if let FnRetTy::Return(ret_ty) = sig.decl.output { + if let TyKind::Never = ret_ty.kind { + return; + } + + matches!(ret_ty.kind, TyKind::Tup([])) + } else { + true + }; + + if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) { + return; + } + + // We shouldn't try to change the signature of a lang item! + if cx.tcx.lang_items().from_def_id(def_id.to_def_id()).is_some() { + return; + } + + let body = cx.tcx.hir().body(body_id); + let typeck_results = cx.tcx.typeck_body(body_id); + let mut visitor = NeverReturnVisitor { + typeck_results, + returns_unit, + found_implicit_return: false, + }; + + if visit_returns(&mut visitor, body.value).is_continue() && visitor.found_implicit_return { + let mut applicability = Applicability::MachineApplicable; + let (lint_span, mut snippet, sugg) = match sig.decl.output { + FnRetTy::DefaultReturn(span) => (span, String::new(), " -> !"), + FnRetTy::Return(ret_ty) => { + let snippet = if let Some(snippet) = snippet_opt(cx, ret_ty.span) { + format!(" a `{snippet}`") + } else { + applicability = Applicability::HasPlaceholders; + String::new() + }; + + (ret_ty.span, snippet, "!") + }, + }; + + snippet.insert_str(0, "function never returns, but is typed to return"); + span_lint_and_sugg( + cx, + NEVER_RETURNS, + lint_span, + snippet, + "replace with", + sugg.into(), + applicability, + ); + } + } +} + +impl LateLintPass<'_> for NeverReturns { + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if let ItemKind::Fn(sig, _, body_id) = item.kind { + let local_def_id = item.owner_id.def_id; + if is_entrypoint_fn(cx, local_def_id.to_def_id()) { + return; + } + + self.check_item_fn(cx, sig, local_def_id, body_id); + } else if let ItemKind::Impl(impl_) = item.kind { + // Do not lint trait impls + if impl_.of_trait.is_some() { + return; + } + + for impl_item in impl_.items { + let ImplItemKind::Fn(sig, body_id) = cx.tcx.hir().impl_item(impl_item.id).kind else { + continue; + }; + + let local_def_id = item.owner_id.def_id; + self.check_item_fn(cx, sig, local_def_id, body_id); + } + } + } +} + +struct NeverReturnVisitor<'tcx> { + typeck_results: &'tcx TypeckResults<'tcx>, + found_implicit_return: bool, + returns_unit: bool, +} + +impl ReturnVisitor for &mut NeverReturnVisitor<'_> { + type Result = std::ops::ControlFlow<()>; + + fn visit_return(&mut self, kind: ReturnType<'_>) -> Self::Result { + let expression = match kind { + ReturnType::Explicit(expr) => expr, + ReturnType::UnitReturnExplicit(_) => { + return Self::Result::Break(()); + }, + ReturnType::Implicit(expr) | ReturnType::MissingElseImplicit(expr) => { + self.found_implicit_return = true; + expr + }, + ReturnType::DivergingImplicit(_) => { + // If this function returns unit, a diverging implicit may just + // be an implicit unit return, in which case we should not lint. + return if self.returns_unit { + Self::Result::Break(()) + } else { + Self::Result::Continue(()) + }; + }, + }; + + if expression.span.from_expansion() { + return Self::Result::Break(()); + } + + let adjustments = self.typeck_results.expr_adjustments(expression); + if adjustments.iter().any(is_never_to_any) { + Self::Result::Continue(()) + } else { + Self::Result::Break(()) + } + } +} + +fn is_never_to_any(adjustment: &Adjustment<'_>) -> bool { + matches!(adjustment.kind, Adjust::NeverToAny) +} diff --git a/clippy_lints/src/unnecessary_literal_bound.rs b/clippy_lints/src/unnecessary_literal_bound.rs index 80ce67111261..f587eacb513a 100644 --- a/clippy_lints/src/unnecessary_literal_bound.rs +++ b/clippy_lints/src/unnecessary_literal_bound.rs @@ -1,10 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::path_res; +use clippy_utils::{ReturnType, ReturnVisitor, path_res, visit_returns}; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::def::Res; -use rustc_hir::intravisit::{FnKind, Visitor}; -use rustc_hir::{Body, Expr, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind, intravisit}; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; use rustc_span::Span; @@ -67,49 +67,36 @@ fn extract_anonymous_ref<'tcx>(hir_ty: &Ty<'tcx>) -> Option<&'tcx Ty<'tcx>> { Some(ty) } -fn is_str_literal(expr: &Expr<'_>) -> bool { - matches!( - expr.kind, - ExprKind::Lit(Lit { - node: LitKind::Str(..), - .. - }), - ) -} - -struct FindNonLiteralReturn; +struct LiteralReturnVisitor; -impl<'hir> Visitor<'hir> for FindNonLiteralReturn { +impl ReturnVisitor for LiteralReturnVisitor { type Result = std::ops::ControlFlow<()>; - type NestedFilter = intravisit::nested_filter::None; + fn visit_return(&mut self, kind: ReturnType<'_>) -> Self::Result { + let expr = match kind { + ReturnType::Implicit(e) | ReturnType::Explicit(e) => e, + ReturnType::UnitReturnExplicit(_) | ReturnType::MissingElseImplicit(_) => { + panic!("Function which returns `&str` has a unit return!") + }, + ReturnType::DivergingImplicit(_) => { + // If this block is implicitly returning `!`, it can return `&'static str`. + return Self::Result::Continue(()); + }, + }; - fn visit_expr(&mut self, expr: &'hir Expr<'hir>) -> Self::Result { - if let ExprKind::Ret(Some(ret_val_expr)) = expr.kind - && !is_str_literal(ret_val_expr) - { - Self::Result::Break(()) + if matches!( + expr.kind, + ExprKind::Lit(Lit { + node: LitKind::Str(..), + .. + }) + ) { + Self::Result::Continue(()) } else { - intravisit::walk_expr(self, expr) + Self::Result::Break(()) } } } -fn check_implicit_returns_static_str(body: &Body<'_>) -> bool { - // TODO: Improve this to the same complexity as the Visitor to catch more implicit return cases. - if let ExprKind::Block(block, _) = body.value.kind - && let Some(implicit_ret) = block.expr - { - return is_str_literal(implicit_ret); - } - - false -} - -fn check_explicit_returns_static_str(expr: &Expr<'_>) -> bool { - let mut visitor = FindNonLiteralReturn; - visitor.visit_expr(expr).is_continue() -} - impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound { fn check_fn( &mut self, @@ -143,7 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound { } // Check for all return statements returning literals - if check_explicit_returns_static_str(body.value) && check_implicit_returns_static_str(body) { + if visit_returns(LiteralReturnVisitor, body.value).is_continue() { span_lint_and_sugg( cx, UNNECESSARY_LITERAL_BOUND, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 18d8ea509577..a1f0f62bf098 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -10,6 +10,7 @@ #![feature(assert_matches)] #![feature(unwrap_infallible)] #![feature(array_windows)] +#![feature(associated_type_defaults)] #![recursion_limit = "512"] #![allow( clippy::missing_errors_doc, @@ -30,6 +31,7 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) extern crate rustc_ast; +extern crate rustc_ast_ir; extern crate rustc_ast_pretty; extern crate rustc_attr; extern crate rustc_const_eval; @@ -69,6 +71,7 @@ pub mod numeric_literal; pub mod paths; pub mod ptr; pub mod qualify_min_const_fn; +mod returns; pub mod source; pub mod str_utils; pub mod sugg; @@ -81,6 +84,7 @@ pub use self::check_proc_macro::{is_from_proc_macro, is_span_if, is_span_match}; pub use self::hir_utils::{ HirEqInterExpr, SpanlessEq, SpanlessHash, both, count_eq, eq_expr_value, hash_expr, hash_stmt, is_bool, over, }; +pub use returns::{ReturnType, ReturnVisitor, visit_returns}; use core::mem; use core::ops::ControlFlow; diff --git a/clippy_utils/src/returns.rs b/clippy_utils/src/returns.rs new file mode 100644 index 000000000000..5205b3d24175 --- /dev/null +++ b/clippy_utils/src/returns.rs @@ -0,0 +1,109 @@ +use std::ops::ControlFlow; + +use rustc_ast::visit::VisitorResult; +use rustc_ast_ir::try_visit; +use rustc_hir::intravisit::{self, Visitor}; +use rustc_hir::{Block, Expr, ExprKind}; + +pub enum ReturnType<'tcx> { + /// An implicit return. + /// + /// This is an expression that evaluates directly to a value, like a literal or operation. + Implicit(&'tcx Expr<'tcx>), + /// An explicit return. + /// + /// This is the return expression of `return `. + Explicit(&'tcx Expr<'tcx>), + /// An explicit unit type return. + /// + /// This is the return expression `return`. + UnitReturnExplicit(&'tcx Expr<'tcx>), + /// A `()` implicit return. + /// + /// The expression is the `ExprKind::If` with no `else` block. + /// + /// ```no_run + /// fn example() -> () { + /// if true { + /// + /// } // no else! + /// } + /// ``` + MissingElseImplicit(&'tcx Expr<'tcx>), + /// A diverging implict return. + /// + /// ```no_run + /// fn example() -> u8 { + /// { todo!(); } + /// } + /// ``` + DivergingImplicit(&'tcx Block<'tcx>), +} + +pub trait ReturnVisitor { + type Result: VisitorResult = (); + + fn visit_return(&mut self, return_type: ReturnType<'_>) -> Self::Result; +} + +struct ExplicitReturnDriver(V); + +impl Visitor<'_> for ExplicitReturnDriver { + type Result = V::Result; + type NestedFilter = intravisit::nested_filter::None; + + fn visit_expr(&mut self, expr: &Expr<'_>) -> Self::Result { + if let ExprKind::Ret(ret_val_expr) = expr.kind { + if let Some(ret_val_expr) = ret_val_expr { + try_visit!(self.0.visit_return(ReturnType::Explicit(ret_val_expr))); + } else { + try_visit!(self.0.visit_return(ReturnType::UnitReturnExplicit(expr))); + } + } + + intravisit::walk_expr(self, expr) + } +} + +fn visit_implicit_returns(visitor: &mut V, expr: &Expr<'_>) -> V::Result +where + V: ReturnVisitor, +{ + let cont = || V::Result::from_branch(ControlFlow::Continue(())); + match expr.kind { + ExprKind::Block(block, _) => { + if let Some(expr) = block.expr { + visit_implicit_returns(visitor, expr) + } else { + visitor.visit_return(ReturnType::DivergingImplicit(block)) + } + }, + ExprKind::If(_, true_block, else_block) => { + try_visit!(visit_implicit_returns(visitor, true_block)); + if let Some(expr) = else_block { + visit_implicit_returns(visitor, expr) + } else { + visitor.visit_return(ReturnType::MissingElseImplicit(expr)) + } + }, + ExprKind::Match(_, arms, _) => { + for arm in arms { + try_visit!(visit_implicit_returns(visitor, arm.body)); + } + + cont() + }, + + _ => visitor.visit_return(ReturnType::Implicit(expr)), + } +} + +pub fn visit_returns(visitor: V, expr: &Expr<'_>) -> V::Result +where + V: ReturnVisitor, +{ + let mut explicit_driver = ExplicitReturnDriver(visitor); + try_visit!(explicit_driver.visit_expr(expr)); + + visit_implicit_returns(&mut explicit_driver.0, expr) +} diff --git a/lintcheck/src/driver.rs b/lintcheck/src/driver.rs index 87ab14ba0cf8..7f5636b5f73d 100644 --- a/lintcheck/src/driver.rs +++ b/lintcheck/src/driver.rs @@ -54,7 +54,7 @@ fn run_clippy(addr: &str) -> Option { } } -pub fn drive(addr: &str) { +pub fn drive(addr: &str) -> ! { process::exit(run_clippy(addr).unwrap_or_else(|| { Command::new("rustc") .args(env::args_os().skip(2)) diff --git a/src/driver.rs b/src/driver.rs index c66837dc998f..c9b64481796a 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -181,7 +181,7 @@ const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/ne #[allow(clippy::too_many_lines)] #[allow(clippy::ignored_unit_patterns)] -pub fn main() { +pub fn main() -> ! { let early_dcx = EarlyDiagCtxt::new(ErrorOutputType::default()); rustc_driver::init_rustc_env_logger(&early_dcx); diff --git a/tests/compile-test.rs b/tests/compile-test.rs index b8e0413e97bc..d620a2781e5a 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -584,7 +584,7 @@ impl LintMetadata { } } - fn applicability_str(&self) -> &str { + fn applicability_str(&self) -> &'static str { match self.applicability { Applicability::MachineApplicable => "MachineApplicable", Applicability::HasPlaceholders => "HasPlaceholders", diff --git a/tests/ui-toml/panic/panic.rs b/tests/ui-toml/panic/panic.rs index b6264c950e45..3ddc54cf263b 100644 --- a/tests/ui-toml/panic/panic.rs +++ b/tests/ui-toml/panic/panic.rs @@ -13,7 +13,7 @@ fn main() { } } -fn issue_13292() { +fn issue_13292() -> ! { panic_any("should lint") } diff --git a/tests/ui/empty_loop.rs b/tests/ui/empty_loop.rs index be347563135c..87584ac96a58 100644 --- a/tests/ui/empty_loop.rs +++ b/tests/ui/empty_loop.rs @@ -5,7 +5,7 @@ extern crate proc_macros; use proc_macros::{external, inline_macros}; -fn should_trigger() { +fn should_trigger() -> ! { loop {} #[allow(clippy::never_loop)] loop { diff --git a/tests/ui/implicit_return.fixed b/tests/ui/implicit_return.fixed index 56fe29b4e677..713614e038e8 100644 --- a/tests/ui/implicit_return.fixed +++ b/tests/ui/implicit_return.fixed @@ -118,6 +118,7 @@ fn loop_macro_test() -> bool { } } +#[expect(clippy::never_returns)] fn divergent_test() -> bool { fn diverge() -> ! { panic!() diff --git a/tests/ui/implicit_return.rs b/tests/ui/implicit_return.rs index f066ce20cfd1..93438d1d3b76 100644 --- a/tests/ui/implicit_return.rs +++ b/tests/ui/implicit_return.rs @@ -118,6 +118,7 @@ fn loop_macro_test() -> bool { } } +#[expect(clippy::never_returns)] fn divergent_test() -> bool { fn diverge() -> ! { panic!() diff --git a/tests/ui/implicit_return.stderr b/tests/ui/implicit_return.stderr index 3b06f26f5a05..d64e3b1a1f5e 100644 --- a/tests/ui/implicit_return.stderr +++ b/tests/ui/implicit_return.stderr @@ -170,7 +170,7 @@ LL + } | error: missing `return` statement - --> tests/ui/implicit_return.rs:130:5 + --> tests/ui/implicit_return.rs:131:5 | LL | true | ^^^^ diff --git a/tests/ui/infinite_loops.rs b/tests/ui/infinite_loops.rs index 0613eddce266..815a85d5b467 100644 --- a/tests/ui/infinite_loops.rs +++ b/tests/ui/infinite_loops.rs @@ -1,7 +1,7 @@ //@no-rustfix //@aux-build:proc_macros.rs -#![allow(clippy::never_loop)] +#![allow(clippy::never_loop, clippy::never_returns)] #![warn(clippy::infinite_loop)] #![feature(async_closure)] diff --git a/tests/ui/needless_continue.rs b/tests/ui/needless_continue.rs index c26a292c8cb5..c6813bc256de 100644 --- a/tests/ui/needless_continue.rs +++ b/tests/ui/needless_continue.rs @@ -55,14 +55,14 @@ fn main() { } } -fn simple_loop() { +fn simple_loop() -> ! { loop { continue; //~^ ERROR: this `continue` expression is redundant } } -fn simple_loop2() { +fn simple_loop2() -> ! { loop { println!("bleh"); continue; @@ -71,7 +71,7 @@ fn simple_loop2() { } #[rustfmt::skip] -fn simple_loop3() { +fn simple_loop3() -> ! { loop { continue //~^ ERROR: this `continue` expression is redundant @@ -79,7 +79,7 @@ fn simple_loop3() { } #[rustfmt::skip] -fn simple_loop4() { +fn simple_loop4() -> ! { loop { println!("bleh"); continue @@ -94,7 +94,7 @@ mod issue_2329 { fn update_condition() {} // only the outer loop has a label - fn foo() { + fn foo() -> ! { 'outer: loop { println!("Entry"); while condition() { @@ -118,7 +118,7 @@ mod issue_2329 { } // both loops have labels - fn bar() { + fn bar() -> ! { 'outer: loop { println!("Entry"); 'inner: while condition() { diff --git a/tests/ui/never_returns.fixed b/tests/ui/never_returns.fixed new file mode 100644 index 000000000000..6a558cc33d99 --- /dev/null +++ b/tests/ui/never_returns.fixed @@ -0,0 +1,48 @@ +#![warn(clippy::never_returns)] +#![allow(clippy::empty_loop)] + +fn stuff() {} + +fn never_returns() -> ! { + //~^ error: function never returns, but is typed to return + loop { + stuff() + } +} + +fn never_returns_with_ty() -> ! { + //~^ error: function never returns, but is typed to return a `u8` + loop { + stuff() + } +} + +fn never_returns_conditionally(cond: bool) -> ! { + //~^ error: function never returns, but is typed to return a `u8` + if cond { std::process::exit(0) } else { panic!() } +} + +fn returns_unit_implicit(cond: bool) { + if cond {} +} + +fn returns_in_loop(cond: bool) -> u8 { + loop { + if cond { + break 1; + } + } +} + +trait ExampleTrait { + fn example(self) -> u8; +} + +// Should not lint, as the return type is required by the trait +impl ExampleTrait for () { + fn example(self) -> u8 { + loop {} + } +} + +fn main() {} diff --git a/tests/ui/never_returns.rs b/tests/ui/never_returns.rs new file mode 100644 index 000000000000..0fe8a276cda0 --- /dev/null +++ b/tests/ui/never_returns.rs @@ -0,0 +1,48 @@ +#![warn(clippy::never_returns)] +#![allow(clippy::empty_loop)] + +fn stuff() {} + +fn never_returns() { + //~^ error: function never returns, but is typed to return + loop { + stuff() + } +} + +fn never_returns_with_ty() -> u8 { + //~^ error: function never returns, but is typed to return a `u8` + loop { + stuff() + } +} + +fn never_returns_conditionally(cond: bool) -> u8 { + //~^ error: function never returns, but is typed to return a `u8` + if cond { std::process::exit(0) } else { panic!() } +} + +fn returns_unit_implicit(cond: bool) { + if cond {} +} + +fn returns_in_loop(cond: bool) -> u8 { + loop { + if cond { + break 1; + } + } +} + +trait ExampleTrait { + fn example(self) -> u8; +} + +// Should not lint, as the return type is required by the trait +impl ExampleTrait for () { + fn example(self) -> u8 { + loop {} + } +} + +fn main() {} diff --git a/tests/ui/never_returns.stderr b/tests/ui/never_returns.stderr new file mode 100644 index 000000000000..53d72a6e173d --- /dev/null +++ b/tests/ui/never_returns.stderr @@ -0,0 +1,23 @@ +error: function never returns, but is typed to return + --> tests/ui/never_returns.rs:6:19 + | +LL | fn never_returns() { + | ^ help: replace with: `-> !` + | + = note: `-D clippy::never-returns` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::never_returns)]` + +error: function never returns, but is typed to return a `u8` + --> tests/ui/never_returns.rs:13:31 + | +LL | fn never_returns_with_ty() -> u8 { + | ^^ help: replace with: `!` + +error: function never returns, but is typed to return a `u8` + --> tests/ui/never_returns.rs:20:47 + | +LL | fn never_returns_conditionally(cond: bool) -> u8 { + | ^^ help: replace with: `!` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/unnecessary_literal_bound.fixed b/tests/ui/unnecessary_literal_bound.fixed index 107e397466d0..5859e2107f71 100644 --- a/tests/ui/unnecessary_literal_bound.fixed +++ b/tests/ui/unnecessary_literal_bound.fixed @@ -5,28 +5,26 @@ struct Struct<'a> { } impl Struct<'_> { - // Should warn fn returns_lit(&self) -> &'static str { + //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments "Hello" } - // Should NOT warn fn returns_non_lit(&self) -> &str { self.not_literal } - // Should warn, does not currently - fn conditionally_returns_lit(&self, cond: bool) -> &str { + fn conditionally_returns_lit(&self, cond: bool) -> &'static str { + //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments if cond { "Literal" } else { "also a literal" } } - // Should NOT warn fn conditionally_returns_non_lit(&self, cond: bool) -> &str { if cond { "Literal" } else { self.not_literal } } - // Should warn fn contionally_returns_literals_explicit(&self, cond: bool) -> &'static str { + //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments if cond { return "Literal"; } @@ -34,7 +32,6 @@ impl Struct<'_> { "also a literal" } - // Should NOT warn fn conditionally_returns_non_lit_explicit(&self, cond: bool) -> &str { if cond { return self.not_literal; @@ -49,14 +46,13 @@ trait ReturnsStr { } impl ReturnsStr for u8 { - // Should warn, even though not useful without trait refinement fn trait_method(&self) -> &'static str { + //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments "Literal" } } impl ReturnsStr for Struct<'_> { - // Should NOT warn fn trait_method(&self) -> &str { self.not_literal } diff --git a/tests/ui/unnecessary_literal_bound.rs b/tests/ui/unnecessary_literal_bound.rs index b371ff9d3a2e..de06180d4165 100644 --- a/tests/ui/unnecessary_literal_bound.rs +++ b/tests/ui/unnecessary_literal_bound.rs @@ -5,28 +5,26 @@ struct Struct<'a> { } impl Struct<'_> { - // Should warn fn returns_lit(&self) -> &str { + //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments "Hello" } - // Should NOT warn fn returns_non_lit(&self) -> &str { self.not_literal } - // Should warn, does not currently fn conditionally_returns_lit(&self, cond: bool) -> &str { + //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments if cond { "Literal" } else { "also a literal" } } - // Should NOT warn fn conditionally_returns_non_lit(&self, cond: bool) -> &str { if cond { "Literal" } else { self.not_literal } } - // Should warn fn contionally_returns_literals_explicit(&self, cond: bool) -> &str { + //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments if cond { return "Literal"; } @@ -34,7 +32,6 @@ impl Struct<'_> { "also a literal" } - // Should NOT warn fn conditionally_returns_non_lit_explicit(&self, cond: bool) -> &str { if cond { return self.not_literal; @@ -49,14 +46,13 @@ trait ReturnsStr { } impl ReturnsStr for u8 { - // Should warn, even though not useful without trait refinement fn trait_method(&self) -> &str { + //~^ error: returning a `str` unnecessarily tied to the lifetime of arguments "Literal" } } impl ReturnsStr for Struct<'_> { - // Should NOT warn fn trait_method(&self) -> &str { self.not_literal } diff --git a/tests/ui/unnecessary_literal_bound.stderr b/tests/ui/unnecessary_literal_bound.stderr index 512b2f9a0afa..c4c82d50584d 100644 --- a/tests/ui/unnecessary_literal_bound.stderr +++ b/tests/ui/unnecessary_literal_bound.stderr @@ -1,5 +1,5 @@ error: returning a `str` unnecessarily tied to the lifetime of arguments - --> tests/ui/unnecessary_literal_bound.rs:9:30 + --> tests/ui/unnecessary_literal_bound.rs:8:30 | LL | fn returns_lit(&self) -> &str { | ^^^^ help: try: `&'static str` @@ -8,16 +8,22 @@ LL | fn returns_lit(&self) -> &str { = help: to override `-D warnings` add `#[allow(clippy::unnecessary_literal_bound)]` error: returning a `str` unnecessarily tied to the lifetime of arguments - --> tests/ui/unnecessary_literal_bound.rs:29:68 + --> tests/ui/unnecessary_literal_bound.rs:17:56 + | +LL | fn conditionally_returns_lit(&self, cond: bool) -> &str { + | ^^^^ help: try: `&'static str` + +error: returning a `str` unnecessarily tied to the lifetime of arguments + --> tests/ui/unnecessary_literal_bound.rs:26:68 | LL | fn contionally_returns_literals_explicit(&self, cond: bool) -> &str { | ^^^^ help: try: `&'static str` error: returning a `str` unnecessarily tied to the lifetime of arguments - --> tests/ui/unnecessary_literal_bound.rs:53:31 + --> tests/ui/unnecessary_literal_bound.rs:49:31 | LL | fn trait_method(&self) -> &str { | ^^^^ help: try: `&'static str` -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed index b8087c6e000f..629a41c8834e 100644 --- a/tests/ui/while_let_on_iterator.fixed +++ b/tests/ui/while_let_on_iterator.fixed @@ -133,7 +133,7 @@ fn refutable2() { } } -fn nested_loops() { +fn nested_loops() -> ! { let a = [42, 1337]; loop { diff --git a/tests/ui/while_let_on_iterator.rs b/tests/ui/while_let_on_iterator.rs index 8e02f59b5126..bd17fdaf879d 100644 --- a/tests/ui/while_let_on_iterator.rs +++ b/tests/ui/while_let_on_iterator.rs @@ -133,7 +133,7 @@ fn refutable2() { } } -fn nested_loops() { +fn nested_loops() -> ! { let a = [42, 1337]; loop {