Fix strictDecoding
by only propagating not-decoded keys when decoding is successful
#29
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.
I encountered a
strictDecoding
-relatedTOMLDecoder
bug while working on Swift Bundler. If a customDecodable
implementation (init(from:)
) attempts to decode with an initial strategy and then attempts a different strategy on failure, any keys not used by the initial failed (and ignored) attempt still get propagated innotDecodedKeys
and end up causing decoding to throw an unused key error right at the end of the decoding process.There were two flavours of this issue;
Decodable
implementation somewhere in the decoding process throws an error, then its unused keys still get propagated.Flavour 1 was relatively easy to fix. I just updated the
decode<T>(_:)
method of each container type to give the inner decoder a freshNotDecodedKeys
instance and then merge the keys, but only if decoding was successful.Flavour 2 is clearly harder to address. In my current patch I've made the assumption that whenever a container gets created from an
InternalTOMLDecoder
, any containers created from the decoder in the past have now been superseded. I feel like there could be some strange situation where someone might create two containers and make use of both of them to construct the final output, but I dunno if that needs to be supported, and it definitely wouldn't be as common as the 'try one thing and then try another' pattern that I was using when I discovered this issue. Let me know what you thinkI've added tests for both flavours.