New lint: Use `Into`/`TryInto` in bounds as opposed to `From`/`TryFrom`

[Kixunil]

When writing generic bounds such as:

fn foo<T>(a: T) where u32: From<T>;

Into should be preferred, like this:

fn foo<T>(a: T) where T: Into<u32>;

Why the former is bad: Into is a superset of From. In some cases coherence rules prevent implementing From but do allow implementing Into. As a result Into is more generic bound than From.

Category: Style, I guess?

[blasrodri]

@Kixunil Why is it that Into is a superset of From? I thought that they were linked 1:1.
Quoting The Book:

The From and Into traits are inherently linked, and this is actually part of its implementation. If you are able to convert type A from type B, then it should be easy to believe that we should be able to convert type B to type A.
Can you please elaborate further? Thanks :)

[Kixunil]

It's possible to implement Into without implementing From. From implies Into, but Into doesn't imply From Of course normally, people should implement From, not Into but that is not always possible due to coherence issues. (Can't remember specific example right now, but I'm sure I did encounter it once.)

[blasrodri]

Thanks, @Kixunil .
So the idea is that whenever it's possible to implement From, and Into was chosen instead, it should be linted?

[MikailBag]

As far as I remember, several months ago coherence checking rules were improved, and now there is no case when impl Into works, but impl From doesn't.

[blasrodri]

@MikailBag

I think there's still the case where you want to implement a From for a type that isn't defined in your crate. In that case, you can always implement Into and it will work.

Source: https://doc.rust-lang.org/std/convert/trait.Into.html

[sinkuu]

re_balance_coherence feature will be stabilized in 1.41. Beta version of doc says "Prior to Rust 1.40, if the destination type was not part of the current crate...".

[Kixunil]

@blasrodri The point of this issue is to lint bounds, not impls. I guess another lint could be created for impls. (Basically lint whether the code is written according to what the 1.41 docs says.)

[xiongmao86]

If no one is working on this, I'll take a stab.

[xiongmao86]

The history of rust-clippy on the component history pages shows that the cilippy is successfully build from 05-30 to 06-02 today. But I have failed to compile the master of 5-31 with nightly-2020-05-31, a commit which pass ci test at that day. I have also tried 5-30's nightly and also failed. This is really strange, since both ci test show passed when I look at the pr of the commit that I tried to compile. And the compile of my fork master update to that remote master is also failed. Does anybody have idea about what's going on, what makes compile failed? I am really curios why this would happen.

Right now I am trying to rustup to the 06-02 to see if I can build the master. But I suspect that would be failing too.

[flip1995]

@xiongmao86 Clippy is now a subtree in the rust repo, which effectively means, that Clippy will never show up as "missing" on this page again. It may be the case, that some commits from the rust repo have to be copied/synced to the Clippy repo. See https://github.com/rust-lang/rust-clippy/blob/master/CONTRIBUTING.md#fixing-build-failures-caused-by-rust for the new process. We're currently looking in pinning a nightly, but aren't quite there yet: #5561

[xiongmao86]

@flip1995, Thanks for replying, I understand subtree is said to be the new way recommended against submodule. I vaguely grabbed the idea. But I couldn't find articles discussing this in detail. I am interesting in why clippy is decided to transit to subtree way. But I didn't find discussion by searching on this repo.

And what is the impact on the way of development: after pinning to the specific version of nightly, I am very happy that I won't need proxy to download rustc, rust-std etc. and the mirror in my country can really speed up my download. But I not sure do we still need to make "rustup" pr, do we still have to change clippy to catch up the change happen in rustc-dev? Or do we shift the responsibility to rustc developers, and here we only focus on lints, and if failed to build, we sync the change?

[flip1995]

@xiongmao86 This is the issue about moving to subtrees: rust-lang/rust#70651

TL;DR: with subtrees rustc contributors have to deal with fixing the Clippy build in the rustc repo, while folks who just want to contribute to Clippy and not clone/compile rustc, can still do this in this repo.

But I not sure do we still need to make "rustup" pr, do we still have to change clippy to catch up the change happen in rustc-dev?

Yes, currently we have to do a subtree sync for every build failure, since we're still testing with rustc master toolchain. In the future, we want to pin a nightly, which would mean that Clippy development is possible with just rustup (instead of RTIM, which is currently used to install the master toolchain).

Internally, we then also can sync whenever we want, and don't have pressure to sync every time a rustc PR broke Clippy.