clippy_lints/src/types/mod.rs

   1 mod borrowed_box;
   2 mod box_vec;
   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:** Checks for use of `Box<Vec<_>>` anywhere in the code.
  24     /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information.
  25     ///
  26     /// **Why is this bad?** `Vec` already keeps its contents in a separate area on
  27     /// the heap. So if you `Box` it, you just add another level of indirection
  28     /// without any benefit whatsoever.
  29     ///
  30     /// **Known problems:** None.
  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     pub BOX_VEC,
  47     perf,
  48     "usage of `Box<Vec<T>>`, vector elements are already on the heap"
  49 }
  50
  51 declare_clippy_lint! {
  52     /// **What it does:** Checks for use of `Vec<Box<T>>` where T: Sized anywhere in the code.
  53     /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information.
  54     ///
  55     /// **Why is this bad?** `Vec` already keeps its contents in a separate area on
  56     /// the heap. So if you `Box` its contents, you just add another level of indirection.
  57     ///
  58     /// **Known problems:** Vec<Box<T: Sized>> makes sense if T is a large type (see [#3530](https://github.com/rust-lang/rust-clippy/issues/3530),
  59     /// 1st comment).
  60     ///
  61     /// **Example:**
  62     /// ```rust
  63     /// struct X {
  64     ///     values: Vec<Box<i32>>,
  65     /// }
  66     /// ```
  67     ///
  68     /// Better:
  69     ///
  70     /// ```rust
  71     /// struct X {
  72     ///     values: Vec<i32>,
  73     /// }
  74     /// ```
  75     pub VEC_BOX,
  76     complexity,
  77     "usage of `Vec<Box<T>>` where T: Sized, vector elements are already on the heap"
  78 }
  79
  80 declare_clippy_lint! {
  81     /// **What it does:** Checks for use of `Option<Option<_>>` in function signatures and type
  82     /// definitions
  83     ///
  84     /// **Why is this bad?** `Option<_>` represents an optional value. `Option<Option<_>>`
  85     /// represents an optional optional value which is logically the same thing as an optional
  86     /// value but has an unneeded extra level of wrapping.
  87     ///
  88     /// If you have a case where `Some(Some(_))`, `Some(None)` and `None` are distinct cases,
  89     /// consider a custom `enum` instead, with clear names for each case.
  90     ///
  91     /// **Known problems:** None.
  92     ///
  93     /// **Example**
  94     /// ```rust
  95     /// fn get_data() -> Option<Option<u32>> {
  96     ///     None
  97     /// }
  98     /// ```
  99     ///
 100     /// Better:
 101     ///
 102     /// ```rust
 103     /// pub enum Contents {
 104     ///     Data(Vec<u8>), // Was Some(Some(Vec<u8>))
 105     ///     NotYetFetched, // Was Some(None)
 106     ///     None,          // Was None
 107     /// }
 108     ///
 109     /// fn get_data() -> Contents {
 110     ///     Contents::None
 111     /// }
 112     /// ```
 113     pub OPTION_OPTION,
 114     pedantic,
 115     "usage of `Option<Option<T>>`"
 116 }
 117
 118 declare_clippy_lint! {
 119     /// **What it does:** Checks for usage of any `LinkedList`, suggesting to use a
 120     /// `Vec` or a `VecDeque` (formerly called `RingBuf`).
 121     ///
 122     /// **Why is this bad?** Gankro says:
 123     ///
 124     /// > The TL;DR of `LinkedList` is that it's built on a massive amount of
 125     /// pointers and indirection.
 126     /// > It wastes memory, it has terrible cache locality, and is all-around slow.
 127     /// `RingBuf`, while
 128     /// > "only" amortized for push/pop, should be faster in the general case for
 129     /// almost every possible
 130     /// > workload, and isn't even amortized at all if you can predict the capacity
 131     /// you need.
 132     /// >
 133     /// > `LinkedList`s are only really good if you're doing a lot of merging or
 134     /// splitting of lists.
 135     /// > This is because they can just mangle some pointers instead of actually
 136     /// copying the data. Even
 137     /// > if you're doing a lot of insertion in the middle of the list, `RingBuf`
 138     /// can still be better
 139     /// > because of how expensive it is to seek to the middle of a `LinkedList`.
 140     ///
 141     /// **Known problems:** False positives – the instances where using a
 142     /// `LinkedList` makes sense are few and far between, but they can still happen.
 143     ///
 144     /// **Example:**
 145     /// ```rust
 146     /// # use std::collections::LinkedList;
 147     /// let x: LinkedList<usize> = LinkedList::new();
 148     /// ```
 149     pub LINKEDLIST,
 150     pedantic,
 151     "usage of LinkedList, usually a vector is faster, or a more specialized data structure like a `VecDeque`"
 152 }
 153
 154 declare_clippy_lint! {
 155     /// **What it does:** Checks for use of `&Box<T>` anywhere in the code.
 156     /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information.
 157     ///
 158     /// **Why is this bad?** Any `&Box<T>` can also be a `&T`, which is more
 159     /// general.
 160     ///
 161     /// **Known problems:** None.
 162     ///
 163     /// **Example:**
 164     /// ```rust,ignore
 165     /// fn foo(bar: &Box<T>) { ... }
 166     /// ```
 167     ///
 168     /// Better:
 169     ///
 170     /// ```rust,ignore
 171     /// fn foo(bar: &T) { ... }
 172     /// ```
 173     pub BORROWED_BOX,
 174     complexity,
 175     "a borrow of a boxed type"
 176 }
 177
 178 declare_clippy_lint! {
 179     /// **What it does:** Checks for use of redundant allocations anywhere in the code.
 180     ///
 181     /// **Why is this bad?** Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Box<T>>`, `Box<&T>`
 182     /// add an unnecessary level of indirection.
 183     ///
 184     /// **Known problems:** None.
 185     ///
 186     /// **Example:**
 187     /// ```rust
 188     /// # use std::rc::Rc;
 189     /// fn foo(bar: Rc<&usize>) {}
 190     /// ```
 191     ///
 192     /// Better:
 193     ///
 194     /// ```rust
 195     /// fn foo(bar: &usize) {}
 196     /// ```
 197     pub REDUNDANT_ALLOCATION,
 198     perf,
 199     "redundant allocation"
 200 }
 201
 202 declare_clippy_lint! {
 203     /// **What it does:** Checks for `Rc<T>` and `Arc<T>` when `T` is a mutable buffer type such as `String` or `Vec`.
 204     ///
 205     /// **Why is this bad?** Expressions such as `Rc<String>` usually have no advantage over `Rc<str>`, since
 206     /// it is larger and involves an extra level of indirection, and doesn't implement `Borrow<str>`.
 207     ///
 208     /// While mutating a buffer type would still be possible with `Rc::get_mut()`, it only
 209     /// works if there are no additional references yet, which usually defeats the purpose of
 210     /// enclosing it in a shared ownership type. Instead, additionally wrapping the inner
 211     /// type with an interior mutable container (such as `RefCell` or `Mutex`) would normally
 212     /// be used.
 213     ///
 214     /// **Known problems:** This pattern can be desirable to avoid the overhead of a `RefCell` or `Mutex` for
 215     /// cases where mutation only happens before there are any additional references.
 216     ///
 217     /// **Example:**
 218     /// ```rust,ignore
 219     /// # use std::rc::Rc;
 220     /// fn foo(interned: Rc<String>) { ... }
 221     /// ```
 222     ///
 223     /// Better:
 224     ///
 225     /// ```rust,ignore
 226     /// fn foo(interned: Rc<str>) { ... }
 227     /// ```
 228     pub RC_BUFFER,
 229     restriction,
 230     "shared ownership of a buffer type"
 231 }
 232
 233 declare_clippy_lint! {
 234     /// **What it does:** Checks for types used in structs, parameters and `let`
 235     /// declarations above a certain complexity threshold.
 236     ///
 237     /// **Why is this bad?** Too complex types make the code less readable. Consider
 238     /// using a `type` definition to simplify them.
 239     ///
 240     /// **Known problems:** None.
 241     ///
 242     /// **Example:**
 243     /// ```rust
 244     /// # use std::rc::Rc;
 245     /// struct Foo {
 246     ///     inner: Rc<Vec<Vec<Box<(u32, u32, u32, u32)>>>>,
 247     /// }
 248     /// ```
 249     pub TYPE_COMPLEXITY,
 250     complexity,
 251     "usage of very complex types that might be better factored into `type` definitions"
 252 }
 253
 254 declare_clippy_lint! {
 255     /// **What it does:** Checks for `Rc<Mutex<T>>`.
 256     ///
 257     /// **Why is this bad?** `Rc` is used in single thread and `Mutex` is used in multi thread.
 258     /// Consider using `Rc<RefCell<T>>` in single thread or `Arc<Mutex<T>>` in multi thread.
 259     ///
 260     /// **Known problems:** None.
 261     ///
 262     /// **Example:**
 263     /// ```rust,ignore
 264     /// use std::rc::Rc;
 265     /// use std::sync::Mutex;
 266     /// fn foo(interned: Rc<Mutex<i32>>) { ... }
 267     /// ```
 268     ///
 269     /// Better:
 270     ///
 271     /// ```rust,ignore
 272     /// use std::rc::Rc;
 273     /// use std::cell::RefCell
 274     /// fn foo(interned: Rc<RefCell<i32>>) { ... }
 275     /// ```
 276     pub RC_MUTEX,
 277     restriction,
 278     "usage of `Rc<Mutex<T>>`"
 279 }
 280
 281 pub struct Types {
 282     vec_box_size_threshold: u64,
 283     type_complexity_threshold: u64,
 284 }
 285
 286 impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]);
 287
 288 impl<'tcx> LateLintPass<'tcx> for Types {
 289     fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) {
 290         let is_in_trait_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_item(id))
 291         {
 292             matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }))
 293         } else {
 294             false
 295         };
 296
 297         self.check_fn_decl(
 298             cx,
 299             decl,
 300             CheckTyContext {
 301                 is_in_trait_impl,
 302                 ..CheckTyContext::default()
 303             },
 304         );
 305     }
 306
 307     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
 308         match item.kind {
 309             ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty(cx, ty, CheckTyContext::default()),
 310             // functions, enums, structs, impls and traits are covered
 311             _ => (),
 312         }
 313     }
 314
 315     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
 316         match item.kind {
 317             ImplItemKind::Const(ty, _) | ImplItemKind::TyAlias(ty) => self.check_ty(
 318                 cx,
 319                 ty,
 320                 CheckTyContext {
 321                     is_in_trait_impl: true,
 322                     ..CheckTyContext::default()
 323                 },
 324             ),
 325             // methods are covered by check_fn
 326             ImplItemKind::Fn(..) => (),
 327         }
 328     }
 329
 330     fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) {
 331         self.check_ty(cx, field.ty, CheckTyContext::default());
 332     }
 333
 334     fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) {
 335         match item.kind {
 336             TraitItemKind::Const(ty, _) | TraitItemKind::Type(_, Some(ty)) => {
 337                 self.check_ty(cx, ty, CheckTyContext::default());
 338             },
 339             TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, sig.decl, CheckTyContext::default()),
 340             TraitItemKind::Type(..) => (),
 341         }
 342     }
 343
 344     fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
 345         if let Some(ty) = local.ty {
 346             self.check_ty(
 347                 cx,
 348                 ty,
 349                 CheckTyContext {
 350                     is_local: true,
 351                     ..CheckTyContext::default()
 352                 },
 353             );
 354         }
 355     }
 356 }
 357
 358 impl Types {
 359     pub fn new(vec_box_size_threshold: u64, type_complexity_threshold: u64) -> Self {
 360         Self {
 361             vec_box_size_threshold,
 362             type_complexity_threshold,
 363         }
 364     }
 365
 366     fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, context: CheckTyContext) {
 367         for input in decl.inputs {
 368             self.check_ty(cx, input, context);
 369         }
 370
 371         if let FnRetTy::Return(ty) = decl.output {
 372             self.check_ty(cx, ty, context);
 373         }
 374     }
 375
 376     /// Recursively check for `TypePass` lints in the given type. Stop at the first
 377     /// lint found.
 378     ///
 379     /// The parameter `is_local` distinguishes the context of the type.
 380     fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: CheckTyContext) {
 381         if hir_ty.span.from_expansion() {
 382             return;
 383         }
 384
 385         if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) {
 386             return;
 387         }
 388
 389         // Skip trait implementations; see issue #605.
 390         if context.is_in_trait_impl {
 391             return;
 392         }
 393
 394         match hir_ty.kind {
 395             TyKind::Path(ref qpath) if !context.is_local => {
 396                 let hir_id = hir_ty.hir_id;
 397                 let res = cx.qpath_res(qpath, hir_id);
 398                 if let Some(def_id) = res.opt_def_id() {
 399                     let mut triggered = false;
 400                     triggered |= box_vec::check(cx, hir_ty, qpath, def_id);
 401                     triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id);
 402                     triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id);
 403                     triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold);
 404                     triggered |= option_option::check(cx, hir_ty, qpath, def_id);
 405                     triggered |= linked_list::check(cx, hir_ty, def_id);
 406                     triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
 407
 408                     if triggered {
 409                         return;
 410                     }
 411                 }
 412                 match *qpath {
 413                     QPath::Resolved(Some(ty), p) => {
 414                         context.is_nested_call = true;
 415                         self.check_ty(cx, ty, context);
 416                         for ty in p.segments.iter().flat_map(|seg| {
 417                             seg.args
 418                                 .as_ref()
 419                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 420                                 .filter_map(|arg| match arg {
 421                                     GenericArg::Type(ty) => Some(ty),
 422                                     _ => None,
 423                                 })
 424                         }) {
 425                             self.check_ty(cx, ty, context);
 426                         }
 427                     },
 428                     QPath::Resolved(None, p) => {
 429                         context.is_nested_call = true;
 430                         for ty in p.segments.iter().flat_map(|seg| {
 431                             seg.args
 432                                 .as_ref()
 433                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 434                                 .filter_map(|arg| match arg {
 435                                     GenericArg::Type(ty) => Some(ty),
 436                                     _ => None,
 437                                 })
 438                         }) {
 439                             self.check_ty(cx, ty, context);
 440                         }
 441                     },
 442                     QPath::TypeRelative(ty, seg) => {
 443                         context.is_nested_call = true;
 444                         self.check_ty(cx, ty, context);
 445                         if let Some(params) = seg.args {
 446                             for ty in params.args.iter().filter_map(|arg| match arg {
 447                                 GenericArg::Type(ty) => Some(ty),
 448                                 _ => None,
 449                             }) {
 450                                 self.check_ty(cx, ty, context);
 451                             }
 452                         }
 453                     },
 454                     QPath::LangItem(..) => {},
 455                 }
 456             },
 457             TyKind::Rptr(ref lt, ref mut_ty) => {
 458                 context.is_nested_call = true;
 459                 if !borrowed_box::check(cx, hir_ty, lt, mut_ty) {
 460                     self.check_ty(cx, mut_ty.ty, context);
 461                 }
 462             },
 463             TyKind::Slice(ty) | TyKind::Array(ty, _) | TyKind::Ptr(MutTy { ty, .. }) => {
 464                 context.is_nested_call = true;
 465                 self.check_ty(cx, ty, context);
 466             },
 467             TyKind::Tup(tys) => {
 468                 context.is_nested_call = true;
 469                 for ty in tys {
 470                     self.check_ty(cx, ty, context);
 471                 }
 472             },
 473             _ => {},
 474         }
 475     }
 476 }
 477
 478 #[derive(Clone, Copy, Default)]
 479 struct CheckTyContext {
 480     is_in_trait_impl: bool,
 481     is_local: bool,
 482     is_nested_call: bool,
 483 }