`refining_impl_trait` only fires on public traits

The refining_impl_trait lint only fires for public traits. It does not fire without the pub keyword in the code sample below:

pub trait Foo {
//^ Required for lint to fire
    fn foo(self) -> impl Debug;
}

impl Foo for u32 {
    fn foo(self) -> String {
//                  ^^^^^^
//  warning: impl trait in impl method signature does not match trait method signature
        self.to_string()
    }
}

Difference from `async_fn_in_trait` lint

Apparently I was part of the discussion of this at one point (see also the zulip topic on this). I think it got lumped together with the discussion about the lint for async fn in traits, though, when there are some important distinctions:

The async fn lint is only temporary to help avoid footguns created by missing language features, and we want to make non-footgunny uses more convenient.
Refinement is a mechanism that will always exist and is fundamental to trait implementations.
Refinement's ability to "punch through" abstraction boundaries can happen accidentally, even within a crate.

The second point is important, because as a user I would expect such a fundamental mechanism to behave independently of whether the trait happens to be crate-public or not. This can lead to false expectations being created about the behavior in the other case.

Violating abstraction boundaries within a crate

As an example of the last point, let's say I as a user want to define a trait that my type implements ahead of actually generalizing my code:

trait Application {
    fn windows(&self) -> Vec<impl Window>;
}
trait Window {
    fn title(&self) -> Option<String>;
}

struct App;
struct Win;

impl Application for App {
    fn windows(&self) -> Vec<Win> { todo!() }
}
impl Window for Win {
    fn title(&self) -> Option<String> { todo!() }
}

fn all_windows(apps: &[App]) -> Vec<Win> {
    apps.iter().map(|a| a.windows()).flatten().collect()
}

Later on, I want to write a test for all_windows. But in order to do that, I have to change it to accept impl Application, which requires changing the output type to impl Window + '_, and possibly changing all the users of all_windows as well. This can get unwieldy quick.

We can say that the user should have used impl Trait from the beginning, but that might be inconvenient when prototyping. If they are leaning on traits to provide the outlines of an abstraction boundary, we should let them opt in before punching through said boundary, IMO.

cc @compiler-errors

rust-lang / rust