Fix & test the check for hidden / ignored snapshots (#637)

Also some usual refactoring

cargo-insta/src/cli.rs

@@ -10,6 +10,7 @@ use insta::Snapshot;
 use insta::_cargo_insta_support::{
     is_ci, SnapshotPrinter, SnapshotUpdate, TestRunner, ToolConfig, UnreferencedSnapshots,
 };
+use itertools::Itertools;
 use semver::Version;
 use serde::Serialize;
 use uuid::Uuid;
@@ -516,7 +517,15 @@ fn process_snapshots(
         if !quiet {
             println!("{}: no snapshots to review", style("done").bold());
             if loc.tool_config.review_warn_undiscovered() {
-                show_undiscovered_hint(loc.find_flags, &snapshot_containers, &roots, &loc.exts);
+                show_undiscovered_hint(
+                    loc.find_flags,
+                    &snapshot_containers
+                        .iter()
+                        .map(|x| x.0.clone())
+                        .collect_vec(),
+                    &roots,
+                    &loc.exts,
+                );
             }
         }
         return Ok(());
@@ -732,7 +741,8 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box<dyn Error>
         )?
     } else {
         let (snapshot_containers, roots) = load_snapshot_containers(&loc)?;
-        let snapshot_count = snapshot_containers.iter().map(|x| x.0.len()).sum::<usize>();
+        let snapshot_containers = snapshot_containers.into_iter().map(|x| x.0).collect_vec();
+        let snapshot_count = snapshot_containers.iter().map(|x| x.len()).sum::<usize>();
         if snapshot_count > 0 {
             eprintln!(
                 "{}: {} snapshot{} to review",
@@ -1160,7 +1170,7 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box<dyn Err
 
 fn show_undiscovered_hint(
     find_flags: FindFlags,
-    snapshot_containers: &[(SnapshotContainer, &Package)],
+    snapshot_containers: &[SnapshotContainer],
     roots: &HashSet<PathBuf>,
     extensions: &[&str],
 ) {
@@ -1169,56 +1179,64 @@ fn show_undiscovered_hint(
         return;
     }
 
-    let mut found_extra = false;
     let found_snapshots = snapshot_containers
         .iter()
-        .filter_map(|x| x.0.snapshot_file())
+        .filter_map(|x| x.snapshot_file())
+        .map(|x| x.to_path_buf())
         .collect::<HashSet<_>>();
 
-    for root in roots {
-        for snapshot in make_snapshot_walker(
-            root,
-            extensions,
-            FindFlags {
-                include_ignored: true,
-                include_hidden: true,
-            },
-        )
-        .filter_map(|e| e.ok())
-        .filter(|x| {
-            let fname = x.file_name().to_string_lossy();
-            // TODO: use `extensions` here
-            fname.ends_with(".snap.new") || fname.ends_with(".pending-snap")
-        }) {
-            if !found_snapshots.contains(snapshot.path()) {
-                found_extra = true;
-                break;
-            }
-        }
-    }
+    let all_snapshots: HashSet<_> = roots
+        .iter()
+        .flat_map(|root| {
+            make_snapshot_walker(
+                root,
+                extensions,
+                FindFlags {
+                    include_ignored: true,
+                    include_hidden: true,
+                },
+            )
+            .filter_map(|e| e.ok())
+            .filter(|x| {
+                let fname = x.file_name().to_string_lossy();
+                extensions
+                    .iter()
+                    .any(|ext| fname.ends_with(&format!(".{}.new", ext)))
+                    || fname.ends_with(".pending-snap")
+            })
+            .map(|x| x.path().to_path_buf())
+        })
+        .collect();
+
+    let missed_snapshots = all_snapshots.difference(&found_snapshots).collect_vec();
 
     // we did not find any extra snapshots
-    if !found_extra {
+    if missed_snapshots.is_empty() {
         return;
     }
 
     let (args, paths) = match (find_flags.include_ignored, find_flags.include_hidden) {
-        (true, false) => ("--include-ignored", "ignored"),
-        (false, true) => ("--include-hidden", "hidden"),
+        (false, true) => ("--include-ignored", "ignored"),
+        (true, false) => ("--include-hidden", "hidden"),
         (false, false) => (
             "--include-ignored and --include-hidden",
             "ignored or hidden",
         ),
         (true, true) => unreachable!(),
     };
 
-    println!(
+    eprintln!(
         "{}: {}",
         style("warning").yellow().bold(),
         format_args!(
-            "found undiscovered snapshots in some paths which are not picked up by cargo \
-            insta. Use {} if you have snapshots in {} paths.",
-            args, paths,
+            "found undiscovered pending snapshots in some paths which are not picked up by cargo \
+            insta. Use {} if you have snapshots in {} paths. Files:\n{}",
+            args,
+            paths,
+            missed_snapshots
+                .iter()
+                .map(|x| x.display().to_string())
+                .join("\n")
         )
     );
 }

cargo-insta/src/container.rs

@@ -15,7 +15,7 @@ pub(crate) enum Operation {
     Skip,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub(crate) struct PendingSnapshot {
     #[allow(dead_code)]
     id: usize,
@@ -45,9 +45,9 @@ impl PendingSnapshot {
 /// A snapshot and its immediate context, which loads & saves the snapshot. It
 /// holds either a single file snapshot, or all the inline snapshots from a
 /// single rust file.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub(crate) struct SnapshotContainer {
-    // Path of the pending snapshot file (generally a `.new` or `.pending-snap` file)
+    // Path of the pending snapshot file (generally a `.snap.new` or `.pending-snap` file)
     pending_path: PathBuf,
     // Path of the target snapshot file (generally a `.snap` file)
     target_path: PathBuf,

cargo-insta/src/inline.rs

@@ -10,13 +10,14 @@ use proc_macro2::TokenTree;
 use syn::__private::ToTokens;
 use syn::spanned::Spanned;
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 struct InlineSnapshot {
     start: (usize, usize),
     end: (usize, usize),
     indentation: usize,
 }
 
+#[derive(Clone)]
 pub(crate) struct FilePatcher {
     filename: PathBuf,
     lines: Vec<String>,

cargo-insta/tests/main.rs

@@ -1977,6 +1977,163 @@ Unused snapshot
     ");
 }
 
+#[test]
+fn test_hidden_snapshots() {
+    let test_project = TestFiles::new()
+        .add_file(
+            "Cargo.toml",
+            r#"
+[package]
+name = "test_hidden_snapshots"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+insta = { path = '$PROJECT_PATH' }
+"#
+            .to_string(),
+        )
+        .add_file(
+            "src/lib.rs",
+            r#"
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn test_snapshot() {
+        insta::assert_snapshot!("Hello, world!");
+    }
+}
+"#
+            .to_string(),
+        )
+        .add_file(
+            "src/snapshots/test_hidden_snapshots__tests__snapshot.snap",
+            r#"---
+source: src/lib.rs
+expression: "\"Hello, world!\""
+---
+Hello, world!
+"#
+            .to_string(),
+        )
+        .add_file(
+            "src/snapshots/.hidden/hidden_snapshot.snap.new",
+            r#"---
+source: src/lib.rs
+expression: "Hidden snapshot"
+---
+Hidden snapshot
+"#
+            .to_string(),
+        )
+        .create_project();
+
+    // Run test without --include-hidden flag
+    let output = test_project
+        .insta_cmd()
+        .args(["test"])
+        .stderr(std::process::Stdio::piped())
+        .output()
+        .unwrap();
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("found undiscovered pending snapshots")
+            && stderr.contains("--include-hidden"),
+        "{}",
+        stderr
+    );
+
+    // Run test with --include-hidden flag
+    let output = test_project
+        .insta_cmd()
+        .args(["test", "--include-hidden"])
+        .stderr(std::process::Stdio::piped())
+        .output()
+        .unwrap();
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        !stderr.contains("found undiscovered pending snapshots"),
+        "{}",
+        stderr
+    );
+}
+
+#[test]
+fn test_ignored_snapshots() {
+    let test_project = TestFiles::new()
+        .add_file(
+            "Cargo.toml",
+            r#"
+[package]
+name = "test_ignored_snapshots"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+insta = { path = '$PROJECT_PATH' }
+"#
+            .to_string(),
+        )
+        .add_file(
+            "src/lib.rs",
+            r#"
+#[test]
+fn test_snapshot() {
+    insta::assert_snapshot!("Hello, world!", @"");
+}
+"#
+            .to_string(),
+        )
+        .add_file(
+            ".gitignore",
+            r#"
+src/
+"#
+            .to_string(),
+        )
+        .create_project();
+
+    // We need to init a git repository in the project directory so it will be ignored
+    let mut git_cmd = Command::new("git");
+    git_cmd.current_dir(&test_project.workspace_dir);
+    git_cmd.args(["init"]);
+    git_cmd.output().unwrap();
+
+    // Run test without --include-ignored flag
+    let output = test_project
+        .insta_cmd()
+        // add the `--hidden` to check it's printing the correct warning
+        .args(["test", "--include-hidden"])
+        .stderr(std::process::Stdio::piped())
+        .output()
+        .unwrap();
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("found undiscovered pending snapshots")
+            && stderr.contains("--include-ignored"),
+        "{}",
+        stderr
+    );
+
+    // Run test with --include-ignored flag
+    let output = test_project
+        .insta_cmd()
+        .args(["test", "--include-ignored"])
+        .stderr(std::process::Stdio::piped())
+        .output()
+        .unwrap();
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        !stderr.contains("found undiscovered pending snapshots"),
+        "{}",
+        stderr
+    );
+}
+
 #[test]
 fn test_binary_unreferenced_delete() {
     let test_project = TestFiles::new()