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