Adding impl IntoIterator for T can be breaking if impl IntoIterator for &T exists

[not-my-profile]

Adding an impl IntoIterator for T can be a breaking change if an impl IntoIterator for &T already exists and the new implementation has a different associated type. Though this pitfall is not specific to IntoIterator and can be illustrated with a simpler example:

pub mod api {
    pub trait IntoX {
        type X;

        fn into_x(self) -> Self::X;
    }

    pub struct Bar;

    impl IntoX for &Bar {
        type X = bool;

        fn into_x(self) -> Self::X {
            true
        }
    }

    // impl IntoX for Bar {
    //     type X = i32;

    //     fn into_x(self) -> Self::X {
    //         3
    //     }
    // }
}

// API consumer code
use api::{Bar, IntoX};

fn test(b: Bar) {
    assert!(b.into_x());
}

Uncommenting the commented out impl will make the given consumer code fail to compile, since the code will then use the new impl which returns a different type.

I looked over the checks listed in #5 and couldn't find any bullet point for this specific issue ... but I may very well be missing it, there are a lot of bullet points 😅 ... I also didn't find a single other mention of IntoIterator in the issue tracker, so I'm just opening a new issue for this.

[obi1kenobi]

Nice catch that this is breaking!

Unfortunately, I believe this is one of those "breaking but not semver-major" changes that Rust allows. I believe the breakage here can be avoided by using a more explicit form like <Bar as IntoX>::into_x(b), which is one of the edge cases that is explicitly defined as breaking but not major.

That said, two things:

• Semver-major or not, this feels like a footgun. Feels like it might be a good candidate for a clippy lint, like "same trait implemented on type and reference with unrelated associated types"?
• In the future, I'd love to have a "this isn't semver-major but it might annoy your downstream users" category of lints, either inside cargo-semver-checks itself or as its own separate tool. If clippy decides this lint isn't a fit, it would be great to have there instead.