Usages of #[doc(hidden)] pub

[gretchenfrage]

I found 13 places in this repo that use #[doc(hidden)] on pub types for various reasons. In my opinion, there is usually a better way to do something than that, and from what I'm seeing, most of these occurrences seem to date back to the 2018-2019 era of the project.

The way I see it:

• If it's needed for tests, mark it #[cfg(test)].
• If it's needed for some weird thing, like perf testing or an interop container, gate it behind a feature flag, and document that that feature flag is not covered by semver commitments. It extra defensiveness desired, mimic the thing the Hyper project does with their RUSTFLAGS='--cfg hyper_unstable_xyz' thing.
• If it's needed for Quinn, then that means that proto is favoring quinn as a privileged user, which is bad ideologically.

---

I found these usages of #[doc(hidden)]:

• quinn::send_stream::SendStream::poll_stopped
  • As far as I can tell, the only usage of this anywhere is literally in the same module
  • Suggestion: Just make it private
  • quinn: Make SendStream::poll_stopped private #2100 resolves
• quinn::connection::Connection::force_key_update
  • Used in interop container
  • Suggestion: Either make part of public API, or gate behind some sort of "interop-container" feature
  • Un-hide force_key_update / initiate_key_update #2101 resolves
• quinn_proto::connection::Connection::initiate_key_update
  • Used in tests
  • Used in Quinn, in the force_key_update function, which is the previous item on this list
  • Suggestion: If make force_key_update part of public API, make this part of public API. If gate force_key_update behind "interop-container" feature, make this #[cfg(any(test, feature = "interop-container"))]
  • Un-hide force_key_update / initiate_key_update #2101 resolves
• quinn_proto::config::EndpointConfig::get_max_udp_payload_size
  • Used in tests
  • Used in Quinn
  • Suggestion: Make part of public API
    • Possibly add getters for all config values?
  • Un-hide get_max_udp_payload_size #2102 resolves
• quinn_proto::connection::streams::ClosedStream::new
  • Used in Quinn
  • Suggestion: Make part of public API, or rework Quinn usage to not need it
  • Implement Default for ClosedStream #2103 resolves
• quinn_proto::fuzzing
  • Already marked #[cfg(fuzzing)]
  • Presumably used in fuzzing
  • Suggestion: Just remove the #[doc(hidden)]? I don't think it'll appear behind docs if it's gated behind fuzzing.
  • proto: Remove superfluous #[doc(hidden)] fuzzing #2104 resolves
• quinn_proto::StreamId.0
  • Used in perf
  • Suggestion: Make part of public API. It could be useful for applications to access this. It doesn't have to be making the field itself public, it could be adding from and into implementations or whatever.
  • proto: replace hidden field with From impl #2099 resolves
• quinn_proto::EcnCodepoint all variants (this counts for 3 of the 13)
  • Used in Quinn
  • Suggestion: Make part of public API.
  • Unhide ecn variants #2105 resolves
• quinn_udp::EcnCodepoint all variants (this counts for 3 of the 13)
  • The code for this is identical to the proto version
  • Used in tests
  • Used in Quinn
  • Suggestion: Make part of public API.
  • Unhide ecn variants #2105 resolves

[djc]

Thanks for doing this analysis!

• quinn::send_stream::SendStream::poll_stopped
  • As far as I can tell, the only usage of this anywhere is literally in the same module
  • Suggestion: Just make it private

Agreed.

• quinn::connection::Connection::force_key_update
  • Used in interop container
  • Suggestion: Either make part of public API, or gate behind some sort of "interop-container" feature

I'd be okay with making this part of the public API.

• quinn_proto::connection::Connection::initiate_key_update
  • Used in tests
  • Used in Quinn, in the force_key_update function, which is the previous item on this list
  • Suggestion: If make force_key_update part of public API, make this part of public API. If gate force_key_update behind "interop-container" feature, make this #[cfg(any(test, feature = "interop-container"))]

Same here.

• quinn_proto::config::EndpointConfig::get_max_udp_payload_size
  • Used in tests
  • Used in Quinn
  • Suggestion: Make part of public API
    • Possibly add getters for all config values?

Makes sense to make this public, not very motivated to make the other stuff public unless there's more of a motivation.

• quinn_proto::connection::streams::ClosedStream::new
  • Used in Quinn
  • Suggestion: Make part of public API, or rework Quinn usage to not need it

Maybe we can derive Default for ClosedStream instead?

• quinn_proto::fuzzing
  • Already marked #[cfg(fuzzing)]
  • Presumably used in fuzzing
  • Suggestion: Just remove the #[doc(hidden)]? I don't think it'll appear behind docs if it's gated behind fuzzing.

Works for me.

• quinn_proto::StreamId.0
  • Used in perf
  • Suggestion: Make part of public API. It could be useful for applications to access this. It doesn't have to be making the field itself public, it could be adding from and into implementations or whatever.

Suggest #2099.

• quinn_proto::EcnCodepoint all variants (this counts for 3 of the 13)

  • Used in Quinn
  • Suggestion: Make part of public API.

• quinn_udp::EcnCodepoint all variants (this counts for 3 of the 13)

  • The code for this is identical to the proto version
  • Used in tests
  • Used in Quinn
  • Suggestion: Make part of public API.

Sounds good to me.

[gretchenfrage]

Congras, we done it