make [manual_map] ignore types that contain dyn (#12712)

fixes: #12659

[`manual_map`] and [`manual_filter`] shares the same check logic, but
this issue doesn't seems like it could affect `manual_filter` (?)

---

changelog: make [`manual_map`] ignore types that contain `dyn`

Author: Alexendoo

clippy_lints/src/matches/manual_utils.rs

@@ -4,8 +4,8 @@ use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::ty::{is_copy, is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function};
 use clippy_utils::{
-    CaptureKind, can_move_expr_to_closure, is_else_clause, is_lint_allowed, is_res_lang_ctor, path_res,
-    path_to_local_id, peel_blocks, peel_hir_expr_refs, peel_hir_expr_while,
+    CaptureKind, can_move_expr_to_closure, expr_requires_coercion, is_else_clause, is_lint_allowed, is_res_lang_ctor,
+    path_res, path_to_local_id, peel_blocks, peel_hir_expr_refs, peel_hir_expr_while,
 };
 use rustc_ast::util::parser::ExprPrecedence;
 use rustc_errors::Applicability;
@@ -73,7 +73,7 @@ where
     }
 
     // `map` won't perform any adjustments.
-    if !cx.typeck_results().expr_adjustments(some_expr.expr).is_empty() {
+    if expr_requires_coercion(cx, expr) {
         return None;
     }
 
@@ -124,6 +124,12 @@ where
     };
 
     let closure_expr_snip = some_expr.to_snippet_with_context(cx, expr_ctxt, &mut app);
+    let closure_body = if some_expr.needs_unsafe_block {
+        format!("unsafe {}", closure_expr_snip.blockify())
+    } else {
+        closure_expr_snip.to_string()
+    };
+
     let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind {
         if !some_expr.needs_unsafe_block
             && let Some(func) = can_pass_as_func(cx, id, some_expr.expr)
@@ -145,20 +151,12 @@ where
                 ""
             };
 
-            if some_expr.needs_unsafe_block {
-                format!("|{annotation}{some_binding}| unsafe {{ {closure_expr_snip} }}")
-            } else {
-                format!("|{annotation}{some_binding}| {closure_expr_snip}")
-            }
+            format!("|{annotation}{some_binding}| {closure_body}")
         }
     } else if !is_wild_none && explicit_ref.is_none() {
         // TODO: handle explicit reference annotations.
         let pat_snip = snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0;
-        if some_expr.needs_unsafe_block {
-            format!("|{pat_snip}| unsafe {{ {closure_expr_snip} }}")
-        } else {
-            format!("|{pat_snip}| {closure_expr_snip}")
-        }
+        format!("|{pat_snip}| {closure_body}")
     } else {
         // Refutable bindings and mixed reference annotations can't be handled by `map`.
         return None;

clippy_utils/src/lib.rs

@@ -118,7 +118,7 @@ use rustc_middle::ty::fast_reject::SimplifiedType;
 use rustc_middle::ty::layout::IntegerExt;
 use rustc_middle::ty::{
     self as rustc_ty, Binder, BorrowKind, ClosureKind, EarlyBinder, FloatTy, GenericArgKind, GenericArgsRef, IntTy, Ty,
-    TyCtxt, TypeVisitableExt, UintTy, UpvarCapture,
+    TyCtxt, TypeFlags, TypeVisitableExt, UintTy, UpvarCapture,
 };
 use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::source_map::SourceMap;
@@ -3508,3 +3508,90 @@ pub fn leaks_droppable_temporary_with_limited_lifetime<'tcx>(cx: &LateContext<'t
     })
     .is_break()
 }
