Pre-RFC: `pub(api)` visibility, and static enforcement of crate-internal API boundaries

language design
1

This is an idea I wrote up while working with a self-written large crate with a number of internal sub APIs in modules. Hopefully it makes sense.

Any feedback is welcome, but I mainly want to see if other people have felt this pain point, if the approach seems reasonable to people for the Rust philosophy, and if I'm somehow treading over previous attempts to do something similar. I'm happy to address the semantics further, but I wanted to get this high-level feedback before spending major effort refining the write up.

Also, this is my first attempt at anything in the realm of Rust RFCs, so be a little kind :slight_smile: .

(Posting as a link to github, as there is a limit of 5 links for new users)

Rendered

EDIT: Updated to sync to head of branch.

2

Our options are now either to use pub(in super::super), or to use pub(in crate::resources), but these add a lot of verbiage to our source file:

// In src/resources/io/file_api/mod.rs

/// A set of resources that can be read individually.
pub(in crate::resources) struct ResourceSet {
  // ...
}

impl ResourceSet {
  /// Open a file path to load resources from it.
  pub(in crate::resources) fn open(path: &Path) -> Result<Self> { 
    // ...
  }

  /// Load a resource with the given ID.
  pub(in crate::resources) fn load_resource(res_id: &str) -> Result<Resource> {
    // ...
  }
}

There’s another option: pub(in crate::resources) struct ResourceSet (or in super::super), and use pub for associated items. This is what I do; I think it’s cleaner for associated items’ visibility to be controlled by the main item’s visibility[1]. I know there’s an issue open about adjusting the unreachable_pub lint (or something like that) to treat associated items differently.

  1. Well, more precisely, “the visibility of associated items intended to have the same visibility as the main item”, I don’t mean to say that none of my associated items are private. ↩︎

3

pub(in path) had to have in added because pub(not_a_keyword) was ambiguous (in struct definitions? I don't remember the exact syntax quirk). It probably blocks pub(api) too. You might have something with a keyboard like variations of pub(in mod) or pub(super api).

The pub visibility is only supposed to be used for items (types, traits, functions, etc.) that are part of the public API of the crate , not the module.

That's not accurate. pub is for the module. It is unfortunately ambiguous (not locally explicit) whether pub means it's part of the crate's public-public API or not.

And I'm disappointed that your proposal isn't the pub to clarify that reachability for pub items. I'd expect pub(api) to be that "absolutely definitely externally pub from outside of the crate", rather than a syntax sugar for internal non-public pub(in path) with a new invention for avoiding repeating the path.

Couldn't you replace !#[api] with use crate::something as api and use pub(in api)?

4

I thought that, generally, all pub items are suggested to be exported from the containing crate. The text of the unreachable_pub rustc lint docs say:

This lint is “allow” by default because it will trigger for a large amount of existing Rust code. Eventually it is desired for this to become warn-by-default.

I don't see it saying that pub methods are an exception here (at least when it's in warn mode).

Regardless, I can work through that example again, either replacing it or at least mentioning it as another example.

5

It shouldn't block pub(api) because, as mentioned, it would become a weak keyword. The set of possible first tokens in a pub(E) context I believe are self, super, and in otherwise. If that is the case, then introducing api as another should not be ambiguous.

While pub can be used within a module, the rustc lints documentation suggests that the only reason this isn't a default warning is that it would break too much code right now, and may change. See my reply to @robofinch about unreachable_pub.

I'm perfectly happy with replacing #![api] with another way of definining the boundary. I provide some alternatives I came up with off the top of my head in the RFC. While it may be possible to import a module as the name api, note that this is unrelated to my suggestion, as the pub(api) visibility modifier is not using api as a name in the current module scope. There are several potential benefits to having an alternate visibility, such as ensuring that non pub(api) items don't escape a module boundary, and to allow for the visibility of an API boundary module and its exported items to be modified with a single visibility at the module definition site.

6

For reference, here’s the issue I’m thinking of: Suggestion: Split `unreachable_pub` lint into multiple lints · Issue #112615 · rust-lang/rust · GitHub

I pretty much agree with @jplatte w.r.t. why I use the lint.

(The lint no longer triggers on public fields of private types, unless I’m sorely mistaken, but it still triggers on public methods of private types.)

2 Likes
7

I think the ambiguity would be nested tuples: struct TupleStruct(pub (u32, u32), u32). Without in, you can't easily disambiguate between a type path and a module path.

This is not too far removed from the ambiguity that made it hard to use crate as shorthand for pub(crate): struct TupleStruct(crate ::type).