Add a couple of docstrings / small refactors (#612)

Helpful to me, hopefully helpful to others

cargo-insta/src/container.rs

@@ -47,7 +47,9 @@ impl PendingSnapshot {
 /// single rust file.
 #[derive(Debug)]
 pub(crate) struct SnapshotContainer {
-    snapshot_path: PathBuf,
+    // Path of the pending snapshot file (generally a `.new` or `.pending-snap` file)
+    pending_path: PathBuf,
+    // Path of the target snapshot file (generally a `.snap` file)
     target_path: PathBuf,
     kind: SnapshotKind,
     snapshots: Vec<PendingSnapshot>,
@@ -56,7 +58,7 @@ pub(crate) struct SnapshotContainer {
 
 impl SnapshotContainer {
     pub(crate) fn load(
-        snapshot_path: PathBuf,
+        pending_path: PathBuf,
         target_path: PathBuf,
         kind: SnapshotKind,
     ) -> Result<SnapshotContainer, Box<dyn Error>> {
@@ -68,7 +70,7 @@ impl SnapshotContainer {
                 } else {
                     Some(Snapshot::from_file(&target_path)?)
                 };
-                let new = Snapshot::from_file(&snapshot_path)?;
+                let new = Snapshot::from_file(&pending_path)?;
                 snapshots.push(PendingSnapshot {
                     id: 0,
                     old,
@@ -79,7 +81,7 @@ impl SnapshotContainer {
                 None
             }
             SnapshotKind::Inline => {
-                let mut pending_vec = PendingInlineSnapshot::load_batch(&snapshot_path)?;
+                let mut pending_vec = PendingInlineSnapshot::load_batch(&pending_path)?;
                 let mut have_new = false;
 
                 let rv = if fs::metadata(&target_path).is_ok() {
@@ -111,16 +113,16 @@ impl SnapshotContainer {
                 // The runtime code will issue something like this:
                 //   PendingInlineSnapshot::new(None, None, line).save(pending_snapshots)?;
                 if !have_new {
-                    fs::remove_file(&snapshot_path)
-                        .map_err(|e| ContentError::FileIo(e, snapshot_path.to_path_buf()))?;
+                    fs::remove_file(&pending_path)
+                        .map_err(|e| ContentError::FileIo(e, pending_path.to_path_buf()))?;
                 }
 
                 rv
             }
         };
 
         Ok(SnapshotContainer {
-            snapshot_path,
+            pending_path,
             target_path,
             kind,
             snapshots,
@@ -141,7 +143,7 @@ impl SnapshotContainer {
 
     pub(crate) fn snapshot_sort_key(&self) -> impl Ord + '_ {
         let path = self
-            .snapshot_path
+            .pending_path
             .file_name()
             .and_then(|x| x.to_str())
             .unwrap_or_default();
@@ -201,32 +203,32 @@ impl SnapshotContainer {
                 patcher.save()?;
             }
             if did_skip {
-                PendingInlineSnapshot::save_batch(&self.snapshot_path, &new_pending)?;
+                PendingInlineSnapshot::save_batch(&self.pending_path, &new_pending)?;
             } else {
-                try_removing_snapshot(&self.snapshot_path);
+                try_removing_snapshot(&self.pending_path);
             }
         } else {
             // should only be one or this is weird
             debug_assert!(self.snapshots.len() == 1);
             for snapshot in self.snapshots.iter() {
                 match snapshot.op {
                     Operation::Accept => {
-                        let snapshot = Snapshot::from_file(&self.snapshot_path).map_err(|e| {
+                        let snapshot = Snapshot::from_file(&self.pending_path).map_err(|e| {
                             // If it's an IO error, pass a ContentError back so
                             // we get a slightly clearer error message
                             match e.downcast::<std::io::Error>() {
                                 Ok(io_error) => Box::new(ContentError::FileIo(
                                     *io_error,
-                                    self.snapshot_path.to_path_buf(),
+                                    self.pending_path.to_path_buf(),
                                 )),
                                 Err(other_error) => other_error,
                             }
                         })?;
                         snapshot.save(&self.target_path)?;
-                        try_removing_snapshot(&self.snapshot_path);
+                        try_removing_snapshot(&self.pending_path);
                     }
                     Operation::Reject => {
-                        try_removing_snapshot(&self.snapshot_path);
+                        try_removing_snapshot(&self.pending_path);
                     }
                     Operation::Skip => {}
                 }

insta/src/output.rs

@@ -226,13 +226,8 @@ pub fn print_snapshot_summary(
     workspace_root: &Path,
     snapshot: &Snapshot,
     snapshot_file: Option<&Path>,
-    mut line: Option<u32>,
+    line: Option<u32>,
 ) {
-    // default to old assertion line from snapshot.
-    if line.is_none() {
-        line = snapshot.metadata().assertion_line();
-    }
-
     if let Some(snapshot_file) = snapshot_file {
         let snapshot_file = workspace_root
             .join(snapshot_file)
@@ -255,11 +250,12 @@ pub fn print_snapshot_summary(
         println!(
             "Source: {}{}",
             style(value.display()).cyan(),
-            if let Some(line) = line {
-                format!(":{}", style(line).bold())
-            } else {
-                "".to_string()
-            }
+            line.or(
+                // default to old assertion line from snapshot.
+                snapshot.metadata().assertion_line()
+            )
+            .map(|line| format!(":{}", style(line).bold()))
+            .unwrap_or_default()
         );
     }
 

insta/src/runtime.rs

@@ -201,7 +201,8 @@ fn get_snapshot_filename(
     })
 }
 
-/// A single snapshot including surrounding context which asserts and save the
+/// The context around a snapshot, such as the reference value, location, etc.
+/// (but not including the generated value). Responsible for saving the
 /// snapshot.
 #[derive(Debug)]
 struct SnapshotAssertionContext<'a> {
@@ -620,7 +621,7 @@ where
 /// assertion with a panic if needed.
 #[allow(clippy::too_many_arguments)]
 pub fn assert_snapshot(
-    refval: ReferenceValue<'_>,
+    refval: ReferenceValue,
     new_snapshot_value: &str,
     manifest_dir: &str,
     function_name: &str,

insta/src/settings.rs

@@ -269,10 +269,10 @@ impl Settings {
 
     /// Sets the input file reference.
     ///
-    /// This value is completely unused by the snapshot testing system but
-    /// it lets you store some meta data with a snapshot that refers you back
-    /// to the input file.  The path stored here is made relative to the
-    /// workspace root before storing with the snapshot.
+    /// This value is completely unused by the snapshot testing system but it
+    /// allows storing some metadata with a snapshot that refers back to the
+    /// input file.  The path stored here is made relative to the workspace root
+    /// before storing with the snapshot.
     pub fn set_input_file<P: AsRef<Path>>(&mut self, p: P) {
         self._private_inner_mut().input_file(p);
     }

insta/src/snapshot.rs

@@ -418,6 +418,8 @@ impl Snapshot {
 
     fn as_content(&self) -> Content {
         let mut fields = vec![("module_name", Content::from(self.module_name.as_str()))];
+        // Note this is currently never used, since this method is only used for
+        // inline snapshots
         if let Some(name) = self.snapshot_name.as_deref() {
             fields.push(("snapshot_name", Content::from(name)));
         }