+
+/// Returns true if the specified `expr` requires coercion,
+/// meaning that it either has a coercion or propagates a coercion from one of its sub expressions.
+///
+/// Similar to [`is_adjusted`], this not only checks if an expression's type was adjusted,
+/// but also going through extra steps to see if it fits the description of [coercion sites].
+///
+/// You should used this when you want to avoid suggesting replacing an expression that is currently
+/// a coercion site or coercion propagating expression with one that is not.
+///
+/// [coercion sites]: https://doc.rust-lang.org/stable/reference/type-coercions.html#coercion-sites
+pub fn expr_requires_coercion<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> bool {
+    let expr_ty_is_adjusted = cx
+        .typeck_results()
+        .expr_adjustments(expr)
+        .iter()
+        // ignore `NeverToAny` adjustments, such as `panic!` call.
+        .any(|adj| !matches!(adj.kind, Adjust::NeverToAny));
+    if expr_ty_is_adjusted {
+        return true;
+    }
+
+    // Identify coercion sites and recursively check if those sites
+    // actually have type adjustments.
+    match expr.kind {
+        ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) if let Some(def_id) = fn_def_id(cx, expr) => {
+            let fn_sig = cx.tcx.fn_sig(def_id).instantiate_identity();
+
+            if !fn_sig.output().skip_binder().has_type_flags(TypeFlags::HAS_TY_PARAM) {
+                return false;
+            }
+
+            let self_arg_count = usize::from(matches!(expr.kind, ExprKind::MethodCall(..)));
+            let mut args_with_ty_param = {
+                fn_sig
+                    .inputs()
+                    .skip_binder()
+                    .iter()
+                    .skip(self_arg_count)
+                    .zip(args)
+                    .filter_map(|(arg_ty, arg)| {
+                        if arg_ty.has_type_flags(TypeFlags::HAS_TY_PARAM) {
+                            Some(arg)
+                        } else {
+                            None
+                        }
+                    })
+            };
+            args_with_ty_param.any(|arg| expr_requires_coercion(cx, arg))
+        },
+        // Struct/union initialization.
+        ExprKind::Struct(qpath, _, _) => {
+            let res = cx.typeck_results().qpath_res(qpath, expr.hir_id);
+            if let Some((_, v_def)) = adt_and_variant_of_res(cx, res) {
+                let generic_args = cx.typeck_results().node_args(expr.hir_id);
+                v_def
+                    .fields
+                    .iter()
+                    .any(|field| field.ty(cx.tcx, generic_args).has_type_flags(TypeFlags::HAS_TY_PARAM))
+            } else {
+                false
+            }
+        },
+        // Function results, including the final line of a block or a `return` expression.
+        ExprKind::Block(
+            &Block {
+                expr: Some(ret_expr), ..
+            },
+            _,
+        )
+        | ExprKind::Ret(Some(ret_expr)) => expr_requires_coercion(cx, ret_expr),
+
+        // ===== Coercion-propagation expressions =====
+        ExprKind::Array(elems) | ExprKind::Tup(elems) => elems.iter().any(|elem| expr_requires_coercion(cx, elem)),
+        // Array but with repeating syntax.
+        ExprKind::Repeat(rep_elem, _) => expr_requires_coercion(cx, rep_elem),
+        // Others that may contain coercion sites.
+        ExprKind::If(_, then, maybe_else) => {
+            expr_requires_coercion(cx, then) || maybe_else.is_some_and(|e| expr_requires_coercion(cx, e))
+        },
+        ExprKind::Match(_, arms, _) => arms
+            .iter()
+            .map(|arm| arm.body)
+            .any(|body| expr_requires_coercion(cx, body)),
+        _ => false,
+    }
+}

tests/ui/manual_map_option.fixed

@@ -113,7 +113,16 @@ fn main() {
     }
 
     // #6811
-    Some(0).map(|x| vec![x]);
+    match Some(0) {
+        Some(x) => Some(vec![x]),
+        None => None,
+    };
+
+    // Don't lint, coercion
+    let x: Option<Vec<&[u8]>> = match Some(()) {
+        Some(_) => Some(vec![b"1234"]),
+        None => None,
+    };
 
     option_env!("").map(String::from);
 

tests/ui/manual_map_option.rs

@@ -170,6 +170,12 @@ fn main() {
         None => None,
     };
 
+    // Don't lint, coercion
+    let x: Option<Vec<&[u8]>> = match Some(()) {
+        Some(_) => Some(vec![b"1234"]),
+        None => None,
+    };
+
     match option_env!("") {
         Some(x) => Some(String::from(x)),
         None => None,

tests/ui/manual_map_option.stderr

@@ -156,16 +156,7 @@ LL | |     };
    | |_____^ help: try: `Some((String::new(), "test")).as_ref().map(|(x, y)| (y, x))`
 
 error: manual implementation of `Option::map`
-  --> tests/ui/manual_map_option.rs:168:5
-   |
-LL | /     match Some(0) {
-LL | |         Some(x) => Some(vec![x]),
-LL | |         None => None,
-LL | |     };
-   | |_____^ help: try: `Some(0).map(|x| vec![x])`
-
-error: manual implementation of `Option::map`
-  --> tests/ui/manual_map_option.rs:173:5
+  --> tests/ui/manual_map_option.rs:179:5
    |
 LL | /     match option_env!("") {
 LL | |         Some(x) => Some(String::from(x)),
@@ -174,7 +165,7 @@ LL | |     };
    | |_____^ help: try: `option_env!("").map(String::from)`
 
 error: manual implementation of `Option::map`
