From a1bab3bc63d183d3d319d52742e31fa54ec7caae Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Thu, 22 Jul 2021 08:40:24 -0400 Subject: [PATCH] Add primitive type support to disallowed_type lint Fix docs of disallowed_type Add ability to name primitive types without import path Move primitive resolution to clippy_utils path_to_res fn Refactor Res matching, fix naming and docs from review Use tcx.def_path_str when emitting the lint --- clippy_lints/src/disallowed_type.rs | 78 +++++++++---------- clippy_utils/src/lib.rs | 5 +- .../ui-toml/toml_disallowed_type/clippy.toml | 2 + .../conf_disallowed_type.rs | 15 ++-- .../conf_disallowed_type.stderr | 58 ++++++++++---- 5 files changed, 94 insertions(+), 64 deletions(-) diff --git a/clippy_lints/src/disallowed_type.rs b/clippy_lints/src/disallowed_type.rs index e4a88c6324e..7c76e2322c2 100644 --- a/clippy_lints/src/disallowed_type.rs +++ b/clippy_lints/src/disallowed_type.rs @@ -2,7 +2,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_hir::{ - def::Res, def_id::DefId, Crate, Item, ItemKind, PolyTraitRef, TraitBoundModifier, Ty, TyKind, UseKind, + def::Res, def_id::DefId, Crate, Item, ItemKind, PolyTraitRef, PrimTy, TraitBoundModifier, Ty, TyKind, UseKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -15,14 +15,12 @@ /// /// **Known problems:** None. /// - /// N.B. There is no way to ban primitive types. - /// /// **Example:** /// /// An example clippy.toml configuration: /// ```toml /// # clippy.toml - /// disallowed-methods = ["std::collections::BTreeMap"] + /// disallowed-types = ["std::collections::BTreeMap"] /// ``` /// /// ```rust,ignore @@ -42,7 +40,8 @@ #[derive(Clone, Debug)] pub struct DisallowedType { disallowed: FxHashSet>, - def_ids: FxHashSet<(DefId, Vec)>, + def_ids: FxHashSet, + prim_tys: FxHashSet, } impl DisallowedType { @@ -53,6 +52,23 @@ pub fn new(disallowed: &FxHashSet) -> Self { .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::>()) .collect(), def_ids: FxHashSet::default(), + prim_tys: FxHashSet::default(), + } + } + + fn check_res_emit(&self, cx: &LateContext<'_>, res: &Res, span: Span) { + match res { + Res::Def(_, did) => { + if self.def_ids.contains(did) { + emit(cx, &cx.tcx.def_path_str(*did), span); + } + }, + Res::PrimTy(prim) => { + if self.prim_tys.contains(prim) { + emit(cx, prim.name_str(), span); + } + }, + _ => {}, } } } @@ -63,60 +79,36 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedType { fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { for path in &self.disallowed { let segs = path.iter().map(ToString::to_string).collect::>(); - if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::>()) - { - self.def_ids.insert((id, path.clone())); + match clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::>()) { + Res::Def(_, id) => { + self.def_ids.insert(id); + }, + Res::PrimTy(ty) => { + self.prim_tys.insert(ty); + }, + _ => {}, } } } fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { - if_chain! { - if let ItemKind::Use(path, UseKind::Single) = &item.kind; - if let Res::Def(_, did) = path.res; - if let Some((_, name)) = self.def_ids.iter().find(|(id, _)| *id == did); - then { - emit(cx, name, item.span,); - } + if let ItemKind::Use(path, UseKind::Single) = &item.kind { + self.check_res_emit(cx, &path.res, item.span); } } fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { - if_chain! { - if let TyKind::Path(path) = &ty.kind; - if let Some(did) = cx.qpath_res(path, ty.hir_id).opt_def_id(); - if let Some((_, name)) = self.def_ids.iter().find(|(id, _)| *id == did); - then { - emit(cx, name, path.span()); - } + if let TyKind::Path(path) = &ty.kind { + self.check_res_emit(cx, &cx.qpath_res(path, ty.hir_id), ty.span); } } fn check_poly_trait_ref(&mut self, cx: &LateContext<'tcx>, poly: &'tcx PolyTraitRef<'tcx>, _: TraitBoundModifier) { - if_chain! { - if let Res::Def(_, did) = poly.trait_ref.path.res; - if let Some((_, name)) = self.def_ids.iter().find(|(id, _)| *id == did); - then { - emit(cx, name, poly.trait_ref.path.span); - } - } + self.check_res_emit(cx, &poly.trait_ref.path.res, poly.trait_ref.path.span); } - - // TODO: if non primitive const generics are a thing - // fn check_generic_arg(&mut self, cx: &LateContext<'tcx>, arg: &'tcx GenericArg<'tcx>) { - // match arg { - // GenericArg::Const(c) => {}, - // } - // } - // fn check_generic_param(&mut self, cx: &LateContext<'tcx>, param: &'tcx GenericParam<'tcx>) { - // match param.kind { - // GenericParamKind::Const { .. } => {}, - // } - // } } -fn emit(cx: &LateContext<'_>, name: &[Symbol], span: Span) { - let name = name.iter().map(|s| s.to_ident_string()).collect::>().join("::"); +fn emit(cx: &LateContext<'_>, name: &str, span: Span) { span_lint( cx, DISALLOWED_TYPE, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 6dfb8c38a76..59f878f8b20 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -72,7 +72,7 @@ use rustc_hir::{ def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Node, Param, Pat, PatKind, Path, - PathSegment, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp, + PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp, }; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; @@ -490,6 +490,9 @@ fn item_child_by_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, name: &str) -> Opt let (krate, first, path) = match *path { [krate, first, ref path @ ..] => (krate, first, path), + [primitive] => { + return PrimTy::from_name(Symbol::intern(primitive)).map_or(Res::Err, Res::PrimTy); + }, _ => return Res::Err, }; let tcx = cx.tcx; diff --git a/tests/ui-toml/toml_disallowed_type/clippy.toml b/tests/ui-toml/toml_disallowed_type/clippy.toml index 2eff854c22c..dac4446703b 100644 --- a/tests/ui-toml/toml_disallowed_type/clippy.toml +++ b/tests/ui-toml/toml_disallowed_type/clippy.toml @@ -6,4 +6,6 @@ disallowed-types = [ "std::thread::Thread", "std::time::Instant", "std::io::Read", + "std::primitive::usize", + "bool" ] diff --git a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs index 567afb5aec1..0871a3073ab 100644 --- a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs +++ b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs @@ -13,13 +13,15 @@ fn bad_return_type() -> fn() -> Sneaky { todo!() } -fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { - todo!() -} +fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) {} -fn trait_obj(_: &dyn std::io::Read) { - todo!() -} +fn trait_obj(_: &dyn std::io::Read) {} + +fn full_and_single_path_prim(_: usize, _: bool) {} + +fn const_generics() {} + +struct GenArg([u8; U]); static BAD: foo::atomic::AtomicPtr<()> = foo::atomic::AtomicPtr::new(std::ptr::null_mut()); @@ -32,4 +34,5 @@ fn main() { let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default(); let _ = syn::Ident::new("", todo!()); let _ = HashMap; + let _: usize = 64_usize; } diff --git a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr index 4e6fd91fba1..90ce7db2cc4 100644 --- a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr +++ b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr @@ -21,68 +21,98 @@ LL | fn bad_return_type() -> fn() -> Sneaky { error: `std::time::Instant` is not allowed according to config --> $DIR/conf_disallowed_type.rs:16:28 | -LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { +LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) {} | ^^^^^^ error: `std::sync::atomic::AtomicU32` is not allowed according to config --> $DIR/conf_disallowed_type.rs:16:39 | -LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { +LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) {} | ^^^^^^^^^^^^^^^^^^^^^^ error: `std::io::Read` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:20:22 + --> $DIR/conf_disallowed_type.rs:18:22 | -LL | fn trait_obj(_: &dyn std::io::Read) { +LL | fn trait_obj(_: &dyn std::io::Read) {} | ^^^^^^^^^^^^^ +error: `usize` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:20:33 + | +LL | fn full_and_single_path_prim(_: usize, _: bool) {} + | ^^^^^ + +error: `bool` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:20:43 + | +LL | fn full_and_single_path_prim(_: usize, _: bool) {} + | ^^^^ + +error: `usize` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:22:28 + | +LL | fn const_generics() {} + | ^^^^^ + +error: `usize` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:24:24 + | +LL | struct GenArg([u8; U]); + | ^^^^^ + error: `std::collections::HashMap` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:28:48 + --> $DIR/conf_disallowed_type.rs:30:48 | LL | let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: `std::collections::HashMap` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:28:12 + --> $DIR/conf_disallowed_type.rs:30:12 | LL | let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `std::time::Instant` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:29:13 + --> $DIR/conf_disallowed_type.rs:31:13 | LL | let _ = Sneaky::now(); | ^^^^^^ error: `std::sync::atomic::AtomicU32` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:30:13 + --> $DIR/conf_disallowed_type.rs:32:13 | LL | let _ = foo::atomic::AtomicU32::new(0); | ^^^^^^^^^^^^^^^^^^^^^^ error: `std::sync::atomic::AtomicU32` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:31:17 + --> $DIR/conf_disallowed_type.rs:33:17 | LL | static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `std::sync::atomic::AtomicU32` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:31:48 + --> $DIR/conf_disallowed_type.rs:33:48 | LL | static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1); | ^^^^^^^^^^^^^^^^^^^^^^ error: `syn::TypePath` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:32:43 + --> $DIR/conf_disallowed_type.rs:34:43 | LL | let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default(); | ^^^^^^^^^^^^^ -error: `proc_macro2::Ident` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:33:13 +error: `syn::Ident` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:35:13 | LL | let _ = syn::Ident::new("", todo!()); | ^^^^^^^^^^ -error: aborting due to 14 previous errors +error: `usize` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:37:12 + | +LL | let _: usize = 64_usize; + | ^^^^^ + +error: aborting due to 19 previous errors