diff --git a/config-build/src/emit.rs b/config-build/src/emit.rs index 78bd17eb6..d5f1c1a82 100644 --- a/config-build/src/emit.rs +++ b/config-build/src/emit.rs @@ -27,6 +27,8 @@ pub(crate) fn registry(config: &SpecConfig, name: &str) -> Result Result { + /// Settings whose key ends here, by their last segment. + leaves: BTreeMap<&'a str, Leaf<'a>>, + /// Groups below this one, by their segment. + groups: BTreeMap<&'a str, Group<'a>>, +} + +struct Leaf<'a> { + /// The whole dotted key. + key: &'a str, + /// The const in `prop`, and the local this is read into. + ident: &'a str, + prop: &'a SpecConfigProp, +} + +/// The `Settings` struct, its nested structs, and the one function that reads them. +pub(crate) fn settings( + props: &[(&String, &SpecConfigProp)], + idents: &[String], + problems: &mut Vec, +) -> String { + let mut root = Group::default(); + for ((key, prop), ident) in props.iter().zip(idents) { + // An old name is not a field: every read of it folds to the setting that replaced it, so a + // field for it would be a second name for one value — and `config set old.key` writing to a + // field nothing reads is exactly the pitchfork bug. + if prop.renamed_to.is_some() { + continue; + } + // A key with a piece that cannot be a name is already refused by name, and building a tree + // from it invents a group called nothing that then collides with its own parent — two + // messages for one mistake, one of them about something the author never wrote. + if !crate::emit::nameable(key) { + continue; + } + let segments: Vec<&str> = key.split('.').collect(); + let (last, path) = segments.split_last().expect("a key has a segment"); + let mut group = &mut root; + for segment in path { + group = group.groups.entry(segment).or_default(); + } + group.leaves.insert(last, Leaf { key, ident, prop }); + } + + let mut structs: BTreeMap = BTreeMap::new(); + check_names(&root, "", "Settings", &mut structs, problems); + + let mut structs = String::new(); + let mut reads = String::new(); + let root_body = emit(&root, "Settings", &mut structs, &mut reads); + + format!( + "\n/// Every setting, as the types a CLI holds them in.\n\ + ///\n\ + /// Read with [`Settings::read`] from a resolution, which is the only way to build one:\n\ + /// the values, and every reason one could not be read, come from the merge.\n\ + #[derive(Debug, Clone, PartialEq)]\n\ + pub struct Settings {{\n{root_body}}}\n\ + {structs}\n\ + impl Settings {{\n \ + /// This resolution's values, as their types.\n \ + ///\n \ + /// Every setting is read before anything is returned, so the error is the whole list of\n \ + /// what is wrong rather than the first thing found.\n \ + pub fn read(\n \ + resolved: &::usage_config::Resolved,\n \ + ) -> ::std::result::Result {{\n \ + let mut fold = resolved.fold();\n{reads} \ + fold.finish()?;\n \ + ::std::result::Result::Ok(Self {{\n{} }})\n }}\n}}\n", + construct(&root, "Settings", 3), + ) +} + +/// One group's fields, its descendants' structs, and the reads that fill them. +fn emit(group: &Group<'_>, name: &str, structs: &mut String, reads: &mut String) -> String { + let mut fields = String::new(); + for (segment, sub) in &group.groups { + let sub_name = format!("{name}{}", camel(segment)); + let body = emit(sub, &sub_name, structs, reads); + let _ = write!( + structs, + "\n/// The `{}` settings.\n\ + #[derive(Debug, Clone, PartialEq)]\n\ + pub struct {sub_name} {{\n{body}}}\n", + crate::emit::one_line(segment) + ); + let _ = writeln!(fields, " pub {}: {sub_name},", field(segment)); + } + for (segment, leaf) in &group.leaves { + let ty = rust_ty(leaf); + if let Some(help) = leaf.prop.help.as_deref() { + let _ = writeln!(fields, " /// {}", one_line(help)); + } + // The key as well as the help, because the field name is a translation of it and a reader + // going from code to a config file needs the name the file uses. + let _ = writeln!(fields, " /// (`{}`)", crate::emit::one_line(leaf.key)); + if optional(leaf) { + let _ = writeln!(fields, " pub {}: Option<{ty}>,", field(segment)); + let _ = writeln!( + reads, + " let {}: Option<{ty}> = fold.optional(prop::{});", + local(leaf.ident), + leaf.ident + ); + } else { + let _ = writeln!(fields, " pub {}: {ty},", field(segment)); + let _ = writeln!( + reads, + " let {}: Option<{ty}> = fold.required(prop::{});", + local(leaf.ident), + leaf.ident + ); + } + } + fields +} + +/// The expression that builds one group, once the fold has proved every value read. +fn construct(group: &Group<'_>, name: &str, depth: usize) -> String { + let pad = " ".repeat(depth); + let mut out = String::new(); + for (segment, sub) in &group.groups { + // Named, not inferred: a nested group is its own struct, and the name is the same one its + // declaration got. + let sub_name = format!("{name}{}", camel(segment)); + let _ = writeln!(out, "{pad}{}: {sub_name} {{", field(segment)); + let _ = write!(out, "{}", construct(sub, &sub_name, depth + 1)); + let _ = writeln!(out, "{pad}}},"); + } + for (segment, leaf) in &group.leaves { + if optional(leaf) { + let name = field(segment); + let read_into = local(leaf.ident); + // `ci` rather than `ci: ci` where the two agree, which they do for every setting whose + // key has one segment: clippy will not have the long form, and nor should it. + if name == read_into { + let _ = writeln!(out, "{pad}{name},"); + } else { + let _ = writeln!(out, "{pad}{name}: {read_into},"); + } + } else { + // The unwrap the fold's own contract allows: `required` returns `None` only when it has + // recorded an error, and `finish` has already turned any error into a return. + // The message is a literal in generated code, so the key goes through the same escaper + // `PropMeta` uses: a key holding a quote or a backslash — `a"b` is a nameable key — + // otherwise ended the literal early and the rest of it read as code. + let message = crate::emit::rust_str(&format!( + "`{}` has a declared default, so the fold has already reported any absence", + leaf.key + )); + let _ = writeln!( + out, + "{pad}{}: {}.expect({message}),", + field(segment), + local(leaf.ident), + ); + } + } + out +} + +/// Whether a field holds an `Option`. +/// +/// Two ways to be optional and they mean the same thing to the code: the spec said `option`, or +/// nothing declared a default, in which case the resolution can perfectly well come back with no +/// value. Writing the field as `T` would mean a failure at run time for a shape the spec allows. +fn optional(leaf: &Leaf<'_>) -> bool { + leaf.prop.default.is_none() && leaf.prop.default_list.is_empty() + || leaf + .prop + .value_type + .as_ref() + .is_some_and(SpecConfigType::is_optional) +} + +/// The Rust type for one setting, without the `Option` around it. +fn rust_ty(leaf: &Leaf<'_>) -> String { + fn of(ty: &SpecConfigType) -> String { + match ty { + SpecConfigType::Base(base) => base_ty(base).to_string(), + // A `set` is a `Vec` too: the merge has already dropped duplicates, and a list keeps + // the order the files were read in — which for a `PATH`-like setting is the meaning. + SpecConfigType::List(inner) | SpecConfigType::Set(inner) => { + format!("Vec<{}>", of(inner)) + } + SpecConfigType::Map(_, value) => { + format!("::std::collections::BTreeMap", of(value)) + } + SpecConfigType::Option(inner) => of(inner), + // A union is not a Rust type. The value arrives as it was written and the CLI decides, + // which is what declaring a union asked for. + SpecConfigType::Union(_) => "::usage_config::Value".to_string(), + } + } + let Some(declared) = leaf.prop.value_type.as_ref() else { + return "String".to_string(); + }; + // Every `Value` this can produce is one the spec asked for: an `object` says its keys are not + // described, a union says usage cannot decide, and a name usage does not know says the same. So + // there is nothing to refuse here — the type is as narrow as the declaration was. + of(declared) +} + +fn base_ty(base: &Base) -> &'static str { + match base { + Base::Bool => "bool", + Base::Int => "i64", + Base::Uint => "u64", + Base::Float => "f64", + Base::String => "String", + Base::Path => "::std::path::PathBuf", + // Read as text, both of them. What makes a string a URL is what the CLI does with it, and + // the crate that owns the duration type owns its spelling — inventing one here would put a + // dependency in every adopter's binary for a value they may only ever print. + Base::Url | Base::Duration => "String", + // A table whose keys the spec does not describe, and a name usage does not know: the value + // as it was written. + Base::Object | Base::Custom(_) => "::usage_config::Value", + } +} + +/// `task` → `Task`, for a nested struct's name. +/// +/// ASCII only, like every other name here: a Rust identifier is not "any alphanumeric character" — +/// `½` is one and is not allowed in an identifier, and a Unicode digit cannot start one — and the +/// consts in `prop` are built from ASCII already, so keeping more here made one key produce names +/// from two alphabets, one of which the adopter could not compile. +fn camel(segment: &str) -> String { + let mut out = String::with_capacity(segment.len()); + let mut upper = true; + for c in segment.chars() { + if !c.is_ascii_alphanumeric() { + upper = true; + continue; + } + if upper { + out.extend(c.to_uppercase()); + upper = false; + } else { + out.push(c); + } + } + out +} + +/// A key segment as a field name. +fn field(segment: &str) -> String { + let mut name: String = segment + .chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) + .collect(); + // A field cannot start with a digit any more than a const can, and `2fa` is a real name for a + // real setting. Prefixed the same way, so the two spellings of one key agree. + if name.starts_with(|c: char| c.is_ascii_digit()) { + name.insert(0, '_'); + } + raw(&name) +} + +/// The local a setting is read into. +/// +/// Prefixed, and that is the whole point: unprefixed, a setting called `fold` generated +/// `let fold = fold.optional(…)` and shadowed the reader's own fold, so every read after it — and +/// `fold.finish()` — stopped compiling. A setting called `SELF` generated `let self`. The prefix +/// goes on every local, so no name a spec can choose reaches either state, and since the consts it +/// is built from are unique, the locals are too. +fn local(ident: &str) -> String { + format!("read_{}", ident.to_lowercase()) +} + +/// `name`, spelled so it can be an identifier. +/// +/// A `let` binding needs this as much as a field does: `match` is an unremarkable name for a +/// setting, and `let match: Option` is not Rust. +fn raw(name: &str) -> String { + if RAW_ALLOWED.contains(&name) { + return format!("r#{name}"); + } + name.to_string() +} + +/// Keywords a field can still be named, spelled `r#`. +/// +/// The four that cannot — `self`, `crate`, `super` and `Self` — are refused by name in +/// [`check_names`], because there is no way to write them. +const RAW_ALLOWED: [&str; 49] = [ + "as", + "break", + "const", + "continue", + "else", + "enum", + "extern", + "false", + "fn", + "for", + "if", + "impl", + "in", + "let", + "loop", + "match", + "mod", + "move", + "mut", + "pub", + "ref", + "return", + "static", + "struct", + "trait", + "true", + "type", + "unsafe", + "use", + "where", + "while", + "async", + "await", + "dyn", + "abstract", + "become", + "box", + "do", + "final", + "macro", + "override", + "priv", + "typeof", + "unsized", + "virtual", + "yield", + "try", + "gen", + "macro_rules", +]; + +/// Keywords no identifier can be, raw or otherwise. +const NEVER_ALLOWED: [&str; 4] = ["self", "crate", "super", "Self"]; + +/// Refuse the names that cannot become fields, and the ones that would collide. +/// +/// Checked on the names that are actually *emitted*, not on the key segments they come from: two +/// segments can differ and still translate to one field (`foo-bar` beside `foo_bar`), and two groups +/// can differ and still translate to one struct (`a` beside `A`, since a type name loses the case +/// of its first letter). Comparing the segments, as this did, refused neither — and the collision +/// surfaced as duplicate items in a file the adopter did not write. The `prop::` consts do not catch +/// these either: `foo-bar.x` and `foo_bar.y` make two distinct consts and one field. +fn check_names( + group: &Group<'_>, + path: &str, + name: &str, + structs: &mut BTreeMap, + problems: &mut Vec, +) { + // Fields are per level: each group is its own struct, so two groups may both have a `timeout`. + // Every entry remembers whether it came from a setting or from a group, because a setting and a + // group wanting one name is a mistake worth its own words. + let mut fields: BTreeMap = BTreeMap::new(); + + for (segment, leaf) in &group.leaves { + check_segment(segment, leaf.key, problems); + if let Some((other, _)) = fields.insert(field(segment), (leaf.key.to_string(), true)) { + collide(&other, leaf.key, &field(segment), problems); + } + } + for (segment, sub) in &group.groups { + let key = format!("{path}{segment}"); + check_segment(segment, &key, problems); + match fields.insert(field(segment), (key.clone(), false)) { + // `python` as a setting and `python.compile` as another: one field name with two things + // to be, a value and a table. The spec can say it and no struct can hold it. + Some((setting, true)) => problems.push(format!( + "`{setting}` is a setting and a group of settings: it cannot be both a value and \ + a table" + )), + Some((other, false)) => collide(&other, &key, &field(segment), problems), + None => {} + } + // Struct names are *not* per level. Every one of them is declared in the same module and + // built by concatenation, so `http.client.x` and `http_client.y` both arrive at + // `SettingsHttpClient` from different depths — compared among siblings, as this was, neither + // was refused and the adopter's crate got two structs with one name. + let sub_name = format!("{name}{}", camel(segment)); + if let Some(other) = structs.insert(sub_name.clone(), key.clone()) { + problems.push(format!( + "`{other}` and `{key}` are both groups named `{sub_name}`: rename one of them" + )); + } + check_names(sub, &format!("{key}."), &sub_name, structs, problems); + } +} + +/// Whether one key segment can be a field at all. +fn check_segment(segment: &str, of: &str, problems: &mut Vec) { + if NEVER_ALLOWED.contains(&segment) { + problems.push(format!( + "`{of}` cannot be a field: `{segment}` is a keyword Rust has no spelling for" + )); + } +} + +fn collide(one: &str, other: &str, name: &str, problems: &mut Vec) { + problems.push(format!( + "`{one}` and `{other}` both generate the field `{name}`: rename one of them" + )); +} + +/// Help text as one line, for a doc comment. +fn one_line(text: &str) -> String { + text.replace(['\n', '\r'], " ") +} diff --git a/config-build/tests/fixtures/hk.usage.kdl b/config-build/tests/fixtures/hk.usage.kdl index 5889691a5..855ada3ae 100644 --- a/config-build/tests/fixtures/hk.usage.kdl +++ b/config-build/tests/fixtures/hk.usage.kdl @@ -48,6 +48,9 @@ config { prop "task.output" type="string" default="prefix" help="How task output is interleaved" + // A keyword as a key, which is ordinary in a config file and needs `r#` in Rust. + prop "match" type="string" default="all" help="Which files to match" + prop "either" type="bool|string" help="A \"union\" only hk understands" prop "timeout" type="option" help="How long to wait, if at all" diff --git a/config-build/tests/generated.rs b/config-build/tests/generated.rs index 78502d100..be846b99e 100644 --- a/config-build/tests/generated.rs +++ b/config-build/tests/generated.rs @@ -183,3 +183,78 @@ fn a_file_read_against_the_generated_registry_means_what_the_spec_said() { let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn the_settings_struct_holds_what_each_type_says() { + // The struct is generated, so this is the compiler agreeing as much as it is a test: every + // field below has to exist, with that type, or this file does not build. + let resolved = resolve(SETTINGS_REGISTRY, Layers::new()).expect("resolves"); + let settings = Settings::read(&resolved).expect("every default fits its field"); + + // A declared default means the field is the value itself — there is nothing to unwrap for a + // setting that always has one. + assert_eq!(settings.jobs, 4u64); + assert_eq!(settings.stash, "git"); + assert!(!settings.trusted); + assert_eq!(settings.ports, vec![80u64, 443]); + // A dotted key is a nested struct, spelled the way the config file spells it. + assert_eq!(settings.task.output, "prefix"); + // No default, or `option<…>`: absent is a state the type can hold, so nothing fails at run + // time for a shape the spec allows. + assert_eq!(settings.exclude, None); + assert_eq!(settings.timeout, None); + assert_eq!(settings.ci, None); + // A keyword is an unremarkable name for a setting, and `r#` is how Rust spells it — for the + // field and for the local the generated reader binds it to, which is what `let match: …` taught + // me the hard way. + assert_eq!(settings.r#match, "all"); + // A union has no Rust type. The value arrives as it was written, which is what declaring one + // asked for. + assert_eq!(settings.either, None); + // And an old name is not a field at all: `concurrency` folds into `jobs`, so a field for it + // would be a second name for one value. + assert_eq!( + SETTINGS_REGISTRY.get(prop::CONCURRENCY).renamed_to, + Some("jobs") + ); +} + +#[test] +fn a_file_fills_the_struct_and_a_bad_value_names_itself() { + let dir = + std::env::temp_dir().join(format!("usage_config_build_struct_{}", 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\"\npath = \"/bin\"\n[task]\noutput = \"interleave\"\n", + ) + .expect("write"); + let layer = FileLayer::at(&path, FileScope::Project); + let resolved = resolve(SETTINGS_REGISTRY, Layers::new().then(&layer)).expect("resolves"); + let settings = Settings::read(&resolved).expect("reads"); + assert_eq!(settings.jobs, 8); + assert_eq!( + settings.exclude, + Some(vec!["target".to_string(), "dist".to_string()]) + ); + assert_eq!( + settings.path, + Some(vec![std::path::PathBuf::from("/bin")]), + "a `list` arrives as paths" + ); + assert_eq!(settings.task.output, "interleave"); + + // And the failure a struct read can still have: a hook writing past the declared type, which + // is the one place the merge did not check. The error names the setting and the hook. + let mut resolved = resolve(SETTINGS_REGISTRY, Layers::new()).expect("resolves"); + resolved.coerced(prop::JOBS, Value::Int(-1), "one job when raw"); + let err = Settings::read(&resolved).expect_err("a `uint` field cannot hold -1"); + assert_eq!( + err.to_string(), + "jobs expected a non-negative integer but has `-1` (set by one job when raw)" + ); + + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/config-build/tests/golden/settings.rs b/config-build/tests/golden/settings.rs index ce850ed17..950d755aa 100644 --- a/config-build/tests/golden/settings.rs +++ b/config-build/tests/golden/settings.rs @@ -39,6 +39,11 @@ pub static SETTINGS_PROPS: &[::usage_config::PropMeta] = &[ envs: &["HK_LOG_LEVEL"], ..::usage_config::PropMeta::new("log_level", ::usage_config::Ty::String) }, + ::usage_config::PropMeta { + default: Some(::usage_config::Const::Str("all")), + help: Some("Which files to match"), + ..::usage_config::PropMeta::new("match", ::usage_config::Ty::String) + }, ::usage_config::PropMeta { parse: Some(::usage_config::Parser::ListByOsPathSeparator), envs: &["HK_PATH"], @@ -94,18 +99,112 @@ pub mod prop { pub const JOBS: PropId = PropId(4); /// `log_level` pub const LOG_LEVEL: PropId = PropId(5); + /// `match` — Which files to match + pub const MATCH: PropId = PropId(6); /// `path` - pub const PATH: PropId = PropId(6); + pub const PATH: PropId = PropId(7); /// `ports` - pub const PORTS: PropId = PropId(7); + pub const PORTS: PropId = PropId(8); /// `stash` — How to stash before a run - pub const STASH: PropId = PropId(8); + pub const STASH: PropId = PropId(9); /// `task.output` — How task output is interleaved - pub const TASK_OUTPUT: PropId = PropId(9); + pub const TASK_OUTPUT: PropId = PropId(10); /// `timeout` — How long to wait, if at all - pub const TIMEOUT: PropId = PropId(10); + pub const TIMEOUT: PropId = PropId(11); /// `trusted` — Whether this checkout may run its own hooks - pub const TRUSTED: PropId = PropId(11); + pub const TRUSTED: PropId = PropId(12); /// `url_replacements` — Rewrite these URL prefixes - pub const URL_REPLACEMENTS: PropId = PropId(12); + pub const URL_REPLACEMENTS: PropId = PropId(13); +} + +/// Every setting, as the types a CLI holds them in. +/// +/// Read with [`Settings::read`] from a resolution, which is the only way to build one: +/// the values, and every reason one could not be read, come from the merge. +#[derive(Debug, Clone, PartialEq)] +pub struct Settings { + pub task: SettingsTask, + /// (`ci`) + pub ci: Option, + /// A "union" only hk understands + /// (`either`) + pub either: Option<::usage_config::Value>, + /// Paths to leave alone + /// (`exclude`) + pub exclude: Option>, + /// How many jobs to run at once + /// (`jobs`) + pub jobs: u64, + /// (`log_level`) + pub log_level: String, + /// Which files to match + /// (`match`) + pub r#match: String, + /// (`path`) + pub path: Option>, + /// (`ports`) + pub ports: Vec, + /// How to stash before a run + /// (`stash`) + pub stash: String, + /// How long to wait, if at all + /// (`timeout`) + pub timeout: Option, + /// Whether this checkout may run its own hooks + /// (`trusted`) + pub trusted: bool, + /// Rewrite these URL prefixes + /// (`url_replacements`) + pub url_replacements: Option<::std::collections::BTreeMap>, +} + +/// The `task` settings. +#[derive(Debug, Clone, PartialEq)] +pub struct SettingsTask { + /// How task output is interleaved + /// (`task.output`) + pub output: String, +} + +impl Settings { + /// This resolution's values, as their types. + /// + /// Every setting is read before anything is returned, so the error is the whole list of + /// what is wrong rather than the first thing found. + pub fn read( + resolved: &::usage_config::Resolved, + ) -> ::std::result::Result { + let mut fold = resolved.fold(); + let read_task_output: Option = fold.required(prop::TASK_OUTPUT); + let read_ci: Option = fold.optional(prop::CI); + let read_either: Option<::usage_config::Value> = fold.optional(prop::EITHER); + let read_exclude: Option> = fold.optional(prop::EXCLUDE); + let read_jobs: Option = fold.required(prop::JOBS); + let read_log_level: Option = fold.required(prop::LOG_LEVEL); + let read_match: Option = fold.required(prop::MATCH); + let read_path: Option> = fold.optional(prop::PATH); + let read_ports: Option> = fold.required(prop::PORTS); + let read_stash: Option = fold.required(prop::STASH); + let read_timeout: Option = fold.optional(prop::TIMEOUT); + let read_trusted: Option = fold.required(prop::TRUSTED); + let read_url_replacements: Option<::std::collections::BTreeMap> = fold.optional(prop::URL_REPLACEMENTS); + fold.finish()?; + ::std::result::Result::Ok(Self { + task: SettingsTask { + output: read_task_output.expect("`task.output` has a declared default, so the fold has already reported any absence"), + }, + ci: read_ci, + either: read_either, + exclude: read_exclude, + jobs: read_jobs.expect("`jobs` has a declared default, so the fold has already reported any absence"), + log_level: read_log_level.expect("`log_level` has a declared default, so the fold has already reported any absence"), + r#match: read_match.expect("`match` has a declared default, so the fold has already reported any absence"), + path: read_path, + ports: read_ports.expect("`ports` has a declared default, so the fold has already reported any absence"), + stash: read_stash.expect("`stash` has a declared default, so the fold has already reported any absence"), + timeout: read_timeout, + trusted: read_trusted.expect("`trusted` has a declared default, so the fold has already reported any absence"), + url_replacements: read_url_replacements, + }) + } } diff --git a/config-build/tests/refusals.rs b/config-build/tests/refusals.rs index 29731ec21..a85a676a5 100644 --- a/config-build/tests/refusals.rs +++ b/config-build/tests/refusals.rs @@ -277,3 +277,171 @@ fn a_spec_that_does_not_parse_says_so_as_the_parser_put_it() { let err = source_of_spec("config {\n prop\n", "mycli.usage.kdl").expect_err("refused"); assert!(matches!(err, Error::Spec(_)), "{err}"); } + +#[test] +fn a_setting_that_is_also_a_group_of_settings_is_refused() { + // `python` as a setting and `python.compile` as another: one field name with two things to be, + // a value and a table. The spec can say it; no struct can hold it. + let problems = + problems(" prop \"python\" type=\"string\"\n prop \"python.compile\" type=\"bool\""); + assert_eq!( + problems, + vec![ + "`python` is a setting and a group of settings: it cannot be both a value and a table" + ] + ); +} + +#[test] +fn two_keys_that_generate_one_field_are_refused() { + // The `prop::` consts do not catch these: `foo-bar.x` and `foo_bar.y` are two distinct consts + // and one field, because a dash and an underscore are the same character in an identifier. The + // check has to be on the names that are emitted, not on the segments they came from. + assert_eq!( + problems(" prop \"foo-bar.x\" type=\"string\"\n prop \"foo_bar.y\" type=\"string\""), + vec![ + "`foo-bar` and `foo_bar` both generate the field `foo_bar`: rename one of them", + "`foo-bar` and `foo_bar` are both groups named `SettingsFooBar`: rename one of them" + ] + ); + + // And two groups whose *type* names collide, which a field name does not: a struct name loses + // the case of its first letter. + assert_eq!( + problems(" prop \"a.x\" type=\"string\"\n prop \"A.y\" type=\"string\""), + vec!["`A` and `a` are both groups named `SettingsA`: rename one of them"] + ); +} + +#[test] +fn two_groups_that_generate_one_struct_are_refused_however_deep_they_are() { + // Struct names are built by concatenation and all declared in one module, so `http.client` and + // `http_client` arrive at `SettingsHttpClient` from different depths. Compared among siblings + // they are not siblings at all, and the adopter's crate got two structs with one name. + assert_eq!( + problems( + " prop \"http.client.timeout\" type=\"string\"\n \ + prop \"http_client.retries\" type=\"uint\"" + ), + vec![ + "`http.client` and `http_client` are both groups named `SettingsHttpClient`: \ + rename one of them" + ] + ); +} + +#[test] +fn a_key_that_needs_escaping_is_escaped_where_it_becomes_a_literal() { + // `a"b` is a nameable key — it has letters in it — and the generated reader quotes the key into + // an `expect` message. Interpolated raw, the quote ended that literal early and the rest of the + // message read as code, in a file the adopter did not write. + let generated = source_of_spec( + &spec(" prop \"a\\\"b\" type=\"uint\" default=1"), + "mycli.usage.kdl", + ) + .expect("should generate"); + assert!( + generated.contains("`a\\\"b` has a declared default"), + "{generated}" + ); + // The same key in `PropMeta`, which was escaped all along, and in a doc comment, where it needs + // no escape and only one line. + assert!( + generated.contains("PropMeta::new(\"a\\\"b\""), + "{generated}" + ); + assert!(generated.contains("/// (`a\"b`)"), "{generated}"); +} + +#[test] +fn a_setting_named_after_the_readers_own_bindings_is_still_a_setting() { + // `fold` is what the generated reader calls its fold, and an unprefixed local shadowed it: every + // read after it, and `fold.finish()`, stopped compiling. Nothing about `fold` is special to a + // config file, so the fix is on this side — every local is prefixed. + let generated = source_of_spec( + &spec(" prop \"fold\" type=\"string\"\n prop \"resolved\" type=\"bool\""), + "mycli.usage.kdl", + ) + .expect("should generate"); + assert!( + generated.contains("let read_fold: Option ="), + "{generated}" + ); + assert!( + generated.contains("let read_resolved: Option ="), + "{generated}" + ); + // And the fields keep the names the config file uses. + assert!( + generated.contains("pub fold: Option,"), + "{generated}" + ); +} + +#[test] +fn a_name_is_built_from_ascii_wherever_it_is_built() { + // A Rust identifier is not "any alphanumeric character": `½` is one and cannot appear in an + // identifier, and a Unicode digit cannot start one. The `prop::` consts were ASCII already, so + // keeping more in the fields made one key produce names from two alphabets — a const the adopter + // could compile beside a field they could not. + let generated = source_of_spec( + &spec(" prop \"caf\u{e9}.si\u{bd}e\" type=\"string\""), + "mycli.usage.kdl", + ) + .expect("should generate"); + assert!( + generated.contains("pub const CAF__SI_E: PropId"), + "{generated}" + ); + assert!(generated.contains("pub caf_: SettingsCaf,"), "{generated}"); + assert!( + generated.contains("pub si_e: Option,"), + "{generated}" + ); + // Every line that *declares* a name is ASCII. The key itself is not — it appears as a string + // literal in `PropMeta`, and as data it is exactly what the spec said. + for line in generated.lines() { + let declares = line.trim_start(); + let declares = declares.starts_with("pub const ") + || declares.starts_with("pub struct ") + || declares.starts_with("pub fn ") + || declares.starts_with("let ") + || (declares.starts_with("pub ") && declares.contains(':')); + assert!( + !declares || line.is_ascii(), + "a generated name kept a character no identifier can hold: {line}" + ); + } +} + +#[test] +fn a_key_that_starts_with_a_digit_is_a_field_all_the_same() { + // `2fa` is a real name for a real setting, and neither a const nor a field can start with a + // digit — so both get the same prefix rather than one of them being invalid Rust. + let generated = source_of_spec( + &spec(" prop \"2fa.token\" type=\"string\""), + "mycli.usage.kdl", + ) + .expect("should generate"); + assert!( + generated.contains("pub const _2FA_TOKEN: PropId"), + "{generated}" + ); + assert!(generated.contains("pub _2fa: Settings2fa,"), "{generated}"); +} + +#[test] +fn a_key_rust_has_no_spelling_for_is_refused() { + // Most keywords are fine as fields — `type` is written `r#type`, and settings called that are + // perfectly ordinary. Four are not: there is no `r#self`, so the field cannot be written at all. + assert_eq!( + problems(" prop \"self\" type=\"string\""), + vec!["`self` cannot be a field: `self` is a keyword Rust has no spelling for"] + ); + + // And the same for a group, whose name becomes a field too. + assert_eq!( + problems(" prop \"crate.name\" type=\"string\""), + vec!["`crate` cannot be a field: `crate` is a keyword Rust has no spelling for"] + ); +} diff --git a/config/src/read.rs b/config/src/read.rs index 177577257..688f6f3bf 100644 --- a/config/src/read.rs +++ b/config/src/read.rs @@ -197,6 +197,15 @@ fn mismatch(expected: &'static str, value: &Value) -> TypeError { } } +impl FromValue for Value { + // A value read as itself, for a field whose type the spec left open: `object` says the keys are + // not described, and a union says usage cannot decide what belongs. Neither is a shape a + // narrower Rust type could hold without the generator inventing one. + fn from_value(value: &Value) -> Result { + Ok(value.clone()) + } +} + impl FromValue for bool { fn from_value(value: &Value) -> Result { match value {