Objects are assumed nullable when Swiftify can't see the API declaration

[amorde]

Whenever a type comes from an API that Swiftify can't see (ex. from an @import SomeFramework;) it assumes the object is nullable.

test.h:

@import Foundation;

@interface SomeOtherObject: NSObject
@end

test.m

#import "SomeObject.h"
@import SomeOtherObjectFramework; // an import that Swiftify can't resolve

@implementation SomeObject

- (void)someMethod
{
    SomeOtherObject *otherObject = [SomeOtherObject makeOtherObject];
    otherObject.someProperty = YES;
    otherObject.someOtherProperty = NO;
}

@end

Resulting conversion to test.swift:

import Foundation
import SomeOtherObjectFramework

class SomeObject: NSObject {
    func someMethod() {
        let otherObject = SomeOtherObject.make()
        otherObject?.someProperty = true
        otherObject?.someOtherProperty = false
    }
}

// an import that Swiftify can't resolve

Note the otherObject? references. (Side note: the // an import that Swiftify can't resolve comment is oddly placed as well, but not really a pressing issue)

If I add NS_ASSUME_NONNULL_BEGIN / NS_ASSUME_NONNULL_END to the .m to try to remove this behavior it still results in optionals:

test.m:

NS_ASSUME_NONNULL_BEGIN

#import "SomeObject.h"
@import SomeOtherObjectFramework; // an import that Swiftify can't resolve

@implementation SomeObject

- (void)someMethod
{
    SomeOtherObject *otherObject = [SomeOtherObject makeOtherObject];
    otherObject.someProperty = YES;
    otherObject.someOtherProperty = NO;
}

@end

NS_ASSUME_NONNULL_END

Resulting conversion to test.swift:

import Foundation
import SomeOtherObjectFramework

class SomeObject: NSObject {
    func someMethod() {
        let otherObject = SomeOtherObject.make()
        otherObject?.someProperty = true
        otherObject?.someOtherProperty = false
    }
}

// an import that Swiftify can't resolve

[alex-swiftify]

Thank you, Eric for reporting. I've confirmed the problem. Swiftify treats the type of otherObject as unknown and doesn't honor the optionality. We'll look at improving this behavior.

[alex-swiftify]

@amorde After looking further, I have found that:

1. Both NS_ASSUME_NONNULL_BEGIN / NS_ASSUME_NONNULL_END macros and the "Optional specifier to be used for pointer type method parameters and properties" converter option currently affect only the following:

• Method parameters
• Return types
• Properties
• Fields

While working on https://swiftify.atlassian.net/browse/SWC-1176, we have tried to support local variables as well, but this was unavoidably increasing the error rate in the output (which is the most important measure for our converter). Thus support for optionality of the local variables has been postponed to https://swiftify.atlassian.net/browse/SWC-991 (unfortunately I can't suggest a timeline for this to be implemented due to the complexity).

2. As designed by Apple, NS_ASSUME_NONNULL_BEGIN / NS_ASSUME_NONNULL_END macros only affect the types of declarations in the Swift generated interfaces, so I suppose that Apple didn't intend for these macros to affect the local variables either.

3. I have found a bug that NS_ASSUME_NONNULL_BEGIN / NS_ASSUME_NONNULL_END macros did not affect the optionality of fields and properties in some cases (example). This has been just fixed and will produce the following after the next converter update:

class SomeObject: NSObject {
    var otherObject: SomeOtherObject

    func someMethod() {
        otherObject = .make()
        otherObject.someProperty = true
        otherObject.someOtherProperty = false
    }
}

4. The fact that the type comes from an API that Swiftify can't see isn't important in your example, since Swiftify uses the declared type of SomeOtherObject *otherObject to consider the pointer type declaration as optional by default.

I hope that the above changes will be helpful, although the support for local variables is still due. Your feedback is welcome.

[amorde]

Thanks Alex! I primarily mentioned the NS_ASSUME_NONNULL_BEGIN / NS_ASSUME_NONNULL_END macros to signal that there seemed to be no way to change the behavior of the conversion. If there's some other way to address it then that'd be great.

Was there some sort of change that allows configuring this? It looks like the pointer optionality setting still doesn't support local variables.

[alex-swiftify]

@amorde The only way you can currently affect the pointer optionality is by using nullability annotations such as _Nonnull (example). I may revisit how many problems arise if we make the "Optional specifier to be used for pointer type method parameters and properties" option affect local variables too, but as far as I remember this resulted in other hard to solve issues.

Are you sure that in your use case you will benefit from always treating pointer declarations as non-optional? I suppose this may lead to many other issues where type was intended to be optional.

I do really suppose that adding nullability annotations whenever needed is the best bet so far.

[amorde]

The issue we're having is we have very large .m files that make heavy use of custom types that swiftify can't see, so the resulting converted code has hundreds of compilation errors from this. I'd rather have to fix a few mismatches in the other direction than have everything assumed optional, or at least would like a configurable option for it.

And in these cases we do have nullability annotations but swiftify can't see them 😅 that's the issue. the APIs are defined elsewhere, we are @import-ing some other module and using the APIs from there

[alex-swiftify]

@amorde Thanks for elaborating on the issue.

Let's focus on determining the minimum viable amount of changes that will improve your conversion results.

Based off this ticket's summary, it looks like you want the "Optional specifier to be used for pointer type method parameters and properties" converter option as well as NS_ASSUME_NONNULL_BEGIN / NS_ASSUME_NONNULL_END macros to affect the optionality of local variables in addition to (already supported) method parameters, return types, properties and fields. Is that correct?

We can enable this for local variables and make this configurable, so that you will be able to test it. How does that sound?

As regarding dealing with declarations that Swiftify cannot see, I can suggest the following options: a) When converting class A that depends on class B, select both classes with checkboxes in the Offline Converter app. This will ensure that the converter is aware of types declared elsewhere. You can also copy commonly used declarations to a header file and check that header file along with the files that you convert. b) use the "Optional specifier to be used for pointer type method parameters and properties" option to override the default behavior. c) wrap the code where you want to avoid optionals into NS_ASSUME_NONNULL_BEGIN / NS_ASSUME_NONNULL_END macros.

I'm open to your suggestions on how you would like this handled better.

[alex-swiftify]

@amorde We have pushed an update to the Offline Converter where "Optional specifier to be used for pointer type declarations" conversion option as well as NS_ASSUME_NONNULL_BEGIN / NS_ASSUME_NONNULL_END macros support local (and global) variables.

You are welcome to test it, and let me know if that helps.

[amorde]

Just tested it out and it works great! thank you so much 🙏

[alex-swiftify]

Awesome, thanks @amorde for confirming. There might be side effects of these changes, so if you find any, let me know.