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

[Xiaodi Wu via swift-evolution]

Right. So IIUC, what you called a serious testability issue earlier in the
thread (inadvertent open inside public) is no more and no less an issue
with @testable than inadvertent sealed inside open (in that neither would
be picked up during such testing), and proper black box testing is what you
need to verify that the public API contract is as you intend it to be.
On Thu, Jul 28, 2016 at 01:15 David Owens II  wrote:

> On Jul 27, 2016, at 8:52 PM, Xiaodi Wu  wrote:
>
> On Wed, Jul 27, 2016 at 10:30 PM, David Owens II via swift-evolution <
> swift-evolution@swift.org> wrote:
>
>
>> > On Jul 27, 2016, at 7:18 PM, John McCall  wrote:
>> >
>> >
>> >> On Jul 27, 2016, at 6:55 PM, David Owens II via swift-evolution <
>> swift-evolution@swift.org> wrote:
>> >>
>> >> Yes, it’s per file. It’s also added in the initial template that Xcode
>> creates with your project. In addition, it’s recommended by many that talk
>> about “how to unit test in Swift.” So, to someone that is not paying
>> scrupulous attention, there is no mechanism to prevent against this today.
>> >
>> > Perhaps we're not doing a good job of messaging this.
>> >
>> > The Xcode template is the way it is because @testable imports are the
>> right default for a *program* written in Swift.  A lot of the code in an
>> application or command-line program isn't really suitable for independent,
>> in-process black-box testing, because it isn't really presenting an
>> integrated API.  If you break it down into components that can be
>> independently tested, that's awesome, and at that point you can switch the
>> imports in your tests over to non-@testable.  But we don't want the test
>> template to create any obstacles to testing, because as you say, people
>> already don't write enough tests.
>>
>> My primary concern is that it’s easy to think you’ve done the right thing
>> and push out a release because all of your testing shows it’s good, only to
>> find you messed up in a way that it’s easy for a tool to validate. Writing
>> new code is probably not going to be the primary source of this, but
>> refactoring a `public` class to an `open` one, where there are already
>> existing tests for that class, probably in a single file and using
>> `@testable`, it’s easy to say, “looks good, tests passed, integrations look
>> good.”
>>
>
> If you're refactoring `public` classes into `open` ones, though, you'd be
> more likely to have forgotten to open a method you intend to make
> overridable, no? And there's no way to change the rules here to have
> `@testable` pick that up…
>
>
> Yes, `@testable` does exactly that. It basically treats the imported
> module as being part of the module it’s being imported into and gives the
> type all of the same access levels. So I’d be able to override a method
> that is *not* marked as open with no issues.
>
> I see nothing in the proposal that restricts a class within the same
> module from subclassing and overriding `public` methods within the *same*
> module that it’s defined in.
>
> So you need to have test cases that have *no* `@testable import
> ModuleToTest` to ensure the API contracts are being tested well. This is
> John’s point, that proper public API (e.g. “black box”) test cases would
> alleviate this issue from happening.
>
> -David
>
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[David Owens II via swift-evolution]

> On Jul 27, 2016, at 8:52 PM, Xiaodi Wu  wrote:
> 
> On Wed, Jul 27, 2016 at 10:30 PM, David Owens II via swift-evolution 
> > wrote:
> 
> > On Jul 27, 2016, at 7:18 PM, John McCall  > > wrote:
> >
> >
> >> On Jul 27, 2016, at 6:55 PM, David Owens II via swift-evolution 
> >> > wrote:
> >>
> >> Yes, it’s per file. It’s also added in the initial template that Xcode 
> >> creates with your project. In addition, it’s recommended by many that talk 
> >> about “how to unit test in Swift.” So, to someone that is not paying 
> >> scrupulous attention, there is no mechanism to prevent against this today.
> >
> > Perhaps we're not doing a good job of messaging this.
> >
> > The Xcode template is the way it is because @testable imports are the right 
> > default for a *program* written in Swift.  A lot of the code in an 
> > application or command-line program isn't really suitable for independent, 
> > in-process black-box testing, because it isn't really presenting an 
> > integrated API.  If you break it down into components that can be 
> > independently tested, that's awesome, and at that point you can switch the 
> > imports in your tests over to non-@testable.  But we don't want the test 
> > template to create any obstacles to testing, because as you say, people 
> > already don't write enough tests.
> 
> My primary concern is that it’s easy to think you’ve done the right thing and 
> push out a release because all of your testing shows it’s good, only to find 
> you messed up in a way that it’s easy for a tool to validate. Writing new 
> code is probably not going to be the primary source of this, but refactoring 
> a `public` class to an `open` one, where there are already existing tests for 
> that class, probably in a single file and using `@testable`, it’s easy to 
> say, “looks good, tests passed, integrations look good.”
> 
> If you're refactoring `public` classes into `open` ones, though, you'd be 
> more likely to have forgotten to open a method you intend to make 
> overridable, no? And there's no way to change the rules here to have 
> `@testable` pick that up…

