diff --git a/PLAN.md b/PLAN.md index 923ef4251..24e84305e 100644 --- a/PLAN.md +++ b/PLAN.md @@ -121,6 +121,11 @@ manpages, and SDKs — never a runtime dependency of somebody else's program. the conversion happens where the struct is built, and a value that will not convert reports the text and the type's own message. `Error` grew one boxed variant and lost `Copy`, and stayed 40 bytes, so nothing on the hot path grew. +- [x] **Enumerated values** — `#[derive(usage::ValueEnum)]` on an enum of bare variants + gives the words a value may be, and a field says `value_enum` to use them. mise has + nine of these. The list is declared once, on the type: the spec, the help, the + completions and the check that rejects a wrong word all read it from there, so none of + them can drift from the type the way a second list on the field would. - [ ] **Values that are not valid UTF-8** — a word reaches a field through `from_utf8_lossy`, so a `PathBuf` holding a non-UTF-8 path gets replacement characters instead of bytes. The partial should hold `OsString` and let `build` diff --git a/argv/src/spec.rs b/argv/src/spec.rs index 05901d9a7..179ba0b94 100644 --- a/argv/src/spec.rs +++ b/argv/src/spec.rs @@ -760,6 +760,17 @@ fn quoted(value: &str) -> String { /// generated parse needs to name the type that accumulates a subcommand's values /// while parsing, and cannot know which module the derive put it in — an /// associated type is how it names it regardless. +/// A type whose values are a fixed set of words. +/// +/// What a CLI calls an enum: `--shell bash`. The words are what the spec lists as +/// `choices`, so declaring them once on the type keeps help, completions and the check that +/// rejects a wrong value reading from the same place — rather than a list in an attribute +/// that has to be kept in step with the type by hand. +pub trait ValueEnum: Sized { + /// Every word this type accepts, in the order it declared them. + const CHOICES: &'static [&'static str]; +} + pub trait CommandArgs: Sized { /// Values collected so far. Partly-filled by construction, since a parse can /// stop early. diff --git a/conformance/tests/typed.rs b/conformance/tests/typed.rs index cfff52635..a06378d6d 100644 --- a/conformance/tests/typed.rs +++ b/conformance/tests/typed.rs @@ -9,6 +9,7 @@ use std::ffi::OsStr; use std::path::PathBuf; use std::str::FromStr; +use usage::Spec as LibSpec; use usage_argv::Error; use usage_derive::Cli; @@ -248,3 +249,76 @@ fn a_conversion_failure_on_a_subcommand_names_the_field() { Ok(_) => panic!("300 does not fit in a u8"), } } + +/// The words a value may be, declared once on the type. +/// +/// mise has nine of these. What matters is that the list reaches the spec — so help and +/// completions offer it — without being written a second time on the field. +#[derive(Debug, PartialEq, usage_derive::ValueEnum)] +enum Interpreter { + Bash, + Zsh, + Fish, + /// Not `power-shell`, which is what the variant name would have given + #[usage(name = "pwsh")] + PowerShell, +} + +/// A CLI with an enumerated value +#[derive(Cli)] +#[usage(bin = "enumerated")] +struct Enumerated { + /// Which shell + #[usage(short = 's', long, value_enum)] + shell: Option, + /// Shells to generate for + #[usage(long, var, value_enum)] + also: Vec, +} + +#[test] +fn a_word_becomes_the_variant_it_names() { + let a = argv(["-s", "zsh", "--also", "bash", "--also", "pwsh"]); + let e = Enumerated::parse_from(&a).expect("should parse"); + assert_eq!(e.shell, Some(Interpreter::Zsh)); + assert_eq!(e.also, [Interpreter::Bash, Interpreter::PowerShell]); +} + +#[test] +fn the_words_reach_the_spec_from_the_type() { + // The point of `value_enum`: the list is declared once, on the type, and the spec has + // it — so `usage g markdown` and the completions offer the same words the parse accepts. + let spec: LibSpec = Enumerated::to_kdl().parse().expect("valid spec"); + let shell = spec.cmd.flags.iter().find(|f| f.name == "shell").unwrap(); + let choices = shell + .arg + .as_ref() + .and_then(|a| a.choices.as_ref()) + .expect("--shell should declare choices"); + assert_eq!(choices.choices, ["bash", "zsh", "fish", "pwsh"]); +} + +#[test] +fn a_wrong_word_lists_what_was_expected() { + // An `InvalidChoice` carrying the list, rather than a conversion error about a type the + // user never named. + let a = argv(["--shell", "csh"]); + match Enumerated::parse_from(&a) { + Err(Error::InvalidChoice { name, choices }) => { + assert_eq!(name, "shell"); + assert_eq!(choices, ["bash", "zsh", "fish", "pwsh"]); + } + Err(other) => panic!("wrong error: {other:?}"), + Ok(_) => panic!("`csh` is not one of the words"), + } +} + +#[test] +fn the_conversion_stands_on_its_own() { + // Whoever converts one by hand gets a message with the words in it, since the check + // above is the parser's and not the type's. + use std::str::FromStr; + assert_eq!(Interpreter::from_str("fish"), Ok(Interpreter::Fish)); + let err = Interpreter::from_str("csh").expect_err("not a shell"); + assert!(err.contains("bash, zsh, fish, pwsh"), "{err}"); +} diff --git a/derive/src/codegen.rs b/derive/src/codegen.rs index 5d615c291..d2d275537 100644 --- a/derive/src/codegen.rs +++ b/derive/src/codegen.rs @@ -15,7 +15,7 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; -use crate::model::{rendered_path, Cli, Field, Kind, Shape, Subcommands}; +use crate::model::{rendered_path, Cli, Field, Kind, Shape, Subcommands, ValueEnum}; pub fn emit(cli: &Cli) -> TokenStream { let ident = &cli.ident; @@ -406,6 +406,12 @@ fn arg_meta(i: usize, field: &Field) -> TokenStream { /// A field's declared choices, as the metadata holds them. fn choices_tokens(field: &Field) -> TokenStream { + // From the type when the field says `value_enum`, so the spec, the help and the check + // all read the list the type declares rather than a copy of it. + if let (true, Some(ty)) = (field.value_enum, field.value_ty.as_ref()) { + let ty = in_module(ty); + return quote!(<#ty as ::usage_argv::spec::ValueEnum>::CHOICES); + } let choices = &field.choices; quote!(&[#(#choices),*]) } @@ -1482,12 +1488,24 @@ fn post_binding(cli: &Cli) -> TokenStream { }); let choice_checks = cli.fields.iter().filter_map(|f| { - if f.choices.is_empty() { + if f.choices.is_empty() && !f.value_enum { return None; } let ident = &f.ident; let name = &f.name; - let choices = &f.choices; + // A `value_enum`'s words live on the type. Checking against them here rather than + // letting the conversion fail is what makes a wrong word an `InvalidChoice` that + // lists what was expected, instead of a message about a type the user did not name. + let choices: TokenStream = match (f.value_enum, f.value_ty.as_ref()) { + (true, Some(ty)) => { + let ty = in_module(ty); + quote!(<#ty as ::usage_argv::spec::ValueEnum>::CHOICES) + } + _ => { + let list = &f.choices; + quote!(&[#(#list),*]) + } + }; let values = match f.shape { Shape::Optional => quote!(partial.#ident.iter()), Shape::Required => quote!(::std::iter::once(&partial.#ident)), @@ -1497,11 +1515,11 @@ fn post_binding(cli: &Cli) -> TokenStream { }; Some(quote! { for value in #values { - if ![#(#choices),*].contains(&value.as_str()) { + if !#choices.contains(&value.as_str()) { return ::std::result::Result::Err( ::usage_argv::Error::InvalidChoice { name: #name, - choices: &[#(#choices),*], + choices: #choices, }, ); } @@ -1663,6 +1681,50 @@ fn post_binding(cli: &Cli) -> TokenStream { } } +/// The word list and the conversion for a value enum. +/// +/// Two impls and nothing else: the words as a `const` the spec can read, and the `FromStr` +/// that every typed field already goes through. Deliberately not a bespoke path — a value +/// enum is a type whose values happen to be listed, so it converts the way any other type +/// does, and the check that rejects a wrong word is the same `choices` check as a +/// hand-written list. +pub fn emit_value_enum(value_enum: &ValueEnum) -> TokenStream { + let ident = &value_enum.ident; + let words: Vec<&String> = value_enum.variants.iter().map(|(_, name)| name).collect(); + let arms = value_enum + .variants + .iter() + .map(|(variant, name)| quote!(#name => ::std::result::Result::Ok(#ident::#variant),)); + // Listed in the message because a wrong word is the common mistake, and the words are + // right here. The `choices` check usually reports this first, with the same list; this + // is what a caller sees who converts one by hand. + let expected = words + .iter() + .map(|w| w.as_str()) + .collect::<::std::vec::Vec<_>>() + .join(", "); + + quote! { + impl ::usage_argv::spec::ValueEnum for #ident { + const CHOICES: &'static [&'static str] = &[#(#words),*]; + } + + impl ::std::str::FromStr for #ident { + type Err = ::std::string::String; + + fn from_str(value: &str) -> ::std::result::Result { + match value { + #(#arms)* + other => ::std::result::Result::Err(::std::format!( + "`{other}` is not one of: {}", + #expected + )), + } + } + } + } +} + #[cfg(test)] mod in_module_tests { use super::in_module; diff --git a/derive/src/lib.rs b/derive/src/lib.rs index c451cbf0a..aa704709d 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -157,6 +157,7 @@ //! | `help_heading = "x"` | the section to list this under in help output | //! | `hide` | keep it out of help and completions | //! | `double_dash = "required"` | a positional only fillable after `--` | +//! | `value_enum` | the words come from the field's type, which derives [`ValueEnum`] | //! | `arg` | force a field to be positional | //! | `overrides = "--other"` | a flag this one displaces, the last given winning | //! | `conflicts = "--other"` | a flag this one cannot be given with | @@ -247,3 +248,34 @@ pub fn derive_subcommands(input: TokenStream) -> TokenStream { Err(e) => e.to_compile_error().into(), } } + +/// Compile an enum into the words one value may be. +/// +/// What a CLI calls an enum — `--shell bash` — and what the spec calls `choices`. The +/// variant's name in kebab-case is the word, unless `name` says otherwise: +/// +/// ```ignore +/// #[derive(usage::ValueEnum)] +/// enum Shell { +/// Bash, +/// Zsh, +/// #[usage(name = "pwsh")] +/// PowerShell, +/// } +/// ``` +/// +/// A variant cannot be `cfg`-ed out: the words are a `const` list, and a list with holes +/// in it would either offer a word nothing answers to or name a variant that is not there. +/// `cfg` the whole enum instead. +/// +/// A field holding one says `value_enum`, which is what puts the words in the spec — so +/// help, completions and the check that rejects a wrong word all read the same list, and +/// none of them can drift from the type. +#[proc_macro_derive(ValueEnum, attributes(usage))] +pub fn derive_value_enum(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); + match model::ValueEnum::from_input(&input) { + Ok(value_enum) => codegen::emit_value_enum(&value_enum).into(), + Err(e) => e.to_compile_error().into(), + } +} diff --git a/derive/src/model.rs b/derive/src/model.rs index e10941473..09e83b3fc 100644 --- a/derive/src/model.rs +++ b/derive/src/model.rs @@ -48,6 +48,11 @@ pub struct Field { pub value_ty: Option, /// Written as `Option>`, so "never given" and "given nothing" differ. pub optional_collection: bool, + /// Whether the words come from the type, via [`ValueEnum`]. + /// + /// The alternative is `choices("a", "b")` written on the field, which is the same list + /// kept in a second place. Both end up in the spec identically. + pub value_enum: bool, pub help: Option, pub long_help: Option, pub env: Option, @@ -432,6 +437,7 @@ impl Field { default: None, help_heading: None, choices: Vec::new(), + value_enum: false, var_min: None, var_max: None, overrides: Vec::new(), @@ -475,6 +481,7 @@ impl Field { let mut hide = false; let mut is_arg = false; let mut choices: Vec = Vec::new(); + let mut value_enum = false; let mut var_min: Option = None; let mut var_max: Option = None; let mut overrides: Vec = Vec::new(); @@ -547,6 +554,7 @@ impl Field { "conflicts" => conflicts = selectors(&meta)?, "required_if" => required_if = selectors(&meta)?, "required_unless" => required_unless = selectors(&meta)?, + "value_enum" => value_enum = flag_value(&meta)?, "var_min" => var_min = Some(int_value(&meta)?), "var_max" => var_max = Some(int_value(&meta)?), "default" => default = Some(string_value(&meta)?), @@ -573,8 +581,8 @@ impl Field { "unknown option `{other}`; a field takes `name`, `long`, \ `short`, `negate`, `global`, `var`, `variadic`, \ `count`, `hide`, `arg`, `env`, `default`, `choices`, \ - `var_min`, `var_max`, `overrides`, `conflicts`, \ - `required_if`, \ + `var_min`, `var_max`, `value_enum`, `overrides`, \ + `conflicts`, `required_if`, \ `required_unless`, `help_heading`, and `double_dash`" ), )); @@ -697,6 +705,20 @@ impl Field { _ => {} } } + if value_enum && !choices.is_empty() { + return Err(syn::Error::new( + span, + "`value_enum` takes the words from the type and `choices` lists them here, \ + so a field says one or the other — two lists is one too many to keep in \ + step", + )); + } + if value_enum && matches!(shape, Shape::Bool | Shape::Count) { + return Err(syn::Error::new( + span, + "`value_enum` describes what a value may be, and this field takes no value", + )); + } if !choices.is_empty() && matches!(shape, Shape::Bool | Shape::Count) { return Err(syn::Error::new( span, @@ -868,6 +890,7 @@ impl Field { default, help_heading, choices, + value_enum, var_min, var_max, overrides, @@ -1407,9 +1430,104 @@ impl Variant { } } +/// An enum whose variants are the words a value may be. +pub struct ValueEnum { + pub ident: syn::Ident, + /// Each variant, and the word it answers to. + pub variants: Vec<(syn::Ident, String)>, +} + +impl ValueEnum { + pub fn from_input(input: &DeriveInput) -> syn::Result { + let Data::Enum(data) = &input.data else { + return Err(syn::Error::new_spanned( + &input.ident, + "usage::ValueEnum describes the words one value may be, so it needs an enum", + )); + }; + if !input.generics.params.is_empty() { + return Err(syn::Error::new_spanned( + &input.generics, + "usage::ValueEnum does not support generic parameters: the word list is a \ + `const`", + )); + } + + let mut variants: Vec<(syn::Ident, String)> = Vec::new(); + for variant in &data.variants { + if !matches!(variant.fields, Fields::Unit) { + return Err(syn::Error::new_spanned( + &variant.fields, + "a value is one word, so each variant is a bare name: a variant holding \ + fields would have nothing to build them from", + )); + } + // A variant that may not exist cannot be listed. `CHOICES` is a `const` array and + // an array literal takes no attributes on its elements, so a `cfg`-ed-out + // variant would either leave a word in the list that nothing answers to, or an + // arm referring to a variant that is not there. Refused rather than + // miscompiled; `cfg` the whole enum, or keep the words and map them yourself. + if let Some(cfg) = variant + .attrs + .iter() + .find(|a| a.path().is_ident("cfg") || a.path().is_ident("cfg_attr")) + { + return Err(syn::Error::new_spanned( + cfg, + "a value's variants are a `const` list of words, which cannot have holes \ + in it: `cfg` the whole enum instead", + )); + } + let mut name = to_kebab(&variant.ident.to_string()); + for attr in attrs(&variant.attrs) { + for meta in nested(attr)? { + let path = meta.path().clone(); + match ident_of(&path).as_str() { + "name" => name = string_value(&meta)?, + other => { + return Err(syn::Error::new_spanned( + path, + format!( + "unknown option `{other}` on a value; a variant takes \ + `name` here" + ), + )); + } + } + } + } + if name.is_empty() { + return Err(syn::Error::new_spanned( + &variant.ident, + "a value with no name would answer to nothing", + )); + } + if let Some((first, _)) = variants.iter().find(|(_, n)| *n == name) { + return Err(dup( + variant.ident.span(), + first.span(), + &format!("`{name}` names two of these values"), + )); + } + variants.push((variant.ident.clone(), name)); + } + if variants.is_empty() { + return Err(syn::Error::new_spanned( + &input.ident, + "an enum with no variants accepts no value at all", + )); + } + + Ok(ValueEnum { + ident: input.ident.clone(), + variants, + }) + } +} + #[cfg(test)] mod tests { - use super::{Cli, Subcommands}; + use super::{Cli, Subcommands, ValueEnum}; fn cli(body: &str) -> syn::Result { Cli::from_input(&syn::parse_str::(body).expect("valid Rust")) @@ -1515,6 +1633,103 @@ mod tests { } } + fn value_enum(body: &str) -> syn::Result { + ValueEnum::from_input(&syn::parse_str::(body).expect("valid Rust")) + } + + #[test] + fn a_value_enum_takes_bare_variants_with_distinct_words() { + let ve = value_enum( + r#" + enum Shell { + Bash, + #[usage(name = "pwsh")] + PowerShell, + } + "#, + ) + .expect("should compile"); + assert_eq!( + ve.variants + .iter() + .map(|(_, w)| w.as_str()) + .collect::>(), + ["bash", "pwsh"] + ); + + // A variant holding fields has nothing to build them from: a value is one word. + let err = match value_enum("enum Shell { Bash, Other(String) }") { + Ok(_) => panic!("should not have compiled"), + Err(e) => e.to_string(), + }; + assert!(err.contains("bare name"), "unhelpful message: {err}"); + + // Two variants answering to one word means one is unreachable. + let err = match value_enum( + r#" + enum Shell { + #[usage(name = "sh")] + Bash, + #[usage(name = "sh")] + Dash, + } + "#, + ) { + Ok(_) => panic!("should not have compiled"), + Err(e) => e.to_string(), + }; + assert!( + err.contains("names two of these values"), + "unhelpful: {err}" + ); + } + + #[test] + fn a_conditional_value_is_refused_rather_than_miscompiled() { + // The word list is a `const` array, so a variant that may not exist would leave + // either a word nothing answers to or an arm naming a variant that is not there. + let err = match value_enum( + r#" + enum Shell { + Bash, + #[cfg(windows)] + PowerShell, + } + "#, + ) { + Ok(_) => panic!("should not have compiled"), + Err(e) => e.to_string(), + }; + assert!( + err.contains("cannot have holes"), + "unhelpful message: {err}" + ); + } + + #[test] + fn value_enum_and_choices_are_the_same_list_twice() { + let err = rejection( + r#" + struct Ex { + #[usage(long, value_enum, choices("a", "b"))] + shell: Option, + } + "#, + ); + assert!(err.contains("one or the other"), "unhelpful message: {err}"); + + // And a switch has no value for a word to be. + let err = rejection( + r#" + struct Ex { + #[usage(long, value_enum)] + force: bool, + } + "#, + ); + assert!(err.contains("takes no value"), "unhelpful message: {err}"); + } + #[test] fn an_alias_cannot_name_a_sibling() { // The parser takes the first table entry that matches, so a name claimed twice