src/tools/clippy/clippy_lints/src/derive.rs

   1 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then};
   2 use clippy_utils::paths;
   3 use clippy_utils::ty::{implements_trait, implements_trait_with_env, is_copy};
   4 use clippy_utils::{is_lint_allowed, match_def_path};
   5 use if_chain::if_chain;
   6 use rustc_errors::Applicability;
   7 use rustc_hir::def_id::DefId;
   8 use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, Visitor};
   9 use rustc_hir::{
  10     self as hir, BlockCheckMode, BodyId, Constness, Expr, ExprKind, FnDecl, Impl, Item, ItemKind, UnsafeSource,
  11     Unsafety,
  12 };
  13 use rustc_lint::{LateContext, LateLintPass};
  14 use rustc_middle::hir::nested_filter;
  15 use rustc_middle::traits::Reveal;
  16 use rustc_middle::ty::{
  17     self, Binder, BoundConstness, Clause, GenericArgKind, GenericParamDefKind, ImplPolarity, ParamEnv, PredicateKind,
  18     TraitPredicate, Ty, TyCtxt,
  19 };
  20 use rustc_session::{declare_lint_pass, declare_tool_lint};
  21 use rustc_span::def_id::LocalDefId;
  22 use rustc_span::source_map::Span;
  23 use rustc_span::sym;
  24
  25 declare_clippy_lint! {
  26     /// ### What it does
  27     /// Checks for deriving `Hash` but implementing `PartialEq`
  28     /// explicitly or vice versa.
  29     ///
  30     /// ### Why is this bad?
  31     /// The implementation of these traits must agree (for
  32     /// example for use with `HashMap`) so it’s probably a bad idea to use a
  33     /// default-generated `Hash` implementation with an explicitly defined
  34     /// `PartialEq`. In particular, the following must hold for any type:
  35     ///
  36     /// ```text
  37     /// k1 == k2 ⇒ hash(k1) == hash(k2)
  38     /// ```
  39     ///
  40     /// ### Example
  41     /// ```ignore
  42     /// #[derive(Hash)]
  43     /// struct Foo;
  44     ///
  45     /// impl PartialEq for Foo {
  46     ///     ...
  47     /// }
  48     /// ```
  49     #[clippy::version = "pre 1.29.0"]
  50     pub DERIVED_HASH_WITH_MANUAL_EQ,
  51     correctness,
  52     "deriving `Hash` but implementing `PartialEq` explicitly"
  53 }
  54
  55 declare_clippy_lint! {
  56     /// ### What it does
  57     /// Checks for deriving `Ord` but implementing `PartialOrd`
  58     /// explicitly or vice versa.
  59     ///
  60     /// ### Why is this bad?
  61     /// The implementation of these traits must agree (for
  62     /// example for use with `sort`) so it’s probably a bad idea to use a
  63     /// default-generated `Ord` implementation with an explicitly defined
  64     /// `PartialOrd`. In particular, the following must hold for any type
  65     /// implementing `Ord`:
  66     ///
  67     /// ```text
  68     /// k1.cmp(&k2) == k1.partial_cmp(&k2).unwrap()
  69     /// ```
  70     ///
  71     /// ### Example
  72     /// ```rust,ignore
  73     /// #[derive(Ord, PartialEq, Eq)]
  74     /// struct Foo;
  75     ///
  76     /// impl PartialOrd for Foo {
  77     ///     ...
  78     /// }
  79     /// ```
  80     /// Use instead:
  81     /// ```rust,ignore
  82     /// #[derive(PartialEq, Eq)]
  83     /// struct Foo;
  84     ///
  85     /// impl PartialOrd for Foo {
  86     ///     fn partial_cmp(&self, other: &Foo) -> Option<Ordering> {
  87     ///        Some(self.cmp(other))
  88     ///     }
  89     /// }
  90     ///
  91     /// impl Ord for Foo {
  92     ///     ...
  93     /// }
  94     /// ```
  95     /// or, if you don't need a custom ordering:
  96     /// ```rust,ignore
  97     /// #[derive(Ord, PartialOrd, PartialEq, Eq)]
  98     /// struct Foo;
  99     /// ```
 100     #[clippy::version = "1.47.0"]
 101     pub DERIVE_ORD_XOR_PARTIAL_ORD,
 102     correctness,
 103     "deriving `Ord` but implementing `PartialOrd` explicitly"
 104 }
 105
 106 declare_clippy_lint! {
 107     /// ### What it does
 108     /// Checks for explicit `Clone` implementations for `Copy`
 109     /// types.
 110     ///
 111     /// ### Why is this bad?
 112     /// To avoid surprising behavior, these traits should
 113     /// agree and the behavior of `Copy` cannot be overridden. In almost all
 114     /// situations a `Copy` type should have a `Clone` implementation that does
 115     /// nothing more than copy the object, which is what `#[derive(Copy, Clone)]`
 116     /// gets you.
 117     ///
 118     /// ### Example
 119     /// ```rust,ignore
 120     /// #[derive(Copy)]
 121     /// struct Foo;
 122     ///
 123     /// impl Clone for Foo {
 124     ///     // ..
 125     /// }
 126     /// ```
 127     #[clippy::version = "pre 1.29.0"]
 128     pub EXPL_IMPL_CLONE_ON_COPY,
 129     pedantic,
 130     "implementing `Clone` explicitly on `Copy` types"
 131 }
 132
 133 declare_clippy_lint! {
 134     /// ### What it does
 135     /// Checks for deriving `serde::Deserialize` on a type that
 136     /// has methods using `unsafe`.
 137     ///
 138     /// ### Why is this bad?
 139     /// Deriving `serde::Deserialize` will create a constructor
 140     /// that may violate invariants hold by another constructor.
 141     ///
 142     /// ### Example
 143     /// ```rust,ignore
 144     /// use serde::Deserialize;
 145     ///
 146     /// #[derive(Deserialize)]
 147     /// pub struct Foo {
 148     ///     // ..
 149     /// }
 150     ///
 151     /// impl Foo {
 152     ///     pub fn new() -> Self {
 153     ///         // setup here ..
 154     ///     }
 155     ///
 156     ///     pub unsafe fn parts() -> (&str, &str) {
 157     ///         // assumes invariants hold
 158     ///     }
 159     /// }
 160     /// ```
 161     #[clippy::version = "1.45.0"]
 162     pub UNSAFE_DERIVE_DESERIALIZE,
 163     pedantic,
 164     "deriving `serde::Deserialize` on a type that has methods using `unsafe`"
 165 }
 166
 167 declare_clippy_lint! {
 168     /// ### What it does
 169     /// Checks for types that derive `PartialEq` and could implement `Eq`.
 170     ///
 171     /// ### Why is this bad?
 172     /// If a type `T` derives `PartialEq` and all of its members implement `Eq`,
 173     /// then `T` can always implement `Eq`. Implementing `Eq` allows `T` to be used
 174     /// in APIs that require `Eq` types. It also allows structs containing `T` to derive
 175     /// `Eq` themselves.
 176     ///
 177     /// ### Example
 178     /// ```rust
 179     /// #[derive(PartialEq)]
 180     /// struct Foo {
 181     ///     i_am_eq: i32,
 182     ///     i_am_eq_too: Vec<String>,
 183     /// }
 184     /// ```
 185     /// Use instead:
 186     /// ```rust
 187     /// #[derive(PartialEq, Eq)]
 188     /// struct Foo {
 189     ///     i_am_eq: i32,
 190     ///     i_am_eq_too: Vec<String>,
 191     /// }
 192     /// ```
 193     #[clippy::version = "1.63.0"]
 194     pub DERIVE_PARTIAL_EQ_WITHOUT_EQ,
 195     nursery,
 196     "deriving `PartialEq` on a type that can implement `Eq`, without implementing `Eq`"
 197 }
 198
 199 declare_lint_pass!(Derive => [
 200     EXPL_IMPL_CLONE_ON_COPY,
 201     DERIVED_HASH_WITH_MANUAL_EQ,
 202     DERIVE_ORD_XOR_PARTIAL_ORD,
 203     UNSAFE_DERIVE_DESERIALIZE,
 204     DERIVE_PARTIAL_EQ_WITHOUT_EQ
 205 ]);
 206
 207 impl<'tcx> LateLintPass<'tcx> for Derive {
 208     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
 209         if let ItemKind::Impl(Impl {
 210             of_trait: Some(ref trait_ref),
 211             ..
 212         }) = item.kind
 213         {
 214             let ty = cx.tcx.type_of(item.owner_id);
 215             let is_automatically_derived = cx.tcx.has_attr(item.owner_id.to_def_id(), sym::automatically_derived);
 216
 217             check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived);
 218             check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived);
 219
 220             if is_automatically_derived {
 221                 check_unsafe_derive_deserialize(cx, item, trait_ref, ty);
 222                 check_partial_eq_without_eq(cx, item.span, trait_ref, ty);
 223             } else {
 224                 check_copy_clone(cx, item, trait_ref, ty);
 225             }
 226         }
 227     }
 228 }
 229
 230 /// Implementation of the `DERIVED_HASH_WITH_MANUAL_EQ` lint.
 231 fn check_hash_peq<'tcx>(
 232     cx: &LateContext<'tcx>,
 233     span: Span,
 234     trait_ref: &hir::TraitRef<'_>,
 235     ty: Ty<'tcx>,
 236     hash_is_automatically_derived: bool,
 237 ) {
 238     if_chain! {
 239         if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait();
 240         if let Some(def_id) = trait_ref.trait_def_id();
 241         if cx.tcx.is_diagnostic_item(sym::Hash, def_id);
 242         then {
 243             // Look for the PartialEq implementations for `ty`
 244             cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| {
 245                 let peq_is_automatically_derived = cx.tcx.has_attr(impl_id, sym::automatically_derived);
 246
 247                 if !hash_is_automatically_derived || peq_is_automatically_derived {
 248                     return;
 249                 }
 250
 251                 let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation");
 252
 253                 // Only care about `impl PartialEq<Foo> for Foo`
 254                 // For `impl PartialEq<B> for A, input_types is [A, B]
 255                 if trait_ref.subst_identity().substs.type_at(1) == ty {
 256                     span_lint_and_then(
 257                         cx,
 258                         DERIVED_HASH_WITH_MANUAL_EQ,
 259                         span,
 260                         "you are deriving `Hash` but have implemented `PartialEq` explicitly",
 261                         |diag| {
 262                             if let Some(local_def_id) = impl_id.as_local() {
 263                                 let hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_def_id);
 264                                 diag.span_note(
 265                                     cx.tcx.hir().span(hir_id),
 266                                     "`PartialEq` implemented here"
 267                                 );
 268                             }
 269                         }
 270                     );
 271                 }
 272             });
 273         }
 274     }
 275 }
 276
 277 /// Implementation of the `DERIVE_ORD_XOR_PARTIAL_ORD` lint.
 278 fn check_ord_partial_ord<'tcx>(
 279     cx: &LateContext<'tcx>,
 280     span: Span,
 281     trait_ref: &hir::TraitRef<'_>,
 282     ty: Ty<'tcx>,
 283     ord_is_automatically_derived: bool,
 284 ) {
 285     if_chain! {
 286         if let Some(ord_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Ord);
 287         if let Some(partial_ord_trait_def_id) = cx.tcx.lang_items().partial_ord_trait();
 288         if let Some(def_id) = &trait_ref.trait_def_id();
 289         if *def_id == ord_trait_def_id;
 290         then {
 291             // Look for the PartialOrd implementations for `ty`
 292             cx.tcx.for_each_relevant_impl(partial_ord_trait_def_id, ty, |impl_id| {
 293                 let partial_ord_is_automatically_derived = cx.tcx.has_attr(impl_id, sym::automatically_derived);
 294
 295                 if partial_ord_is_automatically_derived == ord_is_automatically_derived {
 296                     return;
 297                 }
 298
 299                 let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation");
 300
 301                 // Only care about `impl PartialOrd<Foo> for Foo`
 302                 // For `impl PartialOrd<B> for A, input_types is [A, B]
 303                 if trait_ref.subst_identity().substs.type_at(1) == ty {
 304                     let mess = if partial_ord_is_automatically_derived {
 305                         "you are implementing `Ord` explicitly but have derived `PartialOrd`"
 306                     } else {
 307                         "you are deriving `Ord` but have implemented `PartialOrd` explicitly"
 308                     };
 309
 310                     span_lint_and_then(
 311                         cx,
 312                         DERIVE_ORD_XOR_PARTIAL_ORD,
 313                         span,
 314                         mess,
 315                         |diag| {
 316                             if let Some(local_def_id) = impl_id.as_local() {
 317                                 let hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_def_id);
 318                                 diag.span_note(
 319                                     cx.tcx.hir().span(hir_id),
 320                                     "`PartialOrd` implemented here"
 321                                 );
 322                             }
 323                         }
 324                     );
 325                 }
 326             });
 327         }
 328     }
 329 }
 330
 331 /// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
 332 fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) {
 333     let clone_id = match cx.tcx.lang_items().clone_trait() {
 334         Some(id) if trait_ref.trait_def_id() == Some(id) => id,
 335         _ => return,
 336     };
 337     let Some(copy_id) = cx.tcx.lang_items().copy_trait() else { return };
 338     let (ty_adt, ty_subs) = match *ty.kind() {
 339         // Unions can't derive clone.
 340         ty::Adt(adt, subs) if !adt.is_union() => (adt, subs),
 341         _ => return,
 342     };
 343     // If the current self type doesn't implement Copy (due to generic constraints), search to see if
 344     // there's a Copy impl for any instance of the adt.
 345     if !is_copy(cx, ty) {
 346         if ty_subs.non_erasable_generics().next().is_some() {
 347             let has_copy_impl = cx.tcx.all_local_trait_impls(()).get(&copy_id).map_or(false, |impls| {
 348                 impls
 349                     .iter()
 350                     .any(|&id| matches!(cx.tcx.type_of(id).kind(), ty::Adt(adt, _) if ty_adt.did() == adt.did()))
 351             });
 352             if !has_copy_impl {
 353                 return;
 354             }
 355         } else {
 356             return;
 357         }
 358     }
 359     // Derive constrains all generic types to requiring Clone. Check if any type is not constrained for
 360     // this impl.
 361     if ty_subs.types().any(|ty| !implements_trait(cx, ty, clone_id, &[])) {
 362         return;
 363     }
 364     // `#[repr(packed)]` structs with type/const parameters can't derive `Clone`.
 365     // https://github.com/rust-lang/rust-clippy/issues/10188
 366     if ty_adt.repr().packed()
 367         && ty_subs
 368             .iter()
 369             .any(|arg| matches!(arg.unpack(), GenericArgKind::Type(_) | GenericArgKind::Const(_)))
 370     {
 371         return;
 372     }
 373
 374     span_lint_and_note(
 375         cx,
 376         EXPL_IMPL_CLONE_ON_COPY,
 377         item.span,
 378         "you are implementing `Clone` explicitly on a `Copy` type",
 379         Some(item.span),
 380         "consider deriving `Clone` or removing `Copy`",
 381     );
 382 }
 383
 384 /// Implementation of the `UNSAFE_DERIVE_DESERIALIZE` lint.
 385 fn check_unsafe_derive_deserialize<'tcx>(
 386     cx: &LateContext<'tcx>,
 387     item: &Item<'_>,
 388     trait_ref: &hir::TraitRef<'_>,
 389     ty: Ty<'tcx>,
 390 ) {
 391     fn has_unsafe<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool {
 392         let mut visitor = UnsafeVisitor { cx, has_unsafe: false };
 393         walk_item(&mut visitor, item);
 394         visitor.has_unsafe
 395     }
 396
 397     if_chain! {
 398         if let Some(trait_def_id) = trait_ref.trait_def_id();
 399         if match_def_path(cx, trait_def_id, &paths::SERDE_DESERIALIZE);
 400         if let ty::Adt(def, _) = ty.kind();
 401         if let Some(local_def_id) = def.did().as_local();
 402         let adt_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_def_id);
 403         if !is_lint_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id);
 404         if cx.tcx.inherent_impls(def.did())
 405             .iter()
 406             .map(|imp_did| cx.tcx.hir().expect_item(imp_did.expect_local()))
 407             .any(|imp| has_unsafe(cx, imp));
 408         then {
 409             span_lint_and_help(
 410                 cx,
 411                 UNSAFE_DERIVE_DESERIALIZE,
 412                 item.span,
 413                 "you are deriving `serde::Deserialize` on a type that has methods using `unsafe`",
 414                 None,
 415                 "consider implementing `serde::Deserialize` manually. See https://serde.rs/impl-deserialize.html"
 416             );
 417         }
 418     }
 419 }
 420
 421 struct UnsafeVisitor<'a, 'tcx> {
 422     cx: &'a LateContext<'tcx>,
 423     has_unsafe: bool,
 424 }
 425
 426 impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> {
 427     type NestedFilter = nested_filter::All;
 428
 429     fn visit_fn(&mut self, kind: FnKind<'tcx>, decl: &'tcx FnDecl<'_>, body_id: BodyId, _: Span, id: LocalDefId) {
 430         if self.has_unsafe {
 431             return;
 432         }
 433
 434         if_chain! {
 435             if let Some(header) = kind.header();
 436             if header.unsafety == Unsafety::Unsafe;
 437             then {
 438                 self.has_unsafe = true;
 439             }
 440         }
 441
 442         walk_fn(self, kind, decl, body_id, id);
 443     }
 444
 445     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
 446         if self.has_unsafe {
 447             return;
 448         }
 449
 450         if let ExprKind::Block(block, _) = expr.kind {
 451             if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) {
 452                 self.has_unsafe = true;
 453             }
 454         }
 455
 456         walk_expr(self, expr);
 457     }
 458
 459     fn nested_visit_map(&mut self) -> Self::Map {
 460         self.cx.tcx.hir()
 461     }
 462 }
 463
 464 /// Implementation of the `DERIVE_PARTIAL_EQ_WITHOUT_EQ` lint.
 465 fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) {
 466     if_chain! {
 467         if let ty::Adt(adt, substs) = ty.kind();
 468         if cx.tcx.visibility(adt.did()).is_public();
 469         if let Some(eq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Eq);
 470         if let Some(def_id) = trait_ref.trait_def_id();
 471         if cx.tcx.is_diagnostic_item(sym::PartialEq, def_id);
 472         let param_env = param_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id);
 473         if !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, []);
 474         // If all of our fields implement `Eq`, we can implement `Eq` too
 475         if adt
 476             .all_fields()
 477             .map(|f| f.ty(cx.tcx, substs))
 478             .all(|ty| implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, []));
 479         then {
 480             span_lint_and_sugg(
 481                 cx,
 482                 DERIVE_PARTIAL_EQ_WITHOUT_EQ,
 483                 span.ctxt().outer_expn_data().call_site,
 484                 "you are deriving `PartialEq` and can implement `Eq`",
 485                 "consider deriving `Eq` as well",
 486                 "PartialEq, Eq".to_string(),
 487                 Applicability::MachineApplicable,
 488             )
 489         }
 490     }
 491 }
 492
 493 /// Creates the `ParamEnv` used for the give type's derived `Eq` impl.
 494 fn param_env_for_derived_eq(tcx: TyCtxt<'_>, did: DefId, eq_trait_id: DefId) -> ParamEnv<'_> {
 495     // Initial map from generic index to param def.
 496     // Vec<(param_def, needs_eq)>
 497     let mut params = tcx
 498         .generics_of(did)
 499         .params
 500         .iter()
 501         .map(|p| (p, matches!(p.kind, GenericParamDefKind::Type { .. })))
 502         .collect::<Vec<_>>();
 503
 504     let ty_predicates = tcx.predicates_of(did).predicates;
 505     for (p, _) in ty_predicates {
 506         if let PredicateKind::Clause(Clause::Trait(p)) = p.kind().skip_binder()
 507             && p.trait_ref.def_id == eq_trait_id
 508             && let ty::Param(self_ty) = p.trait_ref.self_ty().kind()
 509             && p.constness == BoundConstness::NotConst
 510         {
 511             // Flag types which already have an `Eq` bound.
 512             params[self_ty.index as usize].1 = false;
 513         }
 514     }
 515
 516     ParamEnv::new(
 517         tcx.mk_predicates(ty_predicates.iter().map(|&(p, _)| p).chain(
 518             params.iter().filter(|&&(_, needs_eq)| needs_eq).map(|&(param, _)| {
 519                 tcx.mk_predicate(Binder::dummy(PredicateKind::Clause(Clause::Trait(TraitPredicate {
 520                     trait_ref: tcx.mk_trait_ref(eq_trait_id, [tcx.mk_param_from_def(param)]),
 521                     constness: BoundConstness::NotConst,
 522                     polarity: ImplPolarity::Positive,
 523                 }))))
 524             }),
 525         )),
 526         Reveal::UserFacing,
 527         Constness::NotConst,
 528     )
 529 }