X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Ftypes%2Fmod.rs;h=69cd49d884cc00ae91674369eeae38cf86c5ac5d;hb=063f8aa094a740b98b0dab49d8361441c8f1d0c4;hp=ad7409fe3a9b7d1fe1f67ce589d850b41ae5a664;hpb=2b20f49841dbfb773d47c008b4278885435483ac;p=rust.git diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index ad7409fe3a9..69cd49d884c 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -1,5 +1,5 @@ mod borrowed_box; -mod box_vec; +mod box_collection; mod linked_list; mod option_option; mod rc_buffer; @@ -21,12 +21,12 @@ declare_clippy_lint! { /// ### What it does - /// Checks for use of `Box>` anywhere in the code. + /// Checks for use of `Box` where T is a collection such as Vec anywhere in the code. /// Check the [Box documentation](https://doc.rust-lang.org/std/boxed/index.html) for more information. /// /// ### Why is this bad? - /// `Vec` already keeps its contents in a separate area on - /// the heap. So if you `Box` it, you just add another level of indirection + /// Collections already keeps their contents in a separate area on + /// the heap. So if you `Box` them, you just add another level of indirection /// without any benefit whatsoever. /// /// ### Example @@ -43,7 +43,8 @@ /// values: Vec, /// } /// ``` - pub BOX_VEC, + #[clippy::version = "1.57.0"] + pub BOX_COLLECTION, perf, "usage of `Box>`, vector elements are already on the heap" } @@ -75,6 +76,7 @@ /// values: Vec, /// } /// ``` + #[clippy::version = "1.33.0"] pub VEC_BOX, complexity, "usage of `Vec>` where T: Sized, vector elements are already on the heap" @@ -113,6 +115,7 @@ /// Contents::None /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub OPTION_OPTION, pedantic, "usage of `Option>`" @@ -152,6 +155,7 @@ /// # use std::collections::LinkedList; /// let x: LinkedList = LinkedList::new(); /// ``` + #[clippy::version = "pre 1.29.0"] pub LINKEDLIST, pedantic, "usage of LinkedList, usually a vector is faster, or a more specialized data structure like a `VecDeque`" @@ -176,6 +180,7 @@ /// ```rust,ignore /// fn foo(bar: &T) { ... } /// ``` + #[clippy::version = "pre 1.29.0"] pub BORROWED_BOX, complexity, "a borrow of a boxed type" @@ -186,7 +191,7 @@ /// Checks for use of redundant allocations anywhere in the code. /// /// ### Why is this bad? - /// Expressions such as `Rc<&T>`, `Rc>`, `Rc>`, `Rc>`, Arc<&T>`, `Arc>`, + /// Expressions such as `Rc<&T>`, `Rc>`, `Rc>`, `Rc>`, `Arc<&T>`, `Arc>`, /// `Arc>`, `Arc>`, `Box<&T>`, `Box>`, `Box>`, `Box>`, add an unnecessary level of indirection. /// /// ### Example @@ -200,6 +205,7 @@ /// ```rust /// fn foo(bar: &usize) {} /// ``` + #[clippy::version = "1.44.0"] pub REDUNDANT_ALLOCATION, perf, "redundant allocation" @@ -234,6 +240,7 @@ /// ```rust,ignore /// fn foo(interned: Rc) { ... } /// ``` + #[clippy::version = "1.48.0"] pub RC_BUFFER, restriction, "shared ownership of a buffer type" @@ -255,6 +262,7 @@ /// inner: Rc>>>, /// } /// ``` + #[clippy::version = "pre 1.29.0"] pub TYPE_COMPLEXITY, complexity, "usage of very complex types that might be better factored into `type` definitions" @@ -287,6 +295,7 @@ /// use std::cell::RefCell /// fn foo(interned: Rc>) { ... } /// ``` + #[clippy::version = "1.55.0"] pub RC_MUTEX, restriction, "usage of `Rc>`" @@ -295,9 +304,10 @@ pub struct Types { vec_box_size_threshold: u64, type_complexity_threshold: u64, + avoid_breaking_exported_api: bool, } -impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]); +impl_lint_pass!(Types => [BOX_COLLECTION, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION, RC_BUFFER, RC_MUTEX, TYPE_COMPLEXITY]); impl<'tcx> LateLintPass<'tcx> for Types { fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _: &Body<'_>, _: Span, id: HirId) { @@ -308,19 +318,31 @@ fn check_fn(&mut self, cx: &LateContext<'_>, _: FnKind<'_>, decl: &FnDecl<'_>, _ false }; + let is_exported = cx.access_levels.is_exported(cx.tcx.hir().local_def_id(id)); + self.check_fn_decl( cx, decl, CheckTyContext { is_in_trait_impl, + is_exported, ..CheckTyContext::default() }, ); } fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + let is_exported = cx.access_levels.is_exported(item.def_id); + match item.kind { - ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty(cx, ty, CheckTyContext::default()), + ItemKind::Static(ty, _, _) | ItemKind::Const(ty, _) => self.check_ty( + cx, + ty, + CheckTyContext { + is_exported, + ..CheckTyContext::default() + }, + ), // functions, enums, structs, impls and traits are covered _ => (), } @@ -328,7 +350,7 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { match item.kind { - ImplItemKind::Const(ty, _) | ImplItemKind::TyAlias(ty) => self.check_ty( + ImplItemKind::Const(ty, _) => self.check_ty( cx, ty, CheckTyContext { @@ -336,21 +358,39 @@ fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) ..CheckTyContext::default() }, ), - // methods are covered by check_fn - ImplItemKind::Fn(..) => (), + // Methods are covered by check_fn. + // Type aliases are ignored because oftentimes it's impossible to + // make type alias declaration in trait simpler, see #1013 + ImplItemKind::Fn(..) | ImplItemKind::TyAlias(..) => (), } } fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) { - self.check_ty(cx, field.ty, CheckTyContext::default()); + let is_exported = cx.access_levels.is_exported(cx.tcx.hir().local_def_id(field.hir_id)); + + self.check_ty( + cx, + field.ty, + CheckTyContext { + is_exported, + ..CheckTyContext::default() + }, + ); } - fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) { + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &TraitItem<'_>) { + let is_exported = cx.access_levels.is_exported(item.def_id); + + let context = CheckTyContext { + is_exported, + ..CheckTyContext::default() + }; + match item.kind { TraitItemKind::Const(ty, _) | TraitItemKind::Type(_, Some(ty)) => { - self.check_ty(cx, ty, CheckTyContext::default()); + self.check_ty(cx, ty, context); }, - TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, sig.decl, CheckTyContext::default()), + TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, sig.decl, context), TraitItemKind::Type(..) => (), } } @@ -370,10 +410,11 @@ fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) { } impl Types { - pub fn new(vec_box_size_threshold: u64, type_complexity_threshold: u64) -> Self { + pub fn new(vec_box_size_threshold: u64, type_complexity_threshold: u64, avoid_breaking_exported_api: bool) -> Self { Self { vec_box_size_threshold, type_complexity_threshold, + avoid_breaking_exported_api, } } @@ -410,17 +451,24 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: let hir_id = hir_ty.hir_id; let res = cx.qpath_res(qpath, hir_id); if let Some(def_id) = res.opt_def_id() { - let mut triggered = false; - triggered |= box_vec::check(cx, hir_ty, qpath, def_id); - triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id); - triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id); - triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold); - triggered |= option_option::check(cx, hir_ty, qpath, def_id); - triggered |= linked_list::check(cx, hir_ty, def_id); - triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id); - - if triggered { - return; + if self.is_type_change_allowed(context) { + // All lints that are being checked in this block are guarded by + // the `avoid_breaking_exported_api` configuration. When adding a + // new lint, please also add the name to the configuration documentation + // in `clippy_lints::utils::conf.rs` + + let mut triggered = false; + triggered |= box_collection::check(cx, hir_ty, qpath, def_id); + triggered |= redundant_allocation::check(cx, hir_ty, qpath, def_id); + triggered |= rc_buffer::check(cx, hir_ty, qpath, def_id); + triggered |= vec_box::check(cx, hir_ty, qpath, def_id, self.vec_box_size_threshold); + triggered |= option_option::check(cx, hir_ty, qpath, def_id); + triggered |= linked_list::check(cx, hir_ty, def_id); + triggered |= rc_mutex::check(cx, hir_ty, qpath, def_id); + + if triggered { + return; + } } } match *qpath { @@ -487,11 +535,21 @@ fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, mut context: _ => {}, } } + + /// This function checks if the type is allowed to change in the current context + /// based on the `avoid_breaking_exported_api` configuration + fn is_type_change_allowed(&self, context: CheckTyContext) -> bool { + !(context.is_exported && self.avoid_breaking_exported_api) + } } +#[allow(clippy::struct_excessive_bools)] #[derive(Clone, Copy, Default)] struct CheckTyContext { is_in_trait_impl: bool, + /// `true` for types on local variables. is_local: bool, + /// `true` for types that are part of the public API. + is_exported: bool, is_nested_call: bool, }