commit dad96b107c0dd35ce2eecfaecc0e6af4988b3bb4
author Noratrieb <48135649+Noratrieb@users.noreply.github.com> Sun Jul 20 14:35:43 2025 +0200
committer Noratrieb <48135649+Noratrieb@users.noreply.github.com> Mon Jul 21 19:32:44 2025 +0200
tree b5a9e7c8d32d8951f85e86624ce299cefb956f97
parent 6707bf0f59485cf054ac1095725df43220e4be20

Use serde for target spec json deserialize

The previous manual parsing of `serde_json::Value` was a lot of
complicated code and extremely error-prone. It was full of janky
behavior like sometimes ignoring type errors, sometimes erroring for
type errors, sometimes warning for type errors, and sometimes just
ICEing for type errors (the icing on the top).

Additionally, many of the error messages about allowed values were out
of date because they were in a completely different place than the
FromStr impls. Overall, the system caused confusion for users.

I also found the old deserialization code annoying to read. Whenever a
`key!` invocation was found, one had to first look for the right macro
arm, and no go to definition could help.

This PR replaces all this manual parsing with a 2-step process involving
serde.
First, the string is parsed into a `TargetSpecJson` struct. This struct
is a 1:1 representation of the spec JSON. It already parses all the
enums and is very simple to read and write.
Then, the fields from this struct are copied into the actual `Target`.
The reason for this two-step process instead of just serializing into a
`Target` is because of a few reasons

 1. There are a few transformations performed between the two formats
 2. The default logic is implemented this way. Otherwise all the default
    field values would have to be spelled out again, which is
    suboptimal. With this logic, they fall out naturally, because
    everything in the json struct is an `Option`.

Overall, the mapping is pretty simple, with the vast majority of fields
just doing a 1:1 mapping that is captured by two macros. I have
deliberately avoided making the macros generic to keep them simple.

All the `FromStr` impls now have the error message right inside them,
which increases the chance of it being up to date. Some "`from_str`"
impls were turned into proper `FromStr` impls to support this.

The new code is much less involved, delegating all the JSON parsing
logic to serde, without any manual type matching.

This change introduces a few breaking changes for consumers. While it is
possible to use this format on stable, it is very much subject to
change, so breaking changes are expected. The hope is also that because
of the way stricter behavior, breaking changes are easier to deal with,
as they come with clearer error messages.

1. Invalid types now always error, everywhere. Previously, they would
   sometimes error, and sometimes just be ignored (which meant the users
   JSON was still broken, just silently!)
2. This now makes use of `deny_unknown_fields` instead of just warning
   on unused fields, which was done previously. Serde doesn't make it
   easy to get such warning behavior, which was the primary reason that
   this now changed. But I think error behavior is very reasonable too.
   If someone has random stale fields in their JSON, it is likely
   because these fields did something at some point but no longer do,
   and the user likely wants to be informed of this so they can figure
   out what to do.

   This is also relevant for the future. If we remove a field but
   someone has it set, it probably makes sense for them to take a look
   whether they need this and should look for alternatives, or whether
   they can just delete it. Overall, the JSON is made more explicit.

This is the only expected breakage, but there could also be small
breakage from small mistakes. All targets roundtrip though, so it can't
be anything too major.