<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">I’m a bit confused here…</div><div class=""><br class=""></div><div class="">Since the result of breaching your subclassing contract is a compile-time error, how are you intending to test that using XCTest?</div><div class=""><br class=""></div><div class="">On the other hand, if you want to write a test that subclasses the class, and uses it as a Library consumer would, why would you use @testable? Wouldn’t you just import the module normally and verify the contract that way (and thus getting a compile failure if the open/public stuff is wrong).</div><div class=""><br class=""></div><div class="">That’s not a Unit Test, that’s a functional test, so I’d probably make that a separate test target anyway.</div><div class=""><br class=""></div><div class="">Scott</div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 27, 2016, at 4:41 PM, David Owens II &lt;<a href="mailto:david@owensd.io" class="">david@owensd.io</a>&gt; wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">I brought this up in review, but there seems to be a serious testability problem here because of how special @testable is.</div><div class=""><br class=""></div><div class=""><pre style="box-sizing: border-box; font-family: Consolas, 'Liberation Mono', Menlo, Courier, monospace; font-size: 14px; margin-top: 0px; margin-bottom: 0px; line-height: 1.45; word-wrap: normal; padding: 16px; overflow: auto; background-color: rgb(247, 247, 247); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; word-break: normal; color: rgb(51, 51, 51);" class=""><span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// This class is not subclassable outside of ModuleA.</span>
<span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">public</span> <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">class</span> NonSubclassableParentClass {
    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// This method is not overridable outside of ModuleA.</span>
    <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">public</span> <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">func</span> <span class="pl-en" style="box-sizing: border-box; color: rgb(121, 93, 163);">foo</span>() {}

    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// This method is not overridable outside of ModuleA because</span>
    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// its class restricts its access level.</span>
    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// It is not invalid to declare it as `open`.</span>
    open <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">func</span> <span class="pl-en" style="box-sizing: border-box; color: rgb(121, 93, 163);">bar</span>() {}

    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// The behavior of `final` methods remains unchanged.</span>
    <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">public</span> <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">final</span> <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">func</span> <span class="pl-en" style="box-sizing: border-box; color: rgb(121, 93, 163);">baz</span>() {}
}</pre><div class=""><br class=""></div></div><div class="">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?</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">-David</div><div class=""><br class=""></div><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Jul 27, 2016, at 3:18 PM, Scott James Remnant via swift-evolution &lt;<a href="mailto:swift-evolution@swift.org" class="">swift-evolution@swift.org</a>&gt; wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">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.<div class=""><br class=""></div><div class="">The selling point for me is this:</div><div class=""><br class=""></div><div class=""><pre style="box-sizing: border-box; font-family: Consolas, 'Liberation Mono', Menlo, Courier, monospace; font-size: 14px; margin-top: 0px; margin-bottom: 0px; line-height: 1.45; word-wrap: normal; padding: 16px; overflow: auto; background-color: rgb(247, 247, 247); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; word-break: normal; color: rgb(51, 51, 51);" class=""><span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// This is allowed since the superclass is `open`.</span>
<span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">class</span> SubclassB <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">:</span> SubclassableParentClass {
    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// This is invalid because it overrides a method that is</span>
    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// defined outside of the current module but is not `open'.</span>
    <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">override</span> <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">func</span> <span class="pl-en" style="box-sizing: border-box; color: rgb(121, 93, 163);">foo</span>() { }

    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// This is allowed since the superclass's method is overridable.</span>
    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// It does not need to be marked `open` because it is defined on</span>
    <span class="pl-c" style="box-sizing: border-box; color: rgb(150, 152, 150);">// an `internal` class.</span>
    <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">override</span> <span class="pl-k" style="box-sizing: border-box; color: rgb(167, 29, 93);">func</span> <span class="pl-en" style="box-sizing: border-box; color: rgb(121, 93, 163);">bar</span>() { }
}
</pre></div><div class=""><br class=""></div><div class="">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`.</div><div class=""><br class=""></div><div class="">Good job, everyone!</div><div class=""><br class=""></div><div class="">Scott</div></div>_______________________________________________<br class="">swift-evolution mailing list<br class=""><a href="mailto:swift-evolution@swift.org" class="">swift-evolution@swift.org</a><br class=""><a href="https://lists.swift.org/mailman/listinfo/swift-evolution" class="">https://lists.swift.org/mailman/listinfo/swift-evolution</a><br class=""></div></blockquote></div><br class=""></div></div></blockquote></div><br class=""></body></html>