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)) {
 317                 matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }))
 318             } else {
 319                 false
 320             };
 321
 322         let is_exported = cx.access_levels.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.access_levels.is_exported(item.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)) =
 356                     cx.tcx.hir().find_by_def_id(cx.tcx.hir().get_parent_item(item.hir_id()))
 357                 {
 358                     matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }))
 359                 } else {
 360                     false
 361                 };
 362
 363                 self.check_ty(
 364                     cx,
 365                     ty,
 366                     CheckTyContext {
 367                         is_in_trait_impl,
 368                         ..CheckTyContext::default()
 369                     },
 370                 );
 371             },
 372             // Methods are covered by check_fn.
 373             // Type aliases are ignored because oftentimes it's impossible to
 374             // make type alias declaration in trait simpler, see #1013
 375             ImplItemKind::Fn(..) | ImplItemKind::TyAlias(..) => (),
 376         }
 377     }
 378
 379     fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) {
 380         let is_exported = cx.access_levels.is_exported(cx.tcx.hir().local_def_id(field.hir_id));
 381
 382         self.check_ty(
 383             cx,
 384             field.ty,
 385             CheckTyContext {
 386                 is_exported,
 387                 ..CheckTyContext::default()
 388             },
 389         );
 390     }
 391
 392     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &TraitItem<'_>) {
 393         let is_exported = cx.access_levels.is_exported(item.def_id);
 394
 395         let context = CheckTyContext {
 396             is_exported,
 397             ..CheckTyContext::default()
 398         };
 399
 400         match item.kind {
 401             TraitItemKind::Const(ty, _) | TraitItemKind::Type(_, Some(ty)) => {
 402                 self.check_ty(cx, ty, context);
 403             },
 404             TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, sig.decl, context),
 405             TraitItemKind::Type(..) => (),
 406         }
 407     }
 408
 409     fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
 410         if let Some(ty) = local.ty {
 411             self.check_ty(
 412                 cx,
 413                 ty,
 414                 CheckTyContext {
 415                     is_local: true,
 416                     ..CheckTyContext::default()
 417                 },
 418             );
 419         }
 420     }
 421 }
 422
 423 impl Types {
 424     pub fn new(vec_box_size_threshold: u64, type_complexity_threshold: u64, avoid_breaking_exported_api: bool) -> Self {
 425         Self {
 426             vec_box_size_threshold,
 427             type_complexity_threshold,
 428             avoid_breaking_exported_api,
 429         }
 430     }
 431
 432     fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, context: CheckTyContext) {
 433         // Ignore functions in trait implementations as they are usually forced by the trait definition.
 434         //
 435         // FIXME: idially we would like to warn *if the compicated type can be simplified*, but it's hard to
 436         // check.
 437         if context.is_in_trait_impl {
 438             return;
 439         }
 440
 441         for input in decl.inputs {
 442             self.check_ty(cx, input, context);
 443         }
 444
 445         if let FnRetTy::Return(ty) = decl.output {
 446             self.check_ty(cx, ty, context);
 447         }
 448     }
 449
 450     /// Recursively check for `TypePass` lints in the given type. Stop at the first
 451     /// lint found.
 452     ///
 453     /// The parameter `is_local` distinguishes the context of the type.
 454     fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: CheckTyContext) {
 455         if hir_ty.span.from_expansion() {
 456             return;
 457         }
 458
 459         // Skip trait implementations; see issue #605.
 460         if context.is_in_trait_impl {
 461             return;
 462         }
 463
 464         if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) {
 465             return;
 466         }
 467
 468         match hir_ty.kind {
 469             TyKind::Path(ref qpath) if !context.is_local => {
 470                 let hir_id = hir_ty.hir_id;
 471                 let res = cx.qpath_res(qpath, hir_id);
 472                 if let Some(def_id) = res.opt_def_id() {
 473                     if self.is_type_change_allowed(context) {
 474                         // All lints that are being checked in this block are guarded by
 475                         // the `avoid_breaking_exported_api` configuration. When adding a
 476                         // new lint, please also add the name to the configuration documentation
 477                         // in `clippy_lints::utils::conf.rs`
 478
 479                         let mut triggered = false;
 480                         triggered |= box_collection::check(cx, hir_ty, qpath, def_id);
 481                         triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id);
 482                         triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id);
 483                         triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold);
 484                         triggered |= option_option::check(cx, hir_ty, qpath, def_id);
 485                         triggered |= linked_list::check(cx, hir_ty, def_id);
 486                         triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
 487
 488                         if triggered {
 489                             return;
 490                         }
 491                     }
 492                 }
 493                 match *qpath {
 494                     QPath::Resolved(Some(ty), p) => {
 495                         context.is_nested_call = true;
 496                         self.check_ty(cx, ty, context);
 497                         for ty in p.segments.iter().flat_map(|seg| {
 498                             seg.args
 499                                 .as_ref()
 500                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 501                                 .filter_map(|arg| match arg {
 502                                     GenericArg::Type(ty) => Some(ty),
 503                                     _ => None,
 504                                 })
 505                         }) {
 506                             self.check_ty(cx, ty, context);
 507                         }
 508                     },
 509                     QPath::Resolved(None, p) => {
 510                         context.is_nested_call = true;
 511                         for ty in p.segments.iter().flat_map(|seg| {
 512                             seg.args
 513                                 .as_ref()
 514                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 515                                 .filter_map(|arg| match arg {
 516                                     GenericArg::Type(ty) => Some(ty),
 517                                     _ => None,
 518                                 })
 519                         }) {
 520                             self.check_ty(cx, ty, context);
 521                         }
 522                     },
 523                     QPath::TypeRelative(ty, seg) => {
 524                         context.is_nested_call = true;
 525                         self.check_ty(cx, ty, context);
 526                         if let Some(params) = seg.args {
 527                             for ty in params.args.iter().filter_map(|arg| match arg {
 528                                 GenericArg::Type(ty) => Some(ty),
 529                                 _ => None,
 530                             }) {
 531                                 self.check_ty(cx, ty, context);
 532                             }
 533                         }
 534                     },
 535                     QPath::LangItem(..) => {},
 536                 }
 537             },
 538             TyKind::Rptr(ref lt, ref mut_ty) => {
 539                 context.is_nested_call = true;
 540                 if !borrowed_box::check(cx, hir_ty, lt, mut_ty) {
 541                     self.check_ty(cx, mut_ty.ty, context);
 542                 }
 543             },
 544             TyKind::Slice(ty) | TyKind::Array(ty, _) | TyKind::Ptr(MutTy { ty, .. }) => {
 545                 context.is_nested_call = true;
 546                 self.check_ty(cx, ty, context);
 547             },
 548             TyKind::Tup(tys) => {
 549                 context.is_nested_call = true;
 550                 for ty in tys {
 551                     self.check_ty(cx, ty, context);
 552                 }
 553             },
 554             _ => {},
 555         }
 556     }
 557
 558     /// This function checks if the type is allowed to change in the current context
 559     /// based on the `avoid_breaking_exported_api` configuration
 560     fn is_type_change_allowed(&self, context: CheckTyContext) -> bool {
 561         !(context.is_exported && self.avoid_breaking_exported_api)
 562     }
 563 }
 564
 565 #[allow(clippy::struct_excessive_bools)]
 566 #[derive(Clone, Copy, Default)]
 567 struct CheckTyContext {
 568     is_in_trait_impl: bool,
 569     /// `true` for types on local variables.
 570     is_local: bool,
 571     /// `true` for types that are part of the public API.
 572     is_exported: bool,
 573     is_nested_call: bool,
 574 }