From 5c13ea3571dbe02a992a85f5a16819aaa9577d98 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Mon, 4 Mar 2024 17:06:50 -0800 Subject: [PATCH] Support None option values. The python options parser allows default=None, so we must support it in the rust parser. A None value can currently only be obtained via a default of None: if a default is not None then the option will have that value if it doesn't take a non-None value from some other source. We don't currently support setting a value to None in args/env/config in Python, so we don't support it here either. However the data structures could support this if we want to in the future. --- src/rust/engine/options/src/lib.rs | 78 +++++++++++++++++++++++++----- 1 file changed, 67 insertions(+), 11 deletions(-) diff --git a/src/rust/engine/options/src/lib.rs b/src/rust/engine/options/src/lib.rs index fbdaae8ebf3..df65ae55199 100644 --- a/src/rust/engine/options/src/lib.rs +++ b/src/rust/engine/options/src/lib.rs @@ -200,6 +200,23 @@ pub struct OptionValue { pub value: T, } +#[derive(Debug)] +pub struct OptionalOptionValue { + pub derivation: Option>, + pub source: Source, + pub value: Option, +} + +impl OptionalOptionValue { + fn unwrap(self) -> OptionValue { + OptionValue { + derivation: self.derivation, + source: self.source, + value: self.value.unwrap(), + } + } +} + #[derive(Debug)] pub struct ListOptionValue { pub derivation: Option>)>>, @@ -357,12 +374,15 @@ impl OptionParser { fn parse_scalar( &self, id: &OptionId, - default: &T, + default: Option<&T>, getter: fn(&Rc, &OptionId) -> Result, String>, - ) -> Result, String> { + ) -> Result, String> { let mut derivation = None; if self.include_derivation { - let mut derivations = vec![(Source::Default, default.to_owned())]; + let mut derivations = vec![]; + if let Some(def) = default { + derivations.push((Source::Default, def.to_owned())); + } for (source_type, source) in self.sources.iter() { if let Some(val) = getter(source, id)? { derivations.push((source_type.clone(), val)); @@ -372,30 +392,65 @@ impl OptionParser { } for (source_type, source) in self.sources.iter().rev() { if let Some(value) = getter(source, id)? { - return Ok(OptionValue { + return Ok(OptionalOptionValue { derivation, source: source_type.clone(), - value, + value: Some(value), }); } } - Ok(OptionValue { + Ok(OptionalOptionValue { derivation, source: Source::Default, - value: default.to_owned(), + value: default.map(|x| x.to_owned()), }) } + pub fn parse_bool_optional( + &self, + id: &OptionId, + default: Option, + ) -> Result, String> { + self.parse_scalar(id, default.as_ref(), |source, id| source.get_bool(id)) + } + + pub fn parse_int_optional( + &self, + id: &OptionId, + default: Option, + ) -> Result, String> { + self.parse_scalar(id, default.as_ref(), |source, id| source.get_int(id)) + } + + pub fn parse_float_optional( + &self, + id: &OptionId, + default: Option, + ) -> Result, String> { + self.parse_scalar(id, default.as_ref(), |source, id| source.get_float(id)) + } + + pub fn parse_string_optional( + &self, + id: &OptionId, + default: Option<&str>, + ) -> Result, String> { + self.parse_scalar(id, default, |source, id| source.get_string(id)) + } + pub fn parse_bool(&self, id: &OptionId, default: bool) -> Result, String> { - self.parse_scalar(id, &default, |source, id| source.get_bool(id)) + self.parse_bool_optional(id, Some(default)) + .map(OptionalOptionValue::unwrap) } pub fn parse_int(&self, id: &OptionId, default: i64) -> Result, String> { - self.parse_scalar(id, &default, |source, id| source.get_int(id)) + self.parse_int_optional(id, Some(default)) + .map(OptionalOptionValue::unwrap) } pub fn parse_float(&self, id: &OptionId, default: f64) -> Result, String> { - self.parse_scalar(id, &default, |source, id| source.get_float(id)) + self.parse_float_optional(id, Some(default)) + .map(OptionalOptionValue::unwrap) } pub fn parse_string( @@ -403,7 +458,8 @@ impl OptionParser { id: &OptionId, default: &str, ) -> Result, String> { - self.parse_scalar(id, default, |source, id| source.get_string(id)) + self.parse_string_optional(id, Some(default)) + .map(OptionalOptionValue::unwrap) } #[allow(clippy::type_complexity)]