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