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

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`]
This commit is contained in:
bors 2021-10-12 00:17:37 +00:00
commit 77dda9c6b1
6 changed files with 88 additions and 39 deletions

View file

@ -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);
}
},
);
}

View file

@ -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();

View file

@ -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.

View file

@ -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" },
]

View file

@ -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();

View file

@ -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