Auto merge of rust-lang#13027 - xFrednet:12998-expect-returns, r=Jarcho

`needless_return`: Support `#[expect]` on the return statement

A fix for rust-lang/rust-clippy#9361 suppresses `clippy::needless_return` if there are any attributes on the `return` statement. This leads to some unexpected behavior, as described in rust-lang/rust-clippy#12998, where adding `#[expect(clippy::needless_return)]` suppresses the lint, but doesn't fulfill the expectation.

I now decided to manually fulfill any expectations, if they are before the attribute check.

---

Closes: rust-lang/rust-clippy#12998

changelog: none

Author: bors

clippy_lints/src/returns.rs

@@ -7,20 +7,21 @@ 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;
 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<dyn Error>> {
@@ -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,

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");

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");

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" };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^