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