SE-0489 followup: DecodingError.valueNotFound mentions null, which is not always right

In SE-0489: Improve EncodingError and DecodingError's printed descriptions, improved the debug descriptions of DecodingError. In particular, the debug description for DecodingError.valueNotFound is now:

"DecodingError.valueNotFound: Expected value of type \(expectedType) but found null instead"

This is in keeping with the docs for the valueNotFound case:

/// An indication that a non-optional value of the given type was expected,
/// but a null value was found.
///
/// As associated values, this case contains the attempted type and context
/// for debugging.
case valueNotFound(Any.Type, Context)

However, I noticed that this error it not used only to indicate null values. It is also used for missing values, like when an unkeyed decoding container runs off the end of an array before it has found all values. Check out swift-foundation’s JSONDecoder.swift:

Since the Swift 6.3 branch is still in prerelease mode, I wonder whether it’s worth adjusting that debug description and doc comment for valueNotFound slightly so it does not imply null in cases where it is not true? I don’t think we have enough information to conditionally say whether we were looking for a value that is null or a missing value in an array, but we could maybe just cut it down to:

"DecodingError.valueNotFound: Expected value of type \(expectedType) but found none"

And then the debug description passed in by the decoder can still do what it does today, and differentiate null vs missing or whatever other context it wants. I’m working on a PR at the moment to improve those debug descriptions in Foundation.

1 Like

Changing the diagnostic message alone would seem to be a bugfix, but the documentation provides a semantic guarantee. Was that guarantee in the documentation shipped in a public release?

Yes, it’s been there for 7 years. But I don’t believe it was ever honored in the sense that JSONDecoder was always using this error case when reaching the end of an unkeyed container, as far as I know.

Here’s a PR for the diagnostic change. What do you recommend for the documentation fix? It’s a tricky situation, since the “official” client of this API has always been slightly out of sync with the documentation, but other clients may be assuming things about it?

Does anyone know of precedent for documentation that was changed in a seemingly-breaking way, but nevertheless brought the API contract in sync with real-world uses?

Ah, if it's always behaved in a way that clearly differs from the documentation (i.e., if the guarantee could never actually be relied on), then it seems reasonable to consider it a documentation bug? Would have the codeowners weigh in on that, of course.

Being, IIRC, the original author of the doc comment for valueNotFound here, I believe it was always incomplete — but also, as far as I (personally) am concerned, not intended to be completely comprehensive about its usage*; i.e., it wouldn't be unreasonable to use valueNotFound to indicate a failure to find a value while decoding in any context while decoding, not only when finding null when decoding from a keyed container.

The behavior of JSONDecoder actually matches the doc comments for UnkeyedDecodingContainer, who themselves have also always specified this:

/// - throws: `DecodingError.valueNotFound` if there are no more values to
///   decode.
mutating func decodeNil() throws -> Bool

/// - throws: `DecodingError.valueNotFound` if the encountered encoded value
///   is null, or of there are no more values to decode.
mutating func decode(_ type: Bool.Type) throws -> Bool

// ... etc.

So, I'd say that it's reasonable to both update the debugDescription of this case and its documentation to spell out that it can be used, e.g., to indicate "no remaining values".


*The documentation for all of the Encoder/Decoder APIs has always had to play double-duty, offering guidance to Encoder/Decoder authors and end-users of the API; in this case, guiding authors about when it's reasonable to throw this error, and to end-users of the API about what may have reasonably happened to cause this error to be thrown — but Decoder authors are not somehow obligated to follow the doc comment here, so vagueness is somewhat warranted.