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
> <[email protected]> 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
> [email protected]
> https://lists.swift.org/mailman/listinfo/swift-evolution
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution
