rustc should warn about private types in re-exported interfaces

[nwin]

If one does not re-export a public type of a private module no warning is emitted. This is something that which is described in #34537 als a “workaround” and thus apparently intended as a feature.

This is actually pretty bad because it effectively renders the whole private_in_public-checker useless as it becomes unreliable. Why do we need a lint, if one can accidentally circumvent it? It hit use here: PistonDevelopers/image#562

mod outer {
    mod inner {
        pub struct Private;
        pub fn interface() -> Private {
            Private
        }
    }
    // This line should cause an error/warning
    pub use self::inner::{interface};
}

use outer::{interface};

fn main() {
    let _ = interface();
}

[petrochenkov]

    // This line should cause an error/warning
    pub use self::inner::{interface};

Why? interface and Private are pub, therefore they are public to all crates and can be reexported anywhere.

[nwin]

No, they can only be re-exported from outer since inner is private. Private cannot be accessed from anywhere else. Even rustdoc won't link it.

[petrochenkov]

That's how the privacy system works. Marking something as pub gives everyone permission to access it.

Private cannot be accessed from anywhere else.

This is allowed. Privacy guarantees that private things cannot be accessed, not that public things can be accessed.

If you need interface to be private to outer you have to specify it like this:

mod outer {
    mod inner {
        pub(outer) struct Private; // or `pub(super)`
        pub(outer) fn interface() -> Private { // or `pub(super)`
            Private
        }
    }
    pub use self::inner::{interface}; // <- becomes an error
}

References: https://github.com/rust-lang/rfcs/blob/master/text/0136-no-privates-in-public.md, https://github.com/rust-lang/rfcs/blob/master/text/1422-pub-restricted.md

[nwin]

Interesting, that syntax does not seem to be documented anywhere. But beside that, you’re missing my point.

If you need interface to be private to outer you have to specify it like this:

I do not want to be interface to be private to outer. I want the complete opposite. I want to prevent that public types are accidentally not accessible. Thus exporting interface should at least give a warning that Private is not accessible.

I understand your point that this is how the privacy system works but a lint would be helpful because it is not the most common case that you want to hide a type in a public interface. Usually one just forgets to export something.

[steveklabnik]

/cc @rust-lang/lang I'm not sure this is a bug.

[nrc]

Not a bug. I think @petrochenkov is correct in his interpretation.

One could imagine adding this check as an off-by-default Clippy lint (cc @Manishearth)

[srdqty]

This seems to be an error to me. It enables one to publicly expose a function without exposing the identifiers that are necessary to declare its type. There is the case of wanting to seal the implementation details of a type, but the name of the type still must be exposed in that case.

Given that the example above compiles successfully, one would expect the example below to also compile successfully (but it doesn't, because it is attempting to explicitly reference a private identifier).

It seems like the type checker has access to type identifiers in a way that is inconsistent with what is in scope. Since it can type check the expression, but a user cannot explicitly declare the type.

mod outer {
    mod inner {
        pub struct Private;
        pub fn interface() -> Private {
            Private
        }
    }
    // This line should cause an error/warning
    pub use self::inner::{interface};
}

use outer::{interface};

fn main() {
    // Does not compile because this type is not in scope here
    // But, remove the explicit type declaration and there is no error.
    // Type inference has access to types that are out of scope.
    let _ : outer::inner::Private = interface();
}

[eddyb]

@srdqty The technical explanation is that the type-checker uses the exact definitions (via numerical IDs) and does not care about privacy of types. The type-system really needs that because you can easily mix all sorts of types in generic code.

But that's not that relevant. The privacy system, as I've been told (by @petrochenkov, who refers to @pnkfelix), is only intended to prevent module-level API leakage, without any crate-wide (or cross-crate) reachability guarantees.

I've gotten some references in rust-lang/rfcs#1671, such as this comment by @petrochenkov on an older draft, and this blog post by @pnkfelix.

[nikomatsakis]

This is actually pretty bad because it effectively renders the whole private_in_public-checker useless as it becomes unreliable.

The goal of the "private-in-public" checker is not to ensure that the all public bits of the API are reachable. Rather, it's goal is to ensure that all private bits are not reachable. That is, we want to be sure that if you declare a type as private, you know that it can't be accessed used from outside the current module. This is important because unsafe code often relies on the assumption that private types are not misused. In contrast, ensuring that your API is "complete and usable" doesn't have safety guarantees (and indeed an API can be "incomplete" in a lot of ways, even if all the types one might want are declared as public).

That said, I agree that it would be reasonable to have a lint that checks to ensure that all public bits of the API are indeed reachable. And yes it probably makes sense to develop such a lint in clippy, at least for now.

[DemiMarie]

Is there any legitimate use for such a type? I can't think of any, since Rust allows zero-sized types, so one could declare a zero-sized struct that should not be constructable by outside code as

put struct Dummy {
   dummy_field: (),
}

[Manishearth]

Exposing "sealed" traits for bounds without allowing people to implement them is one.

Also allows libraries to seal types for stability reasons -- exposing an effectively anonymous type means that you've further restricted how it can be used; and consequently have more freedom to tinker with internals.

[petrochenkov]

Closing as working in accordance with the rules.
Discussion about possibly changing the rules happens in https://internals.rust-lang.org/t/lang-team-minutes-private-in-public-rules/4504/38