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
This commit is contained in:
Devin Ragotzy 2021-07-22 08:40:24 -04:00
parent ceb7a868d3
commit a1bab3bc63
5 changed files with 94 additions and 64 deletions

View File

@ -2,7 +2,7 @@
use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::fx::FxHashSet;
use rustc_hir::{ 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_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_session::{declare_tool_lint, impl_lint_pass};
@ -15,14 +15,12 @@
/// ///
/// **Known problems:** None. /// **Known problems:** None.
/// ///
/// N.B. There is no way to ban primitive types.
///
/// **Example:** /// **Example:**
/// ///
/// An example clippy.toml configuration: /// An example clippy.toml configuration:
/// ```toml /// ```toml
/// # clippy.toml /// # clippy.toml
/// disallowed-methods = ["std::collections::BTreeMap"] /// disallowed-types = ["std::collections::BTreeMap"]
/// ``` /// ```
/// ///
/// ```rust,ignore /// ```rust,ignore
@ -42,7 +40,8 @@
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct DisallowedType { pub struct DisallowedType {
disallowed: FxHashSet<Vec<Symbol>>, disallowed: FxHashSet<Vec<Symbol>>,
def_ids: FxHashSet<(DefId, Vec<Symbol>)>, def_ids: FxHashSet<DefId>,
prim_tys: FxHashSet<PrimTy>,
} }
impl DisallowedType { impl DisallowedType {
@ -53,6 +52,23 @@ pub fn new(disallowed: &FxHashSet<String>) -> Self {
.map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>()) .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>())
.collect(), .collect(),
def_ids: FxHashSet::default(), 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<'_>) { fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) {
for path in &self.disallowed { for path in &self.disallowed {
let segs = path.iter().map(ToString::to_string).collect::<Vec<_>>(); 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<_>>()) match clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::<Vec<_>>()) {
{ Res::Def(_, id) => {
self.def_ids.insert((id, path.clone())); 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>) { 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 ItemKind::Use(path, UseKind::Single) = &item.kind; self.check_res_emit(cx, &path.res, item.span);
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,);
}
} }
} }
fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) {
if_chain! { if let TyKind::Path(path) = &ty.kind {
if let TyKind::Path(path) = &ty.kind; self.check_res_emit(cx, &cx.qpath_res(path, ty.hir_id), ty.span);
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());
}
} }
} }
fn check_poly_trait_ref(&mut self, cx: &LateContext<'tcx>, poly: &'tcx PolyTraitRef<'tcx>, _: TraitBoundModifier) { fn check_poly_trait_ref(&mut self, cx: &LateContext<'tcx>, poly: &'tcx PolyTraitRef<'tcx>, _: TraitBoundModifier) {
if_chain! { self.check_res_emit(cx, &poly.trait_ref.path.res, poly.trait_ref.path.span);
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);
}
}
} }
// 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) { fn emit(cx: &LateContext<'_>, name: &str, span: Span) {
let name = name.iter().map(|s| s.to_ident_string()).collect::<Vec<_>>().join("::");
span_lint( span_lint(
cx, cx,
DISALLOWED_TYPE, DISALLOWED_TYPE,

View File

@ -72,7 +72,7 @@
use rustc_hir::{ use rustc_hir::{
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl, 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, 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_lint::{LateContext, Level, Lint, LintContext};
use rustc_middle::hir::exports::Export; 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 { let (krate, first, path) = match *path {
[krate, first, ref path @ ..] => (krate, first, 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, _ => return Res::Err,
}; };
let tcx = cx.tcx; let tcx = cx.tcx;

View File

@ -6,4 +6,6 @@ disallowed-types = [
"std::thread::Thread", "std::thread::Thread",
"std::time::Instant", "std::time::Instant",
"std::io::Read", "std::io::Read",
"std::primitive::usize",
"bool"
] ]

View File

@ -13,13 +13,15 @@ fn bad_return_type() -> fn() -> Sneaky {
todo!() todo!()
} }
fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) {}
todo!()
}
fn trait_obj(_: &dyn std::io::Read) { fn trait_obj(_: &dyn std::io::Read) {}
todo!()
} 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()); 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 _: std::collections::BTreeMap<(), syn::TypePath> = Default::default();
let _ = syn::Ident::new("", todo!()); let _ = syn::Ident::new("", todo!());
let _ = HashMap; let _ = HashMap;
let _: usize = 64_usize;
} }

View File

@ -21,68 +21,98 @@ LL | fn bad_return_type() -> fn() -> Sneaky {
error: `std::time::Instant` is not allowed according to config error: `std::time::Instant` is not allowed according to config
--> $DIR/conf_disallowed_type.rs:16:28 --> $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 error: `std::sync::atomic::AtomicU32` is not allowed according to config
--> $DIR/conf_disallowed_type.rs:16:39 --> $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 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 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(); LL | let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^
error: `std::collections::HashMap` is not allowed according to config 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(); LL | let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: `std::time::Instant` is not allowed according to config 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(); LL | let _ = Sneaky::now();
| ^^^^^^ | ^^^^^^
error: `std::sync::atomic::AtomicU32` is not allowed according to config 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); LL | let _ = foo::atomic::AtomicU32::new(0);
| ^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^
error: `std::sync::atomic::AtomicU32` is not allowed according to config 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); LL | static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: `std::sync::atomic::AtomicU32` is not allowed according to config 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); LL | static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1);
| ^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^
error: `syn::TypePath` is not allowed according to config 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(); LL | let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default();
| ^^^^^^^^^^^^^ | ^^^^^^^^^^^^^
error: `proc_macro2::Ident` is not allowed according to config error: `syn::Ident` is not allowed according to config
--> $DIR/conf_disallowed_type.rs:33:13 --> $DIR/conf_disallowed_type.rs:35:13
| |
LL | let _ = syn::Ident::new("", todo!()); 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