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, 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<T>` where T is a collection such as 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 /// Collections already keeps their contents in a separate area on 29 /// the heap. So if you `Box` them, 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_COLLECTION, 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 avoid_breaking_exported_api: bool, 299 } 300 301 impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]); 302 303 impl<'tcx> LateLintPass<'tcx> for Types { check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId)304 fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) { 305 let is_in_trait_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_item(id)) 306 { 307 matches!(item.kind, ItemKind::Impl(hir::Impl { of_trait: Some(_), .. })) 308 } else { 309 false 310 }; 311 312 let is_exported = cx.access_levels.is_exported(cx.tcx.hir().local_def_id(id)); 313 314 self.check_fn_decl( 315 cx, 316 decl, 317 CheckTyContext { 318 is_in_trait_impl, 319 is_exported, 320 ..CheckTyContext::default() 321 }, 322 ); 323 } 324 check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>)325 fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { 326 let is_exported = cx.access_levels.is_exported(item.def_id); 327 328 match item.kind { 329 ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty( 330 cx, 331 ty, 332 CheckTyContext { 333 is_exported, 334 ..CheckTyContext::default() 335 }, 336 ), 337 // functions, enums, structs, impls and traits are covered 338 _ => (), 339 } 340 } 341 check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>)342 fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { 343 match item.kind { 344 ImplItemKind::Const(ty, _) | ImplItemKind::TyAlias(ty) => self.check_ty( 345 cx, 346 ty, 347 CheckTyContext { 348 is_in_trait_impl: true, 349 ..CheckTyContext::default() 350 }, 351 ), 352 // methods are covered by check_fn 353 ImplItemKind::Fn(..) => (), 354 } 355 } 356 check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>)357 fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) { 358 let is_exported = cx.access_levels.is_exported(cx.tcx.hir().local_def_id(field.hir_id)); 359 360 self.check_ty( 361 cx, 362 field.ty, 363 CheckTyContext { 364 is_exported, 365 ..CheckTyContext::default() 366 }, 367 ); 368 } 369 check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &TraitItem<'_>)370 fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &TraitItem<'_>) { 371 let is_exported = cx.access_levels.is_exported(item.def_id); 372 373 let context = CheckTyContext { 374 is_exported, 375 ..CheckTyContext::default() 376 }; 377 378 match item.kind { 379 TraitItemKind::Const(ty, _) | TraitItemKind::Type(_, Some(ty)) => { 380 self.check_ty(cx, ty, context); 381 }, 382 TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, sig.decl, context), 383 TraitItemKind::Type(..) => (), 384 } 385 } 386 check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>)387 fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) { 388 if let Some(ty) = local.ty { 389 self.check_ty( 390 cx, 391 ty, 392 CheckTyContext { 393 is_local: true, 394 ..CheckTyContext::default() 395 }, 396 ); 397 } 398 } 399 } 400 401 impl Types { new(vec_box_size_threshold: u64, type_complexity_threshold: u64, avoid_breaking_exported_api: bool) -> Self402 pub fn new(vec_box_size_threshold: u64, type_complexity_threshold: u64, avoid_breaking_exported_api: bool) -> Self { 403 Self { 404 vec_box_size_threshold, 405 type_complexity_threshold, 406 avoid_breaking_exported_api, 407 } 408 } 409 check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, context: CheckTyContext)410 fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, context: CheckTyContext) { 411 for input in decl.inputs { 412 self.check_ty(cx, input, context); 413 } 414 415 if let FnRetTy::Return(ty) = decl.output { 416 self.check_ty(cx, ty, context); 417 } 418 } 419 420 /// Recursively check for `TypePass` lints in the given type. Stop at the first 421 /// lint found. 422 /// 423 /// The parameter `is_local` distinguishes the context of the type. check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: CheckTyContext)424 fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: CheckTyContext) { 425 if hir_ty.span.from_expansion() { 426 return; 427 } 428 429 if !context.is_nested_call && type_complexity::check(cx, hir_ty, self.type_complexity_threshold) { 430 return; 431 } 432 433 // Skip trait implementations; see issue #605. 434 if context.is_in_trait_impl { 435 return; 436 } 437 438 match hir_ty.kind { 439 TyKind::Path(ref qpath) if !context.is_local => { 440 let hir_id = hir_ty.hir_id; 441 let res = cx.qpath_res(qpath, hir_id); 442 if let Some(def_id) = res.opt_def_id() { 443 if self.is_type_change_allowed(context) { 444 // All lints that are being checked in this block are guarded by 445 // the `avoid_breaking_exported_api` configuration. When adding a 446 // new lint, please also add the name to the configuration documentation 447 // in `clippy_lints::utils::conf.rs` 448 449 let mut triggered = false; 450 triggered |= box_collection::check(cx, hir_ty, qpath, def_id); 451 triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id); 452 triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id); 453 triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold); 454 triggered |= option_option::check(cx, hir_ty, qpath, def_id); 455 triggered |= linked_list::check(cx, hir_ty, def_id); 456 triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id); 457 458 if triggered { 459 return; 460 } 461 } 462 } 463 match *qpath { 464 QPath::Resolved(Some(ty), p) => { 465 context.is_nested_call = true; 466 self.check_ty(cx, ty, context); 467 for ty in p.segments.iter().flat_map(|seg| { 468 seg.args 469 .as_ref() 470 .map_or_else(|| [].iter(), |params| params.args.iter()) 471 .filter_map(|arg| match arg { 472 GenericArg::Type(ty) => Some(ty), 473 _ => None, 474 }) 475 }) { 476 self.check_ty(cx, ty, context); 477 } 478 }, 479 QPath::Resolved(None, p) => { 480 context.is_nested_call = true; 481 for ty in p.segments.iter().flat_map(|seg| { 482 seg.args 483 .as_ref() 484 .map_or_else(|| [].iter(), |params| params.args.iter()) 485 .filter_map(|arg| match arg { 486 GenericArg::Type(ty) => Some(ty), 487 _ => None, 488 }) 489 }) { 490 self.check_ty(cx, ty, context); 491 } 492 }, 493 QPath::TypeRelative(ty, seg) => { 494 context.is_nested_call = true; 495 self.check_ty(cx, ty, context); 496 if let Some(params) = seg.args { 497 for ty in params.args.iter().filter_map(|arg| match arg { 498 GenericArg::Type(ty) => Some(ty), 499 _ => None, 500 }) { 501 self.check_ty(cx, ty, context); 502 } 503 } 504 }, 505 QPath::LangItem(..) => {}, 506 } 507 }, 508 TyKind::Rptr(ref lt, ref mut_ty) => { 509 context.is_nested_call = true; 510 if !borrowed_box::check(cx, hir_ty, lt, mut_ty) { 511 self.check_ty(cx, mut_ty.ty, context); 512 } 513 }, 514 TyKind::Slice(ty) | TyKind::Array(ty, _) | TyKind::Ptr(MutTy { ty, .. }) => { 515 context.is_nested_call = true; 516 self.check_ty(cx, ty, context); 517 }, 518 TyKind::Tup(tys) => { 519 context.is_nested_call = true; 520 for ty in tys { 521 self.check_ty(cx, ty, context); 522 } 523 }, 524 _ => {}, 525 } 526 } 527 528 /// This function checks if the type is allowed to change in the current context 529 /// based on the `avoid_breaking_exported_api` configuration is_type_change_allowed(&self, context: CheckTyContext) -> bool530 fn is_type_change_allowed(&self, context: CheckTyContext) -> bool { 531 !(context.is_exported && self.avoid_breaking_exported_api) 532 } 533 } 534 535 #[allow(clippy::struct_excessive_bools)] 536 #[derive(Clone, Copy, Default)] 537 struct CheckTyContext { 538 is_in_trait_impl: bool, 539 /// `true` for types on local variables. 540 is_local: bool, 541 /// `true` for types that are part of the public API. 542 is_exported: bool, 543 is_nested_call: bool, 544 } 545