Skip to content

Introduce coerce_any_ref_to_any #14812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_any_ref_to_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#coerce_any_ref_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
Expand Down
108 changes: 108 additions & 0 deletions clippy_lints/src/coerce_any_ref_to_any.rs
Original file line number Diff line number Diff line change
@@ -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 reborrowed instead.
///
/// ### Why is this bad?
///
/// The intention is usually to get a reference to the `dyn Any` the value derefernces to,
/// rather than getting a `&dyn Any` which is actually referring to the outer container type.
///
/// ### Example
///
/// Here we see that because `Box<dyn Any>` itself implements `Any`, `&Box<dyn Any>`
/// gets 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_ANY_REF_TO_ANY,
nursery,
"coercing to `&dyn Any` when dereferencing could produce a `dyn Any` without coercion is usually not intended"
}
declare_lint_pass!(CoerceAnyRefToAny => [COERCE_ANY_REF_TO_ANY]);

impl<'tcx> LateLintPass<'tcx> for CoerceAnyRefToAny {
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_ANY_REF_TO_ANY,
e.span,
format!("coercing `{expr_ty}` to `&dyn Any` rather than dereferencing to the `dyn Any` inside"),
"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 Some(ExistentialPredicate::Trait(t)) = binder.no_bound_vars() else {
return false;
};
tcx.is_diagnostic_item(sym::Any, t.def_id)
})
}
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_any_ref_to_any::COERCE_ANY_REF_TO_ANY_INFO,
crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO,
crate::collapsible_if::COLLAPSIBLE_ELSE_IF_INFO,
crate::collapsible_if::COLLAPSIBLE_IF_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ mod casts;
mod cfg_not_test;
mod checked_conversions;
mod cloned_ref_to_slice_refs;
mod coerce_any_ref_to_any;
mod cognitive_complexity;
mod collapsible_if;
mod collection_is_never_read;
Expand Down Expand Up @@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
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(coerce_any_ref_to_any::CoerceAnyRefToAny));
// add lints here, do not remove this comment, it's used in `new_lint`
}
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
# begin autogenerated nightly
channel = "nightly-2025-05-01"
# end autogenerated nightly
components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"]
components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt", "rust-analyzer"]
profile = "minimal"
25 changes: 25 additions & 0 deletions tests/ui/coerce_any_ref_to_any.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![warn(clippy::coerce_any_ref_to_any)]

use std::any::Any;

fn main() {
let x: Box<dyn Any> = Box::new(());
let ref_x = &x;

f(&*x);
//~^ coerce_any_ref_to_any

f(&**ref_x);
//~^ coerce_any_ref_to_any

let _: &dyn Any = &*x;
//~^ coerce_any_ref_to_any

f(&42);
f(&Box::new(()));
f(&**ref_x);
f(&*x);
let _: &dyn Any = &*x;
}

fn f(_: &dyn Any) {}
25 changes: 25 additions & 0 deletions tests/ui/coerce_any_ref_to_any.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![warn(clippy::coerce_any_ref_to_any)]

use std::any::Any;

fn main() {
let x: Box<dyn Any> = Box::new(());
let ref_x = &x;

f(&x);
//~^ coerce_any_ref_to_any

f(ref_x);
//~^ coerce_any_ref_to_any

let _: &dyn Any = &x;
//~^ coerce_any_ref_to_any

f(&42);
f(&Box::new(()));
f(&**ref_x);
f(&*x);
let _: &dyn Any = &*x;
}

fn f(_: &dyn Any) {}
23 changes: 23 additions & 0 deletions tests/ui/coerce_any_ref_to_any.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any` rather than dereferencing to the `dyn Any` inside
--> tests/ui/coerce_any_ref_to_any.rs:9:7
|
LL | f(&x);
| ^^ help: consider dereferencing: `&*x`
|
= note: `-D clippy::coerce-any-ref-to-any` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::coerce_any_ref_to_any)]`

error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any` rather than dereferencing to the `dyn Any` inside
--> tests/ui/coerce_any_ref_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` rather than dereferencing to the `dyn Any` inside
--> tests/ui/coerce_any_ref_to_any.rs:15:23
|
LL | let _: &dyn Any = &x;
| ^^ help: consider dereferencing: `&*x`

error: aborting due to 3 previous errors