[swift-evolution] [Review] SE-0169: Improve Interaction Between private Declarations and Extensions

[David Hart]

> On 17 Apr 2017, at 21:11, Nevin Brackett-Rozinsky via swift-evolution <swift-evolution at swift.org> wrote:
> 
> All right, time to dive in!
> 
> First things first, the “helper visible” row in the table I posted is actually unnecessary: a private helper type can have its visible members unmarked (so, “internal”) and they will be available throughout the file.
> 
> Now, if we believe that cross-type sharing ought be avoided, then the primary place “fileprivate” should occur today is when extending a type in the same file. The one other time it might be unavoidable is when extending an existing type to have an initializer that takes the new type. Thus in “Foo.swift” we might have “extension Bar { init(foo: Foo) … }” and need to see hidden details of Foo.
> 
> Similarly, if we think that unrelated types do not belong together in the same file, then the bottom two rows (which deal with multi-type files) can be expunged as well. After all if there are two types which must not have privileged access to one another, they should go in separate files.
> 
> Thus the only place where “private” need appear today is when protecting members of a type from the rest of that same type. However, as has been mentioned on-list, “private” alone does not achieve sufficient encapsulation for this purpose, because a private variable can be seen in all functions located in the main type declaration.
> 
> Moreover, if a type has two separate invariants, and dedicated methods for interacting with them, the only way to hide one invariant from the other’s methods is to use helper types. It seems that “use a private helper type” should be the preferred way to protect invariants, and if the helper type needs to see the rest of the type’s members then it should be nested.
> 
> Putting everything together, an updated version of the table looks like this, where the “Privileged init” row refers to the scenario described earlier:
> 
> 
> No change
> SE–0159
> SE–0169
> Rename
> Simple file
> private
> private
> private
> private
> Extensions
> fileprivate
> private
> private
> private
> Privileged init
> fileprivate
> private
> fileprivate
> private
> Helper hidden
> private
> no hiding
> private
> scoped
> Invariants
> helper type
> no hiding
> helper type
> helper type
> 
> 
> No change
> 
> If we do not make a change, then we will be stuck using “fileprivate” in perpetuity. This might be a purely aesthetic concern, but I would liken it to a splinter in the finger. Yes it is small, but it hurts every time we touch something with it, and it will keep hurting until we yank it out.

I think its important to point out that its more than aesthetics: because the simple file and extensions scenario are so common, it forces us to use both private and fileprivate fairly regularly, which increases the total number of access control to work with on a daily basis. And I think that’s the important point. That’s why open is a success IMHO: because it only needs to be used rarely, it provided the required functionality without increasing the number of access modifiers that needed to be used regularly.

> The existing meaning for “private” is really only useful to protect members of a helper type. It should not be used for any other purpose because it hamstrings the ability to add extensions, and it is insufficient for true encapsulation outside of helper types.
> 
> SE–0159
> 
> If Swift takes the opinionated stance that one should not put things that need to be hidden from each other in the same file, then SE–0159 is the clear winner. After all, if you can’t hide things from other parts of the file at all, then you won’t be tempted to try and thus you will keep your separate types in separate files. This will also simplify the access control model, making it easier to reason about code and decide which visibility to use.
> 
> It will cause churn, however, as existing projects that use sub-file-level hiding must adapt. In the future a submodule system with a visibility level between “private” and “internal” (perhaps “protected”?) could re-enable the use of helper types (in separate files!) to preserve invariants.
> 
> SE–0169
> 
> If we accept the current proposal, then we will only really need “fileprivate” when extending another type to add something like an initializer which requires privileged access to the main type in a file. Additionally, people will be encouraged to use helper types for encapsulating invariants, because the scope-based “private” of today will no longer be a tempting-but-inferior alternative.
> 
> This option also causes churn, though perhaps in a good way as projects which use “private” for encapsulation must switch to the superior design of helper types. Furthermore, a private helper type can be nested in an extension, so its implementation need not occupy space within the main type declaration. It is worth noting than a private nested helper type cannot be extended, and is thus guaranteed to be defined entirely within a single code block, because it is only visible in the outer type and extensions cannot be nested.
> 
> The access control model becomes more complex to explain, though perhaps simpler to understand. However, one concern is that SE–0169 might effectively encourage people to place unrelated types in the same file. After all, one might reason, why would they have carved out this weirdly-specific meaning for “private” if they didn’t expect and intend for me to put several different types in a single file?
> 
> Rename
> 
> If we change the spelling of “private” to “scoped” and of “fileprivate” to “private”, then there will be no extra work for developers because the semantics are identical and migration can be automated. This is the solution of least churn.
> 
> It also means that “private” can be used everywhere except for invariants within helper types, and that is a very good thing. If invariants are marked “scoped”, and nothing else is, then any change away from “scoped” is easy to spot in code review.
> 
> This approach leaves the possibility that people will try to encapsulate with just “scoped” and not use a helper type, which should be addressed in style guides. However it also discourages people from putting unrelated types in the same file, because “private” does not have exceptions carved out that indicate otherwise.
> 
> In the future when we get submodules, then helper types can go in their own files and a discussion on removing “scoped” may take place. That will get us a lot more simplicity, and it will avoid the temporary inability to encapsulate invariants that SE–0159 would bring.
> 
> Conclusion
> 
> The simplicity of SE–0159 is admirable, but the desire for encapsulation of invariants rules it out. Making no change is unacceptable because we would be stuck with “fileprivate”, so the question comes down to SE–0169 and renaming.
> 
> As I see it, renaming is the superior option. It brings less churn because semantics don’t change and migration can be fully automated. It removes “fileprivate” altogether, whereas SE–0169 keeps it around. And renaming avoids the problematic implication of expectations whereby SE–0169 silently encourages people to put unrelated types in one file.
> 
> The fact that SE–0169 is apparently designed specifically to shield types from each other within the same file will inevitably lead people to use it for exactly that purpose.
> 
> During the discussion of SE–0159, a large number of people on both sides of the issue said they would support renaming to “private” and “scoped”. As an option which appeals to developers regardless of whether they use sub-file-level encapsulation, as an option which does not blur the lines about what constitutes a scope, and as an option which preserves all the semantics of our existing access levels, I think that renaming is the best way to solve the “fileprivate” problem.
> 
> The fact that it exactly matches the original intent of SE–0025 is an added bonus.

I don’t think its worth arguing about renaming again. Yes, its technically a better option, but the Core Team will not consider it for the same reasons they rejected SE-0159. I’m under the impression that SE-0169 is the only solution we have.

> Nevin
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170417/39d98dad/attachment.html>