from_over_into suggests docs antipatterns for foreign types

[Fishrock123]

Lint name: from_over_into

I tried this code:

https://github.com/http-rs/surf at today's HEAD: 97790c19664466efa4cf33d9eafee23486a12b3e

I expected to see this happen: no warnings

Instead, this happened:

error: an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true
   --> src/response.rs:324:1
    |
324 | / impl Into<http::Response> for Response {
325 | |     fn into(self) -> http::Response {
326 | |         self.res
327 | |     }
328 | | }
    | |_^
    |
    = help: consider to implement `From` instead
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into

Changing this to From is, shockingly, not an error, despite being a foreign trait on a foreign type.

However, it results in Rust docs which are even more difficult to read than today's.
I will note there is some "super-structing" / inheritance going on here, where surf::Response is essentially a supertype of http_types::Response, and has From/Into each other, and down-referencing via AsRef/AsMut.

I don't think this rule should detect Into<foreign type>.

Docs examples:

Into<Response>:

From<Response>:

Meta

• cargo clippy -V: clippy 0.1.51 (4253153 2021-01-17)
• rustc -Vv:

rustc 1.51.0-nightly (4253153db 2021-01-17)
binary: rustc
commit-hash: 4253153db205251f72ea4493687a31e04a2a8ca0
commit-date: 2021-01-17
host: x86_64-unknown-linux-gnu
release: 1.51.0-nightly
LLVM version: 11.0.1

[Fishrock123]

Refs: #6476

[ghost]

The recommendation is correct even if triggers a rustdoc bug - rust-lang/rust#42066. This should be fixed on the rustdoc side.

[peku33]

Why exactly this isn't an error?
impl Into<String> for MyStruct {...} is OK - implementing foreign trait for local type.
But isn't impl From<MyStruct> for String {...} forbidden? It pretty much sounds like foreign trait for foreign type.
From<MyStruct> is treated as newly constructed type because of templating and it behaves like local?

[peku33]

From docs:
Only implement Into when targeting a version prior to Rust 1.41 and converting to a type outside the current crate. From was not able to do these types of conversions in earlier versions because of Rust's orphaning rules. See Into for more details.
So isn't this a false positive?

[ghost]

Why exactly this isn't an error?
impl Into<String> for MyStruct {...} is OK - implementing foreign trait for local type.
But isn't impl From<MyStruct> for String {...} forbidden? It pretty much sounds like foreign trait for foreign type.
From<MyStruct> is treated as newly constructed type because of templating and it behaves like local?

https://blog.rust-lang.org/2020/01/30/Rust-1.41.0.html#relaxed-restrictions-when-implementing-traits

From docs:
Only implement Into when targeting a version prior to Rust 1.41 and converting to a type outside the current crate. From was not able to do these types of conversions in earlier versions because of Rust's orphaning rules. See Into for more details.
So isn't this a false positive?

That's exactly what the lint is recommending. From instead of Into. How is it a false positive?

[peku33]

@mikerite shouldn't linter follow official docs and guidelines?
quoted text is from here: https://doc.rust-lang.org/std/convert/trait.From.html

[ghost]

One should always prefer implementing From over Into because implementing From automatically provides one with an implementation of Into thanks to the blanket implementation in the standard library.

That's what we're doing. We lint Into implementations and recommend From.

[Fishrock123]

This issue here is that it is hard to deduce the conversions when reading rustdoc. Which means, frankly, we'll continue to ignore this until that improves (hence the "docs antipatterns").

[camsteffen]

Closing per mikerite's comments.