From 70ce0c2c55b18f7c9a4bc8d767c626f8d5948fae Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Fri, 11 Jun 2021 17:25:32 -0400 Subject: [PATCH 1/2] Remove requirement of fully qualified path for disallowed_method/type --- clippy_lints/src/disallowed_method.rs | 18 ++++++++++--- clippy_lints/src/disallowed_type.rs | 27 +++++++++++++------ .../toml_disallowed_method/clippy.toml | 6 ++++- .../ui-toml/toml_disallowed_type/clippy.toml | 6 ++--- .../conf_disallowed_type.stderr | 16 +++++------ 5 files changed, 50 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/disallowed_method.rs b/clippy_lints/src/disallowed_method.rs index ded0a0fff54b..76a0cba30e5e 100644 --- a/clippy_lints/src/disallowed_method.rs +++ b/clippy_lints/src/disallowed_method.rs @@ -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; @@ -52,6 +52,7 @@ declare_clippy_lint! { #[derive(Clone, Debug)] pub struct DisallowedMethod { disallowed: FxHashSet>, + def_ids: FxHashSet<(DefId, Vec)>, } impl DisallowedMethod { @@ -61,6 +62,7 @@ impl DisallowedMethod { .iter() .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::>()) .collect(), + def_ids: FxHashSet::default(), } } } @@ -68,10 +70,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::>(); + 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())); + } + } + } + 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) diff --git a/clippy_lints/src/disallowed_type.rs b/clippy_lints/src/disallowed_type.rs index f8ac7d76d8fb..ab775d99b084 100644 --- a/clippy_lints/src/disallowed_type.rs +++ b/clippy_lints/src/disallowed_type.rs @@ -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}; @@ -47,6 +49,7 @@ declare_clippy_lint! { #[derive(Clone, Debug)] pub struct DisallowedType { disallowed: FxHashSet>, + def_ids: FxHashSet<(DefId, Vec)>, } impl DisallowedType { @@ -56,6 +59,7 @@ impl DisallowedType { .iter() .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::>()) .collect(), + def_ids: FxHashSet::default(), } } } @@ -63,12 +67,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::>(); + 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())); + } + } + } + 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 +92,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 +102,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); } diff --git a/tests/ui-toml/toml_disallowed_method/clippy.toml b/tests/ui-toml/toml_disallowed_method/clippy.toml index c0df3b6e8af5..a3245da68250 100644 --- a/tests/ui-toml/toml_disallowed_method/clippy.toml +++ b/tests/ui-toml/toml_disallowed_method/clippy.toml @@ -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" +] diff --git a/tests/ui-toml/toml_disallowed_type/clippy.toml b/tests/ui-toml/toml_disallowed_type/clippy.toml index 57dfdc584bb3..2eff854c22c3 100644 --- a/tests/ui-toml/toml_disallowed_type/clippy.toml +++ b/tests/ui-toml/toml_disallowed_type/clippy.toml @@ -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", 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 e5ea23cc0bca..4e6fd91fba11 100644 --- a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr +++ b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr @@ -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(); From d4eff81282d4784951ae52ffc3b8fe1c948f872e Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Sun, 13 Jun 2021 15:52:54 -0400 Subject: [PATCH 2/2] fixup! Remove requirement of fully qualified path for disallowed_method/type --- clippy_lints/src/disallowed_method.rs | 11 ++--------- clippy_lints/src/disallowed_type.rs | 11 ++--------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/disallowed_method.rs b/clippy_lints/src/disallowed_method.rs index 76a0cba30e5e..aa1a609afedc 100644 --- a/clippy_lints/src/disallowed_method.rs +++ b/clippy_lints/src/disallowed_method.rs @@ -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 diff --git a/clippy_lints/src/disallowed_type.rs b/clippy_lints/src/disallowed_type.rs index ab775d99b084..e4a88c6324eb 100644 --- a/clippy_lints/src/disallowed_type.rs +++ b/clippy_lints/src/disallowed_type.rs @@ -13,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. /// @@ -29,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