Improve "private type in public interface" message

[spacekookie]

There seems to be an issue with detecting the pub-ness of types on the current nightly (EDIT: and actually stable) branch. Consider the following code:

main.rs:

mod hardware;

fn main() {
    hardware::do_stuff();
}

hardware/mod.rs:

mod parser;

#[derive(Debug)]
struct Hardware {
    foo: u32
}

pub fn do_stuff() {
    let hw = parser::get_hardware();
    println!("{:?}", hw);
}

hardware/parser.rs:

use super::Hardware;

pub fn get_hardware() -> Hardware {
    Hardware { foo: 5 }
}

There are no public types in the hardware module that are exposed in any way. Yet, I get this error from the compiler:

[rustc]
private type `hardware::Hardware` in public interface

can't leak private type

Play Link

[zackmdavis]

Around the time of this issue report, the error message looked like this:

error[E0446]: private type `hardware::Hardware` in public interface
 --> 50430.rs:5:9
  |
5 | /         pub fn get_hardware() -> Hardware {
6 | |             Hardware { foo: 5 }
7 | |         }
  | |_________^ can't leak private type

As part of #51866 (c2d44b2), the message was expanded to include the span of the definition of the private type:

error[E0446]: private type `hardware::Hardware` in public interface
  --> src/main.rs:5:9
   |
5  | /         pub fn get_hardware() -> Hardware {
6  | |             Hardware { foo: 5 }
7  | |         }
   | |_________^ can't leak private type
...
11 |       struct Hardware {
   |       - `hardware::Hardware` declared as private

What's happening internally is that we're looking up the visibility modifier on the HIR node corresponding to the definition of a type, and linting if it's less visible than its parent (in the sense that a return type is the child of a function signature in the relevant traversal). In this example, the problem is that if the parser module is visible, then its public function parser::get_hardware is visible, but its return type isn't, and that's not allowed.

@spacekookie In what way do you think the current error message could be improved? (I guess we could add two maybe-incorrect suggestions to either make the private type more visible, or make the function less visible??) cc @estebank

[spacekookie]

I guess what I'm wondering is if this is a bug or at least something that should really be allowed.

In case this is what's supposed to happen I'm not entirely sure how the message could be made any better though

[estebank]

I believe that by default we should treat get_hardware as pub(super) and Hardware as pub(self) (or get_hardware as pub(in hardware) and Hardware as pub(in hardware), which should be the same) for the purposes of this lint. It is common for some of the visibility lints to not properly account for impossibility of escape in some cases. In the end, the code as posted should behave as proposed, but if get_hardware is pub in a pub mod it should emit the current error:

error[E0446]: restricted type `foo::hardware::Hardware` in public interface
  --> src/main.rs:6:9
   |
6  | /         pub fn get_hardware() -> Hardware {
7  | |             Hardware { foo: 5 }
8  | |         }
   | |_________^ can't leak restricted type
...
12 |       pub(in foo::hardware) struct Hardware {
   |       --------------------- `foo::hardware::Hardware` declared as restricted

Does this sound correct?

---

Even if we don't change anything about the heuristics, we could extend @zackmdavis' PR to also suggest the appropriate visibility annotation that would make it compile.

[hrvolapeter]

Also stumbled onto this problem. The error message is really confusing and initially I started creating bug report only to find out that this is the expected behaviour.

I tried this code:
Playground

#![deny(private_in_public)]
fn main() {
    
}

struct Inner;

mod A {
    use super::*;
    
    pub trait O {
        fn K() -> Inner;
    }
}

I expected to see this happen: Compile. Module A is not crate public so trait A is not crate public.

Instead, this happened: Compile error, private type in public interface