Introduce coerce_container_to_any

7 files changed, 187 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6db04a3d525..d7c40c6d793 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5685,6 +5685,7 @@ Released 2018-09-13
 [`cmp_nan`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_nan
 [`cmp_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_null
 [`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned
+[`coerce_container_to_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#coerce_container_to_any
 [`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
 [`collapsible_else_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_else_if
 [`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
diff --git a/clippy_lints/src/coerce_container_to_any.rs b/clippy_lints/src/coerce_container_to_any.rs
new file mode 100644
index 00000000000..8c12a42ba4e
--- /dev/null
+++ b/clippy_lints/src/coerce_container_to_any.rs
@@ -0,0 +1,108 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use clippy_utils::sym;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::{self, ExistentialPredicate, Ty, TyCtxt};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Protects against unintended coercion of references to container types to `&dyn Any` when the
+    /// container type dereferences to a `dyn Any` which could be directly referenced instead.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// The intention is usually to get a reference to the `dyn Any` the value dereferences to,
+    /// rather than coercing a reference to the container itself to `&dyn Any`.
+    ///
+    /// ### Example
+    ///
+    /// Because `Box<dyn Any>` itself implements `Any`, `&Box<dyn Any>`
+    /// can be coerced to an `&dyn Any` which refers to *the `Box` itself*, rather than the
+    /// inner `dyn Any`.
+    /// ```no_run
+    /// # use std::any::Any;
+    /// let x: Box<dyn Any> = Box::new(0u32);
+    /// let dyn_any_of_box: &dyn Any = &x;
+    ///
+    /// // Fails as we have a &dyn Any to the Box, not the u32
+    /// assert_eq!(dyn_any_of_box.downcast_ref::<u32>(), None);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # use std::any::Any;
+    /// let x: Box<dyn Any> = Box::new(0u32);
+    /// let dyn_any_of_u32: &dyn Any = &*x;
+    ///
+    /// // Succeeds since we have a &dyn Any to the inner u32!
+    /// assert_eq!(dyn_any_of_u32.downcast_ref::<u32>(), Some(&0u32));
+    /// ```
+    #[clippy::version = "1.88.0"]
+    pub COERCE_CONTAINER_TO_ANY,
+    suspicious,
+    "coercing to `&dyn Any` when dereferencing could produce a `dyn Any` without coercion is usually not intended"
+}
+declare_lint_pass!(CoerceContainerToAny => [COERCE_CONTAINER_TO_ANY]);
+
+impl<'tcx> LateLintPass<'tcx> for CoerceContainerToAny {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
+        // If this expression has an effective type of `&dyn Any` ...
+        {
+            let coerced_ty = cx.typeck_results().expr_ty_adjusted(e);
+
+            let ty::Ref(_, coerced_ref_ty, _) = *coerced_ty.kind() else {
+                return;
+            };
+            if !is_dyn_any(cx.tcx, coerced_ref_ty) {
+                return;
+            }
+        }
+
+        let expr_ty = cx.typeck_results().expr_ty(e);
+        let ty::Ref(_, expr_ref_ty, _) = *expr_ty.kind() else {
+            return;
+        };
+        // ... but only due to coercion ...
+        if is_dyn_any(cx.tcx, expr_ref_ty) {
+            return;
+        }
+        // ... and it also *derefs* to `dyn Any` ...
+        let Some((depth, target)) = clippy_utils::ty::deref_chain(cx, expr_ref_ty).enumerate().last() else {
+            return;
+        };
+        if !is_dyn_any(cx.tcx, target) {
+            return;
+        }
+
+        // ... that's probably not intended.
+        let (span, deref_count) = match e.kind {
+            // If `e` was already an `&` expression, skip `*&` in the suggestion
+            ExprKind::AddrOf(_, _, referent) => (referent.span, depth),
+            _ => (e.span, depth + 1),
+        };
+        span_lint_and_sugg(
+            cx,
+            COERCE_CONTAINER_TO_ANY,
+            e.span,
+            format!("coercing `{expr_ty}` to `&dyn Any`"),
+            "consider dereferencing",
+            format!("&{}{}", str::repeat("*", deref_count), snippet(cx, span, "x")),
+            Applicability::MaybeIncorrect,
+        );
+    }
+}
+
+fn is_dyn_any(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool {
+    let ty::Dynamic(traits, ..) = ty.kind() else {
+        return false;
+    };
+    traits.iter().any(|binder| {
+        let ExistentialPredicate::Trait(t) = binder.skip_binder() else {
+            return false;
+        };
+        tcx.is_diagnostic_item(sym::Any, t.def_id)
+    })
+}
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 71388779b08..a387ee1d5a6 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -77,6 +77,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::cfg_not_test::CFG_NOT_TEST_INFO,
     crate::checked_conversions::CHECKED_CONVERSIONS_INFO,
     crate::cloned_ref_to_slice_refs::CLONED_REF_TO_SLICE_REFS_INFO,
+    crate::coerce_container_to_any::COERCE_CONTAINER_TO_ANY_INFO,
     crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO,
     crate::collapsible_if::COLLAPSIBLE_ELSE_IF_INFO,
     crate::collapsible_if::COLLAPSIBLE_IF_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 6ec14486c20..0f620a0ccf6 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -95,6 +95,7 @@ mod casts;
 mod cfg_not_test;
 mod checked_conversions;
 mod cloned_ref_to_slice_refs;
+mod coerce_container_to_any;
 mod cognitive_complexity;
 mod collapsible_if;
 mod collection_is_never_read;
@@ -948,5 +949,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix));
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
     store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
+    store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
diff --git a/tests/ui/coerce_container_to_any.fixed b/tests/ui/coerce_container_to_any.fixed
new file mode 100644
index 00000000000..ae9d3ef9656
--- /dev/null
+++ b/tests/ui/coerce_container_to_any.fixed
@@ -0,0 +1,26 @@
+#![warn(clippy::coerce_container_to_any)]
+
+use std::any::Any;
+
+fn main() {
+    let x: Box<dyn Any> = Box::new(());
+    let ref_x = &x;
+
+    f(&*x);
+    //~^ coerce_container_to_any
+
+    f(&**ref_x);
+    //~^ coerce_container_to_any
+
+    let _: &dyn Any = &*x;
+    //~^ coerce_container_to_any
+
+    f(&42);
+    f(&Box::new(()));
+    f(&Box::new(Box::new(())));
+    f(&**ref_x);
+    f(&*x);
+    let _: &dyn Any = &*x;
+}
+
+fn f(_: &dyn Any) {}
diff --git a/tests/ui/coerce_container_to_any.rs b/tests/ui/coerce_container_to_any.rs
new file mode 100644
index 00000000000..9948bd48e0d
--- /dev/null
+++ b/tests/ui/coerce_container_to_any.rs
@@ -0,0 +1,26 @@
+#![warn(clippy::coerce_container_to_any)]
+
+use std::any::Any;
+
+fn main() {
+    let x: Box<dyn Any> = Box::new(());
+    let ref_x = &x;
+
+    f(&x);
+    //~^ coerce_container_to_any
+
+    f(ref_x);
+    //~^ coerce_container_to_any
+
+    let _: &dyn Any = &x;
+    //~^ coerce_container_to_any
+
+    f(&42);
+    f(&Box::new(()));
+    f(&Box::new(Box::new(())));
+    f(&**ref_x);
+    f(&*x);
+    let _: &dyn Any = &*x;
+}
+
+fn f(_: &dyn Any) {}
diff --git a/tests/ui/coerce_container_to_any.stderr b/tests/ui/coerce_container_to_any.stderr
new file mode 100644
index 00000000000..00ab77e0ce0
--- /dev/null
+++ b/tests/ui/coerce_container_to_any.stderr
@@ -0,0 +1,23 @@
+error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any`
+  --> tests/ui/coerce_container_to_any.rs:9:7
+   |
+LL |     f(&x);
+   |       ^^ help: consider dereferencing: `&*x`
+   |
+   = note: `-D clippy::coerce-container-to-any` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::coerce_container_to_any)]`
+
+error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any`
+  --> tests/ui/coerce_container_to_any.rs:12:7
+   |
+LL |     f(ref_x);
+   |       ^^^^^ help: consider dereferencing: `&**ref_x`
+
+error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any`
+  --> tests/ui/coerce_container_to_any.rs:15:23
+   |
+LL |     let _: &dyn Any = &x;
+   |                       ^^ help: consider dereferencing: `&*x`
+
+error: aborting due to 3 previous errors
+