Skip to content

Commit eef8a8c

Browse files
committed
Introduce coerce_any_ref_to_any
1 parent 40bead0 commit eef8a8c

8 files changed

+186
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5685,6 +5685,7 @@ Released 2018-09-13
56855685
[`cmp_nan`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_nan
56865686
[`cmp_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_null
56875687
[`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned
5688+
[`coerce_any_ref_to_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#coerce_any_ref_to_any
56885689
[`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
56895690
[`collapsible_else_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_else_if
56905691
[`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet;
3+
use clippy_utils::sym;
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{Expr, ExprKind};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_middle::ty::{self, ExistentialPredicate, Ty, TyCtxt};
8+
use rustc_session::declare_lint_pass;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
///
13+
/// Protects against unintended coercion of references to container types to `&dyn Any`
14+
/// when the container type dereferences to a `dyn Any` which could be reborrowed instead.
15+
///
16+
/// ### Why is this bad?
17+
///
18+
/// The intention is usually to get a reference to the `dyn Any` the value derefernces to,
19+
/// rather than getting a `&dyn Any` which is actually referring to the outer container type.
20+
///
21+
/// ### Example
22+
///
23+
/// Here we see that because `Box<dyn Any>` itself implements `Any`, `&Box<dyn Any>`
24+
/// gets coerced to an `&dyn Any` which refers to *the `Box` itself`*, rather than the
25+
/// inner `dyn Any`.
26+
/// ```no_run
27+
/// # use std::any::Any;
28+
/// let x: Box<dyn Any> = Box::new(0u32);
29+
/// let dyn_any_of_box: &dyn Any = &x;
30+
///
31+
/// // Fails as we have a &dyn Any to the Box, not the u32
32+
/// assert_eq!(dyn_any_of_box.downcast_ref::<u32>(), None);
33+
/// ```
34+
/// Use instead:
35+
/// ```no_run
36+
/// # use std::any::Any;
37+
/// let x: Box<dyn Any> = Box::new(0u32);
38+
/// let dyn_any_of_u32: &dyn Any = &*x;
39+
///
40+
/// // Succeeds since we have a &dyn Any to the inner u32!
41+
/// assert_eq!(dyn_any_of_u32.downcast_ref::<u32>(), Some(&0u32));
42+
/// ```
43+
#[clippy::version = "1.88.0"]
44+
pub COERCE_ANY_REF_TO_ANY,
45+
nursery,
46+
"coercing to `&dyn Any` when dereferencing could produce a `dyn Any` without coercion is usually not intended"
47+
}
48+
declare_lint_pass!(CoerceAnyRefToAny => [COERCE_ANY_REF_TO_ANY]);
49+
50+
impl<'tcx> LateLintPass<'tcx> for CoerceAnyRefToAny {
51+
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
52+
// If this expression has an effective type of `&dyn Any` ...
53+
{
54+
let coerced_ty = cx.typeck_results().expr_ty_adjusted(e);
55+
56+
let ty::Ref(_, coerced_ref_ty, _) = *coerced_ty.kind() else {
57+
return;
58+
};
59+
if !is_dyn_any(cx.tcx, coerced_ref_ty) {
60+
return;
61+
}
62+
}
63+
64+
let expr_ty = cx.typeck_results().expr_ty(e);
65+
let ty::Ref(_, expr_ref_ty, _) = *expr_ty.kind() else {
66+
return;
67+
};
68+
// ... but only due to coercion ...
69+
if is_dyn_any(cx.tcx, expr_ref_ty) {
70+
return;
71+
}
72+
// ... and it also *derefs* to `dyn Any` ...
73+
let Some((depth, target)) = clippy_utils::ty::deref_chain(cx, expr_ref_ty).enumerate().last() else {
74+
return;
75+
};
76+
if !is_dyn_any(cx.tcx, target) {
77+
return;
78+
}
79+
80+
// ... that's probably not intended.
81+
let (span, deref_count) = match e.kind {
82+
// If `e` was already an `&` expression, skip `*&` in the suggestion
83+
ExprKind::AddrOf(_, _, referent) => (referent.span, depth),
84+
_ => (e.span, depth + 1),
85+
};
86+
span_lint_and_sugg(
87+
cx,
88+
COERCE_ANY_REF_TO_ANY,
89+
e.span,
90+
format!("coercing `{expr_ty}` to `&dyn Any` rather than dereferencing to the `dyn Any` inside"),
91+
"consider dereferencing",
92+
format!("&{}{}", str::repeat("*", deref_count), snippet(cx, span, "x")),
93+
Applicability::MaybeIncorrect,
94+
);
95+
}
96+
}
97+
98+
fn is_dyn_any(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool {
99+
let ty::Dynamic(traits, ..) = ty.kind() else {
100+
return false;
101+
};
102+
traits.iter().any(|binder| {
103+
let Some(ExistentialPredicate::Trait(t)) = binder.no_bound_vars() else {
104+
return false;
105+
};
106+
tcx.is_diagnostic_item(sym::Any, t.def_id)
107+
})
108+
}

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
7777
crate::cfg_not_test::CFG_NOT_TEST_INFO,
7878
crate::checked_conversions::CHECKED_CONVERSIONS_INFO,
7979
crate::cloned_ref_to_slice_refs::CLONED_REF_TO_SLICE_REFS_INFO,
80+
crate::coerce_any_ref_to_any::COERCE_ANY_REF_TO_ANY_INFO,
8081
crate::cognitive_complexity::COGNITIVE_COMPLEXITY_INFO,
8182
crate::collapsible_if::COLLAPSIBLE_ELSE_IF_INFO,
8283
crate::collapsible_if::COLLAPSIBLE_IF_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ mod casts;
9595
mod cfg_not_test;
9696
mod checked_conversions;
9797
mod cloned_ref_to_slice_refs;
98+
mod coerce_any_ref_to_any;
9899
mod cognitive_complexity;
99100
mod collapsible_if;
100101
mod collection_is_never_read;
@@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
944945
store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
945946
store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix));
946947
store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
948+
store.register_late_pass(|_| Box::new(coerce_any_ref_to_any::CoerceAnyRefToAny));
947949
// add lints here, do not remove this comment, it's used in `new_lint`
948950
}

