Throw error when an invalid key is present in a JSON Apollo configuration

[Iron-Ham]

What are you trying to accomplish?

closes #2942

Notify users when they create an invalid apollo-codegen-config.json.

What approach did you choose and why?

I created an AnyCodingKey struct which cannot fail. I create sets of all the keys present in the input, and then make sure that the input keys are a subset of the CodingKeys defined for that object.

Anything you want to highlight for special attention from reviewers?

Is the error text here sufficient to explain to users that something is wrong?

[netlify]

👷 Deploy request for apollo-ios-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	9235769

[calvincestari]

Thanks @Iron-Ham!

It looks like the key validation has been implemented in:

• FileOutput
• OutputOptions
• ExperimentalFeatures
• base (ApolloCodegenConfiguration)

It's not implemented in these structs though:

• FileInput
• SchemaTypesFileOutput
• ConversionStrategies
• OperationDocumentFormat
• SelectionSetInitializers

We should decide if we're going to enforce a 100% key-correct configuration file or only perform key validation on the most common places we've noticed incorrect configurations.

• If we're going to enforce 100% key-correctness then some of the structs will need to get decode implementations because at the moment we've only done that where custom behaviour was needed.
• If we're only going to validate at the most error-prone structs then we can likely remove validation of ExperimentalFeatures.

I'm not sure it's worth the effort to go the 100% key-correctness route and I think I'd instead choose to just validate the most error-prone. Thoughts?

[calvincestari]

We could maybe improve this by being more explicit in which keys are unrecognized; subtracting validKeys from allKeys?

[AnthonyMDev]

I'm wondering if this could be put into a protocol that automatically calls this function so we don't have to remember to call it in each of the decoder functions? We could call it StrictlyKeyedDecodable.

I'm not sure if that would cause more complexity with the CodingKeys?

[Iron-Ham]

I'm wondering if this could be put into a protocol that automatically calls this function so we don't have to remember to call it in each of the decoder functions?

That was my first thought, but I shied away from it when I realized that Codable (Encodable + Decodable) doesn't have any reference to CodingKeys. Adds a lot of complexity for a function that we'd otherwise call a handful of times in one file.

[AnthonyMDev]

Yupp, that makes sense. If we were using this all over might be worth it, but for this one place, totally fine.

[Iron-Ham]

Agreed that we should really just tackle the most error prone ones. I'll remove experimental features and adjust the error String.

[AnthonyMDev]

Thanks so much for putting this together @Iron-Ham . I think this could be improved by turning it into a protocol, and adding additional test cases, but this is great!

[AnthonyMDev]

I'm wondering if this could be put into a protocol that automatically calls this function so we don't have to remember to call it in each of the decoder functions? We could call it StrictlyKeyedDecodable.

I'm not sure if that would cause more complexity with the CodingKeys?

[calvincestari]

LGTM - @AnthonyMDev, thoughts on the protocol route?

[AnthonyMDev]