Properties of `id<SomeProtocol>` are sometimes declared as `weak var` after conversion

[amorde]

Given:

@import Foundation;

#import "SomeProtocol.h"

@interface SomeObject: NSObject
@property (nonatomic, readonly) id<SomeProtocol> someProperty;
@end

result:

import Foundation

class SomeObject: NSObject {
    private(set) weak var someProperty: SomeProtocol?
}

[alex-swiftify]

Thanks, @amorde for reporting!

As currently implemented, weak var is generated by the converter in the following cases:

• when the weak keyword is specified for the @property declaration
• when the property (or variable) declaration is a protocol reference

The weak keyword is not generated in the following cases (exclusions):

• for @property declarations with a strong keyword
• for protocol property declarations (weak keyword is not allowed inside protocol declarations)
• for collection types like Array/Dictionary/Set
• for Metal classes (MTL*, see #101)

After some Google'ing, and reading the post here:

As a general rule, delegates should be marked as weak because most delegates are referencing classes that they do not own. This is definitely true when a child is using a delegate to communicate with a parent. Using a weak reference for the delegate is what the documentation recommends. (But see this, too.)

... it looks like declaring protocol references as weak is what Apple documentation recommends, although there are always exceptions.

Can you suggest exact changes to our rules explained above with regards to whether we generate the weak keyword or not?

Alternatively, we could introduce a conversion option like "Declare variables or properties of protocol types as weak", but we are trying to have as minimum conversion options as possible to simoplify the testing.

[AlexanderJ]

To me it looks like a delegate is only one of many use cases of this and the recommendation seems to fit that use case. In general it seems safer to not declare this as weak and maybe only do this if the property name contains "delegate". Especially when protocol oriented programming is used this can cause a lot of trouble.

[alex-swiftify]

@AlexanderJ It's an interesting point, although I personally don't find relying on the property name containing "delegate" a good enough solution (while I understand the reasoning for that).

Currently, I don't see a better option than adding a "Declare variables or properties of protocol types as weak" as a configurable option.

[amorde]

The delegate pattern should use weak as you mentioned - however, if the Objective-C code isn't doing this correctly, I don't think the converter should be the one trying to "fix" that issue.

There are many uses for objects of protocol types that are not the delegate pattern, so assuming that any reference to a protocol must be weak does not feel safe to me.

I agree with @AlexanderJ that checking the property name and/or the type name for "delegate" might be better. But if the Objective-C code did not ever use weak then the Swift code shouldn't either

[AlexanderJ]

That's a good point. The Objective-C code might have declared it as weak and only in that case it should be done. Otherwise you change the semantics which is quite dangerous.

[alex-swiftify]

@amorde

The delegate pattern should use weak as you mentioned - however, if the Objective-C code isn't doing this correctly, I don't think the converter should be the one trying to "fix" that issue.

– this is a great point, and completely agreeable.

I agree with @AlexanderJ that checking the property name and/or the type name for "delegate" might be better. But if the Objective-C code did not ever use weak then the Swift code shouldn't either

From what you say, it's not very obvious if you suggest an alternate behavior depending on whether the identifier name or the type name contains delegate:

• if the Objective-C code uses the weak keyword, we should always use the weak keyword in Swiftify (it already works this way)
• if the Objective-C code doesn't use the weak keyword, we should not use the weak keyword in Swiftify (I also don't think we should make an exception if an identifier or a type name contains delegate).

Finally, I'm going to change the rules from the ones we currently use to the following.

Generate weak var in the following case(s):

• when the weak keyword is specified for the @property declaration

Don't generate weak var in the following cases (exclusions):

• for @property declarations with a strong keyword

@amorde @AlexanderJ Please confirm.

[AlexanderJ]

According to the developer documentation a property is strong by default. So unless you see a weak in the property declaration there should be no weak in the Swift translation. https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/EncapsulatingData/EncapsulatingData.html

[alex-swiftify]

Another case to retest - https://github.com/Swiftify-Corp/Swiftify/issues/217

[alex-swiftify]

@amorde @AlexanderJ Fixed by changing the rules to the ones specified below. Note that there are small changes after my last suggestion.

The changes will be available since the next converter update. Your feedback is welcome, as always.

Generate weak var in the following case(s):

• when the weak keyword is specified for the @property declaration

Don't generate weak var in the following cases (exclusions):

• for protocol property declarations (weak keyword is not allowed inside protocol declarations)

Converter output:

import Foundation

class SomeObject: NSObject {
    private(set) var someProperty: SomeProtocol?
}