Changes on swift42 break attributeValueScalarType for enumerations

[atomicbird]

The changes in the swift42 branch are doing a lot for Swiftiness but they currently break the attributeValueScalarType when the type is an enumeration. With a trivial enumeration such as

@objc public enum EventType: UInt16 {
    case conference
    case party
    case meeting
    case concert
}

The generated code for an accessor using attributeValueScalarType = EventType will be

public var eventType: EventType? {
    get {
        let key = Event.Attributes.eventType
        willAccessValue(forKey: key)
        defer { didAccessValue(forKey: key) }

        return primitiveValue(forKey: key) as? EventType
    }
    set {
        let key = Event.Attributes.eventType
        willChangeValue(forKey: key)
        defer { didChangeValue(forKey: key) }

        guard let value = newValue else {
            setPrimitiveValue(nil, forKey: key)
            return
        }
        setPrimitiveValue(value, forKey: key)
    }
}

This will incorrectly use setPrimitiveValue with a value of the enumeration type. The code will crash because the value isn't representable in Objective-C (even though the enum is declared @objc).

In contrast, master currently generates the following:

@NSManaged open
    var type: EventType

This works as one would expect.

The new code generation was added to support optional scalars, which is a good thing. However it breaks this existing behavior.

The obvious fix is to use rawValue to convert to/from the enumeration type in the generated code, however that would break non-enumeration types that don't have a rawValue. For example the trivial typealias MyType = Uint16 should work with attributeValueScalarType, but would be broken by code that assumed it could use rawValue.

It may be that a separate user info key is needed to handle Swift enums.

[atomicbird]

@radianttap This code for attributeValueScalarType originates in your PR. Have you encountered this?

[atomicbird]

On the one hand, implementing #375 would cover this if we required enumerations to support Codable, so fixing both might not be strictly necessary. On the other, it seems like it would be good to continue support for enumerations that have numeric raw types, for example so they can be used in predicates. I'm leaning toward covering both, fixing this issue plus implementing #375.

[atomicbird]

I just pushed 3f3eb0f which should handle this. Feedback welcome-- this branch hasn't been merged yet, after all.

That commit adds a new attributeRawValueEnumType which is intended for use on any attribute corresponding to a Swift enum with a raw value, including numeric raw values and strings. To use it:

• Create an attribute in the data model with the appropriate type (numeric or string)
• Add attributeRawValueEnumType = in the user info for the attribute.

The generated code will look like this:

public var eventType: EventType? {
    get {
        let key = Event.Attributes.eventType
        willAccessValue(forKey: key)
        defer { didAccessValue(forKey: key) }

        guard let primitiveValue = primitiveValue(forKey: key) as? EventType.RawValue else { return nil }
        return EventType(rawValue: primitiveValue)
    }
    set {
        let key = Event.Attributes.eventType
        willChangeValue(forKey: key)
        defer { didChangeValue(forKey: key) }

        guard let value = newValue else {
            setPrimitiveValue(nil, forKey: key)
            return
        }
        setPrimitiveValue(value.rawValue, forKey: key)
    }
}

The Swift template file is getting a little hairy, but that's really nothing new. If I can learn the template language a little better I might be able to clean it up (I see there's a "procedure" command that's starting to sound useful but I don't know how it works yet).

In parallel with these changes I've been working on some updated documentation. I'll also be covering changes in this branch as part of my try! Swift NYC 2018 talk next month.

[radianttap]

The final implementation of the generated conversion here is great, equivalent to what I did with CoreDataRepresentable. I like the use of attributeRawValueEnumType to separate this from previously existing attributeValueScalarType.

Not sure do we need the name to be that specific, maybe just attributeRawValueType, but that just nitpicking.

One specific improvement I have to satisfy is that my protocol has coredataFallback property which can be used to safely implement non-optional attribute. Although it's somewhat dangerous since failing to convert the stored primitive value into enum type equals data corruption and my way of handling it just hides that.

(Looking forward to watch the Try! Swift talk, I hope video is available soon).