Skip to content

adding to restriction a lint that check for multiple inherent implementations #2816

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

Merged
merged 3 commits into from
May 31, 2018
Merged
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
11 changes: 7 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ before_install:
fi

install:
- . $HOME/.nvm/nvm.sh
- nvm install stable
- nvm use stable
- npm install remark-cli remark-lint
- |
if [ -z ${INTEGRATION} ]; then
. $HOME/.nvm/nvm.sh
nvm install stable
nvm use stable
npm install remark-cli remark-lint
fi

matrix:
include:
Expand Down
95 changes: 95 additions & 0 deletions clippy_lints/src/inherent_impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
//! lint on inherent implementations

use rustc::hir::*;
use rustc::lint::*;
use std::collections::HashMap;
use std::default::Default;
use syntax_pos::Span;

/// **What it does:** Checks for multiple inherent implementations of a struct
///
/// **Why is this bad?** Splitting the implementation of a type makes the code harder to navigate.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// struct X;
/// impl X {
/// fn one() {}
/// }
/// impl X {
/// fn other() {}
/// }
/// ```
///
/// Could be written:
///
/// ```rust
/// struct X;
/// impl X {
/// fn one() {}
/// fn other() {}
/// }
/// ```
declare_clippy_lint! {
pub MULTIPLE_INHERENT_IMPL,
restriction,
"Multiple inherent impl that could be grouped"
}

pub struct Pass {
impls: HashMap<def_id::DefId, (Span, Generics)>,
}

impl Default for Pass {
fn default() -> Self {
Pass { impls: HashMap::new() }
}
}

impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(MULTIPLE_INHERENT_IMPL)
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
fn check_item(&mut self, _: &LateContext<'a, 'tcx>, item: &'tcx Item) {
if let Item_::ItemImpl(_, _, _, ref generics, None, _, _) = item.node {
// Remember for each inherent implementation encoutered its span and generics
self.impls
.insert(item.hir_id.owner_def_id(), (item.span, generics.clone()));
}
}

fn check_crate_post(&mut self, cx: &LateContext<'a, 'tcx>, krate: &'tcx Crate) {
if let Some(item) = krate.items.values().nth(0) {
// Retrieve all inherent implementations from the crate, grouped by type
for impls in cx
.tcx
.crate_inherent_impls(item.hir_id.owner_def_id().krate)
.inherent_impls
.values()
{
// Filter out implementations that have generic params (type or lifetime)
let mut impl_spans = impls
.iter()
.filter_map(|impl_def| self.impls.get(impl_def))
.filter(|(_, generics)| generics.params.len() == 0)
.map(|(span, _)| span);
if let Some(initial_span) = impl_spans.nth(0) {
impl_spans.for_each(|additional_span| {
cx.span_lint_note(
MULTIPLE_INHERENT_IMPL,
*additional_span,
"Multiple implementations of this structure",
*initial_span,
"First implementation here",
)
})
}
}
}
}
}
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ pub mod if_let_redundant_pattern_matching;
pub mod if_not_else;
pub mod infallible_destructuring_match;
pub mod infinite_iter;
pub mod inherent_impl;
pub mod inline_fn_without_body;
pub mod int_plus_one;
pub mod invalid_ref;
Expand Down Expand Up @@ -416,6 +417,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
reg.register_early_lint_pass(box multiple_crate_versions::Pass);
reg.register_late_lint_pass(box map_unit_fn::Pass);
reg.register_late_lint_pass(box infallible_destructuring_match::Pass);
reg.register_late_lint_pass(box inherent_impl::Pass::default());


reg.register_lint_group("clippy_restriction", vec![
Expand All @@ -424,6 +426,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
array_indexing::INDEXING_SLICING,
assign_ops::ASSIGN_OPS,
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
inherent_impl::MULTIPLE_INHERENT_IMPL,
literal_representation::DECIMAL_LITERAL_REPRESENTATION,
mem_forget::MEM_FORGET,
methods::CLONE_ON_REF_PTR,
Expand Down
36 changes: 36 additions & 0 deletions tests/ui/impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#![allow(dead_code)]
#![warn(multiple_inherent_impl)]

struct MyStruct;

impl MyStruct {
fn first() {}
}

impl MyStruct {
fn second() {}
}

impl<'a> MyStruct {
fn lifetimed() {}
}

mod submod {
struct MyStruct;
impl MyStruct {
fn other() {}
}

impl super::MyStruct {
fn third() {}
}
}

use std::fmt;
impl fmt::Debug for MyStruct {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "MyStruct {{ }}")
}
}

fn main() {}
35 changes: 35 additions & 0 deletions tests/ui/impl.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: Multiple implementations of this structure
--> $DIR/impl.rs:10:1
|
10 | / impl MyStruct {
11 | | fn second() {}
12 | | }
| |_^
|
= note: `-D multiple-inherent-impl` implied by `-D warnings`
note: First implementation here
--> $DIR/impl.rs:6:1
|
6 | / impl MyStruct {
7 | | fn first() {}
8 | | }
| |_^

error: Multiple implementations of this structure
--> $DIR/impl.rs:24:5
|
24 | / impl super::MyStruct {
25 | | fn third() {}
26 | | }
| |_____^
|
note: First implementation here
--> $DIR/impl.rs:6:1
|
6 | / impl MyStruct {
7 | | fn first() {}
8 | | }
| |_^

error: aborting due to 2 previous errors