From b2e78d3f58c872f6a6bd75312c3026ef69738dd6 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 13 Apr 2025 20:53:39 +0200 Subject: [PATCH] derive: make error messages slightly more readable, add manually-driven test --- derive/src/argument.rs | 3 +- derive/src/flags.rs | 11 ++-- derive/src/help.rs | 8 ++- derive/src/lib.rs | 3 +- examples/test_compile_errors_manually.rs | 84 ++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 10 deletions(-) create mode 100644 examples/test_compile_errors_manually.rs diff --git a/derive/src/argument.rs b/derive/src/argument.rs index 94b84eb..7a09b2b 100644 --- a/derive/src/argument.rs +++ b/derive/src/argument.rs @@ -40,7 +40,8 @@ pub fn parse_arguments_attr(attrs: &[Attribute]) -> ArgumentsAttr { pub fn parse_argument(v: Variant) -> Vec { let ident = v.ident; - let attributes = get_arg_attributes(&v.attrs).unwrap(); + let attributes = get_arg_attributes(&v.attrs) + .expect("can't parse arg attributes, expected one or more strings"); // Return early because we don't need to check the fields if it's not used. if attributes.is_empty() { diff --git a/derive/src/flags.rs b/derive/src/flags.rs index 3d3db27..da7497e 100644 --- a/derive/src/flags.rs +++ b/derive/src/flags.rs @@ -60,8 +60,9 @@ impl Flags { } else if sep == '[' { let optional = val .strip_prefix('=') - .and_then(|s| s.strip_suffix(']')) - .unwrap(); + .expect("expected '=' after '[' in flag pattern") + .strip_suffix(']') + .expect("expected final ']' in flag pattern"); assert!( optional .chars() @@ -74,8 +75,6 @@ impl Flags { self.long.push(Flag { flag: f, value }); } else if let Some(s) = flag.strip_prefix('-') { - assert!(!s.is_empty()); - // There are three possible patterns: // -f // -f value @@ -83,7 +82,9 @@ impl Flags { // First we trim up to the = or [ let mut chars = s.chars(); - let f = chars.next().unwrap(); + let f = chars + .next() + .expect("flag name must be non-empty (cannot be just '-')"); let val: String = chars.collect(); // Now check the cases: diff --git a/derive/src/help.rs b/derive/src/help.rs index 77af149..2c99c3a 100644 --- a/derive/src/help.rs +++ b/derive/src/help.rs @@ -100,12 +100,14 @@ pub fn help_string( pub fn read_help_file(file: &str) -> (String, String, String) { let path = Path::new(file); - let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); + let manifest_dir = + std::env::var("CARGO_MANIFEST_DIR").expect("can only run in paths that are valid UTF-8"); let mut location = PathBuf::from(manifest_dir); location.push(path); let mut contents = String::new(); - let mut f = std::fs::File::open(location).unwrap(); - f.read_to_string(&mut contents).unwrap(); + let mut f = std::fs::File::open(location).expect("cannot open help-string file"); + f.read_to_string(&mut contents) + .expect("cannot read from help-string file"); ( parse_about(&contents), diff --git a/derive/src/lib.rs b/derive/src/lib.rs index 5f6d9d7..cf4fa15 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -152,7 +152,8 @@ pub fn value(input: TokenStream) -> TokenStream { continue; } - let ValueAttr { keys, value } = ValueAttr::parse(&attr).unwrap(); + let ValueAttr { keys, value } = + ValueAttr::parse(&attr).expect("expected comma-separated list of string literals"); let keys = if keys.is_empty() { vec![variant_name.to_lowercase()] diff --git a/examples/test_compile_errors_manually.rs b/examples/test_compile_errors_manually.rs new file mode 100644 index 0000000..7d08f10 --- /dev/null +++ b/examples/test_compile_errors_manually.rs @@ -0,0 +1,84 @@ +use uutils_args::{Arguments, Options, Value}; + +// Using a fully-fledged compile-error testsuite is a bit overkill, but we still +// want to make sure that the `derive` crate generates reasonable error messages. +// That's what this "example" is for. In the following, there are blocks of +// lines, one marked as POSITIVE and multiple lines marked as NEGATIVE. The +// committed version of this file should only contain POSITIVE. In order to run a +// test, comment out the POSITIVE line, and use a NEGATIVE line instead, and +// manually check whether you see a reasonable error message – ideally the error +// message indicated by the comment. One way to do this is: + +// $ cargo build --example test_compile_errors_manually + +#[derive(Value, Debug, Default)] +enum Flavor { + #[default] + #[value("kind", "nice")] + Kind, + #[value("condescending")] // POSITIVE + // #[value(condescending)] // NEGATIVE: "expected comma-separated list of string literals" + Condescending, +} + +#[derive(Arguments)] +#[arguments(file = "examples/hello_world_help.md")] // POSITIVE +// #[arguments(file = "examples/nonexistent.md")] // NEGATIVE: "cannot open help-string file" +// #[arguments(file = "/dev/full")] // NEGATIVE: Causes OOM, FIXME +// #[arguments(file = "/")] // NEGATIVE: "cannot read from help-string file" +// #[arguments(file = "path/to/some/WRITE-ONLY/file")] // NEGATIVE: "cannot open help-string file" +enum Arg { + /// The name to greet + #[arg("-n NAME", "--name[=NAME]", "name=NAME")] // POSITIVE + // #[arg("-")] // NEGATIVE: flag name must be non-empty (cannot be just '-') + // #[arg("-n NAME", "--name[NAME]", "name=NAME")] // NEGATIVE: "expected '=' after '[' in flag pattern" + // #[arg("-n NAME", "--name[=NAME", "name=NAME")] // NEGATIVE: "expected final ']' in flag pattern" + // #[arg(key="name")] // NEGATIVE: "can't parse arg attributes, expected one or more strings" + Name(String), + + /// The number of times to greet + #[arg("-c N", "--count=N")] + Count(u8), + + #[arg("--flavor=FLAVOR")] + Flavor(Flavor), +} + +struct Settings { + name: String, + count: u8, + flavor: Flavor, +} + +impl Options for Settings { + fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> { + match arg { + Arg::Name(n) => self.name = n, + Arg::Count(c) => self.count = c, + Arg::Flavor(flavor) => self.flavor = flavor, + } + Ok(()) + } +} + +fn main() -> Result<(), uutils_args::Error> { + let (settings, _operands) = Settings { + name: String::new(), + count: 1, + flavor: Flavor::Kind, + } + .parse(std::env::args_os()) + .unwrap(); + + for _ in 0..settings.count { + match settings.flavor { + Flavor::Kind => { + println!("Hello, {}!", settings.name); + } + Flavor::Condescending => { + println!("Ugh, {}.", settings.name); + } + } + } + Ok(()) +}