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<Mutex<T>>` may introduce a deadlock in single thread. Consider
 258     /// using `Rc<RefCell<T>>` instead.
 259     /// ```rust
 260     /// fn main() {
 261     ///     use std::rc::Rc;
 262     ///     use std::sync::Mutex;
 263     ///
 264     ///     let a: Rc<Mutex<i32>> = Rc::new(Mutex::new(1));
 265     ///     let a_clone = a.clone();
 266     ///     let mut data = a.lock().unwrap();
 267     ///     println!("{:?}", *a_clone.lock().unwrap());
 268     ///     *data = 10;
 269     ///  }
 270     /// ```
 271     ///
 272     /// **Known problems:** `Rc<RefCell<T>>` may panic in runtime.
 273     ///
 274     /// **Example:**
 275     /// ```rust,ignore
 276     /// use std::rc::Rc;
 277     /// use std::sync::Mutex;
 278     /// fn foo(interned: Rc<Mutex<i32>>) { ... }
 279     /// ```
 280     ///
 281     /// Better:
 282     ///
 283     /// ```rust,ignore
 284     /// use std::rc::Rc;
 285     /// use std::cell::RefCell
 286     /// fn foo(interned: Rc<RefCell<i32>>) { ... }
 287     /// ```
 288     pub RC_MUTEX,
 289     restriction,
 290     "usage of `Rc<Mutex<T>>`"
 291 }
 292
 293 pub struct Types {
 294     vec_box_size_threshold: u64,
 295     type_complexity_threshold: u64,
 296 }
 297
 298 impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]);
 299
 300 impl<'tcx> LateLintPass<'tcx> for Types {
 301     fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) {
 302         let is_in_trait_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_item(id))
 303         {
 304             matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }))
 305         } else {
 306             false
 307         };
 308
 309         self.check_fn_decl(
 310             cx,
 311             decl,
 312             CheckTyContext {
 313                 is_in_trait_impl,
 314                 ..CheckTyContext::default()
 315             },
 316         );
 317     }
 318
 319     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
 320         match item.kind {
 321             ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty(cx, ty, CheckTyContext::default()),
 322             // functions, enums, structs, impls and traits are covered
 323             _ => (),
 324         }
 325     }
 326
 327     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
 328         match item.kind {
 329             ImplItemKind::Const(ty, _) | ImplItemKind::TyAlias(ty) => self.check_ty(
 330                 cx,
 331                 ty,
 332                 CheckTyContext {
 333                     is_in_trait_impl: true,
 334                     ..CheckTyContext::default()
 335                 },
 336             ),
 337             // methods are covered by check_fn
 338             ImplItemKind::Fn(..) => (),
 339         }
 340     }
 341
 342     fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) {
 343         self.check_ty(cx, field.ty, CheckTyContext::default());
 344     }
 345
 346     fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) {
 347         match item.kind {
 348             TraitItemKind::Const(ty, _) | TraitItemKind::Type(_, Some(ty)) => {
 349                 self.check_ty(cx, ty, CheckTyContext::default());
 350             },
 351             TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, sig.decl, CheckTyContext::default()),
 352             TraitItemKind::Type(..) => (),
 353         }
 354     }
 355
 356     fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
 357         if let Some(ty) = local.ty {
 358             self.check_ty(
 359                 cx,
 360                 ty,
 361                 CheckTyContext {
 362                     is_local: true,
 363                     ..CheckTyContext::default()
 364                 },
 365             );
 366         }
 367     }
 368 }
 369
 370 impl Types {
 371     pub fn new(vec_box_size_threshold: u64, type_complexity_threshold: u64) -> Self {
 372         Self {
 373             vec_box_size_threshold,
 374             type_complexity_threshold,
 375         }
 376     }
 377
 378     fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, context: CheckTyContext) {
 379         for input in decl.inputs {
 380             self.check_ty(cx, input, context);
 381         }
 382
 383         if let FnRetTy::Return(ty) = decl.output {
 384             self.check_ty(cx, ty, context);
 385         }
 386     }
 387
 388     /// Recursively check for `TypePass` lints in the given type. Stop at the first
 389     /// lint found.
 390     ///
 391     /// The parameter `is_local` distinguishes the context of the type.
 392     fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: CheckTyContext) {
 393         if hir_ty.span.from_expansion() {
 394             return;
 395         }
 396
 397         if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) {
 398             return;
 399         }
 400
 401         // Skip trait implementations; see issue #605.
 402         if context.is_in_trait_impl {
 403             return;
 404         }
 405
 406         match hir_ty.kind {
 407             TyKind::Path(ref qpath) if !context.is_local => {
 408                 let hir_id = hir_ty.hir_id;
 409                 let res = cx.qpath_res(qpath, hir_id);
 410                 if let Some(def_id) = res.opt_def_id() {
 411                     let mut triggered = false;
 412                     triggered |= box_vec::check(cx, hir_ty, qpath, def_id);
 413                     triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id);
 414                     triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id);
 415                     triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold);
 416                     triggered |= option_option::check(cx, hir_ty, qpath, def_id);
 417                     triggered |= linked_list::check(cx, hir_ty, def_id);
 418                     triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id);
 419
 420                     if triggered {
 421                         return;
 422                     }
 423                 }
 424                 match *qpath {
 425                     QPath::Resolved(Some(ty), p) => {
 426                         context.is_nested_call = true;
 427                         self.check_ty(cx, ty, context);
 428                         for ty in p.segments.iter().flat_map(|seg| {
 429                             seg.args
 430                                 .as_ref()
 431                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 432                                 .filter_map(|arg| match arg {
 433                                     GenericArg::Type(ty) => Some(ty),
 434                                     _ => None,
 435                                 })
 436                         }) {
 437                             self.check_ty(cx, ty, context);
 438                         }
 439                     },
 440                     QPath::Resolved(None, p) => {
 441                         context.is_nested_call = true;
 442                         for ty in p.segments.iter().flat_map(|seg| {
 443                             seg.args
 444                                 .as_ref()
 445                                 .map_or_else(|| [].iter(), |params| params.args.iter())
 446                                 .filter_map(|arg| match arg {
 447                                     GenericArg::Type(ty) => Some(ty),
 448                                     _ => None,
 449                                 })
 450                         }) {
 451                             self.check_ty(cx, ty, context);
 452                         }
 453                     },
 454                     QPath::TypeRelative(ty, seg) => {
 455                         context.is_nested_call = true;
 456                         self.check_ty(cx, ty, context);
 457                         if let Some(params) = seg.args {
 458                             for ty in params.args.iter().filter_map(|arg| match arg {
 459                                 GenericArg::Type(ty) => Some(ty),
 460                                 _ => None,
 461                             }) {
 462                                 self.check_ty(cx, ty, context);
 463                             }
 464                         }
 465                     },
 466                     QPath::LangItem(..) => {},
 467                 }
 468             },
 469             TyKind::Rptr(ref lt, ref mut_ty) => {
 470                 context.is_nested_call = true;
 471                 if !borrowed_box::check(cx, hir_ty, lt, mut_ty) {
 472                     self.check_ty(cx, mut_ty.ty, context);
 473                 }
 474             },
 475             TyKind::Slice(ty) | TyKind::Array(ty, _) | TyKind::Ptr(MutTy { ty, .. }) => {
 476                 context.is_nested_call = true;
 477                 self.check_ty(cx, ty, context);
 478             },
 479             TyKind::Tup(tys) => {
 480                 context.is_nested_call = true;
 481                 for ty in tys {
 482                     self.check_ty(cx, ty, context);
 483                 }
 484             },
 485             _ => {},
 486         }
 487     }
 488 }
 489
 490 #[derive(Clone, Copy, Default)]
 491 struct CheckTyContext {
 492     is_in_trait_impl: bool,
 493     is_local: bool,
 494     is_nested_call: bool,
 495 }