feat: Abort build if config file contains invalid keys.

[airblast-dev]

Closes: #822 (Maybe)

fix(example): Remove extra open field from example config file.

feat: Reject aditional fields in schema.

fix: Flatten Core for schema generation.

fix: Actually deny unknown fields in schema.

The change is a workaround the conflict of #[serde(flatten)], and #[serde(deny_unknown_fields)].

There is a downside of this workaround which may be undesired. Since we use the remaining top level fields to initialize Core, there is no way to flatten multiple structures at the top level without increasing complexity quite a bit (At least i couldn't find an easy way). While it is unlikely we will have a many structs that are flattened at the top level, it is still something worth mentioning.

Example error message:

2024-07-19T11:07:37.501660Z  INFO 🚀 Starting trunk 0.21.0-alpha.3
2024-07-19T11:07:37.501764Z ERROR TOML parse error at line 5, column 1
  |
5 | random_field = ""
  | ^^^^^^^^^^^^
unknown field `random_field`, expected one of `target`, `release`, `dist`, `offline`
, `frozen`, `locked`, `public_url`, `public_url_no_trailing_slash_fix`, `no_default_
features`, `all_features`, `features`, `filehash`, `pattern_script`, `inject_scripts
`, `pattern_preload`, `pattern_params`, `root_certificate`, `accept_invalid_certs`, 
`minify`, `no_sri`, `allow_self_closing_script`

The config file that caused the error:

[build]
target = "index.html"
dist = "dist"

random_field = ""

When the incorrect field random_field is moved to the top the error turns into:

2024-07-19T11:09:04.123285Z  INFO 🚀 Starting trunk 0.21.0-alpha.3
2024-07-19T11:09:04.123391Z ERROR unknown field `random_field`, expected one of `tru
nk-version`, `trunk_version`, `dist`

While the PR is in a working state, I haven't bothered with writing testing yet as I had a few problems.

• The Core's dist field is not being used from what I can tell. Perhaps its a seperate issue but I'm mainly asking since if the intention is to remove Core, and just use Build. In that case this whole PR should be much simpler.
• I am somewhat lost on how to verify that the schema.json is correct. Any of the tools I tried give errors before and after the PR. Taking a look at the file things look fine, but I would be happy if someone is able to verify the generated schema. (I also tried with the yaml config in the examples, still had schema errors)

As mentioned above this is a workaround, I was able to come up with based on the docs of the attribute macro for flatten. So if the PR is closed due it being a workaround I would completely understand.

[ctron]

I am wondering if there could be an alternative to that pattern. What about having a "catch all" field in such structs. And we validate that that's empty? Just an idea. Not sure it works. But it might be less intrusive.

[airblast-dev]

Thats essentially whats happening, with the flattened HashMap in InnerCfg. To do this we create a seperate struct with the field we want flattened being replaced by the HashMap. After we get InnerCfg, we take its remaining fields, and try to fill the Core. Any remaining field after that stage would mean there is a top level field that is incorrect. Inner fields (build.dist, ...) are simply handled by the types in the fields.

I am open to other solutions, its just that this is the only solution I could find that gave out good error messages, and was fully contained into a single module. Its completely possible I am missing something though.

I am wondering if there could be an alternative to that pattern. What about having a "catch all" field in such structs. And we validate that that's empty? Just an idea. Not sure it works. But it might be less intrusive.

The issue with your suggestion is that once we do that we might end up with false alarms when a change is introduced. A flattened field does not guarantee that it will actually use the fields (or rather mark them as used). Which means we cannot flatten Core and store the remaining fields with a flattened HashMap without risking a false alarm. We can technically avoid this by storing the field names as a string and compare it to the keys in the HashMap, however this gives quite a bit of room for mistake when a new field is added/removed (I think Core is getting deprecated anyway so this might not be an issue?). See https://serde.rs/attr-flatten.html#capture-additional-fields. One more thing is the error messages will (in most cases) contain less information compared to the one returned from the deserializer.

See related serde issues: serde-rs/serde#1358, serde-rs/serde#2200, serde-rs/serde#2764

From what I can tell this is mainly a problem with enums but since this isn't explicitly documented I didn't want go down that route.

I am aware this solution isn't ideal but on the other hand any solution that isn't hacky would require changes on serde's end. Based on what I read in the issues it wont be anytime soon as it would require updates to libraries like serde_json, yaml... Which means it will be a while until we get a solution that works well and isn't a hack. Since we only flatten one struct in Configuration this seemed the safest way to do this.