Fix let_and_return with temporary variables, and distinguish between Rust editions (rust-lang#14180)

The first commit fixes rust-lang#14164 by making sure that temporaries with
non-static references are also looked for in expressions coming from
expansion. The shortcut that was done skipped those parts and reported
an absence of short-lived temporaries, which was incorrect.

The second commit distinguishes between edition 2024 and earlier ones.
Starting from edition 2024, the problematic drop order has been fixed,
and block variables, which might be referenced in a block expression,
are freed after the block expression itself. This allows more
`let_and_return` cases to be reported starting with edition 2024,
whereas in earlier editions an intermediary variable was necessary to
reorder the drops.

Incidentally, since Clippy is compiled in edition 2024 mode, the second
commit has led to a fix in
`clippy_lints/src/matches/significant_drop_in_scrutinee.rs`.

changelog: [`let_and_return`]: lint more cases in edition 2024, and fix
a false positive involving short-lived block temporary variables in
earlier editions.

Author: llogiq

clippy_lints/src/matches/significant_drop_in_scrutinee.rs

@@ -199,7 +199,7 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
             return false;
         }
 
-        let result = match ty.kind() {
+        match ty.kind() {
             rustc_middle::ty::Adt(adt, args) => {
                 // if some field has significant drop,
                 adt.all_fields()
@@ -223,9 +223,7 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
             rustc_middle::ty::Tuple(tys) => tys.iter().any(|ty| self.has_sig_drop_attr_impl(ty)),
             rustc_middle::ty::Array(ty, _) | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr_impl(*ty),
             _ => false,
-        };
-
-        result
+        }
     }
 }
 

clippy_lints/src/returns.rs

@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::source::{SpanRangeExt, snippet_with_context};
 use clippy_utils::sugg::has_enclosing_paren;
-use clippy_utils::visitors::{Descend, for_each_expr};
+use clippy_utils::visitors::for_each_expr;
 use clippy_utils::{
     binary_expr_needs_parentheses, fn_def_id, is_from_proc_macro, is_inside_let_else, is_res_lang_ctor,
     leaks_droppable_temporary_with_limited_lifetime, path_res, path_to_local_id, span_contains_cfg,
@@ -21,6 +21,7 @@ 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::edition::Edition;
 use rustc_span::{BytePos, Pos, Span, sym};
 use std::borrow::Cow;
 use std::fmt::Display;
@@ -235,7 +236,7 @@ impl<'tcx> LateLintPass<'tcx> for Return {
             && let Some(initexpr) = &local.init
             && let PatKind::Binding(_, local_id, _, _) = local.pat.kind
             && path_to_local_id(retexpr, local_id)
-            && !last_statement_borrows(cx, initexpr)
+            && (cx.sess().edition() >= Edition::Edition2024 || !last_statement_borrows(cx, initexpr))
             && !initexpr.span.in_external_macro(cx.sess().source_map())
             && !retexpr.span.in_external_macro(cx.sess().source_map())
             && !local.span.from_expansion()
@@ -483,7 +484,7 @@ fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
         {
             ControlFlow::Break(())
         } else {
-            ControlFlow::Continue(Descend::from(!e.span.from_expansion()))
+            ControlFlow::Continue(())
         }
     })
     .is_some()

tests/ui/let_and_return.edition2021.fixed

