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

New lint: Implement From instead of Into #6456

Closed
davidpdrsn opened this issue Dec 15, 2020 · 2 comments · Fixed by #6476
Closed

New lint: Implement From instead of Into #6456

davidpdrsn opened this issue Dec 15, 2020 · 2 comments · Fixed by #6476
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-style Lint: Belongs in the style lint group

Comments

@davidpdrsn
Copy link

What it does

Finds implementations of Into and suggests implementing From instead. That should always be possible since Rust 1.41 as per https://doc.rust-lang.org/stable/std/convert/trait.Into.html#implementing-into-for-conversions-to-external-types-in-old-versions-of-rust.

According the std docs implementing From is preferred since it gives you Into for free where the reverse isn't true:

It is important to understand that Into does not provide a From implementation (as From does with Into). Therefore, you should always try to implement From and then fall back to Into if From can't be implemented.

Categories (optional)

  • Kind: Style.

Drawbacks

There might still be cases where one cannot implement From but implementing Into is possible. If such cases exist and are hard to detect this lint might not be a good idea. Could lead to false positives.

Example

struct StringWrapper(String);

impl Into<StringWrapper> for String {
    fn into(self) -> StringWrapper {
        StringWrapper(self)
    }
}

Could be written as:

struct StringWrapper(String);

impl From<String> for StringWrapper {
    fn from(s: String) -> StringWrapper {
        StringWrapper(s)
    }
}
@giraffate
Copy link
Contributor

related issue: #1567

@ebroto ebroto added L-style Lint: Belongs in the style lint group good-first-issue These issues are a good way to get started with Clippy labels Dec 15, 2020
@1c3t3a
Copy link
Contributor

1c3t3a commented Dec 18, 2020

I would like to work on this!

bors added a commit that referenced this issue Dec 21, 2020
Added from_over_into lint

Closes #6456
Added a lint that searches for implementations of `Into<..>` and suggests to implement `From<..>` instead, as it comes with a default implementation of `Into`. Category: style.
bors added a commit that referenced this issue Dec 22, 2020
Added from_over_into lint

Closes #6456
Added a lint that searches for implementations of `Into<..>` and suggests to implement `From<..>` instead, as it comes with a default implementation of `Into`. Category: style.
@bors bors closed this as completed in 25e9acb Dec 22, 2020
Kouzukii pushed a commit to namib-project/namib_shared that referenced this issue May 23, 2021
May cause builds with an old rust nightly toolchain to fail, since this lint is pretty new (rust-lang/rust-clippy#6456).
moreal added a commit to moreal/bencodex-rs that referenced this issue Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-style Lint: Belongs in the style lint group
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants