SwiftyLab / MetaCodable

Supercharge Swift's Codable implementations with macros meta-programming.
https://swiftpackageindex.com/SwiftyLab/MetaCodable/main/documentation/metacodable
MIT License
629 stars 23 forks source link

CodedBy fails with optional dates decoding #25

Closed murad1981 closed 1 year ago

murad1981 commented 1 year ago

I have the following @Codable model and its HelperCoder struct:

@Codable
struct TestModel {

    @CodedBy(DateCoder())
    var startDate: Date?
}

struct DateCoder: HelperCoder {
    func decode(from decoder: Decoder) throws -> Date? {
        var container = try decoder.unkeyedContainer()
        let dateStr = try container.decode(String.self)
        let df = ISO8601DateFormatter()
        let date = df.date(from: dateStr)
        return date
    }
}

the input JSON is

{
"startDate": "2023-09-20T00:00:00Z"
}

I get this compiler error:

Cannot assign value of type 'Date??' to type 'Date?'

the expansion screenshot is as follows:

Screenshot 2023-10-04 at 3 14 54 PM
soumyamahunt commented 1 year ago

I have the following @Codable model and its HelperCoder struct:

@Codable
struct TestModel {

    @CodedBy(DateCoder())
    var startDate: Date?
}

struct DateCoder: HelperCoder {
    func decode(from decoder: Decoder) throws -> Date? {
        var container = try decoder.unkeyedContainer()
        let dateStr = try container.decode(String.self)
        let df = ISO8601DateFormatter()
        let date = df.date(from: dateStr)
        return date
    }
}

Can you try with returning Date type instead of Date? in DateCoder implementation? That should fix your issue:

 struct DateCoder: HelperCoder {
     func decode(from decoder: Decoder) throws -> Date {
         var container = try decoder.unkeyedContainer()
         let dateStr = try container.decode(String.self)
         let df = ISO8601DateFormatter()
         let date = df.date(from: dateStr)
         return date
     }
 }
murad1981 commented 1 year ago

That solves the syntax error but doesn't solve the problem, there should be a way to decode optional Dates by returning optional Date from the HelperCoder.

There are lots of use cases where we need something like: var someDate: Date? in the models .

If I implement decodeIfPresent(from: ) instead of decode(from:) in the HelperCoder, the compiler auto completes the function signature with the return type of Date?? which should be Date?.

I also tried to use @Default(nil) for the optional date in the model but it also gives an error: Generic parameter 'T' could not be inferred.

soumyamahunt commented 1 year ago

That solves the syntax error but doesn't solve the problem, there should be a way to decode optional Dates by returning optional Date from the HelperCoder.

This use-case can be handled with current implementation as well, HelperCoder has optional requirement decodeIfPresent(from:), which you can implement in your DateCoder, to indicate what happens when you want to decode optional Date.

The default implementation of decodeIfPresent(from:) just returns result from decode(from:) and returns nil if this method throws error.

murad1981 commented 1 year ago

That solves the syntax error but doesn't solve the problem, there should be a way to decode optional Dates by returning optional Date from the HelperCoder.

This use-case can be handled with current implementation as well, HelperCoder has optional requirement decodeIfPresent(from:), which you can implement in your DateCoder, to indicate what happens when you want to decode optional Date.

The default implementation of decodeIfPresent(from:) just returns result from decode(from:) and returns nil if this method throws error.

kindly look at the edited comment above regarding decodeIfPresent(from:) which produces the same problem as well.

soumyamahunt commented 1 year ago

If I implement decodeIfPresent(from: ) instead of decode(from:) in the HelperCoder, the compiler auto completes the function signature with the return type of Date?? which should be Date?.

Can you share the implementation snippet you are trying? I will be able to suggest changes that will make your use-case work.

murad1981 commented 1 year ago

Sample app is attached, please don't return non-optional Date from the decode(from:) method as a workaround solution to the problem, because I need to store nil Date? in the model for some cases. TestCodableMacro.zip

soumyamahunt commented 1 year ago

Sample app is attached, please don't return non-optional Date from the decode(from:) method as a workaround solution to the problem, because I need to store nil Date? in the model for some cases.

As I mentioned, for storing Date? you need to return Date in decode(from:) and return Date? in decodeIfPresent(from:). Your DateCoder implementation should look like this:

struct DateCoder: HelperCoder {
     func decode(from decoder: Decoder) throws -> Date {
         // return Date, this implementation will be used when you are storing Non-optional Date in Model
     }

    func decodeIfPresent(from decoder: Decoder) throws -> Date? {
         // return optional Date, this implementation will be used when you are storing optional Date in Model
     }
 }

Please try to follow this guideline and try to write some test cases for your use cases, please let me know if any test case fails with your implementation code attached.

murad1981 commented 1 year ago

Thank you @soumyamahunt for the last pattern you provided which really solves the issue, but the only take away in this pattern is that I must provide the implementation of decode(from:) to avoid the compiler error: Type 'DateCoder' does not conform to protocol 'HelperCoder', so I just implemented it like this to return a dummy date:

func decode(from decoder: Decoder) -> Date {
         Date()
    }

if you could fix this to only require func decodeIfPresent(from:) when the model value is optional that would be cool

soumyamahunt commented 1 year ago

if you could fix this to only require func decodeIfPresent(from:) when the model value is optional that would be cool

The use of HelperCodable is to provide custom decoder/encoder that will decode/encode a type and it's optional variant. This is the same in principle with the standard library where decode is used to decode a basic type (i.e. Int) decodeIfPresent to decode optional of that type.

You should implement decode(from:) to decode the actual Date type and reuse that implementation in decodeIfPresent(from:) and provide custom logic to handle failure (i.e. returning nil).

Anyway, closing this since there is no issue here.

murad1981 commented 1 year ago

Posting this comment after closing the issue ..

Sorry but that sounds unreasonable to provide implementations for both the basic type and its optional variant for the case when the value in question may or may not return, providing only: decodeIfPresent implementation is enough, like we already do with the init(from decoder:).

Moreover, the implementation of the basic type and its optional variant will not be the same, so it can't be reused in both.

I'm stuck in a similar case, to do a very basic data conversion (string to int), providing a whole struct with double implementations produces too much boilerplate code. (i've emailed. you about it by the way).

@soumyamahunt

rursache commented 7 months ago

I had the same issue as @murad1981 and managed to fix it. While the last messages here went a bit off the rail, the fix is really easy:

@Codable
struct Demo {
    @CodedBy(CustomDateCoder(format: "yyyy-MM-dd"))
    var myDate: Date

    @CodedBy(CustomDateCoder(format: "HH:mm"))
    var myTime: Date?
}
fileprivate struct CustomDateCoder: HelperCoder {
    var format: String

    func decode(from decoder: any Decoder) throws -> Date {
        let container = try decoder.singleValueContainer()
        let dateString = try container.decode(String.self)
        if dateString.isEmpty {
            throw DecodingError.valueNotFound(Date.self, DecodingError.Context(codingPath: decoder.codingPath, debugDescription: "Date string is empty"))
        }
        let dateFormatter = DateFormatter()
        dateFormatter.dateFormat = format
        guard let date = dateFormatter.date(from: dateString) else {
            throw DecodingError.dataCorrupted(DecodingError.Context(codingPath: decoder.codingPath, debugDescription: "Cannot format date"))
        }
        return date
    }
}