Skip to content

Commit 9919060

Browse files
committed
derive: make error messages slightly more readable, add manually-driven test
1 parent 3e7b411 commit 9919060

File tree

5 files changed

+89
-10
lines changed

5 files changed

+89
-10
lines changed

derive/src/argument.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ pub fn parse_arguments_attr(attrs: &[Attribute]) -> ArgumentsAttr {
4040

4141
pub fn parse_argument(v: Variant) -> Vec<Argument> {
4242
let ident = v.ident;
43-
let attributes = get_arg_attributes(&v.attrs).unwrap();
43+
let attributes = get_arg_attributes(&v.attrs).expect("can't parse arg attributes, expected one or more strings");
4444

4545
// Return early because we don't need to check the fields if it's not used.
4646
if attributes.is_empty() {

derive/src/flags.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ impl Flags {
6060
} else if sep == '[' {
6161
let optional = val
6262
.strip_prefix('=')
63-
.and_then(|s| s.strip_suffix(']'))
64-
.unwrap();
63+
.expect("expected '=' after '[' in flag pattern")
64+
.strip_suffix(']')
65+
.expect("expected final ']' in flag pattern");
6566
assert!(
6667
optional
6768
.chars()
@@ -74,16 +75,14 @@ impl Flags {
7475

7576
self.long.push(Flag { flag: f, value });
7677
} else if let Some(s) = flag.strip_prefix('-') {
77-
assert!(!s.is_empty());
78-
7978
// There are three possible patterns:
8079
// -f
8180
// -f value
8281
// -f[value]
8382

8483
// First we trim up to the = or [
8584
let mut chars = s.chars();
86-
let f = chars.next().unwrap();
85+
let f = chars.next().expect("flag name must be non-empty (cannot be just '-')");
8786
let val: String = chars.collect();
8887

8988
// Now check the cases:

derive/src/help.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,12 @@ pub fn help_string(
100100

101101
pub fn read_help_file(file: &str) -> (String, String, String) {
102102
let path = Path::new(file);
103-
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap();
103+
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").expect("can only run in paths that are valid UTF-8");
104104
let mut location = PathBuf::from(manifest_dir);
105105
location.push(path);
106106
let mut contents = String::new();
107-
let mut f = std::fs::File::open(location).unwrap();
108-
f.read_to_string(&mut contents).unwrap();
107+
let mut f = std::fs::File::open(location).expect("cannot open help-string file");
108+
f.read_to_string(&mut contents).expect("cannot read from help-string file");
109109

110110
(
111111
parse_about(&contents),

derive/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ pub fn value(input: TokenStream) -> TokenStream {
152152
continue;
153153
}
154154

155-
let ValueAttr { keys, value } = ValueAttr::parse(&attr).unwrap();
155+
let ValueAttr { keys, value } = ValueAttr::parse(&attr).expect("expected comma-separated list of string literals");
156156

157157
let keys = if keys.is_empty() {
158158
vec![variant_name.to_lowercase()]
+80
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
use uutils_args::{Arguments, Options, Value};
2+
3+
// Using a fully-fledged compile-error testsuite is a bit overkill, but we still
4+
// want to make sure that the `derive` crate generates reasonable error messages.
5+
// That's what this "example" is for. In the following, there are blocks of
6+
// lines, one marked as POSITIVE and multiple lines marked as NEGATIVE. The
7+
// committed version of this file should only contain POSITIVE. In order to run a
8+
// test, comment out the POSITIVE line, and use a NEGATIVE line instead, and
9+
// manually check whether you see a reasonable error message – ideally the error
10+
// message indicated by the comment. One way to do this is:
11+
12+
// $ cargo build --example test_compile_errors_manually
13+
14+
#[derive(Value, Debug, Default)]
15+
enum Flavor {
16+
#[default]
17+
#[value("kind", "nice")]
18+
Kind,
19+
#[value("condescending")] // POSITIVE
20+
// #[value(condescending)] // NEGATIVE: "expected comma-separated list of string literals"
21+
Condescending,
22+
}
23+
24+
#[derive(Arguments)]
25+
#[arguments(file = "examples/hello_world_help.md")] // POSITIVE
26+
// #[arguments(file = "examples/nonexistent.md")] // NEGATIVE: "cannot open help-string file"
27+
// #[arguments(file = "/dev/full")] // NEGATIVE: Causes OOM, FIXME
28+
// #[arguments(file = "/")] // NEGATIVE: "cannot read from help-string file"
29+
// #[arguments(file = "path/to/some/WRITE-ONLY/file")] // NEGATIVE: "cannot open help-string file"
30+
enum Arg {
31+
/// The name to greet
32+
#[arg("-n NAME", "--name[=NAME]", "name=NAME")] // POSITIVE
33+
// #[arg("-")] // NEGATIVE: flag name must be non-empty (cannot be just '-')
34+
// #[arg("-n NAME", "--name[NAME]", "name=NAME")] // NEGATIVE: "expected '=' after '[' in flag pattern"
35+
// #[arg("-n NAME", "--name[=NAME", "name=NAME")] // NEGATIVE: "expected final ']' in flag pattern"
36+
// #[arg(key="name")] // NEGATIVE: "can't parse arg attributes, expected one or more strings"
37+
Name(String),
38+
39+
/// The number of times to greet
40+
#[arg("-c N", "--count=N")]
41+
Count(u8),
42+
43+
#[arg("--flavor=FLAVOR")]
44+
Flavor(Flavor),
45+
}
46+
47+
struct Settings {
48+
name: String,
49+
count: u8,
50+
flavor: Flavor,
51+
}
52+
53+
impl Options<Arg> for Settings {
54+
fn apply(&mut self, arg: Arg) -> Result<(), uutils_args::Error> {
55+
match arg {
56+
Arg::Name(n) => self.name = n,
57+
Arg::Count(c) => self.count = c,
58+
Arg::Flavor(flavor) => self.flavor = flavor,
59+
}
60+
Ok(())
61+
}
62+
}
63+
64+
fn main() -> Result<(), uutils_args::Error> {
65+
let (settings, _operands) = Settings {
66+
name: String::new(),
67+
count: 1,
68+
flavor: Flavor::Kind,
69+
}
70+
.parse(std::env::args_os())
71+
.unwrap();
72+
73+
for _ in 0..settings.count {
74+
match settings.flavor {
75+
Flavor::Kind => { println!("Hello, {}!", settings.name); }
76+
Flavor::Condescending => { println!("Ugh, {}.", settings.name); }
77+
}
78+
}
79+
Ok(())
80+
}

0 commit comments

Comments
 (0)