Auto merge of #7345 - DevinR528:disallowed-fix, r=Manishearth

Remove requirement of fully qualified path for disallowed_method/type

changelog: Remove the need for fully qualified paths in disallowed_method and disallowed_type

After fixing this issue in [import_rename](https://github.com/rust-lang/rust-clippy/pull/7300#discussion_r650127720) I figured I'd fix it for these two.

If this does in fact fix the **Known problems:** section I was planning to remove them from both lints after confirmation.
This commit is contained in:
bors 2021-06-14 14:57:56 +00:00
commit 2d1e9ab980
5 changed files with 54 additions and 41 deletions

View File

@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint;
use clippy_utils::fn_def_id;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::Expr;
use rustc_hir::{def::Res, def_id::DefId, Crate, Expr};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::Symbol;
@ -13,21 +13,14 @@ declare_clippy_lint! {
/// **Why is this bad?** Some methods are undesirable in certain contexts,
/// and it's beneficial to lint for them as needed.
///
/// **Known problems:** Currently, you must write each function as a
/// fully-qualified path. This lint doesn't support aliases or reexported
/// names; be aware that many types in `std` are actually reexports.
///
/// For example, if you want to disallow `Duration::as_secs`, your clippy.toml
/// configuration would look like
/// `disallowed-methods = ["core::time::Duration::as_secs"]` and not
/// `disallowed-methods = ["std::time::Duration::as_secs"]` as you might expect.
/// **Known problems:** None.
///
/// **Example:**
///
/// An example clippy.toml configuration:
/// ```toml
/// # clippy.toml
/// disallowed-methods = ["alloc::vec::Vec::leak", "std::time::Instant::now"]
/// disallowed-methods = ["std::vec::Vec::leak", "std::time::Instant::now"]
/// ```
///
/// ```rust,ignore
@ -52,6 +45,7 @@ declare_clippy_lint! {
#[derive(Clone, Debug)]
pub struct DisallowedMethod {
disallowed: FxHashSet<Vec<Symbol>>,
def_ids: FxHashSet<(DefId, Vec<Symbol>)>,
}
impl DisallowedMethod {
@ -61,6 +55,7 @@ impl DisallowedMethod {
.iter()
.map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>())
.collect(),
def_ids: FxHashSet::default(),
}
}
}
@ -68,10 +63,20 @@ impl DisallowedMethod {
impl_lint_pass!(DisallowedMethod => [DISALLOWED_METHOD]);
impl<'tcx> LateLintPass<'tcx> for DisallowedMethod {
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()));
}
}
}
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(def_id) = fn_def_id(cx, expr) {
let func_path = cx.get_def_path(def_id);
if self.disallowed.contains(&func_path) {
if self.def_ids.iter().any(|(id, _)| def_id == *id) {
let func_path = cx.get_def_path(def_id);
let func_path_string = func_path
.into_iter()
.map(Symbol::to_ident_string)

View File

@ -1,7 +1,9 @@
use clippy_utils::diagnostics::span_lint;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::{def::Res, Item, ItemKind, PolyTraitRef, TraitBoundModifier, Ty, TyKind, UseKind};
use rustc_hir::{
def::Res, def_id::DefId, Crate, Item, ItemKind, PolyTraitRef, TraitBoundModifier, Ty, TyKind, UseKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{Span, Symbol};
@ -11,14 +13,7 @@ declare_clippy_lint! {
///
/// **Why is this bad?** Some types are undesirable in certain contexts.
///
/// **Known problems:** The fully qualified path must be used. This lint
/// doesn't support aliases or reexported names; be aware that many types
/// in `std` are actually reexports.
///
/// For example, if you want to disallow `BTreeMap`, your clippy.toml
/// configuration would look like
/// `disallowed-methods = ["alloc::collections::btree::map::BTreeMap"]` and not
/// `disallowed-methods = ["std::collections::BTreeMap"]` as you might expect.
/// **Known problems:** None.
///
/// N.B. There is no way to ban primitive types.
///
@ -27,7 +22,7 @@ declare_clippy_lint! {
/// An example clippy.toml configuration:
/// ```toml
/// # clippy.toml
/// disallowed-methods = ["alloc::collections::btree::map::BTreeMap"]
/// disallowed-methods = ["std::collections::BTreeMap"]
/// ```
///
/// ```rust,ignore
@ -47,6 +42,7 @@ declare_clippy_lint! {
#[derive(Clone, Debug)]
pub struct DisallowedType {
disallowed: FxHashSet<Vec<Symbol>>,
def_ids: FxHashSet<(DefId, Vec<Symbol>)>,
}
impl DisallowedType {
@ -56,6 +52,7 @@ impl DisallowedType {
.iter()
.map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>())
.collect(),
def_ids: FxHashSet::default(),
}
}
}
@ -63,12 +60,21 @@ impl DisallowedType {
impl_lint_pass!(DisallowedType => [DISALLOWED_TYPE]);
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()));
}
}
}
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(_, id) = path.res;
let use_path = cx.get_def_path(id);
if let Some(name) = self.disallowed.iter().find(|path| **path == use_path);
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,);
}
@ -79,8 +85,7 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedType {
if_chain! {
if let TyKind::Path(path) = &ty.kind;
if let Some(did) = cx.qpath_res(path, ty.hir_id).opt_def_id();
let use_path = cx.get_def_path(did);
if let Some(name) = self.disallowed.iter().find(|path| **path == use_path);
if let Some((_, name)) = self.def_ids.iter().find(|(id, _)| *id == did);
then {
emit(cx, name, path.span());
}
@ -90,8 +95,7 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedType {
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;
let use_path = cx.get_def_path(did);
if let Some(name) = self.disallowed.iter().find(|path| **path == use_path);
if let Some((_, name)) = self.def_ids.iter().find(|(id, _)| *id == did);
then {
emit(cx, name, poly.trait_ref.path.span);
}

View File

@ -1 +1,5 @@
disallowed-methods = ["core::iter::traits::iterator::Iterator::sum", "regex::re_unicode::Regex::is_match", "regex::re_unicode::Regex::new"]
disallowed-methods = [
"std::iter::Iterator::sum",
"regex::Regex::is_match",
"regex::Regex::new"
]

View File

@ -1,7 +1,7 @@
disallowed-types = [
"std::collections::hash::map::HashMap",
"core::sync::atomic::AtomicU32",
"syn::ty::TypePath",
"std::collections::HashMap",
"std::sync::atomic::AtomicU32",
"syn::TypePath",
"proc_macro2::Ident",
"std::thread::Thread",
"std::time::Instant",

View File

@ -1,4 +1,4 @@
error: `core::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:7:1
|
LL | use std::sync::atomic::AtomicU32;
@ -24,7 +24,7 @@ error: `std::time::Instant` is not allowed according to config
LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) {
| ^^^^^^
error: `core::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
|
LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) {
@ -36,13 +36,13 @@ error: `std::io::Read` is not allowed according to config
LL | fn trait_obj(_: &dyn std::io::Read) {
| ^^^^^^^^^^^^^
error: `std::collections::hash::map::HashMap` is not allowed according to config
error: `std::collections::HashMap` is not allowed according to config
--> $DIR/conf_disallowed_type.rs:28:48
|
LL | let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: `std::collections::hash::map::HashMap` is not allowed according to config
error: `std::collections::HashMap` is not allowed according to config
--> $DIR/conf_disallowed_type.rs:28:12
|
LL | let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new();
@ -54,25 +54,25 @@ error: `std::time::Instant` is not allowed according to config
LL | let _ = Sneaky::now();
| ^^^^^^
error: `core::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
|
LL | let _ = foo::atomic::AtomicU32::new(0);
| ^^^^^^^^^^^^^^^^^^^^^^
error: `core::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
|
LL | static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: `core::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
|
LL | static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1);
| ^^^^^^^^^^^^^^^^^^^^^^
error: `syn::ty::TypePath` is not allowed according to config
error: `syn::TypePath` is not allowed according to config
--> $DIR/conf_disallowed_type.rs:32:43
|
LL | let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default();