@@ -0,0 +1,265 @@
+//@revisions: edition2021 edition2024
+//@[edition2021] edition:2021
+//@[edition2024] edition:2024
+
+#![allow(unused)]
+#![warn(clippy::let_and_return)]
+
+use std::cell::RefCell;
+
+fn test() -> i32 {
+    let _y = 0; // no warning
+    
+    5
+    //~^ ERROR: returning the result of a `let` binding from a block
+    //~| NOTE: `-D clippy::let-and-return` implied by `-D warnings`
+}
+
+fn test_inner() -> i32 {
+    if true {
+        
+        5
+        //~^ ERROR: returning the result of a `let` binding from a block
+    } else {
+        0
+    }
+}
+
+fn test_nowarn_1() -> i32 {
+    let mut x = 5;
+    x += 1;
+    x
+}
+
+fn test_nowarn_2() -> i32 {
+    let x = 5;
+    x + 1
+}
+
+fn test_nowarn_3() -> (i32, i32) {
+    // this should technically warn, but we do not compare complex patterns
+    let (x, y) = (5, 9);
+    (x, y)
+}
+
+fn test_nowarn_4() -> i32 {
+    // this should technically warn, but not b/c of clippy::let_and_return, but b/c of useless type
+    let x: i32 = 5;
+    x
+}
+
+fn test_nowarn_5(x: i16) -> u16 {
+    #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
+    let x = x as u16;
+    x
+}
+
+// False positive example
+trait Decode {
+    fn decode<D: std::io::Read>(d: D) -> Result<Self, ()>
+    where
+        Self: Sized;
+}
+
+macro_rules! tuple_encode {
+    ($($x:ident),*) => (
+        impl<$($x: Decode),*> Decode for ($($x),*) {
+            #[inline]
+            #[allow(non_snake_case)]
+            fn decode<D: std::io::Read>(mut d: D) -> Result<Self, ()> {
+                // Shouldn't trigger lint
+                Ok(($({let $x = Decode::decode(&mut d)?; $x }),*))
+            }
+        }
+    );
+}
+
+fn issue_3792() -> String {
+    use std::io::{self, BufRead, Stdin};
+
+    let stdin = io::stdin();
+    // `Stdin::lock` returns `StdinLock<'static>` so `line` doesn't borrow from `stdin`
+    // https://github.com/rust-lang/rust/pull/93965
+    
+    stdin.lock().lines().next().unwrap().unwrap()
+    //~^ ERROR: returning the result of a `let` binding from a block
+}
+
+tuple_encode!(T0, T1, T2, T3, T4, T5, T6, T7);
+
+mod no_lint_if_stmt_borrows {
+    use std::cell::RefCell;
+    use std::rc::{Rc, Weak};
+    struct Bar;
+
+    impl Bar {
+        fn new() -> Self {
+            Bar {}
+        }
+        fn baz(&self) -> u32 {
+            0
+        }
+    }
+
+    fn issue_3324(value: Weak<RefCell<Bar>>) -> u32 {
+        let value = value.upgrade().unwrap();
+        let ret = value.borrow().baz();
+        ret
+        //~[edition2024]^ ERROR: returning the result of a `let` binding from a block
+    }
+
+    fn borrows_in_closure(value: Weak<RefCell<Bar>>) -> u32 {
+        fn f(mut x: impl FnMut() -> u32) -> impl FnMut() -> u32 {
+            x
+        }
+
+        let value = value.upgrade().unwrap();
+        let ret = f(|| value.borrow().baz())();
+        ret
+        //~[edition2024]^ ERROR: returning the result of a `let` binding from a block
+    }
+
+    mod free_function {
+        struct Inner;
+
+        struct Foo<'a> {
+            inner: &'a Inner,
+        }
+
+        impl Drop for Foo<'_> {
+            fn drop(&mut self) {}
+        }
+
+        impl<'a> Foo<'a> {
+            fn new(inner: &'a Inner) -> Self {
+                Self { inner }
+            }
+
+            fn value(&self) -> i32 {
+                42
+            }
+        }
+
+        fn some_foo(inner: &Inner) -> Foo<'_> {
+            Foo { inner }
+        }
+
+        fn test() -> i32 {
+            let x = Inner {};
+            let value = some_foo(&x).value();
+            value
+            //~[edition2024]^ ERROR: returning the result of a `let` binding from a block
+        }
+
+        fn test2() -> i32 {
+            let x = Inner {};
+            let value = Foo::new(&x).value();
+            value
+            //~[edition2024]^ ERROR: returning the result of a `let` binding from a block
+        }
+    }
+}
+
+mod issue_5729 {
+    use std::sync::Arc;
+
+    trait Foo {}
+
+    trait FooStorage {
+        fn foo_cloned(&self) -> Arc<dyn Foo>;
+    }
+
+    struct FooStorageImpl<T: Foo> {
+        foo: Arc<T>,
+    }
+
+    impl<T: Foo + 'static> FooStorage for FooStorageImpl<T> {
+        fn foo_cloned(&self) -> Arc<dyn Foo> {
+            
+            (Arc::clone(&self.foo)) as _
+            //~^ ERROR: returning the result of a `let` binding from a block
+        }
+    }
+}
+
+mod issue_11335 {
+    pub enum E<T> {
+        A(T),
+        B(T),
+    }
+
+    impl<T> E<T> {
+        pub fn inner(&self) -> &T {
+            
+
+            (match self {
+                E::A(x) => x,
+                E::B(x) => x,
+            }) as _
+            //~^ ERROR: returning the result of a `let` binding from a block
+        }
+    }
+}
+
+// https://github.com/rust-lang/rust-clippy/issues/11167
+macro_rules! fn_in_macro {
+    ($b:block) => {
+        fn f() -> usize $b
+    }
+}
+fn_in_macro!({
+    return 1;
+});
+
+fn issue9150() -> usize {
+    let x = 1;
+    #[cfg(any())]
+    panic!("can't see me");
+    x
+}
+
+fn issue12801() {
+    fn left_is_if() -> String {
+        
+        (if true { "a".to_string() } else { "b".to_string() } + "c")
+        //~^ ERROR: returning the result of a `let` binding from a block
+    }
+
+    fn no_par_needed() -> String {
+        
+        "c".to_string() + if true { "a" } else { "b" }
+        //~^ ERROR: returning the result of a `let` binding from a block
+    }
+
+    fn conjunctive_blocks() -> String {
+        
+        ({ "a".to_string() } + "b" + { "c" } + "d")
+        //~^ ERROR: returning the result of a `let` binding from a block
+    }
+
+    #[allow(clippy::overly_complex_bool_expr)]
+    fn other_ops() {
+        let _ = || {
+            
+            (if true { 2 } else { 3 } << 4)
+            //~^ ERROR: returning the result of a `let` binding from a block
+        };
+        let _ = || {
+            
+            ({ true } || { false } && { 2 <= 3 })
+            //~^ ERROR: returning the result of a `let` binding from a block
+        };
+    }
+}
+
+fn issue14164() -> Result<u32, ()> {
+    let v = std::cell::RefCell::new(Some(vec![1]));
+    let r = match &*v.borrow() {
+        Some(v) => Ok(Ok(v[0])),
+        None => Ok(Ok(0)),
+    }?;
+    r
+    //~[edition2024]^ ERROR: returning the result of a `let` binding from a block
+}
+
+fn main() {}