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     /// Any `&Box<T>` can also be a `&T`, which is more
 171     /// general.
 172     ///
 173     /// ### Example
 174     /// ```rust,ignore
 175     /// fn foo(bar: &Box<T>) { ... }
 176     /// ```
 177     ///
 178     /// Better:
 179     ///
 180     /// ```rust,ignore
 181     /// fn foo(bar: &T) { ... }
 182     /// ```
 183     #[clippy::version = "pre 1.29.0"]
 184     pub BORROWED_BOX,
 185     complexity,
 186     "a borrow of a boxed type"
 187 }
 188
 189 declare_clippy_lint! {
 190     /// ### What it does
 191     /// Checks for use of redundant allocations anywhere in the code.
 192     ///
 193     /// ### Why is this bad?
 194     /// Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Arc<T>>`, `Rc<Box<T>>`, `Arc<&T>`, `Arc<Rc<T>>`,
 195     /// `Arc<Arc<T>>`, `Arc<Box<T>>`, `Box<&T>`, `Box<Rc<T>>`, `Box<Arc<T>>`, `Box<Box<T>>`, add an unnecessary level of indirection.
 196     ///
 197     /// ### Example
 198     /// ```rust
 199     /// # use std::rc::Rc;
 200     /// fn foo(bar: Rc<&usize>) {}
 201     /// ```
 202     ///
 203     /// Better:
 204     ///
 205     /// ```rust
 206     /// fn foo(bar: &usize) {}
 207     /// ```
 208     #[clippy::version = "1.44.0"]
 209     pub REDUNDANT_ALLOCATION,
 210     perf,
 211     "redundant allocation"
 212 }
 213
 214 declare_clippy_lint! {
 215     /// ### What it does
 216     /// Checks for `Rc<T>` and `Arc<T>` when `T` is a mutable buffer type such as `String` or `Vec`.
 217     ///
 218     /// ### Why is this bad?
 219     /// Expressions such as `Rc<String>` usually have no advantage over `Rc<str>`, since
 220     /// it is larger and involves an extra level of indirection, and doesn't implement `Borrow<str>`.
 221     ///
 222     /// While mutating a buffer type would still be possible with `Rc::get_mut()`, it only
 223     /// works if there are no additional references yet, which usually defeats the purpose of
 224     /// enclosing it in a shared ownership type. Instead, additionally wrapping the inner
 225     /// type with an interior mutable container (such as `RefCell` or `Mutex`) would normally
 226     /// be used.
 227     ///
 228     /// ### Known problems
 229     /// This pattern can be desirable to avoid the overhead of a `RefCell` or `Mutex` for
 230     /// cases where mutation only happens before there are any additional references.
 231     ///
 232     /// ### Example
 233     /// ```rust,ignore
 234     /// # use std::rc::Rc;
 235     /// fn foo(interned: Rc<String>) { ... }
 236     /// ```
 237     ///
 238     /// Better:
 239     ///
 240     /// ```rust,ignore
 241     /// fn foo(interned: Rc<str>) { ... }
 242     /// ```
 243     #[clippy::version = "1.48.0"]
 244     pub RC_BUFFER,
 245     restriction,
 246     "shared ownership of a buffer type"
 247 }
 248
 249 declare_clippy_lint! {
 250     /// ### What it does
 251     /// Checks for types used in structs, parameters and `let`
 252     /// declarations above a certain complexity threshold.
 253     ///
 254     /// ### Why is this bad?
 255     /// Too complex types make the code less readable. Consider
 256     /// using a `type` definition to simplify them.
 257     ///
 258     /// ### Example
 259     /// ```rust
 260     /// # use std::rc::Rc;
 261     /// struct Foo {
 262     ///     inner: Rc<Vec<Vec<Box<(u32, u32, u32, u32)>>>>,
 263     /// }
 264     /// ```
 265     #[clippy::version = "pre 1.29.0"]
 266     pub TYPE_COMPLEXITY,
 267     complexity,
 268     "usage of very complex types that might be better factored into `type` definitions"
 269 }
 270
 271 declare_clippy_lint! {
 272     /// ### What it does
 273     /// Checks for `Rc<Mutex<T>>`.
 274     ///
 275     /// ### Why is this bad?
 276     /// `Rc` is used in single thread and `Mutex` is used in multi thread.
 277     /// Consider using `Rc<RefCell<T>>` in single thread or `Arc<Mutex<T>>` in multi thread.
 278     ///
 279     /// ### Known problems
 280     /// Sometimes combining generic types can lead to the requirement that a
 281     /// type use Rc in conjunction with Mutex. We must consider those cases false positives, but
 282     /// alas they are quite hard to rule out. Luckily they are also rare.
 283     ///
 284     /// ### Example
 285     /// ```rust,ignore
 286     /// use std::rc::Rc;
 287     /// use std::sync::Mutex;
 288     /// fn foo(interned: Rc<Mutex<i32>>) { ... }
 289     /// ```
 290     ///
 291     /// Better:
 292     ///
 293     /// ```rust,ignore
 294     /// use std::rc::Rc;
 295     /// use std::cell::RefCell
 296     /// fn foo(interned: Rc<RefCell<i32>>) { ... }
 297     /// ```
 298     #[clippy::version = "1.55.0"]
 299     pub RC_MUTEX,
 300     restriction,
 301     "usage of `Rc<Mutex<T>>`"
 302 }
 303
 304 pub struct Types {
 305     vec_box_size_threshold: u64,
 306     type_complexity_threshold: u64,
 307     avoid_breaking_exported_api: bool,
 308 }
 309
 310 impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]);
 311
 312 impl<'tcx> LateLintPass<'tcx> for Types {
 313     fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) {
 314         let is_in_trait_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_item(id))
 315         {
 316             matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }))
 317         } else {
 318             false
 319         };
 320
 321         let is_exported = cx.access_levels.is_exported(cx.tcx.hir().local_def_id(id));
 322
 323         self.check_fn_decl(
 324             cx,
 325             decl,
 326             CheckTyContext {
 327                 is_in_trait_impl,
 328                 is_exported,
 329                 ..CheckTyContext::default()
 330             },
 331         );
 332     }
 333
 334     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
 335         let is_exported = cx.access_levels.is_exported(item.def_id);
 336
 337         match item.kind {
 338             ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty(
 339                 cx,
 340                 ty,
 341                 CheckTyContext {
 342                     is_exported,
 343                     ..CheckTyContext::default()
 344                 },
 345             ),
 346             // functions, enums, structs, impls and traits are covered
 347             _ => (),
 348         }
 349     }
 350
 351     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
 352         match item.kind {
 353             ImplItemKind::Const(ty, _) | ImplItemKind::TyAlias(ty) => self.check_ty(
 354                 cx,
 355                 ty,
 356                 CheckTyContext {
 357                     is_in_trait_impl: true,
 358                     ..CheckTyContext::default()
 359                 },
 360             ),
 361             // methods are covered by check_fn
 362             ImplItemKind::Fn(..) => (),
 363         }
 364     }
 365
 366     fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) {
 367         let is_exported = cx.access_levels.is_exported(cx.tcx.hir().local_def_id(field.hir_id));
 368
 369         self.check_ty(
 370             cx,
 371             field.ty,
 372             CheckTyContext {
 373                 is_exported,
 374                 ..CheckTyContext::default()
 375             },
 376         );
 377     }
 378
 379     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &TraitItem<'_>) {
 380         let is_exported = cx.access_levels.is_exported(item.def_id);
 381
 382         let context = CheckTyContext {
 383             is_exported,
 384             ..CheckTyContext::default()
 385         };
 386
 387         match item.kind {
 388             TraitItemKind::Const(ty, _) | TraitItemKind::Type(_, Some(ty)) => {
 389                 self.check_ty(cx, ty, context);
 390             },
 391             TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, sig.decl, context),
 392             TraitItemKind::Type(..) => (),
 393         }
 394     }
 395
 396     fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
 397         if let Some(ty) = local.ty {
 398             self.check_ty(
 399                 cx,
 400                 ty,
 401                 CheckTyContext {
 402                     is_local: true,
 403                     ..CheckTyContext::default()
 404                 },
 405             );
 406         }
 407     }
 408 }
 409
 410 impl Types {
 411     pub fn new(vec_box_size_threshold: u64, type_complexity_threshold: u64, avoid_breaking_exported_api: bool) -> Self {
 412         Self {
 413             vec_box_size_threshold,
 414             type_complexity_threshold,
 415             avoid_breaking_exported_api,
 416         }
 417     }
 418
 419     fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, context: CheckTyContext) {
 420         for input in decl.inputs {
 421             self.check_ty(cx, input, context);
 422         }
 423
 424         if let FnRetTy::Return(ty) = decl.output {
 425             self.check_ty(cx, ty, context);
 426         }
 427     }
 428
 429     /// Recursively check for `TypePass` lints in the given type. Stop at the first
 430     /// lint found.
 431     ///
 432     /// The parameter `is_local` distinguishes the context of the type.
 433     fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: CheckTyContext) {
 434         if hir_ty.span.from_expansion() {
 435             return;
 436         }
 437
 438         if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) {
 439             return;
 440         }
 441
 442         // Skip trait implementations; see issue #605.
 443         if context.is_in_trait_impl {
 444             return;
 445         }
 446
 447         match hir_ty.kind {
 448             TyKind::Path(ref qpath) if !context.is_local => {
 449                 let hir_id = hir_ty.hir_id;
 450                 let res = cx.qpath_res(qpath, hir_id);
 451                 if let Some(def_id) = res.opt_def_id() {
 452                     if self.is_type_change_allowed(context) {
 453                         // All lints that are being checked in this block are guarded by
 454                         // the `avoid_breaking_exported_api` configuration. When adding a
 455                         // new lint, please also add the name to the configuration documentation
 456                         // in `clippy_lints::utils::conf.rs`
 457
 458                         let mut triggered = false;
 459                         triggered |= box_collection::check(cx, hir_ty, qpath, def_id);
 460                         triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id);
 461                         triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id);
 462                         triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold);
 463                         triggered |= option_option::check(cx, hir_ty, qpath, def_id);
 464                         triggered |= linked_list::check(cx, hir_ty, def_id);
 465                         triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
 466
 467                         if triggered {
 468                             return;
 469                         }
 470                     }
 471                 }
 472                 match *qpath {
 473                     QPath::Resolved(Some(ty), p) => {
 474                         context.is_nested_call = true;
 475                         self.check_ty(cx, ty, context);
 476                         for ty in p.segments.iter().flat_map(|seg| {
 477                             seg.args
 478                                 .as_ref()
 479                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 480                                 .filter_map(|arg| match arg {
 481                                     GenericArg::Type(ty) => Some(ty),
 482                                     _ => None,
 483                                 })
 484                         }) {
 485                             self.check_ty(cx, ty, context);
 486                         }
 487                     },
 488                     QPath::Resolved(None, p) => {
 489                         context.is_nested_call = true;
 490                         for ty in p.segments.iter().flat_map(|seg| {
 491                             seg.args
 492                                 .as_ref()
 493                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 494                                 .filter_map(|arg| match arg {
 495                                     GenericArg::Type(ty) => Some(ty),
 496                                     _ => None,
 497                                 })
 498                         }) {
 499                             self.check_ty(cx, ty, context);
 500                         }
 501                     },
 502                     QPath::TypeRelative(ty, seg) => {
 503                         context.is_nested_call = true;
 504                         self.check_ty(cx, ty, context);
 505                         if let Some(params) = seg.args {
 506                             for ty in params.args.iter().filter_map(|arg| match arg {
 507                                 GenericArg::Type(ty) => Some(ty),
 508                                 _ => None,
 509                             }) {
 510                                 self.check_ty(cx, ty, context);
 511                             }
 512                         }
 513                     },
 514                     QPath::LangItem(..) => {},
 515                 }
 516             },
 517             TyKind::Rptr(ref lt, ref mut_ty) => {
 518                 context.is_nested_call = true;
 519                 if !borrowed_box::check(cx, hir_ty, lt, mut_ty) {
 520                     self.check_ty(cx, mut_ty.ty, context);
 521                 }
 522             },
 523             TyKind::Slice(ty) | TyKind::Array(ty, _) | TyKind::Ptr(MutTy { ty, .. }) => {
 524                 context.is_nested_call = true;
 525                 self.check_ty(cx, ty, context);
 526             },
 527             TyKind::Tup(tys) => {
 528                 context.is_nested_call = true;
 529                 for ty in tys {
 530                     self.check_ty(cx, ty, context);
 531                 }
 532             },
 533             _ => {},
 534         }
 535     }
 536
 537     /// This function checks if the type is allowed to change in the current context
 538     /// based on the `avoid_breaking_exported_api` configuration
 539     fn is_type_change_allowed(&self, context: CheckTyContext) -> bool {
 540         !(context.is_exported && self.avoid_breaking_exported_api)
 541     }
 542 }
 543
 544 #[allow(clippy::struct_excessive_bools)]
 545 #[derive(Clone, Copy, Default)]
 546 struct CheckTyContext {
 547     is_in_trait_impl: bool,
 548     /// `true` for types on local variables.
 549     is_local: bool,
 550     /// `true` for types that are part of the public API.
 551     is_exported: bool,
 552     is_nested_call: bool,
 553 }