Yes, `@testable` does exactly that. It basically treats the imported module as 
being part of the module it’s being imported into and gives the type all of the 
same access levels. So I’d be able to override a method that is *not* marked as 
open with no issues.

I see nothing in the proposal that restricts a class within the same module 
from subclassing and overriding `public` methods within the *same* module that 
it’s defined in.

So you need to have test cases that have *no* `@testable import ModuleToTest` 
to ensure the API contracts are being tested well. This is John’s point, that 
proper public API (e.g. “black box”) test cases would alleviate this issue from 
happening.

-David

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Xiaodi Wu via swift-evolution]

On Wed, Jul 27, 2016 at 10:30 PM, David Owens II via swift-evolution <
swift-evolution@swift.org> wrote:

>
> > On Jul 27, 2016, at 7:18 PM, John McCall  wrote:
> >
> >
> >> On Jul 27, 2016, at 6:55 PM, David Owens II via swift-evolution <
> swift-evolution@swift.org> wrote:
> >>
> >> Yes, it’s per file. It’s also added in the initial template that Xcode
> creates with your project. In addition, it’s recommended by many that talk
> about “how to unit test in Swift.” So, to someone that is not paying
> scrupulous attention, there is no mechanism to prevent against this today.
> >
> > Perhaps we're not doing a good job of messaging this.
> >
> > The Xcode template is the way it is because @testable imports are the
> right default for a *program* written in Swift.  A lot of the code in an
> application or command-line program isn't really suitable for independent,
> in-process black-box testing, because it isn't really presenting an
> integrated API.  If you break it down into components that can be
> independently tested, that's awesome, and at that point you can switch the
> imports in your tests over to non-@testable.  But we don't want the test
> template to create any obstacles to testing, because as you say, people
> already don't write enough tests.
>
> My primary concern is that it’s easy to think you’ve done the right thing
> and push out a release because all of your testing shows it’s good, only to
> find you messed up in a way that it’s easy for a tool to validate. Writing
> new code is probably not going to be the primary source of this, but
> refactoring a `public` class to an `open` one, where there are already
> existing tests for that class, probably in a single file and using
> `@testable`, it’s easy to say, “looks good, tests passed, integrations look
> good.”
>

If you're refactoring `public` classes into `open` ones, though, you'd be
more likely to have forgotten to open a method you intend to make
overridable, no? And there's no way to change the rules here to have
`@testable` pick that up...

> In contrast, a library developer needs to be more conscientious about the
> difference between black-box and white-box testing.  In fact, they should
> really be leaning towards making black-box tests whenever that's reasonably
> practical.  We don't currently have Xcode templates around making Swift
> libraries, though; I think we all agree that there's a lot of room for
> tools improvement there.
>
> Just FYI: The default Frameworks target also uses @testable for the Swift
> unit tests.
>
> Regardless, there are mitigations that can turn this potential (maybe it’s
> not even an issue in practice) test-time error into a near compile-time
> error (using a linter).
>
> -David
> ___
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
>
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[David Owens II via swift-evolution]

> On Jul 27, 2016, at 7:18 PM, John McCall  wrote:
> 
> 
>> On Jul 27, 2016, at 6:55 PM, David Owens II via swift-evolution 
>>  wrote:
>> 
>> Yes, it’s per file. It’s also added in the initial template that Xcode 
>> creates with your project. In addition, it’s recommended by many that talk 
>> about “how to unit test in Swift.” So, to someone that is not paying 
>> scrupulous attention, there is no mechanism to prevent against this today.
> 
> Perhaps we're not doing a good job of messaging this.
> 
> The Xcode template is the way it is because @testable imports are the right 
> default for a *program* written in Swift.  A lot of the code in an 
> application or command-line program isn't really suitable for independent, 
> in-process black-box testing, because it isn't really presenting an 
> integrated API.  If you break it down into components that can be 
> independently tested, that's awesome, and at that point you can switch the 
> imports in your tests over to non-@testable.  But we don't want the test 
> template to create any obstacles to testing, because as you say, people 
> already don't write enough tests.

