diff --git a/clippy_lints/src/methods/type_id_on_box.rs b/clippy_lints/src/methods/type_id_on_box.rs index 4917936a9322..cec5d3b2b6dd 100644 --- a/clippy_lints/src/methods/type_id_on_box.rs +++ b/clippy_lints/src/methods/type_id_on_box.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use crate::methods::TYPE_ID_ON_BOX; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; @@ -5,17 +7,37 @@ use rustc_errors::Applicability; use rustc_hir::Expr; use rustc_lint::LateContext; use rustc_middle::ty::adjustment::{Adjust, Adjustment}; +use rustc_middle::ty::print::with_forced_trimmed_paths; use rustc_middle::ty::{self, ExistentialPredicate, Ty}; use rustc_span::{sym, Span}; -fn is_dyn_any(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { +/// Checks if a [`Ty`] is a `dyn Any` or a `dyn Trait` where `Trait: Any` +/// and returns the name of the trait object. +fn is_dyn_any(cx: &LateContext<'_>, ty: Ty<'_>) -> Option> { if let ty::Dynamic(preds, ..) = ty.kind() { - preds.iter().any(|p| match p.skip_binder() { - ExistentialPredicate::Trait(tr) => cx.tcx.is_diagnostic_item(sym::Any, tr.def_id), - _ => false, + preds.iter().find_map(|p| match p.skip_binder() { + ExistentialPredicate::Trait(tr) => { + if cx.tcx.is_diagnostic_item(sym::Any, tr.def_id) { + Some(Cow::Borrowed("Any")) + } else if cx + .tcx + .super_predicates_of(tr.def_id) + .predicates + .iter() + .any(|(clause, _)| { + matches!(clause.kind().skip_binder(), ty::ClauseKind::Trait(super_tr) + if cx.tcx.is_diagnostic_item(sym::Any, super_tr.def_id())) + }) + { + Some(Cow::Owned(with_forced_trimmed_paths!(cx.tcx.def_path_str(tr.def_id)))) + } else { + None + } + }, + _ => None, }) } else { - false + None } } @@ -26,13 +48,13 @@ pub(super) fn check(cx: &LateContext<'_>, receiver: &Expr<'_>, call_span: Span) && let ty::Ref(_, ty, _) = recv_ty.kind() && let ty::Adt(adt, args) = ty.kind() && adt.is_box() - && is_dyn_any(cx, args.type_at(0)) + && let Some(trait_path) = is_dyn_any(cx, args.type_at(0)) { span_lint_and_then( cx, TYPE_ID_ON_BOX, call_span, - "calling `.type_id()` on a `Box`", + &format!("calling `.type_id()` on `Box`"), |diag| { let derefs = recv_adjusts .iter() @@ -43,13 +65,13 @@ pub(super) fn check(cx: &LateContext<'_>, receiver: &Expr<'_>, call_span: Span) sugg += &snippet(cx, receiver.span, ""); diag.note( - "this returns the type id of the literal type `Box` instead of the \ + "this returns the type id of the literal type `Box<_>` instead of the \ type id of the boxed value, which is most likely not what you want", ) - .note( - "if this is intentional, use `TypeId::of::>()` instead, \ - which makes it more clear", - ) + .note(format!( + "if this is intentional, use `TypeId::of::>()` instead, \ + which makes it more clear" + )) .span_suggestion( receiver.span, "consider dereferencing first", diff --git a/tests/ui/type_id_on_box.fixed b/tests/ui/type_id_on_box.fixed index 538c38b70e6b..bdc45a93e714 100644 --- a/tests/ui/type_id_on_box.fixed +++ b/tests/ui/type_id_on_box.fixed @@ -19,6 +19,21 @@ fn existential() -> impl Any { Box::new(1) as Box } +trait AnySubTrait: Any {} +impl AnySubTrait for T {} + +// `Any` is an indirect supertrait +trait AnySubSubTrait: AnySubTrait {} +impl AnySubSubTrait for T {} + +// This trait mentions `Any` in its predicates, but it is not a subtrait of `Any`. +trait NormalTrait +where + i32: Any, +{ +} +impl NormalTrait for T {} + fn main() { let any_box: Box = Box::new(0usize); let _ = (*any_box).type_id(); @@ -35,4 +50,13 @@ fn main() { let b = BadBox(Box::new(0usize)); let _ = b.type_id(); // Don't lint. This is a call to `::type_id`. Not `std::boxed::Box`! + + let b: Box = Box::new(1); + let _ = (*b).type_id(); // Lint if calling `type_id` on a `dyn Trait` where `Trait: Any` + + let b: Box = Box::new(1); + let _ = b.type_id(); // Known FN - Any is not an "immediate" supertrait + + let b: Box = Box::new(1); + let _ = b.type_id(); // `NormalTrait` does not have `Any` as its supertrait (even though it mentions it in `i32: Any`) } diff --git a/tests/ui/type_id_on_box.rs b/tests/ui/type_id_on_box.rs index f224d273bc23..e087d9989d89 100644 --- a/tests/ui/type_id_on_box.rs +++ b/tests/ui/type_id_on_box.rs @@ -19,6 +19,21 @@ fn existential() -> impl Any { Box::new(1) as Box } +trait AnySubTrait: Any {} +impl AnySubTrait for T {} + +// `Any` is an indirect supertrait +trait AnySubSubTrait: AnySubTrait {} +impl AnySubSubTrait for T {} + +// This trait mentions `Any` in its predicates, but it is not a subtrait of `Any`. +trait NormalTrait +where + i32: Any, +{ +} +impl NormalTrait for T {} + fn main() { let any_box: Box = Box::new(0usize); let _ = any_box.type_id(); @@ -35,4 +50,13 @@ fn main() { let b = BadBox(Box::new(0usize)); let _ = b.type_id(); // Don't lint. This is a call to `::type_id`. Not `std::boxed::Box`! + + let b: Box = Box::new(1); + let _ = b.type_id(); // Lint if calling `type_id` on a `dyn Trait` where `Trait: Any` + + let b: Box = Box::new(1); + let _ = b.type_id(); // Known FN - Any is not an "immediate" supertrait + + let b: Box = Box::new(1); + let _ = b.type_id(); // `NormalTrait` does not have `Any` as its supertrait (even though it mentions it in `i32: Any`) } diff --git a/tests/ui/type_id_on_box.stderr b/tests/ui/type_id_on_box.stderr index 0fce6a37c004..8edecc47c746 100644 --- a/tests/ui/type_id_on_box.stderr +++ b/tests/ui/type_id_on_box.stderr @@ -1,37 +1,48 @@ -error: calling `.type_id()` on a `Box` - --> tests/ui/type_id_on_box.rs:24:13 +error: calling `.type_id()` on `Box` + --> tests/ui/type_id_on_box.rs:39:13 | LL | let _ = any_box.type_id(); | -------^^^^^^^^^^ | | | help: consider dereferencing first: `(*any_box)` | - = note: this returns the type id of the literal type `Box` instead of the type id of the boxed value, which is most likely not what you want + = note: this returns the type id of the literal type `Box<_>` instead of the type id of the boxed value, which is most likely not what you want = note: if this is intentional, use `TypeId::of::>()` instead, which makes it more clear = note: `-D clippy::type-id-on-box` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::type_id_on_box)]` -error: calling `.type_id()` on a `Box` - --> tests/ui/type_id_on_box.rs:28:13 +error: calling `.type_id()` on `Box` + --> tests/ui/type_id_on_box.rs:43:13 | LL | let _ = any_box.type_id(); // 2 derefs are needed here to get to the `dyn Any` | -------^^^^^^^^^^ | | | help: consider dereferencing first: `(**any_box)` | - = note: this returns the type id of the literal type `Box` instead of the type id of the boxed value, which is most likely not what you want + = note: this returns the type id of the literal type `Box<_>` instead of the type id of the boxed value, which is most likely not what you want = note: if this is intentional, use `TypeId::of::>()` instead, which makes it more clear -error: calling `.type_id()` on a `Box` - --> tests/ui/type_id_on_box.rs:34:13 +error: calling `.type_id()` on `Box` + --> tests/ui/type_id_on_box.rs:49:13 | LL | let _ = b.type_id(); | -^^^^^^^^^^ | | | help: consider dereferencing first: `(*b)` | - = note: this returns the type id of the literal type `Box` instead of the type id of the boxed value, which is most likely not what you want + = note: this returns the type id of the literal type `Box<_>` instead of the type id of the boxed value, which is most likely not what you want = note: if this is intentional, use `TypeId::of::>()` instead, which makes it more clear -error: aborting due to 3 previous errors +error: calling `.type_id()` on `Box` + --> tests/ui/type_id_on_box.rs:55:13 + | +LL | let _ = b.type_id(); // Lint if calling `type_id` on a `dyn Trait` where `Trait: Any` + | -^^^^^^^^^^ + | | + | help: consider dereferencing first: `(*b)` + | + = note: this returns the type id of the literal type `Box<_>` instead of the type id of the boxed value, which is most likely not what you want + = note: if this is intentional, use `TypeId::of::>()` instead, which makes it more clear + +error: aborting due to 4 previous errors