First pass at marking types as Sendable

[domkm]

Related to #433

[domkm]

I used @unchecked Sendable on JSONDictionary because of the underlying [String:Any] dictionary. @unchecked can be removed if we change the underlying to [String:Sendable].

[mickael-menu]

Thanks @domkm!

I don't plan to merge strict-concurrency into develop unless we can get rid of all the warnings. I guess the warnings are still there in your PR? If yes, could you rebase to develop without including the changes from strict-concurrency?

I used @unchecked Sendable on JSONDictionary because of the underlying [String:Any] dictionary. @unchecked can be removed if we change the underlying to [String:Sendable].

Using [String: Sendable] is fine by me, it's supposed to contain only JSON values anyway (String, Int, etc.), so Sendable seems appropriate.

[domkm]

I don't plan to merge strict-concurrency into develop unless we can get rid of all the warnings. I guess the warnings are still there in your PR? If yes, could you rebase to develop without including the changes from strict-concurrency?

Sure, I can rebase it on develop if you'd prefer, but may I ask why you don't want to merge with strict concurrency warnings? The warnings highlight potential bugs, many of which are errors in Swift 6. In other words, hiding the warnings just hides issues but doesn't change behavior.

Using [String: Sendable] is fine by me, it's supposed to contain only JSON values anyway (String, Int, etc.), so Sendable seems appropriate.

Cool. Should it be Codable too?

[mickael-menu]

but may I ask why you don't want to merge with strict concurrency warnings? The warnings highlight potential bugs, many of which are errors in Swift 6. In other words, hiding the warnings just hides issues but doesn't change behavior.

We have a strict no warnings policy to merge things in the repo (that's why your PR fails all the checks). The goal is to progressively address these issues before Swift 6, without allowing other types of warnings to slip through.

Cool. Should it be Codable too?

Yes that's a good idea. 👍

[domkm]

Unfortunately, I wasn't able to make JSONDictionary values Codable without cascading issues, as there are many places that assume it can contain NSNull and nil.

I'm also curious, what is the purpose of this JSONDictionary encoding/decoding scheme instead of using Codable directly?

[mickael-menu]

I'm also curious, what is the purpose of this JSONDictionary encoding/decoding scheme instead of using Codable directly?

I don't remember exactly, but it might be because we use JSONSerialization.jsonObject() to parse JSON, which returns Any and can't be casted to [String: Codable].

[domkm]

I don't remember exactly, but it might be because we use JSONSerialization.jsonObject() to parse JSON, which returns Any and can't be casted to [String: Codable].

Would it make sense to make these types, or at least the main ones, Codable? In my project, I extended Locator and Manifest to be Codable, and I would guess that almost every project at least extends Locator.

[mickael-menu]

Would it make sense to make these types, or at least the main ones, Codable? In my project, I extended Locator and Manifest to be Codable, and I would guess that almost every project at least extends Locator.

Sure, they are all meant to be serializable to JSON. JSONDictionary itself could be Codable using a custom implementation.

Note that many of the models have complex (de)serialization rules so we can't use the default Codable implementation.

[mickael-menu]

Thank you!