diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index c11da3147ef4..8ced47b48a43 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -7,6 +7,7 @@ use clippy_utils::{ path_to_local_id, span_contains_cfg, span_find_starting_semi, }; use core::ops::ControlFlow; +use rustc_ast::NestedMetaItem; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::LangItem::ResultErr; @@ -14,13 +15,13 @@ use rustc_hir::{ Block, Body, Expr, ExprKind, FnDecl, HirId, ItemKind, LangItem, MatchSource, Node, OwnerNode, PatKind, QPath, Stmt, StmtKind, }; -use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_lint::{LateContext, LateLintPass, Level, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::adjustment::Adjust; use rustc_middle::ty::{self, GenericArgKind, Ty}; use rustc_session::declare_lint_pass; use rustc_span::def_id::LocalDefId; -use rustc_span::{BytePos, Pos, Span}; +use rustc_span::{sym, BytePos, Pos, Span}; use std::borrow::Cow; use std::fmt::Display; @@ -80,6 +81,9 @@ declare_clippy_lint! { /// ``` #[clippy::version = "pre 1.29.0"] pub NEEDLESS_RETURN, + // This lint requires some special handling in `check_final_expr` for `#[expect]`. + // This handling needs to be updated if the group gets changed. This should also + // be caught by tests. style, "using a return statement like `return expr;` where an expression would suffice" } @@ -91,6 +95,9 @@ declare_clippy_lint! { /// ### Why is this bad? /// The `return` is unnecessary. /// + /// Returns may be used to add attributes to the return expression. Return + /// statements with attributes are therefore be accepted by this lint. + /// /// ### Example /// ```rust,ignore /// fn foo(x: usize) -> Result<(), Box> { @@ -377,13 +384,39 @@ fn check_final_expr<'tcx>( } }; - if !cx.tcx.hir().attrs(expr.hir_id).is_empty() { - return; - } let borrows = inner.map_or(false, |inner| last_statement_borrows(cx, inner)); if borrows { return; } + if ret_span.from_expansion() { + return; + } + + // Returns may be used to turn an expression into a statement in rustc's AST. + // This allows the addition of attributes, like `#[allow]` (See: clippy#9361) + // `#[expect(clippy::needless_return)]` needs to be handled separatly to + // actually fullfil the expectation (clippy::#12998) + match cx.tcx.hir().attrs(expr.hir_id) { + [] => {}, + [attr] => { + if matches!(Level::from_attr(attr), Some(Level::Expect(_))) + && let metas = attr.meta_item_list() + && let Some(lst) = metas + && let [NestedMetaItem::MetaItem(meta_item)] = lst.as_slice() + && let [tool, lint_name] = meta_item.path.segments.as_slice() + && tool.ident.name == sym::clippy + && matches!( + lint_name.ident.name.as_str(), + "needless_return" | "style" | "all" | "warnings" + ) + { + // This is an expectation of the `needless_return` lint + } else { + return; + } + }, + _ => return, + } emit_return_lint(cx, ret_span, semi_spans, &replacement, expr.hir_id); }, @@ -415,10 +448,6 @@ fn emit_return_lint( replacement: &RetReplacement<'_>, at: HirId, ) { - if ret_span.from_expansion() { - return; - } - span_lint_hir_and_then( cx, NEEDLESS_RETURN, diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index 853f685f04c7..fc4129e1db84 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -228,12 +228,41 @@ fn needless_return_macro() -> String { format!("Hello {}", "world!") } -fn issue_9361() -> i32 { - let n = 1; - #[allow(clippy::arithmetic_side_effects)] +fn issue_9361(n: i32) -> i32 { + #[expect(clippy::arithmetic_side_effects)] return n + n; } +mod issue_12998 { + fn expect_lint() -> i32 { + let x = 1; + + #[expect(clippy::needless_return)] + return x; + } + + fn expect_group() -> i32 { + let x = 1; + + #[expect(clippy::style)] + return x; + } + + fn expect_all() -> i32 { + let x = 1; + + #[expect(clippy::all)] + return x; + } + + fn expect_warnings() -> i32 { + let x = 1; + + #[expect(warnings)] + return x; + } +} + fn issue8336(x: i32) -> bool { if x > 0 { println!("something"); diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index e9c1e0e8ae8e..61c7a02008f0 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -236,12 +236,41 @@ fn needless_return_macro() -> String { return format!("Hello {}", "world!"); } -fn issue_9361() -> i32 { - let n = 1; - #[allow(clippy::arithmetic_side_effects)] +fn issue_9361(n: i32) -> i32 { + #[expect(clippy::arithmetic_side_effects)] return n + n; } +mod issue_12998 { + fn expect_lint() -> i32 { + let x = 1; + + #[expect(clippy::needless_return)] + return x; + } + + fn expect_group() -> i32 { + let x = 1; + + #[expect(clippy::style)] + return x; + } + + fn expect_all() -> i32 { + let x = 1; + + #[expect(clippy::all)] + return x; + } + + fn expect_warnings() -> i32 { + let x = 1; + + #[expect(warnings)] + return x; + } +} + fn issue8336(x: i32) -> bool { if x > 0 { println!("something"); diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index 6c891fe7ad3f..ea9c230eafd2 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -483,7 +483,7 @@ LL + format!("Hello {}", "world!") | error: unneeded `return` statement - --> tests/ui/needless_return.rs:248:9 + --> tests/ui/needless_return.rs:277:9 | LL | return true; | ^^^^^^^^^^^ @@ -497,7 +497,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:250:9 + --> tests/ui/needless_return.rs:279:9 | LL | return false; | ^^^^^^^^^^^^ @@ -509,7 +509,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:257:13 + --> tests/ui/needless_return.rs:286:13 | LL | return 10; | ^^^^^^^^^ @@ -524,7 +524,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:260:13 + --> tests/ui/needless_return.rs:289:13 | LL | return 100; | ^^^^^^^^^^ @@ -537,7 +537,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:268:9 + --> tests/ui/needless_return.rs:297:9 | LL | return 0; | ^^^^^^^^ @@ -549,7 +549,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:275:13 + --> tests/ui/needless_return.rs:304:13 | LL | return *(x as *const isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -564,7 +564,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:277:13 + --> tests/ui/needless_return.rs:306:13 | LL | return !*(x as *const isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -577,7 +577,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:284:20 + --> tests/ui/needless_return.rs:313:20 | LL | let _ = 42; | ____________________^ @@ -594,7 +594,7 @@ LL + let _ = 42; | error: unneeded `return` statement - --> tests/ui/needless_return.rs:291:20 + --> tests/ui/needless_return.rs:320:20 | LL | let _ = 42; return; | ^^^^^^^ @@ -606,7 +606,7 @@ LL + let _ = 42; | error: unneeded `return` statement - --> tests/ui/needless_return.rs:303:9 + --> tests/ui/needless_return.rs:332:9 | LL | return Ok(format!("ok!")); | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -618,7 +618,7 @@ LL + Ok(format!("ok!")) | error: unneeded `return` statement - --> tests/ui/needless_return.rs:305:9 + --> tests/ui/needless_return.rs:334:9 | LL | return Err(format!("err!")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -630,7 +630,7 @@ LL + Err(format!("err!")) | error: unneeded `return` statement - --> tests/ui/needless_return.rs:311:9 + --> tests/ui/needless_return.rs:340:9 | LL | return if true { 1 } else { 2 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -642,7 +642,7 @@ LL + if true { 1 } else { 2 } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:315:9 + --> tests/ui/needless_return.rs:344:9 | LL | return if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -654,7 +654,7 @@ LL + (if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else | error: unneeded `return` statement - --> tests/ui/needless_return.rs:336:5 + --> tests/ui/needless_return.rs:365:5 | LL | return { "a".to_string() } + "b" + { "c" }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^