Introduce NestedReferenceNullAsNone compatibility flag

This compat flag changes reclass-rs's serialization of YAML `null` values in
nested references to emit string "None" instead of the new default of
string "null". Unless your inventory depends on this behavior we
strongly recommend not using the compatibility mode.

Author: simu

README-extensions.md

@@ -21,9 +21,17 @@ For example, given the following inventory, the node's internal path is parsed a
 ```
 
 However, optionally, reclass-rs can be configured to handle `compose_node_name` the same way that kapicorp-reclass does, by naively splitting node names on each dot.
-To enable this compatibility mode, set `compat_flags: ['ComposeNodeNameLiteralDots']` in your inventory's `reclass-config.yml`.
+To enable this compatibility mode, set `reclass_rs_compat_flags: ['ComposeNodeNameLiteralDots']` in your inventory's `reclass-config.yml`.
 In compatibility mode, the node's internal path for the previous inventory is `['path', 'to', 'the', 'node']`.
 
+## Handling of YAML `null` values in nested references
+
+By default, reclass-rs resolves YAML `null` values in nested references to the string `null`.
+This behavior ensures that references which resolve to YAML `null` are correctly preserved when using them as reference default values (see below).
+
+Optionally, reclass-rs can be configured to preserve the kapicorp-reclass behavior of resolving YAML `null` values in nested references to string "None".
+To enable this compatibility mode, set `reclass_rs_compat_flags: ['NestedReferenceNullAsNone']` in your inventory's `reclass-config.yml`.
+
 ## Verbose warnings
 
 Reclass-rs supports boolean config option `verbose_warnings`.

README.md

@@ -19,6 +19,7 @@ The implementation currently supports the following features of Kapicorp Reclass
 * Merging referenced lists and dictionaries
 * Constant parameters
 * Nested references
+  * By default reclass-rs uses a non-compatible mode which resolves YAML `null` values in nested references to string "null".
 * References in class names
 * Loading classes with relative names
 * Loading Reclass configuration options from `reclass-config.yaml`

python/reclass_rs/reclass_rs.pyi

@@ -15,6 +15,7 @@ __all__: list[str] = [
 @final
 class CompatFlag(Enum):
     ComposeNodeNameLiteralDots = "ComposeNodeNameLiteralDots"
+    NestedReferenceNullAsNone = "NestedReferenceNullAsNone"
 
 @final
 class Config:

src/config.rs

@@ -25,6 +25,13 @@ pub enum CompatFlag {
     /// file path when rendering fields `path` and `parts` in `NodeInfoMeta` when
     /// `compose-node-name` is enabled.
     ComposeNodeNameLiteralDots,
+    /// This flag enables Python Reclass-compatible rendering of nested references that resolve to
+    /// a YAML `null` value. When this flag is set, YAML `null` values encountered during nested
+    /// reference resolution will be serialized as `"None"`.
+    ///
+    /// By default, if this flag isn't enabled, reclass-rs will serialize YAML `null` values as
+    /// string `"null"` during nested reference resolution.
+    NestedReferenceNullAsNone,
 }
 
 #[pymethods]
@@ -43,6 +50,9 @@ impl TryFrom<&str> for CompatFlag {
             "compose-node-name-literal-dots"
             | "compose_node_name_literal_dots"
             | "ComposeNodeNameLiteralDots" => Ok(Self::ComposeNodeNameLiteralDots),
+            "nested-reference-null-as-none"
+            | "nested_reference_null_as_none"
+            | "NestedReferenceNullAsNone" => Ok(Self::NestedReferenceNullAsNone),
             _ => Err(anyhow!("Unknown compatibility flag '{value}'")),
         }
     }
@@ -171,6 +181,7 @@ pub struct RenderOpts {
     pub ignore_overwritten_missing_references: bool,
     pub verbose_warnings: bool,
     pub(crate) preserve_resolve_error_in_flattened: bool,
+    pub(crate) nested_reference_null_as_none: bool,
 }
 
 #[pyclass(from_py_object)]
@@ -490,6 +501,9 @@ impl From<&Config> for RenderOpts {
         Self {
             ignore_overwritten_missing_references: value.ignore_overwritten_missing_references,
             verbose_warnings: value.verbose_warnings,
+            nested_reference_null_as_none: value
+                .compatflags
+                .contains(&CompatFlag::NestedReferenceNullAsNone),
             ..Default::default()
         }
     }

src/node/mod.rs

@@ -234,7 +234,7 @@ impl Node {
                     let mut state = ResolveState::default();
                     clstoken
                         .render(&root.parameters, &mut state, &r.config.get_render_opts())?
-                        .raw_string()?
+                        .raw_string(&r.config.get_render_opts())?
                 } else {
                     // If Token::parse() returns None, the class name can't contain any references,
                     // just convert cls into an owned String.

src/refs/mod.rs

@@ -46,8 +46,8 @@ impl ResolveState {
     /// Pushes mapping key into the `current_keys` list. If possible, the provided value is
     /// formatted with `raw_string()`. Additionally, unprocessed `String` values are pushed as-is.
     /// This function will return an error when it's called with a `Value::ValueList`.
-    pub(crate) fn push_mapping_key(&mut self, key: &Value) -> Result<()> {
-        let kstr = match key.raw_string() {
+    pub(crate) fn push_mapping_key(&mut self, key: &Value, opts: &RenderOpts) -> Result<()> {
+        let kstr = match key.raw_string(opts) {
             Ok(s) => s,
             Err(_) => match key {
                 Value::String(s) => Ok(s.clone()),
@@ -173,7 +173,7 @@ impl Token {
                 .interpolate(params, state, opts)
         } else {
             Ok(Value::Literal(
-                self.resolve(params, state, opts)?.raw_string()?,
+                self.resolve(params, state, opts)?.raw_string(opts)?,
             ))
         }
     }
@@ -374,7 +374,7 @@ fn interpolate_token_slice(
         while v.is_string() {
             v = v.interpolate(params, &mut st, opts)?;
         }
-        res.push_str(&v.raw_string()?);
+        res.push_str(&v.raw_string(opts)?);
     }
     Ok(res)
 }

src/types/mapping.rs

@@ -401,7 +401,7 @@ impl Mapping {
                 && let Some(p) = p
             {
                 let mut st = state.clone();
-                st.push_mapping_key(k)?;
+                st.push_mapping_key(k, opts)?;
                 if let Some(errmsg) = p.as_resolve_error() {
                     if opts.ignore_overwritten_missing_references {
                         #[cfg(not(feature = "bench"))]
@@ -476,7 +476,7 @@ impl Mapping {
             // either manage to interpolate a value (in which case it doesn't contain a loop) or we
             // don't and the whole interpolation is aborted.
             let mut st = state.clone();
-            st.push_mapping_key(k)?;
+            st.push_mapping_key(k, opts)?;
             let iv = v.interpolate(root, &mut st, opts);
             let v = if let Err(e) = iv {
                 // convert interpolation errors into `Value::ResolveError` when interpolating

src/types/value.rs

@@ -481,13 +481,20 @@ impl Value {
     /// strings which match Python's `str()` for other types.
     ///
     #[inline]
-    pub(crate) fn raw_string(&self) -> Result<String> {
+    pub(crate) fn raw_string(&self, opts: &RenderOpts) -> Result<String> {
         match self {
             Value::Literal(s) => Ok(s.clone()),
-            // We serialize Null as `null`. NOTE(sg): This isn't compatible with Python's str(),
-            // but ensures that a reclass-rs reference default value which is a reference that
-            // resolves to a Null value is preserved correctly.
-            Value::Null => Ok("null".to_string()),
+            // We serialize Null as `null` unless the `NestedReferenceNullAsNone` compat flag is
+            // set, in which case we serialize Null as `None`. The default serialization isn't
+            // compatible with Python's str(), but ensures that a reclass-rs reference default
+            // value which is a reference that resolves to a Null value is preserved correctly.
+            Value::Null => {
+                if opts.nested_reference_null_as_none {
+                    Ok("None".to_string())
+                } else {
+                    Ok("null".to_string())
+                }
+            }
             // We need custom formatting for bool instead of `format!("{b}")`, so that this
             // function returns strings which match Python's `str()` implementation.
             Value::Bool(b) => match b {

src/types/value/value_flattened_tests.rs

@@ -42,7 +42,8 @@ test_flattened_simple! {
 fn test_flattened_string() {
     let v = Value::String("foo".into());
     let mut st = ResolveState::default();
-    st.push_mapping_key(&"test".into()).unwrap();
+    st.push_mapping_key(&"test".into(), &RenderOpts::default())
+        .unwrap();
     v.flattened(&mut st, &RenderOpts::default()).unwrap();
 }
 

src/types/value/value_tests.rs

@@ -294,45 +294,65 @@ fn test_strip_prefix() {
 #[test]
 fn test_raw_string_literal() {
     assert_eq!(
-        Value::Literal("foo".into()).raw_string().unwrap(),
+        Value::Literal("foo".into())
+            .raw_string(&RenderOpts::default())
+            .unwrap(),
         "foo".to_string()
     );
 }
 
 #[test]
 fn test_raw_string_null() {
-    assert_eq!(Value::Null.raw_string().unwrap(), "null".to_string());
+    assert_eq!(
+        Value::Null.raw_string(&RenderOpts::default()).unwrap(),
+        "null".to_string()
+    );
+}
+
+#[test]
+fn test_raw_string_null_as_none() {
+    let opts = RenderOpts {
+        nested_reference_null_as_none: true,
+        ..RenderOpts::default()
+    };
+    assert_eq!(Value::Null.raw_string(&opts).unwrap(), "None".to_string());
 }
 
 #[test]
 fn test_raw_string_number() {
     assert_eq!(
-        Value::Number(5.into()).raw_string().unwrap(),
+        Value::Number(5.into())
+            .raw_string(&RenderOpts::default())
+            .unwrap(),
         "5".to_string()
     );
     assert_eq!(
-        Value::Number((-1).into()).raw_string().unwrap(),
+        Value::Number((-1).into())
+            .raw_string(&RenderOpts::default())
+            .unwrap(),
         "-1".to_string()
     );
     assert_eq!(
-        Value::Number(3.14.into()).raw_string().unwrap(),
+        Value::Number(3.14.into())
+            .raw_string(&RenderOpts::default())
+            .unwrap(),
         "3.14".to_string()
     );
     assert_eq!(
         Value::Number(serde_yaml::Number::from(f64::INFINITY))
-            .raw_string()
+            .raw_string(&RenderOpts::default())
             .unwrap(),
         ".inf".to_string()
     );
     assert_eq!(
         Value::Number(serde_yaml::Number::from(f64::NEG_INFINITY))
-            .raw_string()
+            .raw_string(&RenderOpts::default())
             .unwrap(),
         "-.inf".to_string()
     );
     assert_eq!(
         Value::Number(serde_yaml::Number::from(f64::NAN))
-            .raw_string()
+            .raw_string(&RenderOpts::default())
             .unwrap(),
         ".nan".to_string()
     );
@@ -343,15 +363,15 @@ fn test_raw_string_mapping() {
     let mut m = Value::Mapping(Mapping::from_str("{foo: foo, bar: true, baz: 1.23}").unwrap());
     // turn string values into literals by calling flatten
     m.render(&Mapping::new(), &RenderOpts::default()).unwrap();
-    let mstr = m.raw_string().unwrap();
+    let mstr = m.raw_string(&RenderOpts::default()).unwrap();
     // NOTE(sg): serde_json output is sorted by keys
     assert_eq!(mstr, r#"{"bar":true,"baz":1.23,"foo":"foo"}"#);
 }
 
 #[test]
 fn test_raw_string_sequence() {
     let v = Value::Sequence(vec!["foo".into(), 3.14.into(), Value::Bool(true)]);
-    let vstr = v.raw_string().unwrap();
+    let vstr = v.raw_string(&RenderOpts::default()).unwrap();
     assert_eq!(vstr, r#"["foo",3.14,true]"#);
 }
 
@@ -364,7 +384,7 @@ fn test_raw_string_mapping_nonstring_keys() {
     let m = Value::Mapping(m)
         .rendered(&Mapping::new(), &RenderOpts::default())
         .unwrap();
-    let mstr = m.raw_string().unwrap();
+    let mstr = m.raw_string(&RenderOpts::default()).unwrap();
     // NOTE(sg): serde_json output is sorted by keys
     assert_eq!(mstr, r#"{"3.14":true,"null":1.23,"true":"foo"}"#);
 }