From ea45e2a9cf13b59e6366a6cacb7f0088f9cadeda Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Wed, 2 Jun 2021 07:20:45 -0400 Subject: [PATCH] Add disallowed_types lint, this adds a field to the Conf struct Replace use of node_type -> node_type_opt, fix clippy warnings Don't walk the hir unnecessarily let the visitor do it --- CHANGELOG.md | 1 + clippy_lints/src/disallowed_type.rs | 122 ++++++++++++++++++ clippy_lints/src/lib.rs | 5 + clippy_lints/src/utils/conf.rs | 2 + .../ui-toml/toml_disallowed_type/clippy.toml | 9 ++ .../conf_disallowed_type.rs | 35 +++++ .../conf_disallowed_type.stderr | 88 +++++++++++++ .../toml_unknown_key/conf_unknown_key.stderr | 2 +- 8 files changed, 263 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/disallowed_type.rs create mode 100644 tests/ui-toml/toml_disallowed_type/clippy.toml create mode 100644 tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs create mode 100644 tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 41af8e190dd..13f0fc8f609 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2358,6 +2358,7 @@ Released 2018-09-13 [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord [`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method +[`disallowed_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_type [`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression [`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons diff --git a/clippy_lints/src/disallowed_type.rs b/clippy_lints/src/disallowed_type.rs new file mode 100644 index 00000000000..f8ac7d76d8f --- /dev/null +++ b/clippy_lints/src/disallowed_type.rs @@ -0,0 +1,122 @@ +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_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{Span, Symbol}; + +declare_clippy_lint! { + /// **What it does:** Denies the configured types in clippy.toml. + /// + /// **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. + /// + /// N.B. There is no way to ban primitive types. + /// + /// **Example:** + /// + /// An example clippy.toml configuration: + /// ```toml + /// # clippy.toml + /// disallowed-methods = ["alloc::collections::btree::map::BTreeMap"] + /// ``` + /// + /// ```rust,ignore + /// use std::collections::BTreeMap; + /// // or its use + /// let x = std::collections::BTreeMap::new(); + /// ``` + /// Use instead: + /// ```rust,ignore + /// // A similar type that is allowed by the config + /// use std::collections::HashMap; + /// ``` + pub DISALLOWED_TYPE, + nursery, + "use of a disallowed type" +} +#[derive(Clone, Debug)] +pub struct DisallowedType { + disallowed: FxHashSet>, +} + +impl DisallowedType { + pub fn new(disallowed: &FxHashSet) -> Self { + Self { + disallowed: disallowed + .iter() + .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::>()) + .collect(), + } + } +} + +impl_lint_pass!(DisallowedType => [DISALLOWED_TYPE]); + +impl<'tcx> LateLintPass<'tcx> for DisallowedType { + 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); + then { + emit(cx, name, item.span,); + } + } + } + + fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { + 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); + then { + emit(cx, name, path.span()); + } + } + } + + 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); + 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) { + let name = name.iter().map(|s| s.to_ident_string()).collect::>().join("::"); + span_lint( + cx, + DISALLOWED_TYPE, + span, + &format!("`{}` is not allowed according to config", name), + ); +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e7dd3952b3a..41c635d9e93 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -187,6 +187,7 @@ mod default_numeric_fallback; mod dereference; mod derive; mod disallowed_method; +mod disallowed_type; mod doc; mod double_comparison; mod double_parens; @@ -583,6 +584,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: derive::EXPL_IMPL_CLONE_ON_COPY, derive::UNSAFE_DERIVE_DESERIALIZE, disallowed_method::DISALLOWED_METHOD, + disallowed_type::DISALLOWED_TYPE, doc::DOC_MARKDOWN, doc::MISSING_ERRORS_DOC, doc::MISSING_PANICS_DOC, @@ -1761,6 +1763,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(attrs::EMPTY_LINE_AFTER_OUTER_ATTR), LintId::of(cognitive_complexity::COGNITIVE_COMPLEXITY), LintId::of(disallowed_method::DISALLOWED_METHOD), + LintId::of(disallowed_type::DISALLOWED_TYPE), LintId::of(fallible_impl_from::FALLIBLE_IMPL_FROM), LintId::of(floating_point_arithmetic::IMPRECISE_FLOPS), LintId::of(floating_point_arithmetic::SUBOPTIMAL_FLOPS), @@ -2066,6 +2069,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box if_then_some_else_none::IfThenSomeElseNone::new(msrv)); store.register_early_pass(|| box bool_assert_comparison::BoolAssertComparison); store.register_late_pass(|| box unused_async::UnusedAsync); + let disallowed_types = conf.disallowed_types.iter().cloned().collect::>(); + store.register_late_pass(move || box disallowed_type::DisallowedType::new(&disallowed_types)); } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 0e33ae740d9..e7d901aa9f9 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -190,6 +190,8 @@ define_Conf! { (warn_on_all_wildcard_imports: bool = false), /// Lint: DISALLOWED_METHOD. The list of disallowed methods, written as fully qualified paths. (disallowed_methods: Vec = Vec::new()), + /// Lint: DISALLOWED_TYPE. The list of disallowed types, written as fully qualified paths. + (disallowed_types: Vec = Vec::new()), /// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators. (unreadable_literal_lint_fractions: bool = true), /// Lint: UPPER_CASE_ACRONYMS. Enables verbose mode. Triggers if there is more than one uppercase char next to each other diff --git a/tests/ui-toml/toml_disallowed_type/clippy.toml b/tests/ui-toml/toml_disallowed_type/clippy.toml new file mode 100644 index 00000000000..57dfdc584bb --- /dev/null +++ b/tests/ui-toml/toml_disallowed_type/clippy.toml @@ -0,0 +1,9 @@ +disallowed-types = [ + "std::collections::hash::map::HashMap", + "core::sync::atomic::AtomicU32", + "syn::ty::TypePath", + "proc_macro2::Ident", + "std::thread::Thread", + "std::time::Instant", + "std::io::Read", +] diff --git a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs new file mode 100644 index 00000000000..567afb5aec1 --- /dev/null +++ b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs @@ -0,0 +1,35 @@ +#![warn(clippy::disallowed_type)] + +extern crate quote; +extern crate syn; + +use std::sync as foo; +use std::sync::atomic::AtomicU32; +use std::time::Instant as Sneaky; + +struct HashMap; + +fn bad_return_type() -> fn() -> Sneaky { + todo!() +} + +fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { + todo!() +} + +fn trait_obj(_: &dyn std::io::Read) { + todo!() +} + +static BAD: foo::atomic::AtomicPtr<()> = foo::atomic::AtomicPtr::new(std::ptr::null_mut()); + +#[allow(clippy::diverging_sub_expression)] +fn main() { + let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new(); + let _ = Sneaky::now(); + let _ = foo::atomic::AtomicU32::new(0); + static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1); + let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default(); + let _ = syn::Ident::new("", todo!()); + let _ = HashMap; +} diff --git a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr new file mode 100644 index 00000000000..e5ea23cc0bc --- /dev/null +++ b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr @@ -0,0 +1,88 @@ +error: `core::sync::atomic::AtomicU32` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:7:1 + | +LL | use std::sync::atomic::AtomicU32; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::disallowed-type` implied by `-D warnings` + +error: `std::time::Instant` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:8:1 + | +LL | use std::time::Instant as Sneaky; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `std::time::Instant` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:12:33 + | +LL | fn bad_return_type() -> fn() -> Sneaky { + | ^^^^^^ + +error: `std::time::Instant` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:16:28 + | +LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { + | ^^^^^^ + +error: `core::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) { + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: `std::io::Read` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:20:22 + | +LL | fn trait_obj(_: &dyn std::io::Read) { + | ^^^^^^^^^^^^^ + +error: `std::collections::hash::map::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 + --> $DIR/conf_disallowed_type.rs:28: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:29:13 + | +LL | let _ = Sneaky::now(); + | ^^^^^^ + +error: `core::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 + --> $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 + --> $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 + --> $DIR/conf_disallowed_type.rs:32:43 + | +LL | let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default(); + | ^^^^^^^^^^^^^ + +error: `proc_macro2::Ident` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:33:13 + | +LL | let _ = syn::Ident::new("", todo!()); + | ^^^^^^^^^^ + +error: aborting due to 14 previous errors + diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index a7be00426c4..06d70b66fda 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `third-party` at line 5 column 1 +error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `third-party` at line 5 column 1 error: aborting due to previous error