Merge pull request #157 from projectsyn/fix/flattening-errors-context

Add missing context to errors that occur while flattening values

Author: simu

src/refs/mod.rs

@@ -104,6 +104,11 @@ impl ResolveState {
             "While looking up key '{key}' in reference '{r}' for parameter '{current_key}': {msg}"
         )
     }
+
+    pub(crate) fn render_flattening_error(&self, msg: &str) -> anyhow::Error {
+        let current_key = self.current_key();
+        anyhow!("In {current_key}: {msg}")
+    }
 }
 
 /// Maximum allowed recursion depth for Token::resolve(). We're fairly conservative with the value,
@@ -348,7 +353,7 @@ fn interpolate_string_or_valuelist(
                 i.push(v);
             }
             // Finally we flatten the resulting ValueList into a single Value.
-            Value::ValueList(i).flattened()
+            Value::ValueList(i).flattened(state)
         }
         // Do nothing for other types
         _ => Ok(v.clone()),

src/types/mapping.rs

@@ -405,13 +405,13 @@ impl Mapping {
     ///
     /// Used in `Value::flattened()` to preserve const and override key information when flattening
     /// Mapping values.
-    pub(super) fn flattened(&self) -> Result<Self> {
+    pub(super) fn flattened(&self, state: &mut ResolveState) -> Result<Self> {
         let mut res = Self::new();
         for (k, v) in self {
             // Propagate key properties to the resulting mapping by using `insert_impl()`.
             res.insert_impl(
                 k.clone(),
-                v.flattened()?,
+                v.flattened(state)?,
                 self.is_const(k),
                 self.is_override(k),
             )?;
@@ -436,7 +436,7 @@ impl Mapping {
             let mut st = state.clone();
             st.push_mapping_key(k)?;
             let mut v = v.interpolate(root, &mut st)?;
-            v.flatten()?;
+            v.flatten(&mut st)?;
             // Propagate key properties to the resulting mapping by using `insert_impl()`.
             res.insert_impl(k.clone(), v, self.is_const(k), self.is_override(k))?;
         }

src/types/value.rs

@@ -575,7 +575,7 @@ impl Value {
                     // done with a layer, any references that we saw there have been successfully
                     // resolved, and don't matter for the next layer we're interpolating).
                     let mut st = state.clone();
-                    r.merge(v.interpolate(root, &mut st)?)?;
+                    r.merge(v.interpolate(root, &mut st)?, &mut st)?;
                 }
                 // Depending on the structure of the ValueList, we may end up with a final
                 // interpolated Value which contains more ValueLists due to mapping merges. Such
@@ -601,7 +601,7 @@ impl Value {
     ///
     /// Note that this method will call [`Value::flatten()`] after merging two Mappings to ensure
     /// that the resulting Value doesn't contain any `ValueList` elements.
-    fn merge(&mut self, other: Self) -> Result<()> {
+    fn merge(&mut self, other: Self, state: &mut ResolveState) -> Result<()> {
         if other.is_null() {
             // Any value can be replaced by null,
             let _prev = std::mem::replace(self, other);
@@ -610,7 +610,7 @@ impl Value {
 
         // If `other` is a ValueList, flatten it before trying to merge
         let other = if other.is_value_list() {
-            other.flattened()?
+            other.flattened(state)?
         } else {
             other
         };
@@ -624,21 +624,31 @@ impl Value {
             Self::Mapping(m) => match other {
                 // merge mapping and mapping
                 Self::Mapping(other) => m.merge(&other)?,
-                _ => return Err(anyhow!("Can't merge {} over mapping", other.variant())),
+                _ => {
+                    return Err(state.render_flattening_error(&format!(
+                        "Can't merge {} over mapping",
+                        other.variant()
+                    )))
+                }
             },
             Self::Sequence(s) => match other {
                 // merge sequence and sequence
                 Self::Sequence(mut other) => s.append(&mut other),
-                _ => return Err(anyhow!("Can't merge {} over sequence", other.variant())),
+                _ => {
+                    return Err(state.render_flattening_error(&format!(
+                        "Can't merge {} over sequence",
+                        other.variant()
+                    )))
+                }
             },
             Self::Literal(_) | Self::Bool(_) | Self::Number(_) => {
                 if other.is_mapping() || other.is_sequence() {
                     // We can't merge simple non-null types over mappings or sequences
-                    return Err(anyhow!(
+                    return Err(state.render_flattening_error(&format!(
                         "Can't merge {} over {}",
                         other.variant(),
                         self.variant()
-                    ));
+                    )));
                 }
                 // overwrite self with the value that's being merged
                 let _prev = std::mem::replace(self, other);
@@ -666,42 +676,42 @@ impl Value {
     ///
     /// Note that we don't recommend calling `flattened()` on arbitrary Values. Users should always
     /// prefer calling [`Value::rendered()`] or one of the in-place variations of that method.
-    pub(crate) fn flattened(&self) -> Result<Self> {
+    pub(crate) fn flattened(&self, state: &mut ResolveState) -> Result<Self> {
         match self {
             // Flatten ValueList by iterating over its elements and merging each element into a
             // base Value.
             Self::ValueList(l) => {
                 // NOTE(sg): Empty ValueLists get flattened to Value::Null
                 let mut base = Value::Null;
                 for v in l {
-                    base.merge(v.clone())?;
+                    base.merge(v.clone(), state)?;
                 }
                 Ok(base)
             }
             // Flatten Mapping by flattening each value and inserting it into a new Mapping.
-            Self::Mapping(m) => Ok(Self::Mapping(m.flattened()?)),
+            Self::Mapping(m) => Ok(Self::Mapping(m.flattened(state)?)),
             // Flatten Sequence by flattening each element and inserting it into a new Sequence
             Self::Sequence(s) => {
                 let mut n = Vec::with_capacity(s.len());
                 for v in s {
-                    n.push(v.flattened()?);
+                    n.push(v.flattened(state)?);
                 }
                 Ok(Self::Sequence(n))
             }
             // Simple values are flattened as themselves
             Self::Null | Self::Bool(_) | Self::Literal(_) | Self::Number(_) => Ok(self.clone()),
             // Flattening an unparsed string is an error
-            Self::String(_) => Err(anyhow!(
-                "Can't flatten unparsed String, did you mean to call `rendered()`?"
+            Self::String(_) => Err(state.render_flattening_error(
+                "Can't flatten unparsed String, did you mean to call `rendered()`?",
             )),
         }
     }
 
     /// Flattens the Value in-place.
     ///
     /// See [`Value::flattened()`] for details.
-    pub(super) fn flatten(&mut self) -> Result<()> {
-        let _prev = std::mem::replace(self, self.flattened()?);
+    pub(super) fn flatten(&mut self, state: &mut ResolveState) -> Result<()> {
+        let _prev = std::mem::replace(self, self.flattened(state)?);
         Ok(())
     }
 
@@ -718,7 +728,7 @@ impl Value {
         let mut v = self
             .interpolate(root, &mut state)
             .map_err(|e| anyhow!("While resolving references: {e}"))?;
-        v.flatten()?;
+        v.flatten(&mut state)?;
         Ok(v)
     }
 

src/types/value/value_flattened_tests.rs

@@ -4,7 +4,10 @@ use std::str::FromStr;
 
 #[test]
 fn test_flattened_null() {
-    assert_eq!(Value::Null.flattened().unwrap(), Value::Null);
+    assert_eq!(
+        Value::Null.flattened(&mut ResolveState::default()).unwrap(),
+        Value::Null
+    );
 }
 
 macro_rules! test_flattened_simple {
@@ -14,7 +17,7 @@ macro_rules! test_flattened_simple {
             #[test]
             fn [<test_flattened_simple_ $variant:snake>]() {
                 let v = Value::$variant($val);
-                let f = v.flattened().unwrap();
+                let f = v.flattened(&mut ResolveState::default()).unwrap();
                 assert_eq!(f, $expected);
             }
             }
@@ -31,10 +34,14 @@ test_flattened_simple! {
 }
 
 #[test]
-#[should_panic(expected = "Can't flatten unparsed String, did you mean to call `rendered()`?")]
+#[should_panic(
+    expected = "In test: Can't flatten unparsed String, did you mean to call `rendered()`?"
+)]
 fn test_flattened_string() {
     let v = Value::String("foo".into());
-    v.flattened().unwrap();
+    let mut st = ResolveState::default();
+    st.push_mapping_key(&"test".into()).unwrap();
+    v.flattened(&mut st).unwrap();
 }
 
 #[test]
@@ -61,7 +68,7 @@ fn test_flattened_simple_value_list() {
         Value::Literal("foo".into()),
         Value::Literal("bar".into()),
     ]);
-    let f = v.flattened().unwrap();
+    let f = v.flattened(&mut ResolveState::default()).unwrap();
     assert!(f.is_literal());
     assert_eq!(f, Value::Literal("bar".into()));
 }
@@ -73,7 +80,7 @@ fn test_flattened_mixed_value_list() {
         Value::Null,
         Value::Literal("bar".into()),
     ]);
-    let f = v.flattened().unwrap();
+    let f = v.flattened(&mut ResolveState::default()).unwrap();
     assert!(f.is_literal());
     assert_eq!(f, Value::Literal("bar".into()));
 }
@@ -85,7 +92,7 @@ fn test_flattened_sequence_value_list() {
         Value::Sequence(vec!["baz".into(), "qux".into()]),
         Value::Sequence(vec!["foo".into()]),
     ]);
-    let f = v.flattened().unwrap();
+    let f = v.flattened(&mut ResolveState::default()).unwrap();
     assert_eq!(
         f,
         Value::Sequence(vec![
@@ -106,7 +113,7 @@ fn test_flattened_mapping_value_list() {
             .into(),
         Mapping::from_str("{baz: baz, qux: qux}").unwrap().into(),
     ]);
-    let f = v.flattened().unwrap();
+    let f = v.flattened(&mut ResolveState::default()).unwrap();
     assert!(f.is_mapping());
 
     let m: serde_yaml::Mapping = f.as_mapping().unwrap().clone().into();
@@ -123,7 +130,7 @@ fn test_flattened_null_over_mapping() {
             .into(),
         Value::Null,
     ]);
-    let f = v.flattened().unwrap();
+    let f = v.flattened(&mut ResolveState::default()).unwrap();
     assert!(f.is_null());
     assert_eq!(f, Value::Null);
 }
@@ -134,7 +141,7 @@ fn test_flattened_null_over_sequence() {
         Value::Sequence(vec!["foo".into(), "bar".into()]),
         Value::Null,
     ]);
-    let f = v.flattened().unwrap();
+    let f = v.flattened(&mut ResolveState::default()).unwrap();
     assert!(f.is_null());
     assert_eq!(f, Value::Null);
 }
@@ -145,7 +152,7 @@ fn test_flattened_map_over_sequence_error() {
         Value::Sequence(vec!["foo".into(), "bar".into()]),
         Value::Mapping(Mapping::from_str("foo: foo").unwrap()),
     ]);
-    let f = v.flattened();
+    let f = v.flattened(&mut ResolveState::default());
     assert!(f.is_err());
 }
 
@@ -155,7 +162,7 @@ fn test_flattened_map_over_simple_value_error() {
         Value::Bool(true),
         Value::Mapping(Mapping::from_str("foo: foo").unwrap()),
     ]);
-    let f = v.flattened();
+    let f = v.flattened(&mut ResolveState::default());
     assert!(f.is_err());
 }
 
@@ -165,7 +172,7 @@ fn test_flattened_sequence_over_map_error() {
         Value::Mapping(Mapping::from_str("foo: foo").unwrap()),
         Value::Sequence(vec!["foo".into(), "bar".into()]),
     ]);
-    let f = v.flattened();
+    let f = v.flattened(&mut ResolveState::default());
     assert!(f.is_err());
 }
 
@@ -175,7 +182,7 @@ fn test_flattened_sequence_over_simple_value_error() {
         Value::Bool(true),
         Value::Sequence(vec!["foo".into(), "bar".into()]),
     ]);
-    let f = v.flattened();
+    let f = v.flattened(&mut ResolveState::default());
     assert!(f.is_err());
 }