Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add missing trait implementation lints #3752

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
};
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
130 changes: 130 additions & 0 deletions clippy_lints/src/missing_traits.rs
Original file line number Diff line number Diff line change
@@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where you give a lint its category

concat!("detects missing implementations of ", stringify!($trait_path))
}

#[derive(Default)]
pub struct $struct_name {
impling_types: Option<NodeSet>,
}

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);
1 change: 1 addition & 0 deletions tests/run-pass/ice-2774.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::missing_copy_implementations)]
use std::collections::HashSet;

// See https://github.com/rust-lang/rust-clippy/issues/2774
Expand Down
3 changes: 3 additions & 0 deletions tests/run-pass/ice-3151.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![allow(clippy::missing_copy_implementations)]
#![allow(clippy::missing_debug_implementations)]

#[derive(Clone)]
pub struct HashMap<V, S> {
hash_builder: S,
Expand Down
1 change: 1 addition & 0 deletions tests/run-pass/needless_borrow_fp.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#[deny(clippy::all)]
#[allow(clippy::missing_copy_implementations)]
#[derive(Debug)]
pub enum Error {
Type(&'static str),
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/missing_traits_copy.rs
Original file line number Diff line number Diff line change
@@ -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>(T);

fn main() {}
16 changes: 16 additions & 0 deletions tests/ui/missing_traits_copy.stderr
Original file line number Diff line number Diff line change
@@ -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

38 changes: 38 additions & 0 deletions tests/ui/missing_traits_debug.rs
Original file line number Diff line number Diff line change
@@ -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>(T);

fn main() {}
16 changes: 16 additions & 0 deletions tests/ui/missing_traits_debug.stderr
Original file line number Diff line number Diff line change
@@ -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