From 540f280f287d958a629fb17871e5dd9b0623765e Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:10:30 +0000 Subject: [PATCH] feat(config): generate the settings registry from the spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The join between the two files every CLI in the fleet keeps in step by hand. A spec's `config` block is read here, at build time, and what comes out is `const` — so a setting that is declared is a setting that resolves, and there is no second declaration to forget. Types, defaults, merge policy, scope, named parsers, environment variables in precedence order, bindings, hide, deprecation and renames all cross over. Ids are emitted as consts, because a `PropId` *is* the index into the table: reading a setting costs no lookup, and a typo in a key is a compile error rather than a `None` at run time. A build script is where strictness belongs — the alternative is a warning on every run of a shipped binary for a mistake only the spec's author can fix — so a registry that cannot mean what it says is refused: a `renamed_to` naming a setting that is not there, renames that form a cycle, an old name carrying a default its replacement lacks, a `parse` nobody implements, a `map` keyed by something a config file cannot spell, two keys whose consts collide. All of them at once, because an author fixing a registry wants the list. The generated registry for the fixture spec is checked in and `include!`d by the tests, so `cargo test` compiles it: a generator can be tested by comparing strings, and a string that looks like Rust is not Rust. --- Cargo.lock | 8 + Cargo.toml | 1 + config-build/Cargo.toml | 25 + config-build/examples/gen.rs | 20 + config-build/src/emit.rs | 440 ++++++++++++++++++ config-build/src/lib.rs | 158 +++++++ config-build/tests/fixtures/hk.usage.kdl | 66 +++ .../tests/fixtures/split-settings.usage.kdl | 5 + config-build/tests/fixtures/split.usage.kdl | 5 + config-build/tests/generated.rs | 185 ++++++++ config-build/tests/golden/settings.rs | 111 +++++ config-build/tests/refusals.rs | 279 +++++++++++ lib/src/spec/mod.rs | 19 +- 13 files changed, 1321 insertions(+), 1 deletion(-) create mode 100644 config-build/Cargo.toml create mode 100644 config-build/examples/gen.rs create mode 100644 config-build/src/emit.rs create mode 100644 config-build/src/lib.rs create mode 100644 config-build/tests/fixtures/hk.usage.kdl create mode 100644 config-build/tests/fixtures/split-settings.usage.kdl create mode 100644 config-build/tests/fixtures/split.usage.kdl create mode 100644 config-build/tests/generated.rs create mode 100644 config-build/tests/golden/settings.rs create mode 100644 config-build/tests/refusals.rs diff --git a/Cargo.lock b/Cargo.lock index e5563406c..f266278e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2091,6 +2091,14 @@ dependencies = [ "toml", ] +[[package]] +name = "usage-config-build" +version = "5.1.0" +dependencies = [ + "usage-config", + "usage-lib", +] + [[package]] name = "usage-conformance" version = "0.0.0" diff --git a/Cargo.toml b/Cargo.toml index ca5feab8f..e82693b74 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,7 @@ resolver = "2" members = [ "argv", "config", + "config-build", "derive", "clap_usage", "cli", diff --git a/config-build/Cargo.toml b/config-build/Cargo.toml new file mode 100644 index 000000000..38b988821 --- /dev/null +++ b/config-build/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "usage-config-build" +description = "Generates a usage-config settings registry from a usage spec, at build time" +version = "5.1.0" +edition = "2021" +rust-version = "1.80.0" +homepage = { workspace = true } +documentation = { workspace = true } +repository = { workspace = true } +authors = { workspace = true } +license = { workspace = true } + +# Runs in a build script, so its dependencies cost an adopter's *build* and never ship: the KDL +# parser lives here and `usage-config` carries none of it. +[dependencies] +usage-lib = { workspace = true } + +# The generated code is compiled and run in the tests, which is the only way to know it is code +# rather than plausible text. +[dev-dependencies] +usage-config = { workspace = true, features = ["toml"] } + +[package.metadata.release] +shared-version = true +release = true diff --git a/config-build/examples/gen.rs b/config-build/examples/gen.rs new file mode 100644 index 000000000..f3355b28a --- /dev/null +++ b/config-build/examples/gen.rs @@ -0,0 +1,20 @@ +//! Regenerates the checked-in registry the tests compile. +//! +//! ```sh +//! cargo run -p usage-config-build --example gen > config-build/tests/golden/settings.rs +//! ``` +//! +//! An example rather than a build script, because the point of checking the file in is that a +//! human reads the diff: generated code that nobody looks at is where a generator's mistakes live. + +const SPEC: &str = "config-build/tests/fixtures/hk.usage.kdl"; + +fn main() { + match usage_config_build::source(SPEC) { + Ok(source) => print!("{source}"), + Err(err) => { + eprintln!("{SPEC}: {err}"); + std::process::exit(1); + } + } +} diff --git a/config-build/src/emit.rs b/config-build/src/emit.rs new file mode 100644 index 000000000..78bd17eb6 --- /dev/null +++ b/config-build/src/emit.rs @@ -0,0 +1,440 @@ +//! The spec's settings written out as `const` Rust. +//! +//! Two rules run through all of it. Nothing is guessed: a declaration this crate cannot represent +//! faithfully is refused rather than approximated, because an approximation compiles and then +//! misbehaves at run time in the author's users' hands. And every problem is collected, because an +//! author fixing a registry wants the list rather than one round trip per mistake. + +use std::collections::BTreeMap; +use std::fmt::Write as _; + +use usage::spec::config::{ + SpecConfig, SpecConfigMerge, SpecConfigProp, SpecConfigScope, SpecConfigValue, +}; +use usage::spec::config_type::{Base, SpecConfigType}; + +/// The generated registry, or everything wrong with it. +pub(crate) fn registry(config: &SpecConfig, name: &str) -> Result> { + let mut problems = Vec::new(); + let props: Vec<(&String, &SpecConfigProp)> = config.props.iter().collect(); + + check_keys(&props, &mut problems); + check_renames(&props, &mut problems); + let idents = identifiers(&props, &mut problems); + + let mut body = String::new(); + for (key, prop) in &props { + let _ = writeln!(body, "{}", prop_meta(key, prop, &mut problems)); + } + + if !problems.is_empty() { + return Err(problems); + } + + let mut out = String::new(); + let _ = writeln!( + out, + "// @generated by usage-config-build from `{name}`. Do not edit.\n\ + //\n\ + // Every setting this CLI has, as consts: there is no second declaration of a setting to\n\ + // keep in step with this one, which is the drift this file exists to remove." + ); + let _ = writeln!( + out, + "\npub static SETTINGS_PROPS: &[::usage_config::PropMeta] = &[\n{body}];" + ); + let _ = writeln!( + out, + "\npub const SETTINGS_REGISTRY: ::usage_config::Registry =\n \ + ::usage_config::Registry::new(SETTINGS_PROPS);" + ); + + // Ids as consts, because a `PropId` *is* the index into the table above — so generated code + // reads a setting without looking its name up, and a typo in a key is a compile error rather + // than a `None` at run time. + let _ = writeln!( + out, + "\n/// Each setting's id, which is its position in [`SETTINGS_PROPS`].\n\ + pub mod prop {{\n \ + use ::usage_config::PropId;\n" + ); + for (index, ((key, prop), ident)) in props.iter().zip(&idents).enumerate() { + // Both through `one_line`: a doc comment is a line, and a key can hold a newline as easily + // as help can — after which the rest of the key is code. + let key = one_line(key); + if let Some(help) = prop.help.as_deref() { + let _ = writeln!(out, " /// `{key}` — {}", one_line(help)); + } else { + let _ = writeln!(out, " /// `{key}`"); + } + let _ = writeln!(out, " pub const {ident}: PropId = PropId({index});"); + } + let _ = writeln!(out, "}}"); + Ok(out) +} + +/// One `PropMeta`, as the struct-update form: a field this crate does not set is whatever +/// `PropMeta::new` says it is, so a field added to the runtime does not break generated code. +fn prop_meta(key: &str, prop: &SpecConfigProp, problems: &mut Vec) -> String { + let mut fields = String::new(); + if let Some(default) = default_const(key, prop, problems) { + let _ = writeln!(fields, " default: Some({default}),"); + } + match prop.merge { + SpecConfigMerge::Replace => {} + SpecConfigMerge::Union => { + let _ = writeln!(fields, " merge: ::usage_config::Merge::Union,"); + } + SpecConfigMerge::Deep => { + let _ = writeln!(fields, " merge: ::usage_config::Merge::Deep,"); + } + } + match prop.scope { + SpecConfigScope::Any => {} + SpecConfigScope::Global => { + let _ = writeln!(fields, " scope: ::usage_config::Scope::Global,"); + } + SpecConfigScope::Env => { + let _ = writeln!(fields, " scope: ::usage_config::Scope::Env,"); + } + } + if let Some(parse) = prop.parse.as_deref() { + match parser(parse) { + Some(variant) => { + let _ = writeln!( + fields, + " parse: Some(::usage_config::Parser::{variant})," + ); + } + // Not carried as an extension and not dropped: a spec that names a parser nothing + // implements would resolve values by a rule that does not exist, and every layer + // handing over text would produce one item where the author wrote several. + None => problems.push(format!( + "`{key}` names the parser `{parse}`, which is not one of: {}", + PARSERS + .iter() + .map(|(name, _)| *name) + .collect::>() + .join(", ") + )), + } + } + let envs = environment(prop); + if !envs.is_empty() { + let list: Vec = envs.iter().map(|env| rust_str(env)).collect(); + let _ = writeln!(fields, " envs: &[{}],", list.join(", ")); + } + if !prop.bindings.is_empty() { + let mut pairs = Vec::new(); + for (kind, keys) in &prop.bindings { + for bound in keys { + pairs.push(format!("({}, {})", rust_str(kind), rust_str(bound))); + } + } + let _ = writeln!(fields, " bindings: &[{}],", pairs.join(", ")); + } + if prop.hide { + let _ = writeln!(fields, " hide: true,"); + } + if let Some(why) = prop.deprecated.as_deref() { + let _ = writeln!(fields, " deprecated: Some({}),", rust_str(why)); + } + if let Some(to) = prop.renamed_to.as_deref() { + let _ = writeln!(fields, " renamed_to: Some({}),", rust_str(to)); + } + if let Some(help) = prop.help.as_deref() { + let _ = writeln!(fields, " help: Some({}),", rust_str(help)); + } + let new = format!( + "::usage_config::PropMeta::new({}, {})", + rust_str(key), + ty(key, prop.value_type.as_ref(), problems), + ); + // A setting with nothing but a key and a type is written as the call itself: the update form + // with no fields in it is the same value spelled twice, and clippy says so. + if fields.is_empty() { + return format!(" {new},"); + } + format!(" ::usage_config::PropMeta {{\n{fields} ..{new}\n }},") +} + +/// The runtime `Ty` expression for a declared type. +/// +/// Borrowed inners are fine in a `const`: `Ty::List(&Ty::String)` promotes, which is why the +/// runtime's type is shaped this way. +fn ty(key: &str, declared: Option<&SpecConfigType>, problems: &mut Vec) -> String { + let Some(declared) = declared else { + // No `type=` at all. The spec's own default is a string, and saying so beats inventing + // something cleverer for a property whose author did not say. + return "::usage_config::Ty::String".to_string(); + }; + match declared { + SpecConfigType::Base(base) => base_ty(base).to_string(), + SpecConfigType::List(inner) => format!( + "::usage_config::Ty::List(&{})", + ty(key, Some(inner), problems) + ), + SpecConfigType::Set(inner) => format!( + "::usage_config::Ty::Set(&{})", + ty(key, Some(inner), problems) + ), + SpecConfigType::Option(inner) => { + format!( + "::usage_config::Ty::Option(&{})", + ty(key, Some(inner), problems) + ) + } + SpecConfigType::Map(k, value) => { + // A table's keys are text in every format a settings file is written in, so a map + // keyed by anything else is a promise the runtime cannot keep — and one it would keep + // *quietly*, handing the CLI string keys for a `map`. + if !matches!(k, Base::String | Base::Custom(_)) { + problems.push(format!( + "`{key}` is a `map<{k}, …>`, and a settings table's keys are text: \ + write `map`" + )); + } + format!( + "::usage_config::Ty::Map(&{})", + ty(key, Some(value), problems) + ) + } + // A union, or a name this usage does not know: the spec has said usage cannot decide what + // belongs here, and `Any` is the runtime's word for exactly that — nothing coerced, nothing + // refused, the field's own type the only judge. + SpecConfigType::Union(_) => "::usage_config::Ty::Any".to_string(), + } +} + +fn base_ty(base: &Base) -> &'static str { + match base { + Base::Bool => "::usage_config::Ty::Bool", + Base::String => "::usage_config::Ty::String", + Base::Int => "::usage_config::Ty::Int", + Base::Uint => "::usage_config::Ty::Uint", + Base::Float => "::usage_config::Ty::Float", + Base::Path => "::usage_config::Ty::Path", + Base::Url => "::usage_config::Ty::Url", + Base::Duration => "::usage_config::Ty::Duration", + Base::Object => "::usage_config::Ty::Object", + Base::Custom(_) => "::usage_config::Ty::Any", + } +} + +/// The declared default as a `Const`, or nothing when there is none. +fn default_const(key: &str, prop: &SpecConfigProp, problems: &mut Vec) -> Option { + let has_default = prop.default.is_some() || !prop.default_list.is_empty(); + // Asked before either form of default is returned, because a *list* default returned early and + // skipped this — the one shape of the mistake that was generated instead of refused. + // + // An old name that carries a default its replacement does not have would seed a value under a + // key nothing reads: the merge folds a rename to its target, whose own default is what shows + // up. Left to run time it is a warning on every run of a shipped binary for a mistake only the + // author can fix, so it is refused here. + if let (Some(to), true) = (prop.renamed_to.as_deref(), has_default) { + problems.push(format!( + "`{key}` is renamed to `{to}` and declares a default of its own: put the default on \ + the setting that replaced it" + )); + } + // Both forms at once is something the spec accepts and nothing can mean: returning the list and + // ignoring the scalar dropped a value the author wrote, and I had claimed here that the parser + // refused it — it does not. I checked. + if prop.default.is_some() && !prop.default_list.is_empty() { + problems.push(format!( + "`{key}` declares a default twice, as `default=` and as a `default` node: keep one" + )); + } + // A list default is a child node rather than a value, and either may be the one that is there. + if !prop.default_list.is_empty() { + let items: Vec = prop.default_list.iter().map(value_const).collect(); + return Some(format!( + "::usage_config::Const::List(&[{}])", + items.join(", ") + )); + } + prop.default.as_ref().map(value_const) +} + +fn value_const(value: &SpecConfigValue) -> String { + match value { + SpecConfigValue::Bool(b) => format!("::usage_config::Const::Bool({b})"), + SpecConfigValue::Int(i) => format!("::usage_config::Const::Int({i})"), + // `{:?}` rather than `{}`: `1.0` written as `1` is an integer literal, and a `f64` field + // will not take one. + SpecConfigValue::Float(f) => format!("::usage_config::Const::Float({f:?})"), + SpecConfigValue::String(s) => format!("::usage_config::Const::Str({})", rust_str(s)), + } +} + +/// Both spellings of the environment, in precedence order. +/// +/// `env=` is the singular form and `env` the child node; the spec keeps them in step, and the +/// higher-precedence one comes first. +fn environment(prop: &SpecConfigProp) -> Vec { + let mut envs: Vec = Vec::new(); + if let Some(env) = prop.env.as_deref() { + envs.push(env.to_string()); + } + for env in &prop.envs { + if !envs.iter().any(|seen| seen == env) { + envs.push(env.clone()); + } + } + envs +} + +const PARSERS: [(&str, &str); 4] = [ + ("list_by_comma", "ListByComma"), + ("list_by_colon", "ListByColon"), + ("list_by_os_path_separator", "ListByOsPathSeparator"), + ("set_by_comma", "SetByComma"), +]; + +fn parser(name: &str) -> Option<&'static str> { + PARSERS + .iter() + .find(|(spec, _)| *spec == name) + .map(|(_, variant)| *variant) +} + +/// The const name for each key, and a problem for any two that would collide. +fn identifiers(props: &[(&String, &SpecConfigProp)], problems: &mut Vec) -> Vec { + let mut idents = Vec::with_capacity(props.len()); + let mut seen: BTreeMap = BTreeMap::new(); + for (key, _) in props { + let ident = ident_of(key); + if let Some(other) = seen.get(&ident) { + // `task.output` and `task_output` are two settings and one const. Generating both + // would not compile, which is a good outcome reported badly: the message names the + // pair rather than leaving the author to read generated code. + problems.push(format!( + "`{key}` and `{other}` both generate `prop::{ident}`: rename one of them" + )); + } else { + seen.insert(ident.clone(), (*key).clone()); + } + idents.push(ident); + } + idents +} + +fn ident_of(key: &str) -> String { + let mut out = String::with_capacity(key.len()); + for c in key.chars() { + match c { + 'a'..='z' | 'A'..='Z' | '0'..='9' => out.extend(c.to_uppercase()), + // A dot, a dash, anything else: one underscore, so `task.output` and `task-output` + // read the same way a Rust const is written. + _ => out.push('_'), + } + } + // A const cannot start with a digit, and a key may. + if out.starts_with(|c: char| c.is_ascii_digit()) { + out.insert(0, '_'); + } + out +} + +/// Refuse a key that cannot become a name. +/// +/// An empty key generated `pub const : PropId = …`, and an empty part of one (`task..output`) +/// generated a field with no name: both are code the *adopter's* crate fails to compile, in a file +/// they did not write and cannot fix. +fn check_keys(props: &[(&String, &SpecConfigProp)], problems: &mut Vec) { + for (key, _) in props { + if key.is_empty() { + problems.push("a setting with no name cannot be generated".to_string()); + } else if !nameable(key) { + problems.push(format!( + "`{key}` has a part with no name in it: every piece of a dotted key needs an \ + ASCII letter or digit" + )); + } + } +} + +/// Whether every part of `key` can be an identifier. +/// +/// A part needs an ASCII letter or digit in it, which is the alphabet the names are actually built +/// from: `ident_of` keeps those and turns everything else into an underscore. Asking the wider +/// question — is any character of it alphanumeric — accepted `é` and then generated `_` for it, so +/// the two disagreed about the same key. Rust would take `É` as an identifier, but what an arbitrary +/// letter uppercases to is not something to bet a generated name on. +/// +/// Empty (`task..output`) a part generated a field with no name; made only of characters outside +/// that alphabet (`-`) it generated an *anonymous* const — legal Rust, and unreferenceable, so the +/// reader that names it does not compile — and a field called `_`, which is not legal at all. +pub(crate) fn nameable(key: &str) -> bool { + !key.is_empty() + && key + .split('.') + .all(|part| part.chars().any(|c| c.is_ascii_alphanumeric())) +} + +/// Refuse a `renamed_to` that cannot be followed. +fn check_renames(props: &[(&String, &SpecConfigProp)], problems: &mut Vec) { + let by_key: BTreeMap<&str, &SpecConfigProp> = props + .iter() + .map(|(key, prop)| (key.as_str(), *prop)) + .collect(); + for (key, prop) in props { + let Some(to) = prop.renamed_to.as_deref() else { + continue; + }; + if !by_key.contains_key(to) { + problems.push(format!( + "`{key}` is renamed to `{to}`, which is not a setting" + )); + continue; + } + // Walking the chain from here, bounded by how many settings there are. A cycle makes + // every lookup of these keys answer `None` at run time — the setting becomes unreachable + // rather than wrong, which is the hardest kind of bug to see. + let mut seen = vec![key.as_str()]; + let mut current = to; + for _ in 0..props.len() { + if seen.contains(¤t) { + problems.push(format!( + "the renames of `{}` form a cycle: {} → {current}", + seen[0], + seen.join(" → ") + )); + break; + } + seen.push(current); + match by_key.get(current).and_then(|p| p.renamed_to.as_deref()) { + Some(next) => current = next, + None => break, + } + } + } +} + +/// `text` as a Rust string literal. +pub(crate) fn rust_str(text: &str) -> String { + let mut out = String::with_capacity(text.len() + 2); + out.push('"'); + for c in text.chars() { + match c { + '"' => out.push_str("\\\""), + '\\' => out.push_str("\\\\"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + // A control character written raw is a literal a compiler accepts and a reader cannot + // see. `\u{…}` is the same string, spelled. + c if c.is_control() => { + let _ = write!(out, "\\u{{{:x}}}", c as u32); + } + c => out.push(c), + } + } + out.push('"'); + out +} + +/// Help text as one line, for a doc comment on a generated const. +pub(crate) fn one_line(text: &str) -> String { + text.replace(['\n', '\r'], " ") +} diff --git a/config-build/src/lib.rs b/config-build/src/lib.rs new file mode 100644 index 000000000..663f5699f --- /dev/null +++ b/config-build/src/lib.rs @@ -0,0 +1,158 @@ +//! A spec's `config` block, as the registry `usage-config` resolves against. +//! +//! Every CLI in the jdx fleet declares its settings in one file and resolves them in another, and +//! the two are kept in step by hand. They drift every time: hk declares eighteen `sources.cli` +//! bindings and reads five, pitchfork generates five settings that its own `settings get` cannot +//! reach, fnox's docs describe a layer that does not exist. This crate is the join. The spec is +//! read here, at build time, and what comes out is `const` — so a setting that is declared is a +//! setting that resolves, and there is no second place to forget to update. +//! +//! ```no_run +//! // build.rs +//! usage_config_build::generate("mycli.usage.kdl").expect("settings"); +//! ``` +//! +//! ```ignore +//! // src/settings.rs +//! include!(concat!(env!("OUT_DIR"), "/settings.rs")); +//! ``` +//! +//! Nothing generated here allocates or parses at run time: `PropMeta` is `const`-constructible +//! down to its defaults, so a binary that reads no settings pays nothing for having them, and one +//! that reads all of them pays a `static`. +//! +//! # What it refuses +//! +//! A build script is the right place to be strict, because the alternative is a warning on every +//! run of a shipped binary for a mistake only the author can fix. So a registry is refused when it +//! cannot mean what it says: a `renamed_to` naming a setting that is not there, renames that form a +//! cycle, an old name carrying a default its replacement lacks, a `parse` nobody implements, a +//! `map` keyed by something a config file cannot spell, or two keys whose generated names collide. +//! All of them at once, rather than the first — an author fixing a registry wants the list. + +use std::fmt; +use std::path::{Path, PathBuf}; +use std::str::FromStr as _; + +mod emit; + +/// Read `spec` and write the registry to `$OUT_DIR/settings.rs`. +/// +/// Returns the path written, and tells cargo to re-run when the spec changes. +pub fn generate(spec: impl AsRef) -> Result { + let out_dir = std::env::var("OUT_DIR").map_err(|_| Error::NoOutDir)?; + let out = PathBuf::from(out_dir).join("settings.rs"); + generate_to(spec, &out)?; + Ok(out) +} + +/// Read `spec` and write the registry to `out`. +/// +/// For a caller that keeps generated code in the repository rather than in `OUT_DIR` — which is +/// how this crate tests itself, and how a CLI that wants its registry reviewable does it. +pub fn generate_to(spec: impl AsRef, out: impl AsRef) -> Result<(), Error> { + let spec = spec.as_ref(); + // Before anything can fail, so a spec that does not parse is still watched and the next build + // is not a stale success. + println!("cargo::rerun-if-changed={}", spec.display()); + let source = source(spec)?; + // And every file the spec *included*, which is where a CLI with many settings keeps them — so + // watching only the file the build script names left editing the settings rebuilding nothing. + for watched in watched(spec)?.into_iter().skip(1) { + println!("cargo::rerun-if-changed={}", watched.display()); + } + let out = out.as_ref(); + if let Some(parent) = out.parent() { + std::fs::create_dir_all(parent).map_err(|err| Error::Io { + path: parent.to_path_buf(), + why: err.to_string(), + })?; + } + // Only when it differs, so a checked-in registry keeps its mtime and nothing downstream + // rebuilds for a generator that produced the same bytes. + if std::fs::read_to_string(out).is_ok_and(|existing| existing == source) { + return Ok(()); + } + std::fs::write(out, source).map_err(|err| Error::Io { + path: out.to_path_buf(), + why: err.to_string(), + }) +} + +/// Every file a build should watch: the spec, then each `include`, recursively. +/// +/// [`generate_to`] prints these for cargo. A build script doing something more elaborate — writing +/// the registry somewhere of its own, or generating other things from the same spec — wants the list +/// rather than the printing. +pub fn watched(spec: impl AsRef) -> Result, Error> { + let parsed = + usage::Spec::parse_file(spec.as_ref()).map_err(|err| Error::Spec(err.to_string()))?; + Ok(parsed.sources) +} + +/// The Rust source a spec's `config` block becomes. +pub fn source(spec: impl AsRef) -> Result { + let spec = spec.as_ref(); + let parsed = usage::Spec::parse_file(spec).map_err(|err| Error::Spec(err.to_string()))?; + let name = spec + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| spec.display().to_string()); + source_of(&parsed.config, &name) +} + +/// The same, from a spec that is already in memory. +/// +/// A relative `include` resolves against the file a spec was read from, so a spec whose settings +/// live in their own file has to go through [`source`]; this is for a caller holding one text. +pub fn source_of_spec(spec: &str, name: &str) -> Result { + let parsed = usage::Spec::from_str(spec).map_err(|err| Error::Spec(err.to_string()))?; + source_of(&parsed.config, name) +} + +fn source_of(config: &usage::spec::config::SpecConfig, name: &str) -> Result { + if config.props.is_empty() { + return Err(Error::NoSettings); + } + emit::registry(config, name).map_err(Error::Registry) +} + +/// Why a registry could not be generated. +#[derive(Debug)] +pub enum Error { + /// The spec itself did not parse. + Spec(String), + /// The spec parsed and declares no settings, which is not something to generate an empty + /// registry for: a build script asking for one has been pointed at the wrong file. + NoSettings, + /// Everything wrong with the settings, rather than the first thing. + Registry(Vec), + /// [`generate`] was called outside a build script. + NoOutDir, + Io { + path: PathBuf, + why: String, + }, +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Spec(why) => write!(f, "{why}"), + Self::NoSettings => f.write_str("this spec declares no `config` settings"), + Self::Registry(problems) => { + for (i, problem) in problems.iter().enumerate() { + if i > 0 { + f.write_str("\n")?; + } + f.write_str(problem)?; + } + Ok(()) + } + Self::NoOutDir => f.write_str("OUT_DIR is not set; call this from a build script"), + Self::Io { path, why } => write!(f, "{}: {why}", path.display()), + } + } +} + +impl std::error::Error for Error {} diff --git a/config-build/tests/fixtures/hk.usage.kdl b/config-build/tests/fixtures/hk.usage.kdl new file mode 100644 index 000000000..5889691a5 --- /dev/null +++ b/config-build/tests/fixtures/hk.usage.kdl @@ -0,0 +1,66 @@ +name "hk" +bin "hk" + +// A settings block with one of everything this generator has to carry, modelled on hk's own +// registry — the fleet's most featureful, and the first CLI that will adopt this. +config { + source "git" name="git config" doc_hint="git config `{key}`" + source "pkl" name="hk.pkl" + + file "/etc/hk/config.toml" scope="system" + file "~/.config/hk/config.toml" scope="global" + file "hk.toml" findup=#true + + prop "jobs" type="uint" default=4 default_note="0 = one per core" \ + help="How many jobs to run at once" { + env "HK_JOBS" "HK_JOB" + source "git" "hk.jobs" + cli "--jobs" "-j" + } + + prop "exclude" type="list" merge="union" parse="list_by_comma" \ + help="Paths to leave alone" { + env "HK_EXCLUDE" + source "pkl" "exclude" "defaults.exclude" + } + + prop "path" type="list" parse="list_by_os_path_separator" { + env "HK_PATH" + } + + prop "stash" type="string" default="git" help="How to stash before a run" { + choices { + choice "git" help="git stash" + choice "patch-file" help="A patch on disk" + choice "none" + } + } + + prop "url_replacements" type="map" merge="deep" \ + help="Rewrite these URL prefixes" + + prop "trusted" type="bool" scope="global" default=#false \ + help="Whether this checkout may run its own hooks" + + prop "ci" type="bool" scope="env" hide=#true { + env "CI" + } + + prop "task.output" type="string" default="prefix" help="How task output is interleaved" + + prop "either" type="bool|string" help="A \"union\" only hk understands" + + prop "timeout" type="option" help="How long to wait, if at all" + + prop "log_level" type="string" default="info" { + env "HK_LOG_LEVEL" + x "hk.parse_env" "lowercase" + } + + prop "concurrency" type="uint" deprecated="Use jobs instead." renamed_to="jobs" \ + deprecated_warn_at="2026.12.0" + + prop "ports" type="list" { + default 80 443 + } +} diff --git a/config-build/tests/fixtures/split-settings.usage.kdl b/config-build/tests/fixtures/split-settings.usage.kdl new file mode 100644 index 000000000..2b8db29a1 --- /dev/null +++ b/config-build/tests/fixtures/split-settings.usage.kdl @@ -0,0 +1,5 @@ +config { + prop "jobs" type="uint" default=2 help="How many jobs to run at once" { + env "SPLIT_JOBS" + } +} diff --git a/config-build/tests/fixtures/split.usage.kdl b/config-build/tests/fixtures/split.usage.kdl new file mode 100644 index 000000000..dc47f1bf6 --- /dev/null +++ b/config-build/tests/fixtures/split.usage.kdl @@ -0,0 +1,5 @@ +name "split" +bin "split" + +// The shape a CLI with many settings uses: the settings live in a file of their own. +include file="./split-settings.usage.kdl" diff --git a/config-build/tests/generated.rs b/config-build/tests/generated.rs new file mode 100644 index 000000000..78502d100 --- /dev/null +++ b/config-build/tests/generated.rs @@ -0,0 +1,185 @@ +//! The generated registry, compiled and resolved against. +//! +//! A generator can be tested by comparing strings, and a string that looks like Rust is not Rust. +//! So the output of `usage-config-build` for [the fixture spec](fixtures/hk.usage.kdl) is checked +//! in beside this file and included here: it is compiled by `cargo test`, and every assertion +//! below is against the registry that compilation produced. + +use std::collections::BTreeMap; + +use usage_config::{ + resolve, FileLayer, FileScope, Layers, Merge, Parser, Scope, SourceKind, Ty, Value, +}; + +include!("golden/settings.rs"); + +#[test] +fn every_declared_default_resolves_as_its_type() { + // No layers at all: what a CLI gets from its registry alone, which is also the only thing a + // `const` default can be wrong about. + let resolved = resolve(SETTINGS_REGISTRY, Layers::new()).expect("resolves"); + let mut fold = resolved.fold(); + + let jobs: Option = fold.required(prop::JOBS); + let stash: Option = fold.required(prop::STASH); + let trusted: Option = fold.required(prop::TRUSTED); + let output: Option = fold.required(prop::TASK_OUTPUT); + let ports: Option> = fold.required(prop::PORTS); + let timeout: Option = fold.optional(prop::TIMEOUT); + fold.finish().expect("every default fits its type"); + + assert_eq!(jobs, Some(4)); + assert_eq!(stash, Some("git".to_string())); + assert_eq!(trusted, Some(false)); + assert_eq!(output, Some("prefix".to_string())); + // A list default is a child node in the spec and stays typed all the way here: three numbers, + // not three strings. + assert_eq!(ports, Some(vec![80, 443])); + // No default and an `option`: absent is a state, not a failure. + assert_eq!(timeout, None); +} + +#[test] +fn a_generated_id_is_the_setting_it_names() { + // The one thing emitting ids as indices can get wrong, and it would be silent: every read + // would answer about the wrong setting. So each const is checked against the name it claims. + for id in SETTINGS_REGISTRY.ids() { + let key = SETTINGS_REGISTRY.get(id).key; + // `lookup_exact`, not `lookup`: the folding one answers about the setting that *replaced* + // an old name, which is right for reading a value and wrong for asking where a name lives. + assert_eq!( + SETTINGS_REGISTRY.lookup_exact(key), + Some(id), + "`{key}` is not where the registry says it is" + ); + } + assert_eq!(SETTINGS_REGISTRY.get(prop::JOBS).key, "jobs"); + assert_eq!(SETTINGS_REGISTRY.get(prop::TASK_OUTPUT).key, "task.output"); + assert_eq!( + SETTINGS_REGISTRY.get(prop::URL_REPLACEMENTS).key, + "url_replacements" + ); +} + +#[test] +fn what_the_spec_said_about_each_setting_is_what_the_registry_holds() { + let meta = |id| SETTINGS_REGISTRY.get(id); + + // Types, including the two that are not a name: a union is `Any`, because the spec has said + // usage cannot decide what belongs there. + assert_eq!(meta(prop::JOBS).ty, Ty::Uint); + assert_eq!(meta(prop::EXCLUDE).ty, Ty::List(&Ty::String)); + assert_eq!(meta(prop::PATH).ty, Ty::List(&Ty::Path)); + assert_eq!(meta(prop::URL_REPLACEMENTS).ty, Ty::Map(&Ty::String)); + assert_eq!(meta(prop::TIMEOUT).ty, Ty::Option(&Ty::Duration)); + assert_eq!(meta(prop::EITHER).ty, Ty::Any); + + // Merge policy, scope, and the named parser: each of these is a `prop` attribute that a + // hand-written registry has to remember to copy, and every one of them is a real setting in + // hk's own file. + assert_eq!(meta(prop::EXCLUDE).merge, Merge::Union); + assert_eq!(meta(prop::URL_REPLACEMENTS).merge, Merge::Deep); + assert_eq!(meta(prop::JOBS).merge, Merge::Replace); + assert_eq!(meta(prop::TRUSTED).scope, Scope::Global); + assert_eq!(meta(prop::CI).scope, Scope::Env); + assert_eq!(meta(prop::JOBS).scope, Scope::Any); + assert_eq!(meta(prop::EXCLUDE).parse, Some(Parser::ListByComma)); + assert_eq!(meta(prop::PATH).parse, Some(Parser::ListByOsPathSeparator)); + assert_eq!(meta(prop::JOBS).parse, None); + assert!(meta(prop::CI).hide); + assert!(!meta(prop::JOBS).hide); + + // The environment in precedence order, and help as the spec wrote it — quotes and all. + assert_eq!(meta(prop::JOBS).envs, &["HK_JOBS", "HK_JOB"]); + assert_eq!(meta(prop::CI).envs, &["CI"]); + assert_eq!(meta(prop::JOBS).help, Some("How many jobs to run at once")); + assert_eq!( + meta(prop::EITHER).help, + Some("A \"union\" only hk understands") + ); + + // Bindings, which is the whole mechanism behind hk's git and pkl layers: a custom layer asks + // the registry for its own kind and iterates what it finds. + assert_eq!( + SETTINGS_REGISTRY + .bindings(SourceKind::new("git")) + .collect::>(), + vec![(prop::JOBS, "hk.jobs")] + ); + assert_eq!( + SETTINGS_REGISTRY + .bindings(SourceKind::new("pkl")) + .collect::>(), + vec![ + (prop::EXCLUDE, "exclude"), + (prop::EXCLUDE, "defaults.exclude") + ] + ); +} + +#[test] +fn a_renamed_setting_folds_into_the_one_that_replaced_it() { + let resolved = resolve(SETTINGS_REGISTRY, Layers::new()).expect("resolves"); + // The old name still answers, about the setting that replaced it. + assert_eq!(resolved.get_key("concurrency"), Some(&Value::Int(4))); + assert_eq!( + SETTINGS_REGISTRY.get(prop::CONCURRENCY).renamed_to, + Some("jobs") + ); + let explained = usage_config::explain(&resolved, "concurrency").expect("declared"); + assert!( + explained.starts_with("concurrency is now jobs\n"), + "{explained}" + ); + assert!( + explained.contains("deprecated: Use jobs instead."), + "{explained}" + ); +} + +#[test] +fn a_file_read_against_the_generated_registry_means_what_the_spec_said() { + // End to end, which is the point of the whole crate: a spec declared these settings, a build + // generated the registry, and a config file is read through it with nothing hand-written in + // between. + let dir = std::env::temp_dir().join(format!("usage_config_build_{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("dir"); + let path = dir.join("hk.toml"); + std::fs::write( + &path, + "jobs = 8\nexclude = \"target,dist\"\nconcurrency = 2\n\ + [url_replacements]\n\"git@github.com:\" = \"https://github.com/\"\n", + ) + .expect("write"); + + let layer = FileLayer::at(&path, FileScope::Project); + let resolved = resolve(SETTINGS_REGISTRY, Layers::new().then(&layer)).expect("resolves"); + let mut fold = resolved.fold(); + let jobs: Option = fold.required(prop::JOBS); + let exclude: Option> = fold.required(prop::EXCLUDE); + let replacements: Option> = fold.required(prop::URL_REPLACEMENTS); + fold.finish().expect("reads"); + + // The file's value beats the declared default… + assert_eq!(jobs, Some(8)); + // …the named parser splits what the file wrote as one string… + assert_eq!( + exclude, + Some(vec!["target".to_string(), "dist".to_string()]) + ); + // …a `map` arrives as a table… + assert_eq!( + replacements.as_ref().and_then(|m| m.get("git@github.com:")), + Some(&"https://github.com/".to_string()) + ); + // …and the deprecated key in the same file is folded, with something said about it. + let warnings = usage_config::explain::warnings(&resolved); + assert!( + warnings + .iter() + .any(|w| w.contains("concurrency is deprecated")), + "{warnings:?}" + ); + + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/config-build/tests/golden/settings.rs b/config-build/tests/golden/settings.rs new file mode 100644 index 000000000..ce850ed17 --- /dev/null +++ b/config-build/tests/golden/settings.rs @@ -0,0 +1,111 @@ +// @generated by usage-config-build from `hk.usage.kdl`. Do not edit. +// +// Every setting this CLI has, as consts: there is no second declaration of a setting to +// keep in step with this one, which is the drift this file exists to remove. + +pub static SETTINGS_PROPS: &[::usage_config::PropMeta] = &[ + ::usage_config::PropMeta { + scope: ::usage_config::Scope::Env, + envs: &["CI"], + hide: true, + ..::usage_config::PropMeta::new("ci", ::usage_config::Ty::Bool) + }, + ::usage_config::PropMeta { + deprecated: Some("Use jobs instead."), + renamed_to: Some("jobs"), + ..::usage_config::PropMeta::new("concurrency", ::usage_config::Ty::Uint) + }, + ::usage_config::PropMeta { + help: Some("A \"union\" only hk understands"), + ..::usage_config::PropMeta::new("either", ::usage_config::Ty::Any) + }, + ::usage_config::PropMeta { + merge: ::usage_config::Merge::Union, + parse: Some(::usage_config::Parser::ListByComma), + envs: &["HK_EXCLUDE"], + bindings: &[("pkl", "exclude"), ("pkl", "defaults.exclude")], + help: Some("Paths to leave alone"), + ..::usage_config::PropMeta::new("exclude", ::usage_config::Ty::List(&::usage_config::Ty::String)) + }, + ::usage_config::PropMeta { + default: Some(::usage_config::Const::Int(4)), + envs: &["HK_JOBS", "HK_JOB"], + bindings: &[("git", "hk.jobs")], + help: Some("How many jobs to run at once"), + ..::usage_config::PropMeta::new("jobs", ::usage_config::Ty::Uint) + }, + ::usage_config::PropMeta { + default: Some(::usage_config::Const::Str("info")), + envs: &["HK_LOG_LEVEL"], + ..::usage_config::PropMeta::new("log_level", ::usage_config::Ty::String) + }, + ::usage_config::PropMeta { + parse: Some(::usage_config::Parser::ListByOsPathSeparator), + envs: &["HK_PATH"], + ..::usage_config::PropMeta::new("path", ::usage_config::Ty::List(&::usage_config::Ty::Path)) + }, + ::usage_config::PropMeta { + default: Some(::usage_config::Const::List(&[::usage_config::Const::Int(80), ::usage_config::Const::Int(443)])), + ..::usage_config::PropMeta::new("ports", ::usage_config::Ty::List(&::usage_config::Ty::Uint)) + }, + ::usage_config::PropMeta { + default: Some(::usage_config::Const::Str("git")), + help: Some("How to stash before a run"), + ..::usage_config::PropMeta::new("stash", ::usage_config::Ty::String) + }, + ::usage_config::PropMeta { + default: Some(::usage_config::Const::Str("prefix")), + help: Some("How task output is interleaved"), + ..::usage_config::PropMeta::new("task.output", ::usage_config::Ty::String) + }, + ::usage_config::PropMeta { + help: Some("How long to wait, if at all"), + ..::usage_config::PropMeta::new("timeout", ::usage_config::Ty::Option(&::usage_config::Ty::Duration)) + }, + ::usage_config::PropMeta { + default: Some(::usage_config::Const::Bool(false)), + scope: ::usage_config::Scope::Global, + help: Some("Whether this checkout may run its own hooks"), + ..::usage_config::PropMeta::new("trusted", ::usage_config::Ty::Bool) + }, + ::usage_config::PropMeta { + merge: ::usage_config::Merge::Deep, + help: Some("Rewrite these URL prefixes"), + ..::usage_config::PropMeta::new("url_replacements", ::usage_config::Ty::Map(&::usage_config::Ty::String)) + }, +]; + +pub const SETTINGS_REGISTRY: ::usage_config::Registry = + ::usage_config::Registry::new(SETTINGS_PROPS); + +/// Each setting's id, which is its position in [`SETTINGS_PROPS`]. +pub mod prop { + use ::usage_config::PropId; + + /// `ci` + pub const CI: PropId = PropId(0); + /// `concurrency` + pub const CONCURRENCY: PropId = PropId(1); + /// `either` — A "union" only hk understands + pub const EITHER: PropId = PropId(2); + /// `exclude` — Paths to leave alone + pub const EXCLUDE: PropId = PropId(3); + /// `jobs` — How many jobs to run at once + pub const JOBS: PropId = PropId(4); + /// `log_level` + pub const LOG_LEVEL: PropId = PropId(5); + /// `path` + pub const PATH: PropId = PropId(6); + /// `ports` + pub const PORTS: PropId = PropId(7); + /// `stash` — How to stash before a run + pub const STASH: PropId = PropId(8); + /// `task.output` — How task output is interleaved + pub const TASK_OUTPUT: PropId = PropId(9); + /// `timeout` — How long to wait, if at all + pub const TIMEOUT: PropId = PropId(10); + /// `trusted` — Whether this checkout may run its own hooks + pub const TRUSTED: PropId = PropId(11); + /// `url_replacements` — Rewrite these URL prefixes + pub const URL_REPLACEMENTS: PropId = PropId(12); +} diff --git a/config-build/tests/refusals.rs b/config-build/tests/refusals.rs new file mode 100644 index 000000000..29731ec21 --- /dev/null +++ b/config-build/tests/refusals.rs @@ -0,0 +1,279 @@ +//! What a registry has to be refused for, and the one thing it must stay identical to. +//! +//! A build script is where strictness belongs: the alternative to refusing a declaration that +//! cannot mean what it says is a warning on every run of a shipped binary, for a mistake only the +//! author of the spec can fix. + +use usage_config_build::{source, source_of_spec, Error}; + +/// A spec with `config { … }` around the settings under test. +fn spec(settings: &str) -> String { + format!("name \"mycli\"\nbin \"mycli\"\nconfig {{\n{settings}\n}}\n") +} + +/// Every problem a spec's settings are refused for. +fn problems(settings: &str) -> Vec { + match source_of_spec(&spec(settings), "mycli.usage.kdl") { + Ok(generated) => panic!("should have been refused, generated:\n{generated}"), + Err(Error::Registry(problems)) => problems, + Err(other) => panic!("refused for the wrong reason: {other}"), + } +} + +#[test] +fn the_checked_in_registry_is_what_the_generator_produces() { + // The generated file is checked in so that `cargo test` compiles it — which is the only way to + // know the generator emits code rather than plausible text. That only works while the two + // agree, so this is the test that fails when a change to the emitter is not regenerated: + // + // cargo run -p usage-config-build --example gen > config-build/tests/golden/settings.rs + let generated = source("tests/fixtures/hk.usage.kdl").expect("the fixture is valid"); + let checked_in = std::fs::read_to_string("tests/golden/settings.rs").expect("golden"); + assert_eq!( + generated, checked_in, + "tests/golden/settings.rs is stale; regenerate it with the command in this test" + ); +} + +#[test] +fn an_included_spec_is_watched_and_read() { + // `include` is how a CLI with many settings keeps them in a file of their own, which makes that + // file the one most likely to be edited — and watching only the file the build script names left + // editing the settings rebuilding nothing, so the generated registry went stale in silence. + let watched = usage_config_build::watched("tests/fixtures/split.usage.kdl").expect("parses"); + let names: Vec = watched + .iter() + .map(|p| { + p.file_name() + .unwrap_or_default() + .to_string_lossy() + .into_owned() + }) + .collect(); + assert_eq!( + names, + vec!["split.usage.kdl", "split-settings.usage.kdl"], + "the spec and what it included" + ); + + // And the settings in the included file are the ones generated, which is the other half of it. + let generated = source("tests/fixtures/split.usage.kdl").expect("generates"); + assert!(generated.contains("PropMeta::new(\"jobs\""), "{generated}"); + assert!(generated.contains("SPLIT_JOBS"), "{generated}"); +} + +#[test] +fn a_rename_to_a_setting_that_is_not_there_is_refused() { + // The runtime folds an old name into its replacement by looking the replacement up. Naming one + // that does not exist leaves every value written under the old key silently unread. + let problems = problems(" prop \"old\" renamed_to=\"new\"\n prop \"jobs\" type=\"uint\""); + assert_eq!( + problems, + vec!["`old` is renamed to `new`, which is not a setting"] + ); +} + +#[test] +fn renames_that_go_round_in_a_circle_are_refused() { + // A cycle makes `lookup` give up and answer `None`, so both settings become unreachable rather + // than wrong — the hardest kind of bug to notice, and one only the spec's author can fix. + let problems = problems( + " prop \"a\" renamed_to=\"b\"\n \ + prop \"b\" renamed_to=\"c\"\n \ + prop \"c\" renamed_to=\"a\"", + ); + assert!( + problems.iter().any(|p| p.contains("form a cycle")), + "{problems:?}" + ); + // Named from each end, because the author has to find the loop and any of its names will do. + assert_eq!(problems.len(), 3, "{problems:?}"); +} + +#[test] +fn an_old_name_with_a_default_of_its_own_is_refused() { + // The merge folds a rename to its target, whose default is the one that shows up — so a + // default on the old name is seeded where nothing reads it. Left to run time this is a + // warning on every run of a shipped binary. + let problems = problems( + " prop \"concurrency\" type=\"uint\" default=8 renamed_to=\"jobs\"\n \ + prop \"jobs\" type=\"uint\" default=4", + ); + assert_eq!( + problems, + vec![ + "`concurrency` is renamed to `jobs` and declares a default of its own: put the \ + default on the setting that replaced it" + ] + ); +} + +#[test] +fn an_old_name_with_a_list_default_is_refused_too() { + // The same mistake in the other spelling. A list default is a child node rather than a value, + // and returning it early skipped the check above — so this was the one shape of an alias + // carrying a default that was generated instead of refused. + let problems = problems( + " prop \"old_ports\" type=\"list\" renamed_to=\"ports\" {\n \ + default 80 443\n }\n \ + prop \"ports\" type=\"list\"", + ); + assert_eq!( + problems, + vec![ + "`old_ports` is renamed to `ports` and declares a default of its own: put the default \ + on the setting that replaced it" + ] + ); +} + +#[test] +fn a_default_declared_twice_is_refused() { + // `default=1` beside a `default 80 443` node. The spec takes both — I checked, rather than + // trusting the comment I had written saying it did not — and there is no reading of a property + // that has two defaults. Generated, the scalar was dropped and nothing said so. + assert_eq!( + problems( + " prop \"ports\" type=\"list\" default=1 {\n default 80 443\n }" + ), + vec!["`ports` declares a default twice, as `default=` and as a `default` node: keep one"] + ); +} + +#[test] +fn a_key_that_cannot_be_a_name_is_refused() { + // KDL takes `prop ""` perfectly happily, and it generated `pub const : PropId = …` — code the + // *adopter's* crate fails to compile, in a file they did not write. + assert_eq!( + problems(" prop \"\" type=\"string\""), + vec!["a setting with no name cannot be generated"] + ); + // And a piece of a dotted key with no name in it: empty, + assert_eq!( + problems(" prop \"task..output\" type=\"string\""), + vec![ + "`task..output` has a part with no name in it: every piece of a dotted key needs an \ + ASCII letter or digit" + ] + ); + // or nothing but separators, where every character becomes an underscore. That generates an + // *anonymous* const — legal Rust, and unreferenceable, so the code naming it does not compile. + assert_eq!( + problems(" prop \"-\" type=\"string\""), + vec![ + "`-` has a part with no name in it: every piece of a dotted key needs an ASCII letter \ + or digit" + ] + ); +} + +#[test] +fn a_key_that_holds_a_newline_stays_on_its_doc_comment() { + // A doc comment is a line. A key can hold a newline as easily as help text can, and the rest of + // it then reads as code — in a file the adopter did not write. + let generated = source_of_spec( + &spec(" prop \"a\\nb\" type=\"string\""), + "mycli.usage.kdl", + ) + .expect("should generate"); + // A doc comment needs no escapes, only a single line: the newline becomes a space. + assert!(generated.contains("/// `a b`"), "{generated}"); + for line in generated.lines() { + let trimmed = line.trim_start(); + assert!( + trimmed.starts_with("///") || trimmed.starts_with("//") || !trimmed.starts_with("b`"), + "the key ended its own comment:\n{generated}" + ); + } +} + +#[test] +fn a_key_with_no_ascii_in_it_is_refused() { + // `é` is a letter, and `ident_of` builds names from ASCII — so it became `_`, and the promised + // `prop::` const was an anonymous one nothing can refer to. Rust would take `É` as an identifier; + // what an arbitrary letter uppercases to is not something to bet a generated name on. + assert_eq!( + problems(" prop \"é\" type=\"string\""), + vec![ + "`é` has a part with no name in it: every piece of a dotted key needs an ASCII letter \ + or digit" + ] + ); +} + +#[test] +fn a_parser_nothing_implements_is_refused() { + // Not dropped and not carried: a spec naming a parser that does not exist would have its + // values split by a rule nobody wrote, so a layer handing over text produces one item where + // the author meant several. + let problems = problems(" prop \"paths\" type=\"list\" parse=\"split_on_vibes\""); + assert_eq!(problems.len(), 1, "{problems:?}"); + assert!( + problems[0].starts_with("`paths` names the parser `split_on_vibes`, which is not one of: "), + "{problems:?}" + ); + // The message lists what there is, because the author's next move is to pick one. + assert!(problems[0].contains("list_by_comma"), "{problems:?}"); +} + +#[test] +fn a_table_keyed_by_something_a_file_cannot_spell_is_refused() { + // Keys in TOML and JSON are text. A `map` would be honoured by handing the CLI string + // keys — quietly, which is the worst way to not support something. + let problems = problems(" prop \"ports\" type=\"map\""); + assert_eq!( + problems, + vec![ + "`ports` is a `map`, and a settings table's keys are text: \ + write `map`" + ] + ); +} + +#[test] +fn two_keys_that_generate_one_const_are_refused() { + // `task.output` and `task_output` are two settings and one `prop::TASK_OUTPUT`. Generating + // both would not compile — a good outcome reported badly, in a file the author did not write. + let problems = problems( + " prop \"task.output\" type=\"string\"\n prop \"task_output\" type=\"string\"", + ); + assert_eq!( + problems, + vec![ + "`task_output` and `task.output` both generate `prop::TASK_OUTPUT`: rename one of them" + ] + ); +} + +#[test] +fn everything_wrong_is_reported_at_once() { + // Three mistakes, one build. Reporting the first would make fixing a registry a sequence of + // builds, which is how the fleet's own generators behave and why nobody enjoys editing them. + let problems = problems( + " prop \"a\" parse=\"nope\"\n \ + prop \"b\" renamed_to=\"nowhere\"\n \ + prop \"c\" type=\"map\"", + ); + assert_eq!(problems.len(), 3, "{problems:?}"); + let all = problems.join("\n"); + assert!(all.contains("`a` names the parser `nope`"), "{all}"); + assert!(all.contains("`b` is renamed to `nowhere`"), "{all}"); + assert!(all.contains("`c` is a `map`"), "{all}"); +} + +#[test] +fn a_spec_with_no_settings_is_not_an_empty_registry() { + // A build script that generates an empty registry has been pointed at the wrong file, and the + // failure it would otherwise produce is "no such setting" for every read in the CLI. + let err = source_of_spec("name \"mycli\"\nbin \"mycli\"\n", "mycli.usage.kdl") + .expect_err("should be refused"); + assert!(matches!(err, Error::NoSettings), "{err}"); + assert_eq!(err.to_string(), "this spec declares no `config` settings"); +} + +#[test] +fn a_spec_that_does_not_parse_says_so_as_the_parser_put_it() { + // Nothing here re-words a KDL error: the spec's own parser has the line and column. + let err = source_of_spec("config {\n prop\n", "mycli.usage.kdl").expect_err("refused"); + assert!(matches!(err, Error::Spec(_)), "{err}"); +} diff --git a/lib/src/spec/mod.rs b/lib/src/spec/mod.rs index 6718a673e..165a7780a 100644 --- a/lib/src/spec/mod.rs +++ b/lib/src/spec/mod.rs @@ -19,7 +19,7 @@ use log::{info, warn}; use serde::Serialize; use std::fmt::{Display, Formatter}; use std::iter::once; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::str::FromStr; use xx::file; @@ -41,6 +41,16 @@ pub struct Spec { pub version: Option, pub usage: String, pub complete: IndexMap, + /// Every file this spec was read from: its own path, then each `include`, recursively. + /// + /// What a build script has to watch. A generator that watches only the file it was pointed at + /// rebuilds nothing when an included file changes — and `include` is how a CLI with many + /// settings keeps them in a file of their own, so that is the file most likely to be edited. + /// + /// Not serialized: it is where the spec came from rather than part of what it says, and `usage g + /// json` describes the latter. + #[serde(skip)] + pub sources: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub source_code_link_template: Option, @@ -178,6 +188,11 @@ impl Spec { let mut schema = Self { ..Default::default() }; + // The file being read, before anything in it can fail: a build script that watches this list + // should watch a spec that does not parse too, or the next build is a stale success. + if !ctx.file.as_os_str().is_empty() { + schema.sources.push(ctx.file.clone()); + } for node in kdl.nodes().iter().map(|n| NodeHelper::new(ctx, n)) { match node.name() { "name" => schema.name = node.arg(0)?.ensure_string()?, @@ -339,6 +354,8 @@ impl Spec { merge_opt!(unknown_flags); merge_extend!(complete); merge_extend!(examples); + // An included spec brings the files *it* read, which is how a nested include is watched. + merge_extend!(sources); if !other.config.is_empty() { self.config.merge(&other.config);