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