From 9f9c890b54c1c17725b640b4d30d98538652407b Mon Sep 17 00:00:00 2001
From: Armin Ronacher <armin.ronacher@active-4.com>
Date: Mon, 19 Feb 2024 17:28:26 +0100
Subject: [PATCH] Clean up error location reporting for yaml parsing errors

---
 src/content/mod.rs  |  9 ++++-----
 src/content/yaml.rs | 22 +++++++++++++---------
 src/env.rs          |  4 ++--
 src/snapshot.rs     | 11 ++---------
 4 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/content/mod.rs b/src/content/mod.rs
index 2526dbf2..ed7bf603 100644
--- a/src/content/mod.rs
+++ b/src/content/mod.rs
@@ -20,7 +20,7 @@ use std::fmt;
 /// An internal error type for content related errors.
 #[derive(Debug)]
 pub enum Error {
-    FailedParsingYaml(Option<std::path::PathBuf>),
+    FailedParsingYaml(std::path::PathBuf),
     UnexpectedDataType,
     #[cfg(feature = "_cargo_insta_internal")]
     MissingField,
@@ -29,10 +29,9 @@ pub enum Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Self::FailedParsingYaml(None) => f.write_str("Failed parsing the provided YAML text"),
-            Self::FailedParsingYaml(Some(pathbuf)) => f.write_str(
-                format!("Failed parsing the YAML from {:?}", pathbuf.as_os_str()).as_str(),
-            ),
+            Self::FailedParsingYaml(p) => {
+                f.write_str(format!("Failed parsing the YAML from {:?}", p.display()).as_str())
+            }
             Self::UnexpectedDataType => {
                 f.write_str("The present data type wasn't what was expected")
             }
diff --git a/src/content/yaml.rs b/src/content/yaml.rs
index 76ab9f13..15a961d5 100644
--- a/src/content/yaml.rs
+++ b/src/content/yaml.rs
@@ -1,18 +1,20 @@
+use std::path::Path;
+
 use crate::content::{Content, Error};
 
 use yaml_rust::{yaml::Hash as YamlObj, Yaml as YamlValue};
 
-pub fn parse_str(s: &str) -> Result<Content, Error> {
-    let mut blobs =
-        yaml_rust::YamlLoader::load_from_str(s).map_err(|_| Error::FailedParsingYaml(None))?;
+pub fn parse_str(s: &str, filename: &Path) -> Result<Content, Error> {
+    let mut blobs = yaml_rust::YamlLoader::load_from_str(s)
+        .map_err(|_| Error::FailedParsingYaml(filename.to_path_buf()))?;
 
     match (blobs.pop(), blobs.pop()) {
-        (Some(blob), None) => from_yaml_blob(blob).map_err(Into::into),
-        _ => Err(Error::FailedParsingYaml(None)),
+        (Some(blob), None) => from_yaml_blob(blob, filename),
+        _ => Err(Error::FailedParsingYaml(filename.to_path_buf())),
     }
 }
 
-fn from_yaml_blob(blob: YamlValue) -> Result<Content, Error> {
+fn from_yaml_blob(blob: YamlValue, filename: &Path) -> Result<Content, Error> {
     match blob {
         YamlValue::Null => Ok(Content::None),
         YamlValue::Boolean(b) => Ok(Content::from(b)),
@@ -25,18 +27,20 @@ fn from_yaml_blob(blob: YamlValue) -> Result<Content, Error> {
         YamlValue::Array(seq) => {
             let seq = seq
                 .into_iter()
-                .map(from_yaml_blob)
+                .map(|x| from_yaml_blob(x, filename))
                 .collect::<Result<_, Error>>()?;
             Ok(Content::Seq(seq))
         }
         YamlValue::Hash(obj) => {
             let obj = obj
                 .into_iter()
-                .map(|(k, v)| Ok((from_yaml_blob(k)?, from_yaml_blob(v)?)))
+                .map(|(k, v)| Ok((from_yaml_blob(k, filename)?, from_yaml_blob(v, filename)?)))
                 .collect::<Result<_, Error>>()?;
             Ok(Content::Map(obj))
         }
-        YamlValue::BadValue | YamlValue::Alias(_) => Err(Error::FailedParsingYaml(None)),
+        YamlValue::BadValue | YamlValue::Alias(_) => {
+            Err(Error::FailedParsingYaml(filename.to_path_buf()))
+        }
     }
 }
 
diff --git a/src/env.rs b/src/env.rs
index bed510d6..8014ed5e 100644
--- a/src/env.rs
+++ b/src/env.rs
@@ -129,9 +129,9 @@ impl ToolConfig {
         let mut cfg = None;
         for choice in &[".config/insta.yaml", "insta.yaml", ".insta.yaml"] {
             let path = workspace_dir.join(choice);
-            match fs::read_to_string(path) {
+            match fs::read_to_string(&path) {
                 Ok(s) => {
-                    cfg = Some(yaml::parse_str(&s).map_err(Error::Deserialize)?);
+                    cfg = Some(yaml::parse_str(&s, &path).map_err(Error::Deserialize)?);
                     break;
                 }
                 // ideally we would not swallow all errors here but unfortunately there are
diff --git a/src/snapshot.rs b/src/snapshot.rs
index 195ef5d6..4d90c78b 100644
--- a/src/snapshot.rs
+++ b/src/snapshot.rs
@@ -44,7 +44,7 @@ impl PendingInlineSnapshot {
         let mut rv: Vec<Self> = contents
             .lines()
             .map(|line| {
-                let value = yaml::parse_str(line)?;
+                let value = yaml::parse_str(line, p)?;
                 Self::from_content(value)
             })
             .collect::<Result<_, Box<dyn Error>>>()?;
@@ -286,14 +286,7 @@ impl Snapshot {
                     break;
                 }
             }
-            let content = yaml::parse_str(&buf).map_err(|e| {
-                // Add file context to error
-                if matches!(e, content::Error::FailedParsingYaml(None)) {
-                    content::Error::FailedParsingYaml(Some(p.to_path_buf()))
-                } else {
-                    e
-                }
-            })?;
+            let content = yaml::parse_str(&buf, p)?;
             MetaData::from_content(content)?
         // legacy format
         } else {