[swift-evolution] [swift-evolution-announce] [Accepted with Revision] SE-0177: Allow distinguishing between public access and public overridability

[David Owens II]

I brought this up in review, but there seems to be a serious testability problem here because of how special @testable is.

// This class is not subclassable outside of ModuleA.
public class NonSubclassableParentClass {
    // This method is not overridable outside of ModuleA.
    public func foo() {}

    // This method is not overridable outside of ModuleA because
    // its class restricts its access level.
    // It is not invalid to declare it as `open`.
    open func bar() {}

    // The behavior of `final` methods remains unchanged.
    public final func baz() {}
}

In a unit test, I *can* subclass `NonSubclassableParentClass`, the access level of `NonSubclassableParentClass` is irrelevant. There’s now no programatic way to ensure that I’m actually testing the contract that I had intended to provide to the consumers of my framework (that my class is subclassable). Is this really the intention?

The “fix”, on the authors end, is to create another target that consumes the output framework to ensure my contract is actually what I wanted (and make sure that it’s not a special test target!). No one is going to do this.

Sure, maybe a code review might catch it. Or I can write a custom linter to validate for this. Do we really want `open` members in non `open` types? The issue with `public` members on `internal` types is much less concerning as the `internal` type isn’t being exposed to others to begin with.

-David


> On Jul 27, 2016, at 3:18 PM, Scott James Remnant via swift-evolution <swift-evolution at swift.org> wrote:
> 
> I realize that there’s no review needed, but I actually wanted to give a hearty 👏 to the authors and commenters of this proposal, because I genuinely think we’ve reached something good in the result.
> 
> The selling point for me is this:
> 
> // This is allowed since the superclass is `open`.
> class SubclassB : SubclassableParentClass {
>     // This is invalid because it overrides a method that is
>     // defined outside of the current module but is not `open'.
>     override func foo() { }
> 
>     // This is allowed since the superclass's method is overridable.
>     // It does not need to be marked `open` because it is defined on
>     // an `internal` class.
>     override func bar() { }
> }
> 
> This feels super-clean; it gives Library developers `open` for their APIs, without confusing app developers, and still requires that sub-classing Library developers think about `open`.
> 
> Good job, everyone!
> 
> Scott
> _______________________________________________
> 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/20160727/84bbfd2a/attachment.html>