Auto merge of #7791 - JamesHinshelwood:reason-in-disallowed-type, r=g…

…iraffate Allow giving reasons for `disallowed_types` Similar to #7609 but for the `disallowed_type` lint. The permitted form of configuration is the same as for `disallowed_methods`. changelog: Allow giving reasons for [`disallowed_type`]
rust-lang · Oct 12, 2021 · 77dda9c · 77dda9c
2 parents 7272366 + 886cbb1
commit 77dda9c
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 39 deletions.
diff --git a/clippy_lints/src/disallowed_type.rs b/clippy_lints/src/disallowed_type.rs
@@ -1,12 +1,14 @@
-use clippy_utils::diagnostics::span_lint;
+use clippy_utils::diagnostics::span_lint_and_then;
 
-use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::{
     def::Res, def_id::DefId, Item, ItemKind, PolyTraitRef, PrimTy, TraitBoundModifier, Ty, TyKind, UseKind,
 };
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::{Span, Symbol};
+use rustc_span::Span;
+
+use crate::utils::conf;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -19,7 +21,15 @@ declare_clippy_lint! {
     /// An example clippy.toml configuration:
     /// ```toml
     /// # clippy.toml
-    /// disallowed-types = ["std::collections::BTreeMap"]
+    /// disallowed-types = [
+    ///     # Can use a string as the path of the disallowed type.
+    ///     "std::collections::BTreeMap",
+    ///     # Can also use an inline table with a `path` key.
+    ///     { path = "std::net::TcpListener" },
+    ///     # When using an inline table, can add a `reason` for why the type
+    ///     # is disallowed.
+    ///     { path = "std::net::Ipv4Addr", reason = "no IPv4 allowed" },
+    /// ]
     /// ```
     ///
     /// ```rust,ignore
@@ -38,33 +48,30 @@ declare_clippy_lint! {
 }
 #[derive(Clone, Debug)]
 pub struct DisallowedType {
-    disallowed: FxHashSet<Vec<Symbol>>,
-    def_ids: FxHashSet<DefId>,
-    prim_tys: FxHashSet<PrimTy>,
+    conf_disallowed: Vec<conf::DisallowedType>,
+    def_ids: FxHashMap<DefId, Option<String>>,
+    prim_tys: FxHashMap<PrimTy, Option<String>>,
 }
 
 impl DisallowedType {
-    pub fn new(disallowed: &FxHashSet<String>) -> Self {
+    pub fn new(conf_disallowed: Vec<conf::DisallowedType>) -> Self {
         Self {
-            disallowed: disallowed
-                .iter()
-                .map(|s| s.split("::").map(Symbol::intern).collect::<Vec<_>>())
-                .collect(),
-            def_ids: FxHashSet::default(),
-            prim_tys: FxHashSet::default(),
+            conf_disallowed,
+            def_ids: FxHashMap::default(),
+            prim_tys: FxHashMap::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);
+                if let Some(reason) = self.def_ids.get(did) {
+                    emit(cx, &cx.tcx.def_path_str(*did), span, reason.as_deref());
                 }
             },
             Res::PrimTy(prim) => {
-                if self.prim_tys.contains(prim) {
-                    emit(cx, prim.name_str(), span);
+                if let Some(reason) = self.prim_tys.get(prim) {
+                    emit(cx, prim.name_str(), span, reason.as_deref());
                 }
             },
             _ => {},
@@ -76,14 +83,21 @@ impl_lint_pass!(DisallowedType => [DISALLOWED_TYPE]);
 
 impl<'tcx> LateLintPass<'tcx> for DisallowedType {
     fn check_crate(&mut self, cx: &LateContext<'_>) {
-        for path in &self.disallowed {
-            let segs = path.iter().map(ToString::to_string).collect::<Vec<_>>();
-            match clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::<Vec<_>>()) {
+        for conf in &self.conf_disallowed {
+            let (path, reason) = match conf {
+                conf::DisallowedType::Simple(path) => (path, None),
+                conf::DisallowedType::WithReason { path, reason } => (
+                    path,
+                    reason.as_ref().map(|reason| format!("{} (from clippy.toml)", reason)),
+                ),
+            };
+            let segs: Vec<_> = path.split("::").collect();
+            match clippy_utils::path_to_res(cx, &segs) {
                 Res::Def(_, id) => {
-                    self.def_ids.insert(id);
+                    self.def_ids.insert(id, reason);
                 },
                 Res::PrimTy(ty) => {
-                    self.prim_tys.insert(ty);
+                    self.prim_tys.insert(ty, reason);
                 },
                 _ => {},
             }
@@ -107,11 +121,16 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedType {
     }
 }
 
-fn emit(cx: &LateContext<'_>, name: &str, span: Span) {
-    span_lint(
+fn emit(cx: &LateContext<'_>, name: &str, span: Span, reason: Option<&str>) {
+    span_lint_and_then(
         cx,
         DISALLOWED_TYPE,
         span,
         &format!("`{}` is not allowed according to config", name),
+        |diag| {
+            if let Some(reason) = reason {
+                diag.note(reason);
+            }
+        },
     );
 }
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -757,8 +757,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(bool_assert_comparison::BoolAssertComparison));
     store.register_early_pass(move || Box::new(module_style::ModStyle));
     store.register_late_pass(|| Box::new(unused_async::UnusedAsync));
-    let disallowed_types = conf.disallowed_types.iter().cloned().collect::<FxHashSet<_>>();
-    store.register_late_pass(move || Box::new(disallowed_type::DisallowedType::new(&disallowed_types)));
+    let disallowed_types = conf.disallowed_types.clone();
+    store.register_late_pass(move || Box::new(disallowed_type::DisallowedType::new(disallowed_types.clone())));
     let import_renames = conf.enforced_import_renames.clone();
     store.register_late_pass(move || Box::new(missing_enforced_import_rename::ImportRename::new(import_renames.clone())));
     let scripts = conf.allowed_scripts.clone();

diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
@@ -23,6 +23,14 @@ pub enum DisallowedMethod {
     WithReason { path: String, reason: Option<String> },
 }
 
+/// A single disallowed type, used by the `DISALLOWED_TYPE` lint.
+#[derive(Clone, Debug, Deserialize)]
+#[serde(untagged)]
+pub enum DisallowedType {
+    Simple(String),
+    WithReason { path: String, reason: Option<String> },
+}
+
 /// Conf with parse errors
 #[derive(Default)]
 pub struct TryConf {
@@ -255,7 +263,7 @@ define_Conf! {
     /// Lint: DISALLOWED_TYPE.
     ///
     /// The list of disallowed types, written as fully qualified paths.
-    (disallowed_types: Vec<String> = Vec::new()),
+    (disallowed_types: Vec<crate::utils::conf::DisallowedType> = Vec::new()),
     /// Lint: UNREADABLE_LITERAL.
     ///
     /// Should the fraction of a decimal be linted to include separators.

diff --git a/tests/ui-toml/toml_disallowed_type/clippy.toml b/tests/ui-toml/toml_disallowed_type/clippy.toml
@@ -7,5 +7,9 @@ disallowed-types = [
     "std::time::Instant",
     "std::io::Read",
     "std::primitive::usize",
-    "bool"
+    "bool",
+    # can give path and reason with an inline table
+    { path = "std::net::Ipv4Addr", reason = "no IPv4 allowed" },
+    # can use an inline table but omit reason
+    { path = "std::net::TcpListener" },
 ]
diff --git a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs
@@ -25,6 +25,10 @@ struct GenArg<const U: usize>([u8; U]);
 
 static BAD: foo::atomic::AtomicPtr<()> = foo::atomic::AtomicPtr::new(std::ptr::null_mut());
 
+fn ip(_: std::net::Ipv4Addr) {}
+
+fn listener(_: std::net::TcpListener) {}
+
 #[allow(clippy::diverging_sub_expression)]
 fn main() {
     let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new();

diff --git a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr
@@ -60,59 +60,73 @@ error: `usize` is not allowed according to config
 LL | struct GenArg<const U: usize>([u8; U]);
    |                        ^^^^^
 
+error: `std::net::Ipv4Addr` is not allowed according to config
+  --> $DIR/conf_disallowed_type.rs:28:10
+   |
+LL | fn ip(_: std::net::Ipv4Addr) {}
+   |          ^^^^^^^^^^^^^^^^^^
+   |
+   = note: no IPv4 allowed (from clippy.toml)
+
+error: `std::net::TcpListener` is not allowed according to config
+  --> $DIR/conf_disallowed_type.rs:30:16
+   |
+LL | fn listener(_: std::net::TcpListener) {}
+   |                ^^^^^^^^^^^^^^^^^^^^^
+
 error: `std::collections::HashMap` is not allowed according to config
-  --> $DIR/conf_disallowed_type.rs:30:48
+  --> $DIR/conf_disallowed_type.rs:34: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:30:12
+  --> $DIR/conf_disallowed_type.rs:34: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:31:13
+  --> $DIR/conf_disallowed_type.rs:35:13
    |
 LL |     let _ = Sneaky::now();
    |             ^^^^^^
 
 error: `std::sync::atomic::AtomicU32` is not allowed according to config
-  --> $DIR/conf_disallowed_type.rs:32:13
+  --> $DIR/conf_disallowed_type.rs:36:13
    |
 LL |     let _ = foo::atomic::AtomicU32::new(0);
    |             ^^^^^^^^^^^^^^^^^^^^^^
 
 error: `std::sync::atomic::AtomicU32` is not allowed according to config
-  --> $DIR/conf_disallowed_type.rs:33:17
+  --> $DIR/conf_disallowed_type.rs:37: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:33:48
+  --> $DIR/conf_disallowed_type.rs:37: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:34:43
+  --> $DIR/conf_disallowed_type.rs:38:43
    |
 LL |     let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default();
    |                                           ^^^^^^^^^^^^^
 
 error: `syn::Ident` is not allowed according to config
-  --> $DIR/conf_disallowed_type.rs:35:13
+  --> $DIR/conf_disallowed_type.rs:39:13
    |
 LL |     let _ = syn::Ident::new("", todo!());
    |             ^^^^^^^^^^
 
 error: `usize` is not allowed according to config
-  --> $DIR/conf_disallowed_type.rs:37:12
+  --> $DIR/conf_disallowed_type.rs:41:12
    |
 LL |     let _: usize = 64_usize;
    |            ^^^^^
 
-error: aborting due to 19 previous errors
+error: aborting due to 21 previous errors