My primary concern is that it’s easy to think you’ve done the right thing and 
push out a release because all of your testing shows it’s good, only to find 
you messed up in a way that it’s easy for a tool to validate. Writing new code 
is probably not going to be the primary source of this, but refactoring a 
`public` class to an `open` one, where there are already existing tests for 
that class, probably in a single file and using `@testable`, it’s easy to say, 
“looks good, tests passed, integrations look good.”

> In contrast, a library developer needs to be more conscientious about the 
> difference between black-box and white-box testing.  In fact, they should 
> really be leaning towards making black-box tests whenever that's reasonably 
> practical.  We don't currently have Xcode templates around making Swift 
> libraries, though; I think we all agree that there's a lot of room for tools 
> improvement there.

Just FYI: The default Frameworks target also uses @testable for the Swift unit 
tests.

Regardless, there are mitigations that can turn this potential (maybe it’s not 
even an issue in practice) test-time error into a near compile-time error 
(using a linter).

-David
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[David Owens II via swift-evolution]

Yes, it’s per file. It’s also added in the initial template that Xcode creates 
with your project. In addition, it’s recommended by many that talk about “how 
to unit test in Swift.” So, to someone that is not paying scrupulous attention, 
there is no mechanism to prevent against this today.

This also assumes that people write tests… which, well, we know is not the case 
at all.

I guess time will tell if this should be a warning/error or not depending on 
the usage of the various Swift linters that I’m sure will start to become 
prevalent. I know it’s something I’ll be enabling in my code.

It seems this design is acceptable and by-design.

-David


> On Jul 27, 2016, at 6:36 PM, Brent Royal-Gordon  
> wrote:
> 
>> On Jul 27, 2016, at 4:41 PM, David Owens II via swift-evolution 
>>  wrote:
>> 
>> 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?
> 
> I could be wrong, but isn't `@testable import` applied per-file? So if you 
> keep code users should actually be able to write in one file with a 
> non-`@testable` import, and mocks and other testing hacks in a different file 
> with an `@testable` import, the first file should fail to compile if you use 
> anything that's not normally permitted.
> 
> In a future version of Swift, we might consider refining this by requiring 
> people to apply `@testable` directly to declarations which treat something 
> closed as if it's open, but it seems like even the current feature set does 
> not make testing impossible.
> 
> -- 
> Brent Royal-Gordon
> Architechies
> 

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Scott James Remnant via swift-evolution]

I’m a bit confused here…

Since the result of breaching your subclassing contract is a compile-time 
error, how are you intending to test that using XCTest?

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).

That’s not a Unit Test, that’s a functional test, so I’d probably make that a 
separate test target anyway.

Scott


> On Jul 27, 2016, at 4:41 PM, David Owens II  wrote:
> 
> 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 
>> > 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@swift.org 
>> https://lists.swift.org/mailman/listinfo/swift-evolution
> 

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Matthew Johnson via swift-evolution]

> On Jul 27, 2016, at 8:47 PM, Brent Royal-Gordon  
> wrote:
> 
>> On Jul 27, 2016, at 6:43 PM, Matthew Johnson  wrote:
>> 
>>> In a future version of Swift, we might consider refining this by requiring 
>>> people to apply `@testable` directly to declarations which treat something 
>>> closed as if it's open, but it seems like even the current feature set does 
>>> not make testing impossible.
>> 
>> +1 to @testable on declarations.  I really do not like making things 
>> internal when they should be private just because a test needs to inspect 
>> state.
> 
> Whoa, that wasn't what I was suggesting at all. I was suggesting that the 
> *test suite* should mark the forbidden subclass/override with `@testable`.

Sorry, I misread that.  But I still stand by the enhancement to @testable I 
mentioned.  There are times when access control is wider than necessary to 
support tests and it always sucks doing that.

> 
> -- 
> Brent Royal-Gordon
> Architechies
> 

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Brent Royal-Gordon via swift-evolution]

> On Jul 27, 2016, at 6:43 PM, Matthew Johnson  wrote:
> 
>> In a future version of Swift, we might consider refining this by requiring 
>> people to apply `@testable` directly to declarations which treat something 
>> closed as if it's open, but it seems like even the current feature set does 
>> not make testing impossible.
> 
> +1 to @testable on declarations.  I really do not like making things internal 
> when they should be private just because a test needs to inspect state.

Whoa, that wasn't what I was suggesting at all. I was suggesting that the *test 
suite* should mark the forbidden subclass/override with `@testable`.

