p-x9 / AssociatedObject

🔗 Swift Macro for allowing variable declarations even in class extensions
MIT License
130 stars 4 forks source link

Fix Bug: Optional & ImplicitlyUnwrappedOptional #25

Closed mlch911 closed 9 months ago

mlch911 commented 9 months ago

Xcode will give this an error.

// before
@AssociatedObject(.OBJC_ASSOCIATION_COPY)
var test: String!

// expanded
var string: String! {
    get {
        if let value = objc_getAssociatedObject(
            self,
            &Self.__associated_stringKey
        ) as? String! {   // <-- This `String!` is not allowed.
            return value
        }
        return nil
    }
    set {
        objc_setAssociatedObject(
            self,
            &Self.__associated_stringKey,
            newValue,
            .OBJC_ASSOCIATION_ASSIGN
        )
    }
}
mlch911 commented 9 months ago

Should re-run the CI.

p-x9 commented 9 months ago

When defined as follows,

class A {
    @AssociatedObject(.OBJC_ASSOCIATION_RETAIN_NONATOMIC)
    var hello: String? = "default"
}

With this implementation, I think that a nil assignment would return the default value.

let a = A()
print(a.hello) // => default
a.hello = nil
print(a.hello)  // => default

However, in the current main branch implementation, the default values are not reflected in the first acquisition as follows:

let a = A()
print(a.hello) // => nil
a.hello = "modified"
print(a.hello)  // => modified

Therefore, I feel that we need to find a new way to implement this again.

p-x9 commented 9 months ago

In the meantime, since it is difficult to update test cases, we are planning to rewrite the tests using the following library.

https://github.com/pointfreeco/swift-macro-testing

p-x9 commented 9 months ago

The following branch resolved the above issue.

https://github.com/p-x9/AssociatedObject/pull/27

In addition, I thought it would be a better way to replace Wrapped! with Wrapped? in the case of ImplicitlyUnwrappedOptional. (To avoid returning defaultValue after assigning nil)

mlch911 commented 9 months ago

That's better!