Rust Changed My Mind: Private access

When offering something dangerous, the utility must be great enough to warrant the costs.

I’ve taken the position that features that are overwhelmingly used poorly and have workable alternatives shouldn’t be offered to the programmer. This is why I don’t think inheritance or exceptions should be supported. There are features that are commonly misused that are good though. Extensions is one such example. The difference is that their rate of misuse is much lower and that there isn’t a clear alternative for the useful purpose they serve in enabling functional programming.

While I’ve always thought this way, I never expected to have my mind changed to include allowing private access in unit tests.

Rust’s private

Per the book:

If an item is private, it may be accessed by the current module and its descendants.

In a sense, it’s kind of like protected in other languages but instead of only applying to the inheritance tree it applies to namespaces.

In testing

You’ll hear that unit testing a private function is a smell. If it’s complicated enough to need to test, then it’s complicated enough to be extracted into a new type / abstraction. This is technically correct 💺. In fact I’ve yet to ever encounter a single exception to this rule.

Valid private functions are either so simple they shouldn’t be tested:

impl Foo {
    pub fn should_secure(&self, request_path: &Option<&str>) -> bool {
        if FeatureControl::is_enabled("secure_requests")
            && request_in_scope(request_path)
            && valid_provisioning_state()
    }

    // These functions add readability, but any unit unit test is totally
    // redundant and would just restate the function's logic as an assertion.
    fn request_in_scope(request_path: &Option<&str>) -> bool {
        request.is_some_and(|x| x.starts_with("/privileged/"))
    }

    fn valid_provisioning_state() -> bool {
        self.key_provisioned && self.vtpm_enabled && self.secure_boot_enabled
    } 
}

But, you’ll also frequently find it makes sense to extract something more complicated into a private function to reuse code, better label one small thing, or because it is tied to the type in some way (say a static factory). Now, nothing stops you from extracting these into a new type internal to your module. In these scenarios though, fidelity to the principle gets a bit obtuse.

Another competing interest is that just because something can be public or module internal doesn’t mean it should be. A key element of designing libraries is to never make things public until a user needs it. This allows you to make breaking changes without making a breaking change release.

Both of these apply to this snippet:

impl RequestValidator for StandardRequestValidator {
    // Here, these functions are all rather complicated. There's also no reason for
    // them to be public in the context of the library, they're implementation
    // details of the validator.
    //
    // They also don't make meaningful sense to extract, because these details are
    // what the type is.
    fn authenticate(&self, key: &LatchableKey, request: &Request<Vec<u8>>) -> bool {            
        StandardRequestValidator::is_valid_request_date(request)
            && self.is_valid_authorization(key, request)            
            && StandardRequestValidator::is_valid_claims_header(request)
    }
}

This shouldn’t impact your ability to write tests though. If you’ve written enough C++, you’ve probably hit this issue. Private isn’t an option for testing, but C++ also doesn’t have an “internal” concept natively. You can try to use export controls, but even then you’re now committed to including the tests in the same shared library even if you otherwise wouldn’t want to.

The point is, some flexibility is warranted here.

The other side

I have also had the opposite experience. Where the behavior I go to extract is complicated enough that I obviously should extract it into a new abstract concept. This happens when you extract a private function from a private function, extract many, see that there are groups of related private functions, etc.

That doesn’t invalidate the first scenario though. What I think I have moved on is that this level of adherence to design principles doesn’t make a real world difference and isn’t considering human nature. Taxing smoking didn’t stop people from smoking. It didn’t work with sugar or alcohol either. When you don’t let people test private functions, I’ve come to find they just often won’t write unit tests or worse won’t even extract the function.

In the same file

I still don’t like having my unit tests in the same file though. It’s gross, and it means I need two copies of the file open if I want to see code and the test at same time.

Yes, I’ve gotten used to it. Still doesn’t mean I like it though. Doesn’t mean it’s not an inferior organization 😉.

In code

I’m still unsure how I feel about all descendants getting private access. In Rust, a powerful paradigm is using the type system to make invalid states unrepresentable. This can rely upon internal state being private. If the descendants can access these fields, they can violate the invariants. You say well that code shouldn’t make this mistake, but that doesn’t scale.

So far I haven’t seen this issue come up in practice though. This is probably because these types of objects should generally be at the bottom of module hierarchies. Time will tell if exceptions are lurking out there that I just haven’t encountered yet.