Skip to content

Commit d406cb3

Browse files
committed
make get_program_options no-fail
an error here means the program options are not well-formed, but that's generally not recoverable and represents a logic error that should be caught be `conf(test)` on the main structure
1 parent 92dd4d6 commit d406cb3

File tree

14 files changed

+45
-62
lines changed

14 files changed

+45
-62
lines changed

conf_derive/src/proc_macro_options/field_item/flatten_item.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -323,11 +323,14 @@ impl FlattenItem {
323323
// Check for positional args in flatten optional
324324
for opt in __inner_options__ {
325325
if opt.is_positional {
326-
return Err(::conf::Error::positional_in_flatten_optional(
327-
#field_name,
328-
<#inner_type as ::conf::Conf>::get_name(),
329-
&opt.id
330-
));
326+
panic!(
327+
"{}",
328+
::conf::Error::positional_in_flatten_optional(
329+
#field_name,
330+
<#inner_type as ::conf::Conf>::get_name(),
331+
&opt.id
332+
)
333+
);
331334
}
332335
}
333336
}
@@ -337,7 +340,7 @@ impl FlattenItem {
337340

338341
let push_expr = quote! {
339342
let mut #was_skipped_ident = [false; #skip_short_len];
340-
let __inner_options__ = <#inner_type as ::conf::Conf>::get_program_options()?;
343+
let __inner_options__ = <#inner_type as ::conf::Conf>::get_program_options();
341344
#positional_check
342345
#program_options_ident.extend(
343346
__inner_options__.iter().cloned().map(
@@ -354,7 +357,8 @@ impl FlattenItem {
354357
.filter_map(
355358
|(short_form, was_skipped)| if was_skipped { None } else { Some(short_form) }
356359
).collect();
357-
return Err(
360+
panic!(
361+
"{}",
358362
::conf::Error::skip_short_not_found(
359363
not_skipped,
360364
#field_name,

conf_derive/src/proc_macro_options/mod.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,24 +119,19 @@ impl GenConfStruct {
119119
.struct_item
120120
.gen_post_process_program_options(&program_options_ident)?;
121121

122-
// Note: fields_push_program_options is allowed to early return with ? on an error
123122
Ok(quote! {
124-
fn get_program_options() -> Result<&'static [::conf::ProgramOption], ::conf::Error> {
123+
fn get_program_options() -> &'static [::conf::ProgramOption] {
125124
static CACHED: ::std::sync::OnceLock<Vec<::conf::ProgramOption>> = ::std::sync::OnceLock::new();
126125

127-
if CACHED.get().is_none() {
126+
CACHED.get_or_init(|| {
128127
let mut #program_options_ident = vec![];
129128

130129
#(#fields_push_program_options)*
131130

132131
#struct_post_process_program_options
133132

134-
let _ = CACHED.set(#program_options_ident);
135-
}
136-
137-
let cached = CACHED.get().unwrap();
138-
139-
Ok(cached.as_ref())
133+
#program_options_ident
134+
}).as_ref()
140135
}
141136
})
142137
}

src/traits.rs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ pub trait Conf: Sized {
9090
#[doc(hidden)]
9191
fn get_parser(parsed_env: &ParsedEnv) -> Result<Parser<'_>, Error> {
9292
let parser_config = Self::get_parser_config()?;
93-
let program_options = Self::get_program_options()?;
93+
let program_options = Self::get_program_options();
9494
let subcommands = Self::get_subcommands(parsed_env)?;
9595
Parser::new(parser_config, program_options, subcommands, parsed_env)
9696
}
@@ -108,7 +108,7 @@ pub trait Conf: Sized {
108108
// Users shouldn't generally call this, because the returned data is implementation details,
109109
// and may change without a semver breaking change to the crate version.
110110
#[doc(hidden)]
111-
fn get_program_options() -> Result<&'static [ProgramOption], Error>;
111+
fn get_program_options() -> &'static [ProgramOption];
112112
// Get the subcommands that are declared on this Conf.
113113
//
114114
// These come from `conf(subcommand)` being used on a field, and `derive(Subcommand)` being used
@@ -135,16 +135,7 @@ pub trait Conf: Sized {
135135
fn any_program_options_appeared<'a>(
136136
conf_context: &ConfContext<'a>,
137137
) -> Result<Option<(&'a str, ConfValueSource<&'a str>)>, InnerError> {
138-
// This unwrap is unfortunate but this code is only called when an earlier call to
139-
// Self::get_program_options has succeeded, since we have to call that to
140-
// instantiate the parser, and we have to do that before getting a ConfContext.
141-
// The only place in the library where a `ConfContext` is created where one doesn't already
142-
// exist is in `try_parse_from`, and the ConfContext::new function is pub(crate).
143-
// And we have to call get_program_options before that point, which calls it
144-
// recursively on all the constituent structures.
145-
// So I don't think this unwrap will panic unless get_program_options is implemented in a
146-
// non-deterministic way, which it shouldn't be.
147-
let program_options = Self::get_program_options().unwrap();
138+
let program_options = Self::get_program_options();
148139
for opt in program_options {
149140
if let Some(value_source) = conf_context.option_appears(&opt.id)? {
150141
return Ok(Some((&opt.id, value_source)));

tests/test_env_aliases.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ struct TestEnvAliases {
1414

1515
#[test]
1616
fn test_env_aliases_get_program_options() {
17-
let opts = TestEnvAliases::get_program_options().unwrap();
17+
let opts = TestEnvAliases::get_program_options();
1818

1919
let mut iter = opts.iter();
2020
let opt = iter.next().unwrap();
@@ -96,7 +96,7 @@ struct TestEnvAliases2 {
9696

9797
#[test]
9898
fn test_env_aliases2_get_program_options() {
99-
let opts = TestEnvAliases2::get_program_options().unwrap();
99+
let opts = TestEnvAliases2::get_program_options();
100100

101101
let mut iter = opts.iter();
102102
let opt = iter.next().unwrap();

tests/test_env_only_param.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ struct TestEnvOnly {
1919
#[test]
2020
fn test_env_only_get_program_options() {
2121
let parser_config = TestEnvOnly::get_parser_config().unwrap();
22-
let opts = TestEnvOnly::get_program_options().unwrap();
22+
let opts = TestEnvOnly::get_program_options();
2323

2424
assert!(!parser_config.no_help_flag);
2525

tests/test_flags.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ struct TestFlags {
1717
#[test]
1818
fn test_flags_get_program_options() {
1919
let parser_config = TestFlags::get_parser_config().unwrap();
20-
let opts = TestFlags::get_program_options().unwrap();
20+
let opts = TestFlags::get_program_options();
2121

2222
assert!(!parser_config.no_help_flag);
2323
assert!(parser_config.about.is_none());

tests/test_flatten.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ struct TestClientConfig {
3030
#[test]
3131
fn test_client_config_program_options() {
3232
let parser_config = TestClientConfig::get_parser_config().unwrap();
33-
let opts = TestClientConfig::get_program_options().unwrap();
33+
let opts = TestClientConfig::get_program_options();
3434

3535
assert!(!parser_config.no_help_flag);
3636
assert!(parser_config.about.is_none());
@@ -135,7 +135,7 @@ struct TestDbToolConfig {
135135
#[test]
136136
fn test_db_tool_config_program_options() {
137137
let parser_config = TestDbToolConfig::get_parser_config().unwrap();
138-
let opts = TestDbToolConfig::get_program_options().unwrap();
138+
let opts = TestDbToolConfig::get_program_options();
139139

140140
assert!(!parser_config.no_help_flag);
141141
assert!(parser_config.about.is_none());
@@ -265,7 +265,7 @@ struct TestServiceConfig {
265265
#[test]
266266
fn test_service_config_program_options() {
267267
let parser_config = TestServiceConfig::get_parser_config().unwrap();
268-
let opts = TestServiceConfig::get_program_options().unwrap();
268+
let opts = TestServiceConfig::get_program_options();
269269

270270
assert!(!parser_config.no_help_flag);
271271
assert!(parser_config.about.is_none());
@@ -512,7 +512,7 @@ struct FrobConfig {
512512
#[test]
513513
fn frob_config_program_options() {
514514
let parser_config = FrobConfig::get_parser_config().unwrap();
515-
let opts = FrobConfig::get_program_options().unwrap();
515+
let opts = FrobConfig::get_program_options();
516516

517517
assert!(!parser_config.no_help_flag);
518518
assert!(parser_config.about.is_none());
@@ -845,7 +845,7 @@ struct FrobConfig2 {
845845
#[test]
846846
fn frob_config2_program_options() {
847847
let parser_config = FrobConfig2::get_parser_config().unwrap();
848-
let opts = FrobConfig2::get_program_options().unwrap();
848+
let opts = FrobConfig2::get_program_options();
849849

850850
assert!(!parser_config.no_help_flag);
851851
assert!(parser_config.about.is_none());

tests/test_flatten_optional.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ struct TestFlattenOptional {
2828

2929
#[test]
3030
fn test_flatten_optional_get_program_options() {
31-
let opts = TestFlattenOptional::get_program_options().unwrap();
31+
let opts = TestFlattenOptional::get_program_options();
3232

3333
let mut iter = opts.iter();
3434

tests/test_help.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ struct SubsystemOptions {
2323
#[test]
2424
fn test_subsystem_options_help() {
2525
let parser_config = SubsystemOptions::get_parser_config().unwrap();
26-
let opts = SubsystemOptions::get_program_options().unwrap();
26+
let opts = SubsystemOptions::get_program_options();
2727

2828
let env = Default::default();
2929
let parser = Parser::new(parser_config, opts, &[], &env).unwrap();
@@ -66,7 +66,7 @@ struct OtherSubsystemOptions {
6666
#[test]
6767
fn test_other_subsystem_options_help() {
6868
let parser_config = OtherSubsystemOptions::get_parser_config().unwrap();
69-
let opts = OtherSubsystemOptions::get_program_options().unwrap();
69+
let opts = OtherSubsystemOptions::get_program_options();
7070

7171
let env = Default::default();
7272
let parser = Parser::new(parser_config, opts, &[], &env).unwrap();
@@ -128,7 +128,7 @@ struct SystemOptions {
128128
#[test]
129129
fn test_system_options_help() {
130130
let parser_config = SystemOptions::get_parser_config().unwrap();
131-
let opts = SystemOptions::get_program_options().unwrap();
131+
let opts = SystemOptions::get_program_options();
132132

133133
let env = Default::default();
134134
let parser = Parser::new(parser_config, opts, &[], &env).unwrap();

tests/test_params.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ struct TestParams {
2525
#[test]
2626
fn test_params_get_program_options() {
2727
let parser_config = TestParams::get_parser_config().unwrap();
28-
let opts = TestParams::get_program_options().unwrap();
28+
let opts = TestParams::get_program_options();
2929

3030
assert!(!parser_config.no_help_flag);
3131
assert!(parser_config.about.is_none());

0 commit comments

Comments
 (0)