Route Dictionary through key-aware encoding in type-erased Codable paths - #46
Open
vincentborko wants to merge 1 commit into
Open
Route Dictionary through key-aware encoding in type-erased Codable paths#46vincentborko wants to merge 1 commit into
vincentborko wants to merge 1 commit into
Conversation
A Swift Dictionary is represented in Kotlin as a real LinkedHashMap that also
conforms to Collection<Tuple2<K,V>> (hence Sequence<Tuple2>) to satisfy Swift's
Sequence surface. skip-lib's Codable dispatch has a statically-typed fast path
(the reified Dictionary overloads) and a type-erased path (the generic
encode<T>/encodeIfPresent<T> overrides plus the codableUnkeyed/SingleValue/
DictionaryKeyed free helpers). The type-erased path only checked `is Sequence<*>`,
so a Dictionary reaching it — e.g. an optional dictionary whose encodeIfPresent
resolves to the generic overload, or a Dictionary nested as a value inside
another Dictionary — was iterated as a flat sequence of raw Tuple2 pairs. Those
bare Tuple2 values are neither primitives nor Encodable, producing
"skip.lib.Tuple2 cannot be cast to skip.lib.Encodable" and, where it did not
crash, silent JSON-shape corruption ({"a":1} serialized as [["a",1]]).
Add an explicit `is Dictionary<*,*>` branch ahead of every `is Sequence<*>`
check that routes to the existing key-aware encoding, choosing keyed-object vs
unkeyed-array form from the runtime key type (Int/String -> object, else array)
to mirror the reified overloads.
Fixes skiptools/skip-foundation#62
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
JSONEncoder().encode(...)on aCodabletype with a populated dictionary property can crash with:Reported as skiptools/skip-foundation#62. A non-optional, statically-typed dictionary property already works, so the failure is easy to miss until real (non-empty) data flows through an optional or nested dictionary.
Root cause
A Swift
Dictionary<K, V>is backed by a realLinkedHashMapbut also conforms toCollection<Tuple2<K, V>>(henceSequence<Tuple2>) to provide Swift'sSequence/Collectionsurface. skip-lib's Codable dispatch has two tiers:inline reifiedencode(value: Dictionary<K, V>, …)overloads, which iterate.storageand preserve keys; andDictionary<K, V>type has been lost — the genericencode<T>/encodeIfPresent<T>overrides on the containers, and the freecodableUnkeyedEncode/codableSingleValueEncode/codableDictionaryKeyedEncodehelpers.The type-erased path checked only
is Sequence<*>before its fallthrough. Because aDictionaryis aSequence<Tuple2>, any dictionary reaching this path is iterated as a flat array of rawTuple2pairs; eachTuple2then matches none of the primitive cases and falls through tocontainer.encode(value), which the concrete encoder eventually force-casts toEncodable— throwing onTuple2. Where it happens not to crash, it silently corrupts shape ({"a":1}→[["a",1]]).Verified on Robolectric, the literal skip-foundation#62 repro reaches this path via an optional dictionary:
container.encodeIfPresent(optionalDict, …)resolves to the genericencodeIfPresent<T>→ genericencode<T>override, not the reified overload. A nested[String: [String: String]]reaches it throughcodableDictionaryKeyedEncodefor the inner dictionary value.Fix
Add an explicit
is Dictionary<*, *>branch ahead of everyis Sequence<*>check in the type-erased dispatch (theTopLevelEncoder/ keyed / unkeyed / single-valueencode<T>overrides and the three free helpers), routing to the existing key-aware encoding. The keyed-object vs unkeyed-array decision is recovered from the runtime key type (Int/String→ object, else array), mirroring the reified overloads. The existingencodeAsArrayhelpers are relaxed toDictionary<*, *>so they can be shared.Testing
swift test(native): all 174 tests pass.JSONEncoderround-trip over optional and nested dictionaries). Against unpatched skip-lib it reproduces theTuple2 cannot be cast to Encodablecrash on Robolectric; with this change it passes, producing the correct JSON object shape. Verified by building skip-foundation against this branch as a local dependency.Companion PR (defense-in-depth + regression test): skiptools/skip-foundation#127
swift testruns locally.