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

Lint for duplicate impl section #1024

Closed
mspiegel opened this issue Jun 18, 2016 · 4 comments
Closed

Lint for duplicate impl section #1024

mspiegel opened this issue Jun 18, 2016 · 4 comments
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-style Lint: Belongs in the style lint group T-middle Type: Probably requires verifiying types

Comments

@mspiegel
Copy link

I'm a novice to rust and noticed recently that I had accidentally created two implementation sections within the same module. Both sections were impl's that did not have a trait declaration:

impl Foobar {
   ...stuff here
}

impl Foobar {
   ...more stuff here
}

It's probably a good practice to warn on duplicate impls or is there something I am not considering?

@mcarton mcarton added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types A-lint Area: New lints L-style Lint: Belongs in the style lint group labels Jun 18, 2016
@Manishearth
Copy link
Member

Duplicate impls aren't really an antipattern, but this could be an allow lint

@llogiq
Copy link
Contributor

llogiq commented Jun 19, 2016

I see duplicate impls in conjunction with #[cfg(..)]s to unify implementations for different systems. Otherwise I consider them a net loss in readability.

@mcarton
Copy link
Member

mcarton commented Jun 19, 2016

Different impl sections can have different generic bounds.
I also like to (ab?)use that to make sections in the documentation:

pub struct Foo;

/// # Constructors
impl Foo {
    /// Make a default `Foo`
    pub fn new() -> Foo {}

    /// Make a `Foo` with some `Bar`
    pub fn with_bar() -> Foo {}
}

/// # Operations on `Foo`s
impl Foo {
    /// Do something with a `Foo`
    pub fn do_sth(self) {}
}

looks like this:

screen shot 2016-06-19 at 13 28 16

But:

pub struct Foo;

impl Foo {
    /// # Constructors
    /// Make a default `Foo`
    pub fn new() -> Foo { … }

    /// Make a `Foo` with some `Bar`
    pub fn with_bar() -> Foo {}

    /// # Operations on `Foo`s
    /// Do something with a `Foo`
    pub fn do_sth(self) {}
}

looks like this:

screen shot 2016-06-19 at 13 31 27

Ok for an allow-by-default lint though.

@camsteffen
Copy link
Contributor

Duplicates #414

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-style Lint: Belongs in the style lint group T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

5 participants