Swiftify-Corp / Swiftify

43 stars 24 forks source link

Properties of a custom class type are always declared as implicitly unwrapped optionals #199

Closed amorde closed 2 years ago

amorde commented 2 years ago

Input:

Header

@import Foundation;

#import "SomeSharedObject.h"

@interface SomeObject: NSObject
@property (nonatomic, nonnull, readonly) SomeSharedObject *someProperty;
@end

Implementation

#import "SomeObject.h"

@interface SomeObject ()
@end

@implementation SomeObject
@end

Result:

import Foundation

class SomeObject: NSObject {
    private(set) var someProperty: SomeSharedObject!
}

Note that the header annotates the property with the nonnull attribute but the property is still declared as an implicitly-unwrapped optional (!)

alex-swiftify commented 2 years ago

Thanks, @amorde for reporting!

After reviewing, I agree that we should change nonnull properties to not generate a non-optional SomeSharedObject instead of an IUO (SomeSharedObject !).

However, this may affect the compilation and add a lot of compiler errors to the generated output for cases when SomeSharedObject has no default value. Question. From your perspective, do you think we should still generate private(set) var someProperty: SomeSharedObject although this won't compile because the property has no initializer?

For records, here is what Xcode's "generated interface" vs Swiftify generate depending on the property nullability specifier:

Xcode:

Swiftify:

amorde commented 2 years ago

Yeah that's a good point, but this actually happens even when the value is initialized. I was simply trying to make the smallest reproducible sample to report here.

For example:

// SomeObject.h

@interface SomeObject: NSObject

@property (nonatomic, nonnull, readonly) SomeSharedObject *someProperty;

- (nonnull instancetype)initWithValue:(nonnull SomeSharedObject *)value;

@end
// SomeObject.m

#import "SomeObject.h"

@interface SomeObject ()
@end

@implementation SomeObject

- (nonnull instancetype)initWithValue:(nonnull SomeSharedObject *)value;
{
    self = [super init];
    if (self) {
        _value = value;
    }
    return self
}

@end

This code still results in the IUO:

//  Converted to Swift 5.5 by Swiftify v5.5.40471 - https://swiftify.com/
import Foundation

class SomeObject: NSObject {
    private(set) var someProperty: SomeSharedObject!

    init(value: SomeSharedObject) {
        super.init()
        self.value = value
    }
}

If you can detect if there's an initialized value (or not) it makes sense to me to adjust for that, but otherwise using the nullability annotations as the source of truth would be helpful.

alex-swiftify commented 2 years ago

@amorde Makes sense.

While it should be possible to detect whether the property is initialized or not, this is more difficult/error-prone. For example, if one converts a header (.h) file without the source (.m) file, we won't be able to detect whether a property is initialized or not. Additionally, I'd prefer to make the conversion rules as easy as possible.

Do you think if we'll change the rules to the following (regardless of whether the property is initialized or not), that would be a good enough solution?

nonnull -> SomeSharedObject

In my opinion, if the Objective-C code declares the property as nonnull, it will either have an initializer (i.e. for object types), or rely on the fact that properties of ordinary types like int will get initialized with default values (e.g. 0) - in these cases, Swiftify is able to generate that default initializer as well.

alex-swiftify commented 2 years ago

@amorde Fixed as suggested above (will be available since the next converter update).

Converter output:

import Foundation

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