rust-toolchain.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
# begin autogenerated nightly
33
channel = "nightly-2025-05-01"
44
# end autogenerated nightly
5-
components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"]
5+
components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt", "rust-analyzer"]
66
profile = "minimal"

tests/ui/coerce_any_ref_to_any.fixed

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#![warn(clippy::coerce_any_ref_to_any)]
2+
3+
use std::any::Any;
4+
5+
fn main() {
6+
let x: Box<dyn Any> = Box::new(());
7+
let ref_x = &x;
8+
9+
f(&*x);
10+
//~^ coerce_any_ref_to_any
11+
12+
f(&**ref_x);
13+
//~^ coerce_any_ref_to_any
14+
15+
let _: &dyn Any = &*x;
16+
//~^ coerce_any_ref_to_any
17+
18+
f(&42);
19+
f(&Box::new(()));
20+
f(&**ref_x);
21+
f(&*x);
22+
let _: &dyn Any = &*x;
23+
}
24+
25+
fn f(_: &dyn Any) {}

tests/ui/coerce_any_ref_to_any.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#![warn(clippy::coerce_any_ref_to_any)]
2+
3+
use std::any::Any;
4+
5+
fn main() {
6+
let x: Box<dyn Any> = Box::new(());
7+
let ref_x = &x;
8+
9+
f(&x);
10+
//~^ coerce_any_ref_to_any
11+
12+
f(ref_x);
13+
//~^ coerce_any_ref_to_any
14+
15+
let _: &dyn Any = &x;
16+
//~^ coerce_any_ref_to_any
17+
18+
f(&42);
19+
f(&Box::new(()));
20+
f(&**ref_x);
21+
f(&*x);
22+
let _: &dyn Any = &*x;
23+
}
24+
25+
fn f(_: &dyn Any) {}

tests/ui/coerce_any_ref_to_any.stderr

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any` rather than dereferencing to the `dyn Any` inside
2+
--> tests/ui/coerce_any_ref_to_any.rs:9:7
3+
|
4+
LL | f(&x);
5+
| ^^ help: consider dereferencing: `&*x`
6+
|
7+
= note: `-D clippy::coerce-any-ref-to-any` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::coerce_any_ref_to_any)]`
9+
10+
error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any` rather than dereferencing to the `dyn Any` inside
11+
--> tests/ui/coerce_any_ref_to_any.rs:12:7
12+
|
13+
LL | f(ref_x);
14+
| ^^^^^ help: consider dereferencing: `&**ref_x`
15+
16+
error: coercing `&std::boxed::Box<dyn std::any::Any>` to `&dyn Any` rather than dereferencing to the `dyn Any` inside
17+
--> tests/ui/coerce_any_ref_to_any.rs:15:23
18+
|
19+
LL | let _: &dyn Any = &x;
20+
| ^^ help: consider dereferencing: `&*x`
21+
22+
error: aborting due to 3 previous errors
23+

0 commit comments

Comments
 (0)