fix: check temporaries in receiver

rust-lang · Jan 23, 2025 · 28f651d · 28f651d
1 parent 9857a40
commit 28f651d
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 24 deletions.
diff --git a/clippy_lints/src/methods/return_and_then.rs b/clippy_lints/src/methods/return_and_then.rs
@@ -1,37 +1,41 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_applicability};
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{is_expr_final_block_expr, is_expr_used_or_unified, peel_blocks};
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
+use rustc_middle::ty::{self, GenericArg, Ty};
 use rustc_span::sym;
+use std::ops::ControlFlow;
 
-use super::RETURN_AND_THEN;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_applicability};
+use clippy_utils::ty::get_type_diagnostic_name;
+use clippy_utils::visitors::for_each_unconsumed_temporary;
+use clippy_utils::{is_expr_final_block_expr, peel_blocks};
 
-fn is_final_call(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
-    if !is_expr_used_or_unified(cx.tcx, expr) {
-        return false;
-    }
-    is_expr_final_block_expr(cx.tcx, expr)
-}
+use super::RETURN_AND_THEN;
 
-/// lint if `and_then` is the last call in the function
+/// lint if `and_then` is the last expression in a block and
+/// there are no temporaries in the receiver
 pub(super) fn check<'tcx>(
-    cx: &LateContext<'_>,
+    cx: &LateContext<'tcx>,
     expr: &hir::Expr<'_>,
-    recv: &'tcx hir::Expr<'_>,
+    recv: &'tcx hir::Expr<'tcx>,
     arg: &'tcx hir::Expr<'_>,
 ) {
-    if !is_final_call(cx, expr) {
+    if !is_expr_final_block_expr(cx.tcx, expr) {
         return;
     }
 
     let recv_type = cx.typeck_results().expr_ty(recv);
-    let is_option = is_type_diagnostic_item(cx, recv_type, sym::Option);
-    let is_result = is_type_diagnostic_item(cx, recv_type, sym::Result);
+    if !matches!(get_type_diagnostic_name(cx, recv_type), Some(sym::Option | sym::Result)) {
+        return;
+    }
 
-    if !is_option && !is_result {
+    let has_ref_type = matches!(recv_type.kind(), ty::Adt(_, args) if args
+        .first()
+        .and_then(|arg0: &GenericArg<'tcx>| GenericArg::as_type(*arg0))
+        .is_some_and(Ty::is_ref));
+    let has_temporaries = for_each_unconsumed_temporary(cx, recv, |_| ControlFlow::Break(())).is_break();
+    if has_ref_type && has_temporaries {
         return;
     }
 
@@ -44,7 +48,7 @@ pub(super) fn check<'tcx>(
 
     let mut applicability = Applicability::MachineApplicable;
     let arg_snip = snippet_with_applicability(cx, closure_arg.span, "_", &mut applicability);
-    let recv_snip = snippet_with_applicability(cx, recv.span, "..", &mut applicability);
+    let recv_snip = snippet_with_applicability(cx, recv.span, "_", &mut applicability);
     let body_snip = snippet_with_applicability(cx, closure_expr.span, "..", &mut applicability);
     let inner = match body_snip.strip_prefix('{').and_then(|s| s.strip_suffix('}')) {
         Some(s) => s.trim_start_matches('\n').trim_end(),

diff --git a/tests/ui/return_and_then.fixed b/tests/ui/return_and_then.fixed
@@ -15,7 +15,7 @@ fn main() {
 
     fn test_call_chain() -> Option<i32> {
         let n = gen_option(1)?;
-        test_opt_func(Some(n))
+        test_opt_block(Some(n))
     }
 
     fn test_res_block(opt: Result<i32, i32>) -> Result<i32, i32> {
@@ -27,6 +27,24 @@ fn main() {
         let n = opt?;
         test_res_block(Ok(n))
     }
+
+    // should not lint
+    fn test_tmp_ref() -> Option<String> {
+        String::from("<BOOM>")
+            .strip_prefix("<")
+            .and_then(|s| s.strip_suffix(">").map(String::from))
+    }
+
+    // should not lint
+    fn test_unconsumed_tmp() -> Option<i32> {
+        [1, 2, 3]
+            .iter()
+            .map(|x| x + 1)
+            .collect::<Vec<_>>() // temporary Vec created here
+            .as_slice() // creates temporary slice
+            .first() // creates temporary reference
+            .and_then(|x| test_opt_block(Some(*x)))
+    }
 }
 
 fn gen_option(n: i32) -> Option<i32> {

diff --git a/tests/ui/return_and_then.rs b/tests/ui/return_and_then.rs
@@ -14,7 +14,7 @@ fn main() {
     }
 
     fn test_call_chain() -> Option<i32> {
-        gen_option(1).and_then(|n| test_opt_func(Some(n)))
+        gen_option(1).and_then(|n| test_opt_block(Some(n)))
     }
 
     fn test_res_block(opt: Result<i32, i32>) -> Result<i32, i32> {
@@ -24,6 +24,24 @@ fn main() {
     fn test_res_func(opt: Result<i32, i32>) -> Result<i32, i32> {
         opt.and_then(|n| test_res_block(Ok(n)))
     }
+
+    // should not lint
+    fn test_tmp_ref() -> Option<String> {
+        String::from("<BOOM>")
+            .strip_prefix("<")
+            .and_then(|s| s.strip_suffix(">").map(String::from))
+    }
+
+    // should not lint
+    fn test_unconsumed_tmp() -> Option<i32> {
+        [1, 2, 3]
+            .iter()
+            .map(|x| x + 1)
+            .collect::<Vec<_>>() // temporary Vec created here
+            .as_slice() // creates temporary slice
+            .first() // creates temporary reference
+            .and_then(|x| test_opt_block(Some(*x)))
+    }
 }
 
 fn gen_option(n: i32) -> Option<i32> {

diff --git a/tests/ui/return_and_then.stderr b/tests/ui/return_and_then.stderr
@@ -33,13 +33,13 @@ LL +         test_opt_block(Some(n))
 error: use the question mark operator instead of an `and_then` call
   --> tests/ui/return_and_then.rs:17:9
    |
-LL |         gen_option(1).and_then(|n| test_opt_func(Some(n)))
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |         gen_option(1).and_then(|n| test_opt_block(Some(n)))
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 help: try
    |
 LL ~         let n = gen_option(1)?;
-LL +         test_opt_func(Some(n))
+LL +         test_opt_block(Some(n))
    |
 
 error: use the question mark operator instead of an `and_then` call