-- 
Brent Royal-Gordon
Architechies

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Matthew Johnson via swift-evolution]

> On Jul 27, 2016, at 8:36 PM, Brent Royal-Gordon via swift-evolution 
>  wrote:
> 
>> On Jul 27, 2016, at 4:41 PM, David Owens II via swift-evolution 
>>  wrote:
>> 
>> 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?
> 
> I could be wrong, but isn't `@testable import` applied per-file? So if you 
> keep code users should actually be able to write in one file with a 
> non-`@testable` import, and mocks and other testing hacks in a different file 
> with an `@testable` import, the first file should fail to compile if you use 
> anything that's not normally permitted.
> 
> In a future version of Swift, we might consider refining this by requiring 
> people to apply `@testable` directly to declarations which treat something 
> closed as if it's open, but it seems like even the current feature set does 
> not make testing impossible.

+1 to @testable on declarations.  I really do not like making things internal 
when they should be private just because a test needs to inspect state.

> 
> -- 
> Brent Royal-Gordon
> Architechies
> 
> ___
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Brent Royal-Gordon via swift-evolution]

> On Jul 27, 2016, at 4:41 PM, David Owens II via swift-evolution 
>  wrote:
> 
> 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?

I could be wrong, but isn't `@testable import` applied per-file? So if you keep 
code users should actually be able to write in one file with a non-`@testable` 
import, and mocks and other testing hacks in a different file with an 
`@testable` import, the first file should fail to compile if you use anything 
that's not normally permitted.

In a future version of Swift, we might consider refining this by requiring 
people to apply `@testable` directly to declarations which treat something 
closed as if it's open, but it seems like even the current feature set does not 
make testing impossible.

-- 
Brent Royal-Gordon
Architechies

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Jordan Rose via swift-evolution]

Other than warning on open methods in non-open classes, I can’t think of a good 
way to avoid this and still preserve the rest of the testing model. After all, 
@testable allows you to override internal methods too.

(Just to make things clear, @testable only applies to the current file, so it’s 
possible to have a single test bundle with both black-box and glass-box tests, 
as long as they’re in different files.)

Jordan


> On Jul 27, 2016, at 17:40, David Owens II via swift-evolution 
>  wrote:
> 
> While arguably true, that's a philosophical debate. There are plenty of 
> reasons to use @testable, and if that's what people are using, then I think 
> there is a valid concern here. 
> 
> Sent from my iPhone
> 
> On Jul 27, 2016, at 5:22 PM, John McCall  > wrote:
> 
>>> On Jul 27, 2016, at 4:41 PM, David Owens II via swift-evolution 
>>> > wrote:
>>> 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?
>> 
>> A "black box" unit test emulating consumer behavior has no business using a 
>> @testable import.  It should just use the external API of the library.
>> 
>> John.
>> 
>>> 
>>> 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 
 > 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@swift.org 
 https://lists.swift.org/mailman/listinfo/swift-evolution 
 
>>> 
>>> ___
>>> swift-evolution mailing list
>>> swift-evolution@swift.org 
>>> https://lists.swift.org/mailman/listinfo/swift-evolution 
>>> 
>> 
> ___
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[David Owens II via swift-evolution]

While arguably true, that's a philosophical debate. There are plenty of reasons 
to use @testable, and if that's what people are using, then I think there is a 
valid concern here. 

Sent from my iPhone

> On Jul 27, 2016, at 5:22 PM, John McCall  wrote:
> 
>> On Jul 27, 2016, at 4:41 PM, David Owens II via swift-evolution 
>>  wrote:
>> 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?
> 
> A "black box" unit test emulating consumer behavior has no business using a 
> @testable import.  It should just use the external API of the library.
> 
> John.
> 
>> 
>> 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 
>>>  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@swift.org
>>> https://lists.swift.org/mailman/listinfo/swift-evolution
>> 
>> ___
>> swift-evolution mailing list
>> swift-evolution@swift.org
>> https://lists.swift.org/mailman/listinfo/swift-evolution
> 
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[John McCall via swift-evolution]

> On Jul 27, 2016, at 4:41 PM, David Owens II via swift-evolution 
>  wrote:
> 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?

A "black box" unit test emulating consumer behavior has no business using a 
@testable import.  It should just use the external API of the library.

John.

> 
> 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 
>> > 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@swift.org 
>> https://lists.swift.org/mailman/listinfo/swift-evolution
> 
> ___
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[David Owens II via swift-evolution]

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 
>  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@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

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

[Scott James Remnant via swift-evolution]

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@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution