Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Abort build if config file contains invalid keys. #823

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Trunk.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ ignore = []
addresses = ["127.0.0.1"]
# The port to serve on.
port = 8080
# Open a browser tab once the initial build is complete.
open = false
# Whether to disable fallback to index.html for missing files.
no_spa = false
# Disable auto-reload of the web app.
Expand Down
22 changes: 15 additions & 7 deletions schemas/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"$ref": "#/definitions/Watch"
}
},
"additionalProperties": false,
"definitions": {
"AddressFamily": {
"type": "string",
Expand Down Expand Up @@ -221,7 +222,8 @@
"default": "index.html",
"type": "string"
}
}
},
"additionalProperties": false
},
"Clean": {
"description": "Config options for the serve system.",
Expand All @@ -240,7 +242,8 @@
"null"
]
}
}
},
"additionalProperties": false
},
"Hook": {
"description": "Config options for build system hooks.",
Expand Down Expand Up @@ -269,7 +272,8 @@
}
]
}
}
},
"additionalProperties": false
},
"Hooks": {
"description": "Newtype for handling `Vec<Hook>`",
Expand Down Expand Up @@ -372,7 +376,8 @@
"default": false,
"type": "boolean"
}
}
},
"additionalProperties": false
},
"Serve": {
"description": "Config options for the serve system.",
Expand Down Expand Up @@ -528,7 +533,8 @@
}
]
}
}
},
"additionalProperties": false
},
"Tools": {
"description": "Config options for automatic application downloads.",
Expand Down Expand Up @@ -566,7 +572,8 @@
"null"
]
}
}
},
"additionalProperties": false
},
"Uri": {
"type": "string",
Expand All @@ -593,7 +600,8 @@
"type": "string"
}
}
}
},
"additionalProperties": false
},
"WsProtocol": {
"description": "WebSocket protocol",
Expand Down
1 change: 1 addition & 0 deletions src/config/models/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use std::{

/// Config options for the build system.
#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct Build {
/// The index HTML file to drive the bundling process
#[serde(default = "default::target")]
Expand Down
1 change: 1 addition & 0 deletions src/config/models/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::path::PathBuf;

/// Config options for the serve system.
#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct Clean {
/// The output dir for all final assets
#[serde(default, skip_serializing_if = "Option::is_none")]
Expand Down
1 change: 1 addition & 0 deletions src/config/models/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::path::PathBuf;

/// Config options for the core project.
#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct Core {
#[serde(default)]
// align that with cargo's `rust-version`
Expand Down
1 change: 1 addition & 0 deletions src/config/models/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use serde::{Deserialize, Serialize};
/// Config options for build system hooks.
#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
#[serde(deny_unknown_fields)]
pub struct Hook {
/// The stage in the build process to execute this hook.
pub stage: PipelineStage,
Expand Down
132 changes: 107 additions & 25 deletions src/config/models/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub use clean::*;
pub use core::*;
pub use hook::*;
pub use proxy::*;
use serde_json::Value;
pub use serve::*;
pub use tools::*;
pub use watch::*;
Expand All @@ -28,9 +29,9 @@ mod test;

use anyhow::{bail, Context, Result};
use schemars::JsonSchema;
use serde::Deserialize;
use serde::{de::IntoDeserializer, Deserialize};
use source::Source;
use std::path::PathBuf;
use std::{collections::HashMap, path::PathBuf};
use tracing::log;

/// Common configuration model functionality
Expand All @@ -41,33 +42,114 @@ pub trait ConfigModel {
}
}

/// The persisted Trunk configuration model
#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, JsonSchema)]
pub struct Configuration {
#[serde(flatten)]
pub core: Core,

#[serde(default)]
pub build: Build,

#[serde(default)]
pub tools: Tools,

#[serde(default)]
pub hooks: Hooks,
// Generator macro for the Configuration structure.
//
// Any field that does not require flattening (#[serde(flatten)]) should be defined inside the
// macro.
macro_rules! config_gen {
($(#[$meta:meta])* $vis:vis $ident:ident, $($(#[$field_meta:meta])* $field_vis:vis $field:ident: $ty:ty),*) => {
$(
#[$meta]
)*
$vis struct $ident {
#[serde(default)]
pub build: Build,

#[serde(default)]
pub tools: Tools,

#[serde(default)]
pub hooks: Hooks,

#[serde(default)]
pub watch: Watch,

#[serde(default)]
pub serve: Serve,

#[serde(default)]
pub clean: Clean,

#[serde(default)]
#[serde(alias = "proxy")]
pub proxies: Proxies,
$(
$(
#[$field_meta]
)*
$field_vis $field: $ty
)*
}
};
}

#[serde(default)]
pub watch: Watch,
config_gen! {
#[doc="The persisted Trunk configuration model"]
#[derive(Clone, Debug, Default, PartialEq, Eq, JsonSchema)]
// The deny unknown fields here is actually not for serde, but for `schemars::JsonSchema`.
#[serde(deny_unknown_fields)]
pub Configuration,
// The flatten is fine as we are not using it for serde, but for `schemars::JsonSchema`.
// Meaning there isn't any conflicts between "flatten", and "deny_unknown_fields".
#[serde(flatten)]
pub core: Core
}

#[serde(default)]
pub serve: Serve,
// HACK: We do not have a way to properly use `#[serde(flatten)]`, and `#[serde(deny_unknown_fields)]` together.
// However, we can still implement `Deserialize` manually so that `Configuration` returns cleaner errors.
// Once the mentioned attributes can be used together, this implementation can simply be replaced
// with two attributes on the structs field.
impl<'de> Deserialize<'de> for Configuration {
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
// Collect the configuration into a temporary struct, and store extra fields, and values
// inside the hashmap.
//
// The extra fields stored in the `HashMap`, are handled by `Core::deserialize` later.
config_gen! {
#[derive(Deserialize)]
InnerCfg,
#[serde(flatten)]
extras: HashMap<String, Value>
}

#[serde(default)]
pub clean: Clean,
impl From<(Core, InnerCfg)> for Configuration {
fn from((core, inner_cfg): (Core, InnerCfg)) -> Self {
Configuration {
core,
build: inner_cfg.build,
tools: inner_cfg.tools,
hooks: inner_cfg.hooks,
watch: inner_cfg.watch,
serve: inner_cfg.serve,
clean: inner_cfg.clean,
proxies: inner_cfg.proxies,
}
}
}

#[serde(default)]
#[serde(alias = "proxy")]
pub proxies: Proxies,
// This can still return an error due to unknown fields from the structs other fields
// which deny them (such as `Build`, `Serve`...).
// However any top level fields defined are stored in `extras`.
let inner_cfg = InnerCfg::deserialize(deserializer)?;

let extras = inner_cfg.extras.clone();

// Use the extra fields from `InnerCfg` to initialize the `Core`.
// This is essentially the same as flattening and denying unknown fields.
// Since the top level unknown fields come here, and `Core` denies unknown fields, it is
// returned as an error.
//
// NOTE: Top level key/field errors will return less information compared to the error from
// `InnerCfg`. The most important information, which is the key/field name will always be
// returned.
let core =
Core::deserialize(extras.into_deserializer()).map_err(serde::de::Error::custom)?;

Ok((core, inner_cfg).into())
}
}

impl ConfigModel for Configuration {
Expand Down
1 change: 1 addition & 0 deletions src/config/models/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use serde::Deserialize;

/// Config options for building proxies.
#[derive(Clone, Debug, PartialEq, Eq, Deserialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct Proxy {
/// The URL of the backend to which requests are to be proxied.
pub backend: Uri,
Expand Down
1 change: 1 addition & 0 deletions src/config/models/serve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use tracing::log;

/// Config options for the serve system.
#[derive(Clone, Debug, PartialEq, Eq, Deserialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct Serve {
/// A single address to serve on.
// This is required for the TOML to allow a single "address" field as before
Expand Down
1 change: 1 addition & 0 deletions src/config/models/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use serde::Deserialize;
// at all, this struct is used for both configuration as well as CLI arguments.
#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Args, JsonSchema)]
#[command(next_help_heading = "Tools")]
#[serde(deny_unknown_fields)]
pub struct Tools {
/// Version of `dart-sass` to use.
#[serde(default)]
Expand Down
1 change: 1 addition & 0 deletions src/config/models/watch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::path::PathBuf;

/// Config options for the watch system.
#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, JsonSchema)]
#[serde(deny_unknown_fields)]
pub struct Watch {
/// Watch specific file(s) or folder(s) [default: build target parent folder]
#[serde(default, skip_serializing_if = "Vec::is_empty")]
Expand Down