Merge pull request #133 from BenWiederhake/dev-derive-panic-no-unwrap

derive: make error messages slightly more readable, add manually-driven test

derive/src/argument.rs

@@ -40,7 +40,8 @@ pub fn parse_arguments_attr(attrs: &[Attribute]) -> ArgumentsAttr {
 
 pub fn parse_argument(v: Variant) -> Vec<Argument> {
     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() {

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:

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),

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()]

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<Arg> 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(())
+}