Skip to content

Commit 70a8b80

Browse files
committed
fix some edge cases around serde-only fields
1 parent 3d91ea3 commit 70a8b80

File tree

4 files changed

+453
-2
lines changed

4 files changed

+453
-2
lines changed

conf_derive/src/proc_macro_options/field_item/flag_item.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,11 @@ impl FlagItem {
239239
self.serde.is_some() && !self.get_serde_skip()
240240
}
241241

242+
/// Returns true if this field has a CLI or env source (registered with clap)
243+
pub fn has_cli_or_env_source(&self) -> bool {
244+
self.short_switch.is_some() || self.long_switch.is_some() || self.env_name.is_some()
245+
}
246+
242247
pub fn gen_push_program_options(
243248
&self,
244249
program_options_ident: &Ident,
@@ -289,6 +294,18 @@ impl FlagItem {
289294
) -> Result<(TokenStream, bool), Error> {
290295
let id = self.field_name.to_string();
291296

297+
// If there's no CLI or env source, this flag wasn't registered with clap,
298+
// so we just return the default value (false)
299+
if !self.has_cli_or_env_source() {
300+
return Ok((
301+
quote! {
302+
let _ = #conf_context_ident;
303+
Ok(false)
304+
},
305+
false,
306+
));
307+
}
308+
292309
Ok((
293310
quote! {
294311
let (_src, val) = #conf_context_ident.get_boolean_opt(#id)?;
@@ -309,6 +326,34 @@ impl FlagItem {
309326

310327
let try_from = self.get_serde_try_from();
311328

329+
// If there's no CLI or env source, this flag wasn't registered with clap,
330+
// so we just use the serde value directly
331+
if !self.has_cli_or_env_source() {
332+
let _ = id; // suppress unused warning
333+
if let Some(_try_from_type) = try_from {
334+
return Ok((
335+
quote! {
336+
let _ = #conf_context_ident;
337+
<bool as ::core::convert::TryFrom<_>>::try_from(#doc_val)
338+
.map_err(|err| ::conf::InnerError::serde(
339+
#doc_name,
340+
#field_name_str,
341+
err
342+
))
343+
},
344+
false,
345+
));
346+
} else {
347+
return Ok((
348+
quote! {
349+
let _ = #conf_context_ident;
350+
Ok(#doc_val)
351+
},
352+
false,
353+
));
354+
}
355+
}
356+
312357
if let Some(_try_from_type) = try_from {
313358
// When try_from is set, #doc_val has type #try_from_type.
314359
// To pick this value for the field, we use TryFrom::try_from to convert.

conf_derive/src/proc_macro_options/field_item/repeat_item.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,14 @@ impl RepeatItem {
383383
self.serde.is_some() && !self.get_serde_skip()
384384
}
385385

386+
/// Returns true if this field has a CLI or env source (registered with clap)
387+
pub fn has_cli_or_env_source(&self) -> bool {
388+
self.short_switch.is_some()
389+
|| self.long_switch.is_some()
390+
|| self.env_name.is_some()
391+
|| self.is_positional
392+
}
393+
386394
/// Generate a routine that pushes a ::conf::ProgramOption corresponding to
387395
/// this field, onto a mut Vec<ProgramOption> that is in scope.
388396
///
@@ -627,6 +635,22 @@ impl RepeatItem {
627635
&self,
628636
conf_context_ident: &Ident,
629637
) -> Result<(TokenStream, bool), syn::Error> {
638+
// If there's no CLI or env source, this repeat wasn't registered with clap,
639+
// so we just return the default value (empty Vec)
640+
if !self.has_cli_or_env_source() {
641+
let field_type = &self.field_type;
642+
return Ok((
643+
quote! {
644+
{
645+
let _ = #conf_context_ident;
646+
let result: #field_type = Default::default();
647+
Ok(result)
648+
}
649+
},
650+
false,
651+
));
652+
}
653+
630654
self.gen_initializer_helper(conf_context_ident, None)
631655
}
632656

@@ -641,6 +665,52 @@ impl RepeatItem {
641665
doc_name: &Ident,
642666
doc_val: &Ident,
643667
) -> Result<(TokenStream, bool), Error> {
668+
// If there's no CLI or env source, this repeat wasn't registered with clap,
669+
// so we just use the serde value directly
670+
if !self.has_cli_or_env_source() {
671+
let try_from = self.get_serde_try_from();
672+
673+
if let Some(_try_from_type) = try_from {
674+
// Convert each element via TryFrom
675+
let field_name_str = self.field_name.to_string();
676+
let inner_type = type_is_vec(&self.field_type)?
677+
.ok_or_else(|| Error::new(self.field_type.span(), "Expected Vec<T> type"))?;
678+
679+
return Ok((
680+
quote! {
681+
{
682+
let _ = #conf_context_ident;
683+
let mut __result__ = Vec::with_capacity(#doc_val.len());
684+
for __item__ in #doc_val {
685+
match <#inner_type as ::core::convert::TryFrom<_>>::try_from(__item__) {
686+
Ok(__converted__) => __result__.push(__converted__),
687+
Err(__err__) => {
688+
return Err(vec![::conf::InnerError::serde(
689+
#doc_name,
690+
#field_name_str,
691+
__err__
692+
)]);
693+
}
694+
}
695+
}
696+
Ok(__result__)
697+
}
698+
},
699+
false,
700+
));
701+
} else {
702+
return Ok((
703+
quote! {
704+
{
705+
let _ = #conf_context_ident;
706+
Ok(#doc_val)
707+
}
708+
},
709+
false,
710+
));
711+
}
712+
}
713+
644714
let use_value_parser = self
645715
.serde
646716
.as_ref()

src/parser.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,9 +335,12 @@ impl<'a> Parser<'a> {
335335
let mut buf = String::new();
336336
option.print(&mut buf, Some(env))?;
337337
Ok(MaybeArg::EnvOnly(buf))
338-
} else if option.default_value.is_some() || option.has_serde_source {
338+
} else if option.default_value.is_some()
339+
|| option.has_serde_source
340+
|| !option.is_required
341+
{
339342
// This option is not visible in CLI help since it can only come from
340-
// default values or serde deserialization
343+
// default values, serde deserialization, or is optional (defaults to None)
341344
Ok(MaybeArg::NotArgsOrEnv)
342345
} else {
343346
panic!(

0 commit comments

Comments
 (0)