let_and_return: do not lint if last statement borrows

Author: ebroto

clippy_lints/src/let_and_return.rs

@@ -1,8 +1,12 @@
 use if_chain::if_chain;
 use rustc_errors::Applicability;
-use rustc_hir::{Block, ExprKind, PatKind, StmtKind};
+use rustc_hir::def_id::DefId;
+use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
+use rustc_hir::{Block, Expr, ExprKind, PatKind, StmtKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::hir::map::Map;
 use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty::subst::GenericArgKind;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 use crate::utils::{in_macro, match_qpath, snippet_opt, span_lint_and_then};
@@ -49,6 +53,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetReturn {
             if let PatKind::Binding(.., ident, _) = local.pat.kind;
             if let ExprKind::Path(qpath) = &retexpr.kind;
             if match_qpath(qpath, &[&*ident.name.as_str()]);
+            if !last_statement_borrows(cx, initexpr);
             if !in_external_macro(cx.sess(), initexpr.span);
             if !in_external_macro(cx.sess(), retexpr.span);
             if !in_external_macro(cx.sess(), local.span);
@@ -80,3 +85,57 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetReturn {
         }
     }
 }
+
+fn last_statement_borrows<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
+    let mut visitor = BorrowVisitor { cx, borrows: false };
+    walk_expr(&mut visitor, expr);
+    visitor.borrows
+}
+
+struct BorrowVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'a, 'tcx>,
+    borrows: bool,
+}
+
+impl BorrowVisitor<'_, '_> {
+    fn fn_def_id(&self, expr: &Expr<'_>) -> Option<DefId> {
+        match &expr.kind {
+            ExprKind::MethodCall(..) => self.cx.tables.type_dependent_def_id(expr.hir_id),
+            ExprKind::Call(
+                Expr {
+                    kind: ExprKind::Path(qpath),
+                    ..
+                },
+                ..,
+            ) => self.cx.tables.qpath_res(qpath, expr.hir_id).opt_def_id(),
+            _ => None,
+        }
+    }
+}
+
+impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
+    type Map = Map<'tcx>;
+
+    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+        if self.borrows {
+            return;
+        }
+
+        if let Some(def_id) = self.fn_def_id(expr) {
+            self.borrows = self
+                .cx
+                .tcx
+                .fn_sig(def_id)
+                .output()
+                .skip_binder()
+                .walk()
+                .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)));
+        }
+
+        walk_expr(self, expr);
+    }
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
+    }
+}

clippy_lints/src/utils/mod.rs

@@ -400,7 +400,7 @@ pub fn method_calls<'tcx>(
 /// Matches an `Expr` against a chain of methods, and return the matched `Expr`s.
 ///
 /// For example, if `expr` represents the `.baz()` in `foo.bar().baz()`,
-/// `matched_method_chain(expr, &["bar", "baz"])` will return a `Vec`
+/// `method_chain_args(expr, &["bar", "baz"])` will return a `Vec`
 /// containing the `Expr`s for
 /// `.bar()` and `.baz()`
 pub fn method_chain_args<'a>(expr: &'a Expr<'_>, methods: &[&str]) -> Option<Vec<&'a [Expr<'a>]>> {

tests/ui/let_and_return.rs

@@ -67,4 +67,72 @@ macro_rules! tuple_encode {
 
 tuple_encode!(T0, T1, T2, T3, T4, T5, T6, T7);
 
+mod no_lint_if_stmt_borrows {
+    mod issue_3792 {
+        use std::io::{self, BufRead, Stdin};
+
+        fn read_line() -> String {
+            let stdin = io::stdin();
+            let line = stdin.lock().lines().next().unwrap().unwrap();
+            line
+        }
+    }
+
+    mod issue_3324 {
+        use std::cell::RefCell;
+        use std::rc::{Rc, Weak};
+
+        fn test(value: Weak<RefCell<Bar>>) -> u32 {
+            let value = value.upgrade().unwrap();
+            let ret = value.borrow().baz();
+            ret
+        }
+
+        struct Bar {}
+
+        impl Bar {
+            fn new() -> Self {
+                Bar {}
+            }
+            fn baz(&self) -> u32 {
+                0
+            }
+        }
+
+        fn main() {
+            let a = Rc::new(RefCell::new(Bar::new()));
+            let b = Rc::downgrade(&a);
+            test(b);
+        }
+    }
+
+    mod free_function {
+        struct Inner;
+
+        struct Foo<'a> {
+            inner: &'a Inner,
+        }
+
+        impl Drop for Foo<'_> {
+            fn drop(&mut self) {}
+        }
+
+        impl Foo<'_> {
+            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
+        }
+    }
+}
+
 fn main() {}