Add primitive type support to disallowed_type lint

DevinR528 · DevinR528 · commit a1bab3bc63d1 · 2021-07-27T10:01:38.000-04:00
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
diff --git a/clippy_lints/src/disallowed_type.rs b/clippy_lints/src/disallowed_type.rs
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint;
 
 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 @@ declare_clippy_lint! {
     ///
     /// **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 @@ declare_clippy_lint! {
 #[derive(Clone, Debug)]
 pub struct DisallowedType {
     disallowed: FxHashSet<Vec<Symbol>>,
-    def_ids: FxHashSet<(DefId, Vec<Symbol>)>,
+    def_ids: FxHashSet<DefId>,
+    prim_tys: FxHashSet<PrimTy>,
 }
 
 impl DisallowedType {
@@ -53,6 +52,23 @@ impl DisallowedType {
                 .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>())
                 .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::<Vec<_>>();
-            if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::<Vec<_>>())
-            {
-                self.def_ids.insert((id, path.clone()));
+            match clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::<Vec<_>>()) {
+                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::<Vec<_>>().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
@@ -72,7 +72,7 @@ use rustc_hir::LangItem::{ResultErr, ResultOk};
 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 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Res {
 
     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
@@ -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
@@ -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<const C: usize>() {}
+
+struct GenArg<const U: usize>([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
@@ -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<const C: usize>() {}
+   |                            ^^^^^
+
+error: `usize` is not allowed according to config
+  --> $DIR/conf_disallowed_type.rs:24:24
+   |
+LL | struct GenArg<const U: usize>([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
 

Original file line number	Diff line number	Diff line change
`@@ -6,4 +6,6 @@ disallowed-types = [`
`6`	`6`	`"std::thread::Thread",`
`7`	`7`	`"std::time::Instant",`
`8`	`8`	`"std::io::Read",`
	`9`	`+ "std::primitive::usize",`
	`10`	`+ "bool"`
`9`	`11`	`]`