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