-  --> tests/ui/manual_map_option.rs:193:12
+  --> tests/ui/manual_map_option.rs:199:12
    |
 LL |       } else if let Some(x) = Some(0) {
    |  ____________^
@@ -185,7 +176,7 @@ LL | |     };
    | |_____^ help: try: `{ Some(0).map(|x| x + 1) }`
 
 error: manual implementation of `Option::map`
-  --> tests/ui/manual_map_option.rs:201:12
+  --> tests/ui/manual_map_option.rs:207:12
    |
 LL |       } else if let Some(x) = Some(0) {
    |  ____________^
@@ -195,5 +186,5 @@ LL | |         None
 LL | |     };
    | |_____^ help: try: `{ Some(0).map(|x| x + 1) }`
 
-error: aborting due to 21 previous errors
+error: aborting due to 20 previous errors
 

tests/ui/manual_map_option_2.fixed

@@ -40,9 +40,14 @@ fn main() {
         None => None,
     };
 
-    // Lint. `s` is captured by reference, so no lifetime issues.
     let s = Some(String::new());
+    // Lint. `s` is captured by reference, so no lifetime issues.
     let _ = s.as_ref().map(|x| { if let Some(ref s) = s { (x.clone(), s) } else { panic!() } });
+    // Don't lint this, type of `s` is coercioned from `&String` to `&str`
+    let x: Option<(String, &str)> = match &s {
+        Some(x) => Some({ if let Some(ref s) = s { (x.clone(), s) } else { panic!() } }),
+        None => None,
+    };
 
     // Issue #7820
     unsafe fn f(x: u32) -> u32 {
@@ -54,3 +59,89 @@ fn main() {
     let _ = Some(0).map(|x| unsafe { f(x) });
     let _ = Some(0).map(|x| unsafe { f(x) });
 }
+
+// issue #12659
+mod with_type_coercion {
+    trait DummyTrait {}
+
+    fn foo<T: DummyTrait, F: Fn() -> Result<T, ()>>(f: F) {
+        // Don't lint
+        let _: Option<Result<Box<dyn DummyTrait>, ()>> = match Some(0) {
+            Some(_) => Some(match f() {
+                Ok(res) => Ok(Box::new(res)),
+                _ => Err(()),
+            }),
+            None => None,
+        };
+
+        let _: Option<Box<&[u8]>> = match Some(()) {
+            Some(_) => Some(Box::new(b"1234")),
+            None => None,
+        };
+
+        let x = String::new();
+        let _: Option<Box<&str>> = match Some(()) {
+            Some(_) => Some(Box::new(&x)),
+            None => None,
+        };
+
+        let _: Option<&str> = match Some(()) {
+            Some(_) => Some(&x),
+            None => None,
+        };
+
+        //~v ERROR: manual implementation of `Option::map`
+        let _ = Some(0).map(|_| match f() {
+                Ok(res) => Ok(Box::new(res)),
+                _ => Err(()),
+            });
+    }
+
+    #[allow(clippy::redundant_allocation)]
+    fn bar() {
+        fn f(_: Option<Box<&[u8]>>) {}
+        fn g(b: &[u8]) -> Box<&[u8]> {
+            Box::new(b)
+        }
+
+        let x: &[u8; 4] = b"1234";
+        f(match Some(()) {
+            Some(_) => Some(Box::new(x)),
+            None => None,
+        });
+
+        //~v ERROR: manual implementation of `Option::map`
+        let _: Option<Box<&[u8]>> = Some(0).map(|_| g(x));
+    }
+
+    fn with_fn_ret(s: &Option<String>) -> Option<(String, &str)> {
+        // Don't lint, `map` doesn't work as the return type is adjusted.
+        match s {
+            Some(x) => Some({ if let Some(ref s) = s { (x.clone(), s) } else { panic!() } }),
+            None => None,
+        }
+    }
+
+    fn with_fn_ret_2(s: &Option<String>) -> Option<(String, &str)> {
+        if true {
+            // Don't lint, `map` doesn't work as the return type is adjusted.
+            return match s {
+                Some(x) => Some({ if let Some(ref s) = s { (x.clone(), s) } else { panic!() } }),
+                None => None,
+            };
+        }
+        None
+    }
+
+    #[allow(clippy::needless_late_init)]
+    fn with_fn_ret_3<'a>(s: &'a Option<String>) -> Option<(String, &'a str)> {
+        let x: Option<(String, &'a str)>;
+        x = {
+            match s {
+                Some(x) => Some({ if let Some(ref s) = s { (x.clone(), s) } else { panic!() } }),
+                None => None,
+            }
+        };
+        x
+    }
+}