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