src/tools/clippy/clippy_lints/src/types/mod.rs

   1 mod borrowed_box;
   2 mod box_collection;
   3 mod linked_list;
   4 mod option_option;
   5 mod rc_buffer;
   6 mod rc_mutex;
   7 mod redundant_allocation;
   8 mod type_complexity;
   9 mod utils;
  10 mod vec_box;
  11
  12 use rustc_hir as hir;
  13 use rustc_hir::intravisit::FnKind;
  14 use rustc_hir::{
  15     Body, FnDecl, FnRetTy, GenericArg, HirId, ImplItem, ImplItemKind, Item, ItemKind, Local, MutTy, QPath, TraitItem,
  16     TraitItemKind, TyKind,
  17 };
  18 use rustc_lint::{LateContext, LateLintPass};
  19 use rustc_session::{declare_tool_lint, impl_lint_pass};
  20 use rustc_span::source_map::Span;
  21
  22 declare_clippy_lint! {
  23     /// ### What it does
  24     /// Checks for use of `Box<T>` where T is a collection such as Vec anywhere in the code.
  25     /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information.
  26     ///
  27     /// ### Why is this bad?
  28     /// Collections already keeps their contents in a separate area on
  29     /// the heap. So if you `Box` them, you just add another level of indirection
  30     /// without any benefit whatsoever.
  31     ///
  32     /// ### Example
  33     /// ```rust,ignore
  34     /// struct X {
  35     ///     values: Box<Vec<Foo>>,
  36     /// }
  37     /// ```
  38     ///
  39     /// Better:
  40     ///
  41     /// ```rust,ignore
  42     /// struct X {
  43     ///     values: Vec<Foo>,
  44     /// }
  45     /// ```
  46     #[clippy::version = "1.57.0"]
  47     pub BOX_COLLECTION,
  48     perf,
  49     "usage of `Box<Vec<T>>`, vector elements are already on the heap"
  50 }
  51
  52 declare_clippy_lint! {
  53     /// ### What it does
  54     /// Checks for use of `Vec<Box<T>>` where T: Sized anywhere in the code.
  55     /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information.
  56     ///
  57     /// ### Why is this bad?
  58     /// `Vec` already keeps its contents in a separate area on
  59     /// the heap. So if you `Box` its contents, you just add another level of indirection.
  60     ///
  61     /// ### Known problems
  62     /// Vec<Box<T: Sized>> makes sense if T is a large type (see [#3530](https://github.com/rust-lang/rust-clippy/issues/3530),
  63     /// 1st comment).
  64     ///
  65     /// ### Example
  66     /// ```rust
  67     /// struct X {
  68     ///     values: Vec<Box<i32>>,
  69     /// }
  70     /// ```
  71     ///
  72     /// Better:
  73     ///
  74     /// ```rust
  75     /// struct X {
  76     ///     values: Vec<i32>,
  77     /// }
  78     /// ```
  79     #[clippy::version = "1.33.0"]
  80     pub VEC_BOX,
  81     complexity,
  82     "usage of `Vec<Box<T>>` where T: Sized, vector elements are already on the heap"
  83 }
  84
  85 declare_clippy_lint! {
  86     /// ### What it does
  87     /// Checks for use of `Option<Option<_>>` in function signatures and type
  88     /// definitions
  89     ///
  90     /// ### Why is this bad?
  91     /// `Option<_>` represents an optional value. `Option<Option<_>>`
  92     /// represents an optional optional value which is logically the same thing as an optional
  93     /// value but has an unneeded extra level of wrapping.
  94     ///
  95     /// If you have a case where `Some(Some(_))`, `Some(None)` and `None` are distinct cases,
  96     /// consider a custom `enum` instead, with clear names for each case.
  97     ///
  98     /// ### Example
  99     /// ```rust
 100     /// fn get_data() -> Option<Option<u32>> {
 101     ///     None
 102     /// }
 103     /// ```
 104     ///
 105     /// Better:
 106     ///
 107     /// ```rust
 108     /// pub enum Contents {
 109     ///     Data(Vec<u8>), // Was Some(Some(Vec<u8>))
 110     ///     NotYetFetched, // Was Some(None)
 111     ///     None,          // Was None
 112     /// }
 113     ///
 114     /// fn get_data() -> Contents {
 115     ///     Contents::None
 116     /// }
 117     /// ```
 118     #[clippy::version = "pre 1.29.0"]
 119     pub OPTION_OPTION,
 120     pedantic,
 121     "usage of `Option<Option<T>>`"
 122 }
 123
 124 declare_clippy_lint! {
 125     /// ### What it does
 126     /// Checks for usage of any `LinkedList`, suggesting to use a
 127     /// `Vec` or a `VecDeque` (formerly called `RingBuf`).
 128     ///
 129     /// ### Why is this bad?
 130     /// Gankro says:
 131     ///
 132     /// > The TL;DR of `LinkedList` is that it's built on a massive amount of
 133     /// pointers and indirection.
 134     /// > It wastes memory, it has terrible cache locality, and is all-around slow.
 135     /// `RingBuf`, while
 136     /// > "only" amortized for push/pop, should be faster in the general case for
 137     /// almost every possible
 138     /// > workload, and isn't even amortized at all if you can predict the capacity
 139     /// you need.
 140     /// >
 141     /// > `LinkedList`s are only really good if you're doing a lot of merging or
 142     /// splitting of lists.
 143     /// > This is because they can just mangle some pointers instead of actually
 144     /// copying the data. Even
 145     /// > if you're doing a lot of insertion in the middle of the list, `RingBuf`
 146     /// can still be better
 147     /// > because of how expensive it is to seek to the middle of a `LinkedList`.
 148     ///
 149     /// ### Known problems
 150     /// False positives – the instances where using a
 151     /// `LinkedList` makes sense are few and far between, but they can still happen.
 152     ///
 153     /// ### Example
 154     /// ```rust
 155     /// # use std::collections::LinkedList;
 156     /// let x: LinkedList<usize> = LinkedList::new();
 157     /// ```
 158     #[clippy::version = "pre 1.29.0"]
 159     pub LINKEDLIST,
 160     pedantic,
 161     "usage of LinkedList, usually a vector is faster, or a more specialized data structure like a `VecDeque`"
 162 }
 163
 164 declare_clippy_lint! {
 165     /// ### What it does
 166     /// Checks for use of `&Box<T>` anywhere in the code.
 167     /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information.
 168     ///
 169     /// ### Why is this bad?
 170     /// A `&Box<T>` parameter requires the function caller to box `T` first before passing it to a function.
 171     /// Using `&T` defines a concrete type for the parameter and generalizes the function, this would also
 172     /// auto-deref to `&T` at the function call site if passed a `&Box<T>`.
 173     ///
 174     /// ### Example
 175     /// ```rust,ignore
 176     /// fn foo(bar: &Box<T>) { ... }
 177     /// ```
 178     ///
 179     /// Better:
 180     ///
 181     /// ```rust,ignore
 182     /// fn foo(bar: &T) { ... }
 183     /// ```
 184     #[clippy::version = "pre 1.29.0"]
 185     pub BORROWED_BOX,
 186     complexity,
 187     "a borrow of a boxed type"
 188 }
 189
 190 declare_clippy_lint! {
 191     /// ### What it does
 192     /// Checks for use of redundant allocations anywhere in the code.
 193     ///
 194     /// ### Why is this bad?
 195     /// Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Arc<T>>`, `Rc<Box<T>>`, `Arc<&T>`, `Arc<Rc<T>>`,
 196     /// `Arc<Arc<T>>`, `Arc<Box<T>>`, `Box<&T>`, `Box<Rc<T>>`, `Box<Arc<T>>`, `Box<Box<T>>`, add an unnecessary level of indirection.
 197     ///
 198     /// ### Example
 199     /// ```rust
 200     /// # use std::rc::Rc;
 201     /// fn foo(bar: Rc<&usize>) {}
 202     /// ```
 203     ///
 204     /// Better:
 205     ///
 206     /// ```rust
 207     /// fn foo(bar: &usize) {}
 208     /// ```
 209     #[clippy::version = "1.44.0"]
 210     pub REDUNDANT_ALLOCATION,
 211     perf,
 212     "redundant allocation"
 213 }
 214
 215 declare_clippy_lint! {
 216     /// ### What it does
 217     /// Checks for `Rc<T>` and `Arc<T>` when `T` is a mutable buffer type such as `String` or `Vec`.
 218     ///
 219     /// ### Why is this bad?
 220     /// Expressions such as `Rc<String>` usually have no advantage over `Rc<str>`, since
 221     /// it is larger and involves an extra level of indirection, and doesn't implement `Borrow<str>`.
 222     ///
 223     /// While mutating a buffer type would still be possible with `Rc::get_mut()`, it only
 224     /// works if there are no additional references yet, which usually defeats the purpose of
 225     /// enclosing it in a shared ownership type. Instead, additionally wrapping the inner
 226     /// type with an interior mutable container (such as `RefCell` or `Mutex`) would normally
 227     /// be used.
 228     ///
 229     /// ### Known problems
 230     /// This pattern can be desirable to avoid the overhead of a `RefCell` or `Mutex` for
 231     /// cases where mutation only happens before there are any additional references.
 232     ///
 233     /// ### Example
 234     /// ```rust,ignore
 235     /// # use std::rc::Rc;
 236     /// fn foo(interned: Rc<String>) { ... }
 237     /// ```
 238     ///
 239     /// Better:
 240     ///
 241     /// ```rust,ignore
 242     /// fn foo(interned: Rc<str>) { ... }
 243     /// ```
 244     #[clippy::version = "1.48.0"]
 245     pub RC_BUFFER,
 246     restriction,
 247     "shared ownership of a buffer type"
 248 }
 249
 250 declare_clippy_lint! {
 251     /// ### What it does
 252     /// Checks for types used in structs, parameters and `let`
 253     /// declarations above a certain complexity threshold.
 254     ///
 255     /// ### Why is this bad?
 256     /// Too complex types make the code less readable. Consider
 257     /// using a `type` definition to simplify them.
 258     ///
 259     /// ### Example
 260     /// ```rust
 261     /// # use std::rc::Rc;
 262     /// struct Foo {
 263     ///     inner: Rc<Vec<Vec<Box<(u32, u32, u32, u32)>>>>,
 264     /// }
 265     /// ```
 266     #[clippy::version = "pre 1.29.0"]
 267     pub TYPE_COMPLEXITY,
 268     complexity,
 269     "usage of very complex types that might be better factored into `type` definitions"
 270 }
 271
 272 declare_clippy_lint! {
 273     /// ### What it does
 274     /// Checks for `Rc<Mutex<T>>`.
 275     ///
 276     /// ### Why is this bad?
 277     /// `Rc` is used in single thread and `Mutex` is used in multi thread.
 278     /// Consider using `Rc<RefCell<T>>` in single thread or `Arc<Mutex<T>>` in multi thread.
 279     ///
 280     /// ### Known problems
 281     /// Sometimes combining generic types can lead to the requirement that a
 282     /// type use Rc in conjunction with Mutex. We must consider those cases false positives, but
 283     /// alas they are quite hard to rule out. Luckily they are also rare.
 284     ///
 285     /// ### Example
 286     /// ```rust,ignore
 287     /// use std::rc::Rc;
 288     /// use std::sync::Mutex;
 289     /// fn foo(interned: Rc<Mutex<i32>>) { ... }
 290     /// ```
 291     ///
 292     /// Better:
 293     ///
 294     /// ```rust,ignore
 295     /// use std::rc::Rc;
 296     /// use std::cell::RefCell
 297     /// fn foo(interned: Rc<RefCell<i32>>) { ... }
 298     /// ```
 299     #[clippy::version = "1.55.0"]
 300     pub RC_MUTEX,
 301     restriction,
 302     "usage of `Rc<Mutex<T>>`"
 303 }
 304
 305 pub struct Types {
 306     vec_box_size_threshold: u64,
 307     type_complexity_threshold: u64,
 308     avoid_breaking_exported_api: bool,
 309 }
 310
 311 impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]);
 312
 313 impl<'tcx> LateLintPass<'tcx> for Types {
 314     fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) {
 315         let is_in_trait_impl =
 316             if let Some(hir::Node::Item(item)) = cx.tcx.hir().find_by_def_id(cx.tcx.hir().get_parent_item(id).def_id) {
 317                 matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }))
 318             } else {
 319                 false
 320             };
 321
 322         let is_exported = cx.effective_visibilities.is_exported(cx.tcx.hir().local_def_id(id));
 323
 324         self.check_fn_decl(
 325             cx,
 326             decl,
 327             CheckTyContext {
 328                 is_in_trait_impl,
 329                 is_exported,
 330                 ..CheckTyContext::default()
 331             },
 332         );
 333     }
 334
 335     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
 336         let is_exported = cx.effective_visibilities.is_exported(item.owner_id.def_id);
 337
 338         match item.kind {
 339             ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty(
 340                 cx,
 341                 ty,
 342                 CheckTyContext {
 343                     is_exported,
 344                     ..CheckTyContext::default()
 345                 },
 346             ),
 347             // functions, enums, structs, impls and traits are covered
 348             _ => (),
 349         }
 350     }
 351
 352     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
 353         match item.kind {
 354             ImplItemKind::Const(ty, _) => {
 355                 let is_in_trait_impl = if let Some(hir::Node::Item(item)) = cx
 356                     .tcx
 357                     .hir()
 358                     .find_by_def_id(cx.tcx.hir().get_parent_item(item.hir_id()).def_id)
 359                 {
 360                     matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }))
 361                 } else {
 362                     false
 363                 };
 364
 365                 self.check_ty(
 366                     cx,
 367                     ty,
 368                     CheckTyContext {
 369                         is_in_trait_impl,
 370                         ..CheckTyContext::default()
 371                     },
 372                 );
 373             },
 374             // Methods are covered by check_fn.
 375             // Type aliases are ignored because oftentimes it's impossible to
 376             // make type alias declaration in trait simpler, see #1013
 377             ImplItemKind::Fn(..) | ImplItemKind::Type(..) => (),
 378         }
 379     }
 380
 381     fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) {
 382         let is_exported = cx.effective_visibilities.is_exported(cx.tcx.hir().local_def_id(field.hir_id));
 383
 384         self.check_ty(
 385             cx,
 386             field.ty,
 387             CheckTyContext {
 388                 is_exported,
 389                 ..CheckTyContext::default()
 390             },
 391         );
 392     }
 393
 394     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &TraitItem<'_>) {
 395         let is_exported = cx.effective_visibilities.is_exported(item.owner_id.def_id);
 396
 397         let context = CheckTyContext {
 398             is_exported,
 399             ..CheckTyContext::default()
 400         };
 401
 402         match item.kind {
 403             TraitItemKind::Const(ty, _) | TraitItemKind::Type(_, Some(ty)) => {
 404                 self.check_ty(cx, ty, context);
 405             },
 406             TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, sig.decl, context),
 407             TraitItemKind::Type(..) => (),
 408         }
 409     }
 410
 411     fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
 412         if let Some(ty) = local.ty {
 413             self.check_ty(
 414                 cx,
 415                 ty,
 416                 CheckTyContext {
 417                     is_local: true,
 418                     ..CheckTyContext::default()
 419                 },
 420             );
 421         }
 422     }
 423 }
 424
 425 impl Types {
 426     pub fn new(vec_box_size_threshold: u64, type_complexity_threshold: u64, avoid_breaking_exported_api: bool) -> Self {
 427         Self {
 428             vec_box_size_threshold,
 429             type_complexity_threshold,
 430             avoid_breaking_exported_api,
 431         }
 432     }
 433
 434     fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, context: CheckTyContext) {
 435         // Ignore functions in trait implementations as they are usually forced by the trait definition.
 436         //
 437         // FIXME: ideally we would like to warn *if the complicated type can be simplified*, but it's hard
 438         // to check.
 439         if context.is_in_trait_impl {
 440             return;
 441         }
 442
 443         for input in decl.inputs {
 444             self.check_ty(cx, input, context);
 445         }
 446
 447         if let FnRetTy::Return(ty) = decl.output {
 448             self.check_ty(cx, ty, context);
 449         }
 450     }
 451
 452     /// Recursively check for `TypePass` lints in the given type. Stop at the first
 453     /// lint found.
 454     ///
 455     /// The parameter `is_local` distinguishes the context of the type.
 456     fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: CheckTyContext) {
 457         if hir_ty.span.from_expansion() {
 458             return;
 459         }
 460
 461         // Skip trait implementations; see issue #605.
 462         if context.is_in_trait_impl {
 463             return;
 464         }
 465
 466         if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) {
 467             return;
 468         }
 469
 470         match hir_ty.kind {
 471             TyKind::Path(ref qpath) if !context.is_local => {
 472                 let hir_id = hir_ty.hir_id;
 473                 let res = cx.qpath_res(qpath, hir_id);
 474                 if let Some(def_id) = res.opt_def_id() {
 475                     if self.is_type_change_allowed(context) {
 476                         // All lints that are being checked in this block are guarded by
 477                         // the `avoid_breaking_exported_api` configuration. When adding a
 478                         // new lint, please also add the name to the configuration documentation
 479                         // in `clippy_lints::utils::conf.rs`
 480
 481                         let mut triggered = false;
 482                         triggered |= box_collection::check(cx, hir_ty, qpath, def_id);
 483                         triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id);
 484                         triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id);
 485                         triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold);
 486                         triggered |= option_option::check(cx, hir_ty, qpath, def_id);
 487                         triggered |= linked_list::check(cx, hir_ty, def_id);
 488                         triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
 489
 490                         if triggered {
 491                             return;
 492                         }
 493                     }
 494                 }
 495                 match *qpath {
 496                     QPath::Resolved(Some(ty), p) => {
 497                         context.is_nested_call = true;
 498                         self.check_ty(cx, ty, context);
 499                         for ty in p.segments.iter().flat_map(|seg| {
 500                             seg.args
 501                                 .as_ref()
 502                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 503                                 .filter_map(|arg| match arg {
 504                                     GenericArg::Type(ty) => Some(ty),
 505                                     _ => None,
 506                                 })
 507                         }) {
 508                             self.check_ty(cx, ty, context);
 509                         }
 510                     },
 511                     QPath::Resolved(None, p) => {
 512                         context.is_nested_call = true;
 513                         for ty in p.segments.iter().flat_map(|seg| {
 514                             seg.args
 515                                 .as_ref()
 516                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 517                                 .filter_map(|arg| match arg {
 518                                     GenericArg::Type(ty) => Some(ty),
 519                                     _ => None,
 520                                 })
 521                         }) {
 522                             self.check_ty(cx, ty, context);
 523                         }
 524                     },
 525                     QPath::TypeRelative(ty, seg) => {
 526                         context.is_nested_call = true;
 527                         self.check_ty(cx, ty, context);
 528                         if let Some(params) = seg.args {
 529                             for ty in params.args.iter().filter_map(|arg| match arg {
 530                                 GenericArg::Type(ty) => Some(ty),
 531                                 _ => None,
 532                             }) {
 533                                 self.check_ty(cx, ty, context);
 534                             }
 535                         }
 536                     },
 537                     QPath::LangItem(..) => {},
 538                 }
 539             },
 540             TyKind::Rptr(lt, ref mut_ty) => {
 541                 context.is_nested_call = true;
 542                 if !borrowed_box::check(cx, hir_ty, lt, mut_ty) {
 543                     self.check_ty(cx, mut_ty.ty, context);
 544                 }
 545             },
 546             TyKind::Slice(ty) | TyKind::Array(ty, _) | TyKind::Ptr(MutTy { ty, .. }) => {
 547                 context.is_nested_call = true;
 548                 self.check_ty(cx, ty, context);
 549             },
 550             TyKind::Tup(tys) => {
 551                 context.is_nested_call = true;
 552                 for ty in tys {
 553                     self.check_ty(cx, ty, context);
 554                 }
 555             },
 556             _ => {},
 557         }
 558     }
 559
 560     /// This function checks if the type is allowed to change in the current context
 561     /// based on the `avoid_breaking_exported_api` configuration
 562     fn is_type_change_allowed(&self, context: CheckTyContext) -> bool {
 563         !(context.is_exported && self.avoid_breaking_exported_api)
 564     }
 565 }
 566
 567 #[allow(clippy::struct_excessive_bools)]
 568 #[derive(Clone, Copy, Default)]
 569 struct CheckTyContext {
 570     is_in_trait_impl: bool,
 571     /// `true` for types on local variables.
 572     is_local: bool,
 573     /// `true` for types that are part of the public API.
 574     is_exported: bool,
 575     is_nested_call: bool,
 576 }