From a593366694d3de88f648150ec00b3f284d8ca500 Mon Sep 17 00:00:00 2001 From: Bryant Mairs Date: Fri, 8 Feb 2019 20:24:57 -0800 Subject: [PATCH 1/2] Allow macro calls in declare_clippy_lint!() --- clippy_lints/src/lib.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5a9364eddb69..b29abe9ba8a4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -41,34 +41,34 @@ use toml; // as said in the README.md of this repository. If this changes, please update README.md. #[macro_export] macro_rules! declare_clippy_lint { - { pub $name:tt, style, $description:tt } => { + { pub $name:tt, style, $description:expr } => { declare_tool_lint! { pub clippy::$name, Warn, $description, report_in_external_macro: true } }; - { pub $name:tt, correctness, $description:tt } => { + { pub $name:tt, correctness, $description:expr } => { declare_tool_lint! { pub clippy::$name, Deny, $description, report_in_external_macro: true } }; - { pub $name:tt, complexity, $description:tt } => { + { pub $name:tt, complexity, $description:expr } => { declare_tool_lint! { pub clippy::$name, Warn, $description, report_in_external_macro: true } }; - { pub $name:tt, perf, $description:tt } => { + { pub $name:tt, perf, $description:expr } => { declare_tool_lint! { pub clippy::$name, Warn, $description, report_in_external_macro: true } }; - { pub $name:tt, pedantic, $description:tt } => { + { pub $name:tt, pedantic, $description:expr } => { declare_tool_lint! { pub clippy::$name, Allow, $description, report_in_external_macro: true } }; - { pub $name:tt, restriction, $description:tt } => { + { pub $name:tt, restriction, $description:expr } => { declare_tool_lint! { pub clippy::$name, Allow, $description, report_in_external_macro: true } }; - { pub $name:tt, cargo, $description:tt } => { + { pub $name:tt, cargo, $description:expr } => { declare_tool_lint! { pub clippy::$name, Allow, $description, report_in_external_macro: true } }; - { pub $name:tt, nursery, $description:tt } => { + { pub $name:tt, nursery, $description:expr } => { declare_tool_lint! { pub clippy::$name, Allow, $description, report_in_external_macro: true } }; - { pub $name:tt, internal, $description:tt } => { + { pub $name:tt, internal, $description:expr } => { declare_tool_lint! { pub clippy::$name, Allow, $description, report_in_external_macro: true } }; - { pub $name:tt, internal_warn, $description:tt } => { + { pub $name:tt, internal_warn, $description:expr } => { declare_tool_lint! { pub clippy::$name, Warn, $description, report_in_external_macro: true } }; } From 992e1cc6abb8b6680d59c3d2b8b4d2c6ec330065 Mon Sep 17 00:00:00 2001 From: Bryant Mairs Date: Fri, 8 Feb 2019 20:21:32 -0800 Subject: [PATCH 2/2] Add lint for missing Copy and Debug impls --- clippy_lints/src/lib.rs | 5 ++ clippy_lints/src/missing_traits.rs | 130 +++++++++++++++++++++++++++ tests/run-pass/ice-2774.rs | 1 + tests/run-pass/ice-3151.rs | 3 + tests/run-pass/needless_borrow_fp.rs | 1 + tests/ui/missing_traits_copy.rs | 36 ++++++++ tests/ui/missing_traits_copy.stderr | 16 ++++ tests/ui/missing_traits_debug.rs | 38 ++++++++ tests/ui/missing_traits_debug.stderr | 16 ++++ 9 files changed, 246 insertions(+) create mode 100644 clippy_lints/src/missing_traits.rs create mode 100644 tests/ui/missing_traits_copy.rs create mode 100644 tests/ui/missing_traits_copy.stderr create mode 100644 tests/ui/missing_traits_debug.rs create mode 100644 tests/ui/missing_traits_debug.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b29abe9ba8a4..436a563086e3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -150,6 +150,7 @@ pub mod misc_early; pub mod missing_const_for_fn; pub mod missing_doc; pub mod missing_inline; +pub mod missing_traits; pub mod multiple_crate_versions; pub mod mut_mut; pub mod mut_reference; @@ -493,6 +494,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box types::RefToMut); reg.register_late_lint_pass(box assertions_on_constants::AssertionsOnConstants); reg.register_late_lint_pass(box missing_const_for_fn::MissingConstForFn); + reg.register_late_lint_pass(box missing_traits::MissingCopyImplementations::default()); + reg.register_late_lint_pass(box missing_traits::MissingDebugImplementations::default()); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -706,6 +709,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { misc_early::REDUNDANT_CLOSURE_CALL, misc_early::UNNEEDED_FIELD_PATTERN, misc_early::ZERO_PREFIXED_LITERAL, + missing_traits::MISSING_COPY_IMPLEMENTATIONS, + missing_traits::MISSING_DEBUG_IMPLEMENTATIONS, mut_reference::UNNECESSARY_MUT_PASSED, mutex_atomic::MUTEX_ATOMIC, needless_bool::BOOL_COMPARISON, diff --git a/clippy_lints/src/missing_traits.rs b/clippy_lints/src/missing_traits.rs new file mode 100644 index 000000000000..3d8698591212 --- /dev/null +++ b/clippy_lints/src/missing_traits.rs @@ -0,0 +1,130 @@ +use crate::utils::span_lint; +use rustc::hir::*; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc::{declare_tool_lint, lint_array}; +use rustc::util::nodemap::NodeSet; + +macro_rules! missing_impl { + ($trait:ident, $trait_path:path, $lint_constant:ident, $struct_name:ident, + $trait_check:ident) => ( + declare_clippy_lint! { + pub $lint_constant, + correctness, + concat!("detects missing implementations of ", stringify!($trait_path)) + } + + #[derive(Default)] + pub struct $struct_name { + impling_types: Option, + } + + impl $struct_name { + pub fn new() -> $struct_name { + $struct_name { impling_types: None } + } + } + + impl LintPass for $struct_name { + fn name(&self) -> &'static str { + stringify!($struct_name) + } + + fn get_lints(&self) -> LintArray { + lint_array!($lint_constant) + } + } + + impl<'a, 'tcx> LateLintPass<'a, 'tcx> for $struct_name { + fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) { + if !cx.access_levels.is_reachable(item.id) { + return; + } + + match item.node { + ItemKind::Struct(..) | + ItemKind::Union(..) | + ItemKind::Enum(..) => {} + _ => return, + } + + let x = match cx.tcx.lang_items().$trait_check() { + Some(x) => x, + None => return, + }; + + if self.impling_types.is_none() { + let mut impls = NodeSet::default(); + cx.tcx.for_each_impl(x, |d| { + if let Some(ty_def) = cx.tcx.type_of(d).ty_adt_def() { + if let Some(node_id) = cx.tcx.hir().as_local_node_id(ty_def.did) { + impls.insert(node_id); + } + } + }); + + self.impling_types = Some(impls); + } + + if !self.impling_types.as_ref().unwrap().contains(&item.id) { + span_lint( + cx, + $lint_constant, + item.span, + concat!("type does not implement `", stringify!($trait_path), "`; \ + consider adding #[derive(", stringify!($trait), ")] or a manual \ + implementation")) + } + } + } + ) +} + +/// **What it does:** Checks for `Copy` implementations missing from structs and enums +/// +/// **Why is this bad?** `Copy` is a core trait that should be implemented for +/// all types as much as possible. +/// +/// For more, see the [Rust API Guidelines](https://rust-lang-nursery.github.io/api-guidelines/interoperability.html#c-common-traits) +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// // Bad +/// struct Foo; +/// +/// // Good +/// #[derive(Copy)] +/// struct Bar; +/// ``` +missing_impl!( + Copy, + Copy, + MISSING_COPY_IMPLEMENTATIONS, + MissingCopyImplementations, + copy_trait); + +/// **What it does:** Checks for `Debug` implementations missing from structs and enums +/// +/// **Why is this bad?** `Debug` is a core trait that should be implemented for +/// all types as much as possible. +/// +/// For more, see the [Rust API Guidelines](https://rust-lang-nursery.github.io/api-guidelines/interoperability.html#c-common-traits) +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// // Bad +/// struct Foo; +/// +/// // Good +/// #[derive(Debug)] +/// struct Bar; +/// ``` +missing_impl!( + Debug, + Debug, + MISSING_DEBUG_IMPLEMENTATIONS, + MissingDebugImplementations, + debug_trait); diff --git a/tests/run-pass/ice-2774.rs b/tests/run-pass/ice-2774.rs index 2cc19ae32b80..bfb2600e5b13 100644 --- a/tests/run-pass/ice-2774.rs +++ b/tests/run-pass/ice-2774.rs @@ -1,3 +1,4 @@ +#![allow(clippy::missing_copy_implementations)] use std::collections::HashSet; // See https://github.com/rust-lang/rust-clippy/issues/2774 diff --git a/tests/run-pass/ice-3151.rs b/tests/run-pass/ice-3151.rs index a03dd05e7d31..a79c23b1146e 100644 --- a/tests/run-pass/ice-3151.rs +++ b/tests/run-pass/ice-3151.rs @@ -1,3 +1,6 @@ +#![allow(clippy::missing_copy_implementations)] +#![allow(clippy::missing_debug_implementations)] + #[derive(Clone)] pub struct HashMap { hash_builder: S, diff --git a/tests/run-pass/needless_borrow_fp.rs b/tests/run-pass/needless_borrow_fp.rs index 4f61c76828db..98778833a2ac 100644 --- a/tests/run-pass/needless_borrow_fp.rs +++ b/tests/run-pass/needless_borrow_fp.rs @@ -1,4 +1,5 @@ #[deny(clippy::all)] +#[allow(clippy::missing_copy_implementations)] #[derive(Debug)] pub enum Error { Type(&'static str), diff --git a/tests/ui/missing_traits_copy.rs b/tests/ui/missing_traits_copy.rs new file mode 100644 index 000000000000..218eb126127a --- /dev/null +++ b/tests/ui/missing_traits_copy.rs @@ -0,0 +1,36 @@ +#![feature(untagged_unions)] +#![allow(dead_code)] +#![allow(clippy::all)] +#![warn(clippy::missing_copy_implementations)] + +pub enum A {} + +#[derive(Clone, Copy)] +pub enum B {} + +pub enum C {} + +impl Copy for C {} +impl Clone for C { + fn clone(&self) -> C { *self } +} + +pub struct Foo; + +#[derive(Clone, Copy)] +pub struct Bar; + +pub struct Baz; + +impl Copy for Baz {} +impl Clone for Baz { + fn clone(&self) -> Baz { *self } +} + +struct PrivateStruct; + +enum PrivateEnum {} + +struct GenericType(T); + +fn main() {} diff --git a/tests/ui/missing_traits_copy.stderr b/tests/ui/missing_traits_copy.stderr new file mode 100644 index 000000000000..5b0c55cbe4de --- /dev/null +++ b/tests/ui/missing_traits_copy.stderr @@ -0,0 +1,16 @@ +error: type does not implement `Copy`; consider adding #[derive(Copy)] or a manual implementation + --> $DIR/missing_traits_copy.rs:6:1 + | +LL | pub enum A {} + | ^^^^^^^^^^^^^ + | + = note: `-D clippy::missing-copy-implementations` implied by `-D warnings` + +error: type does not implement `Copy`; consider adding #[derive(Copy)] or a manual implementation + --> $DIR/missing_traits_copy.rs:18:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/missing_traits_debug.rs b/tests/ui/missing_traits_debug.rs new file mode 100644 index 000000000000..604b5c25db97 --- /dev/null +++ b/tests/ui/missing_traits_debug.rs @@ -0,0 +1,38 @@ +#![feature(untagged_unions)] +#![allow(dead_code)] +#![allow(clippy::all)] +#![warn(clippy::missing_debug_implementations)] + +pub enum A {} + +#[derive(Debug)] +pub enum B {} + +pub enum C {} + +impl std::fmt::Debug for C { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.debug_struct("C").finish() + } +} + +pub struct Foo; + +#[derive(Debug)] +pub struct Bar; + +pub struct Baz; + +impl std::fmt::Debug for Baz { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.debug_struct("Baz").finish() + } +} + +struct PrivateStruct; + +enum PrivateEnum {} + +struct GenericType(T); + +fn main() {} diff --git a/tests/ui/missing_traits_debug.stderr b/tests/ui/missing_traits_debug.stderr new file mode 100644 index 000000000000..3d7fb2516c9a --- /dev/null +++ b/tests/ui/missing_traits_debug.stderr @@ -0,0 +1,16 @@ +error: type does not implement `Debug`; consider adding #[derive(Debug)] or a manual implementation + --> $DIR/missing_traits_debug.rs:6:1 + | +LL | pub enum A {} + | ^^^^^^^^^^^^^ + | + = note: `-D clippy::missing-debug-implementations` implied by `-D warnings` + +error: type does not implement `Debug`; consider adding #[derive(Debug)] or a manual implementation + --> $DIR/missing_traits_debug.rs:19:1 + | +LL | pub struct Foo; + | ^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors +