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