temporalio / sdk-core

Core Temporal SDK that can be used as a base for language specific Temporal SDKs
MIT License
250 stars 68 forks source link

Use unreachable_pub lint to make visibility more explicit #726

Closed djc closed 2 months ago

djc commented 2 months ago

What was changed

Enable unreachable_pub in all of the crates in this workspace.

Why?

I use this lint in a warn-by-default mode (with CI denying it by default) in all of the library crates I maintain to help make visibility explicit. Unfortunately the Rust behavior is a little strange in that an item can be declared to be pub without actually being exported in a way that it's visible from outside the crate. This makes it harder to confidently review changes adding new types or changing visibility because the actual impact on public API is dependent on the path from the crate root to the item, which is visible in the diff.

As such, this would systematically help avoid issues such as the one fixed in #724, where items were declared pub while still being unusable from downstream crates.

Putting pub(crate) or pub(super) is a little boring and slightly noisy but has the advantage of explicitly signaling visibility, and also helps the compiler issue warnings for fields that are not being read.

Checklist

  1. Closes: no issue
  2. How was this tested: ran cargo lint and cargo test-lint after switching unreachable_pub to warn
  3. Any docs updates needed: no