Warn only if snapshots are found in hidden or ignored locations (#334)

cargo-insta/src/cargo.rs

@@ -7,9 +7,7 @@ use std::process;
 
 use serde::Deserialize;
 
-use crate::container::SnapshotContainer;
 use crate::utils::err_msg;
-use crate::walk::{find_snapshots, FindFlags};
 
 #[derive(Deserialize, Clone, Debug)]
 pub struct Target {
@@ -57,11 +55,7 @@ impl Package {
         &self.version
     }
 
-    pub fn iter_snapshot_containers<'a>(
-        &self,
-        extensions: &'a [&'a str],
-        find_flags: FindFlags,
-    ) -> impl Iterator<Item = Result<SnapshotContainer, Box<dyn Error>>> + 'a {
+    pub fn find_snapshot_roots<'a>(&self) -> Vec<PathBuf> {
         let mut roots = Vec::new();
 
         // the manifest path's parent is always a snapshot container.  For
@@ -101,8 +95,6 @@ impl Package {
         }
 
         reduced_roots
-            .into_iter()
-            .flat_map(move |root| find_snapshots(&root, extensions, find_flags))
     }
 }
 

cargo-insta/src/cli.rs

@@ -19,7 +19,7 @@ use uuid::Uuid;
 use crate::cargo::{find_packages, get_cargo, get_package_metadata, Package};
 use crate::container::{Operation, SnapshotContainer};
 use crate::utils::{err_msg, QuietExit};
-use crate::walk::{find_snapshots, make_deletion_walker, FindFlags};
+use crate::walk::{find_snapshots, make_deletion_walker, make_snapshot_walker, FindFlags};
 
 /// A helper utility to work with insta snapshots.
 #[derive(StructOpt, Debug)]
@@ -361,20 +361,31 @@ fn handle_target_args(target_args: &TargetArgs) -> Result<LocationInfo<'_>, Box<
 
 fn load_snapshot_containers<'a>(
     loc: &'a LocationInfo,
-) -> Result<Vec<(SnapshotContainer, Option<&'a Package>)>, Box<dyn Error>> {
+) -> Result<
+    (
+        Vec<(SnapshotContainer, Option<&'a Package>)>,
+        HashSet<PathBuf>,
+    ),
+    Box<dyn Error>,
+> {
+    let mut roots = HashSet::new();
     let mut snapshot_containers = vec![];
     if let Some(ref packages) = loc.packages {
         for package in packages.iter() {
-            for snapshot_container in package.iter_snapshot_containers(&loc.exts, loc.find_flags) {
-                snapshot_containers.push((snapshot_container?, Some(package)));
+            for root in package.find_snapshot_roots() {
+                roots.insert(root.clone());
+                for snapshot_container in find_snapshots(&root, &loc.exts, loc.find_flags) {
+                    snapshot_containers.push((snapshot_container?, Some(package)));
+                }
             }
         }
     } else {
+        roots.insert(loc.workspace_root.clone());
         for snapshot_container in find_snapshots(&loc.workspace_root, &loc.exts, loc.find_flags) {
             snapshot_containers.push((snapshot_container?, None));
         }
     }
-    Ok(snapshot_containers)
+    Ok((snapshot_containers, roots))
 }
 
 fn process_snapshots(
@@ -385,14 +396,16 @@ fn process_snapshots(
 ) -> Result<(), Box<dyn Error>> {
     let term = Term::stdout();
 
-    let mut snapshot_containers = load_snapshot_containers(&loc)?;
+    let (mut snapshot_containers, roots) = load_snapshot_containers(&loc)?;
 
     let snapshot_count = snapshot_containers.iter().map(|x| x.0.len()).sum();
 
     if snapshot_count == 0 {
         if !quiet {
             println!("{}: no snapshots to review", style("done").bold());
-            show_ignore_hint(loc.find_flags);
+            if loc.tool_config.review_warn_undiscovered() {
+                show_undiscovered_hint(loc.find_flags, &snapshot_containers, &roots, &loc.exts);
+            }
         }
         return Ok(());
     }
@@ -584,8 +597,7 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box<dyn Error>> {
             },
         )?
     } else {
-        let loc = handle_target_args(&cmd.target_args)?;
-        let snapshot_containers = load_snapshot_containers(&loc)?;
+        let (snapshot_containers, roots) = load_snapshot_containers(&loc)?;
         let snapshot_count = snapshot_containers.iter().map(|x| x.0.len()).sum::<usize>();
         if snapshot_count > 0 {
             eprintln!(
@@ -598,7 +610,9 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box<dyn Error>> {
             return Err(QuietExit(1).into());
         } else {
             println!("{}: no snapshots to review", style("info").bold());
-            show_ignore_hint(loc.find_flags);
+            if loc.tool_config.review_warn_undiscovered() {
+                show_undiscovered_hint(loc.find_flags, &snapshot_containers, &roots, &loc.exts);
+            }
         }
     }
 
@@ -868,7 +882,7 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box<dyn Err
     }
 
     let loc = handle_target_args(&cmd.target_args)?;
-    let mut snapshot_containers = load_snapshot_containers(&loc)?;
+    let (mut snapshot_containers, _) = load_snapshot_containers(&loc)?;
 
     for (snapshot_container, _package) in snapshot_containers.iter_mut() {
         let target_file = snapshot_container.target_file().to_path_buf();
@@ -901,22 +915,65 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box<dyn Err
     Ok(())
 }
 
-fn show_ignore_hint(find_flags: FindFlags) {
+fn show_undiscovered_hint(
+    find_flags: FindFlags,
+    snapshot_containers: &[(SnapshotContainer, Option<&Package>)],
+    roots: &HashSet<PathBuf>,
+    extensions: &[&str],
+) {
+    // there is nothing to do if we already search everything.
+    if find_flags.include_hidden && find_flags.include_ignored {
+        return;
+    }
+
+    let mut found_extra = false;
+    let found_snapshots = snapshot_containers
+        .iter()
+        .filter_map(|x| x.0.snapshot_file())
+        .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();
+            fname.ends_with(".snap.new") || fname.ends_with(".pending-snap")
+        }) {
+            if !found_snapshots.contains(snapshot.path()) {
+                found_extra = true;
+                break;
+            }
+        }
+    }
+
+    // we did not find any extra snapshots
+    if !found_extra {
+        return;
+    }
+
     let (args, paths) = match (find_flags.include_ignored, find_flags.include_hidden) {
-        (true, true) => return,
         (true, false) => ("--include-ignored", "ignored"),
         (false, true) => ("--include-hidden", "hidden"),
         (false, false) => (
             "--include-ignored and --include-hidden",
             "ignored or hidden",
         ),
+        (true, true) => unreachable!(),
     };
 
     println!(
         "{}: {}",
         style("warning").yellow().bold(),
         format_args!(
-            "some paths are not picked up by cargo insta, use {} if you have snapshots in {} paths.",
+            "found undiscovered snapshots in some paths which are not picked up by cargo \
+            insta. Use {} if you have snapshots in {} paths.",
             args, paths,
         )
     );

cargo-insta/src/walk.rs

@@ -23,8 +23,41 @@ fn is_hidden(entry: &DirEntry) -> bool {
         .unwrap_or(false)
 }
 
+/// Finds all snapshots
+pub fn find_snapshots<'a>(
+    root: &Path,
+    extensions: &'a [&'a str],
+    flags: FindFlags,
+) -> impl Iterator<Item = Result<SnapshotContainer, Box<dyn Error>>> + 'a {
+    make_snapshot_walker(&root, extensions, flags)
+        .filter_map(|e| e.ok())
+        .filter_map(move |e| {
+            let fname = e.file_name().to_string_lossy();
+            if fname.ends_with(".new") {
+                let new_path = e.into_path();
+                let mut old_path = new_path.clone();
+                old_path.set_extension("");
+                Some(SnapshotContainer::load(
+                    new_path,
+                    old_path,
+                    SnapshotContainerKind::External,
+                ))
+            } else if fname.starts_with('.') && fname.ends_with(".pending-snap") {
+                let mut target_path = e.path().to_path_buf();
+                target_path.set_file_name(&fname[1..fname.len() - 13]);
+                Some(SnapshotContainer::load(
+                    e.path().to_path_buf(),
+                    target_path,
+                    SnapshotContainerKind::Inline,
+                ))
+            } else {
+                None
+            }
+        })
+}
+
 /// Creates a walker for snapshots.
-fn make_snapshot_walker(path: &Path, extensions: &[&str], flags: FindFlags) -> Walk {
+pub fn make_snapshot_walker(path: &Path, extensions: &[&str], flags: FindFlags) -> Walk {
     let mut builder = WalkBuilder::new(path);
     builder.standard_filters(!flags.include_ignored);
     if flags.include_hidden {
@@ -48,7 +81,9 @@ fn make_snapshot_walker(path: &Path, extensions: &[&str], flags: FindFlags) -> W
     builder.build()
 }
 
-/// A walker that deletes.
+/// A walker that is used by the snapshot deletion code.
+///
+/// This really should be using the same logic as the main snapshot walker but today is is not.
 pub fn make_deletion_walker(
     workspace_root: &Path,
     known_packages: Option<&[Package]>,
@@ -68,9 +103,7 @@ pub fn make_deletion_walker(
             })
             .collect()
     } else {
-        let mut hs = HashSet::new();
-        hs.insert(workspace_root.to_path_buf());
-        hs
+        Some(workspace_root.to_path_buf()).into_iter().collect()
     };
 
     WalkBuilder::new(workspace_root)
@@ -109,36 +142,3 @@ pub fn make_deletion_walker(
         })
         .build()
 }
-
-/// Finds all snapshots
-pub fn find_snapshots<'a>(
-    root: &Path,
-    extensions: &'a [&'a str],
-    flags: FindFlags,
-) -> impl Iterator<Item = Result<SnapshotContainer, Box<dyn Error>>> + 'a {
-    make_snapshot_walker(&root, extensions, flags)
-        .filter_map(|e| e.ok())
-        .filter_map(move |e| {
-            let fname = e.file_name().to_string_lossy();
-            if fname.ends_with(".new") {
-                let new_path = e.into_path();
-                let mut old_path = new_path.clone();
-                old_path.set_extension("");
-                Some(SnapshotContainer::load(
-                    new_path,
-                    old_path,
-                    SnapshotContainerKind::External,
-                ))
-            } else if fname.starts_with('.') && fname.ends_with(".pending-snap") {
-                let mut target_path = e.path().to_path_buf();
-                target_path.set_file_name(&fname[1..fname.len() - 13]);
-                Some(SnapshotContainer::load(
-                    e.path().to_path_buf(),
-                    target_path,
-                    SnapshotContainerKind::Inline,
-                ))
-            } else {
-                None
-            }
-        })
-}

src/env.rs

@@ -117,6 +117,8 @@ pub struct ToolConfig {
     review_include_ignored: bool,
     #[cfg(feature = "_cargo_insta_internal")]
     review_include_hidden: bool,
+    #[cfg(feature = "_cargo_insta_internal")]
+    review_warn_undiscovered: bool,
 }
 
 impl ToolConfig {
@@ -236,6 +238,10 @@ impl ToolConfig {
             review_include_ignored: resolve(&cfg, &["review", "include_ignored"])
                 .and_then(|x| x.as_bool())
                 .unwrap_or(false),
+            #[cfg(feature = "_cargo_insta_internal")]
+            review_warn_undiscovered: resolve(&cfg, &["review", "warn_undiscovered"])
+                .and_then(|x| x.as_bool())
+                .unwrap_or(true),
         })
     }
 
@@ -294,6 +300,10 @@ impl ToolConfig {
     pub fn review_include_ignored(&self) -> bool {
         self.review_include_ignored
     }
+
+    pub fn review_warn_undiscovered(&self) -> bool {
+        self.review_warn_undiscovered
+    }
 }
 
 /// How snapshots are supposed to be updated

src/lib.rs

@@ -220,6 +220,9 @@
 //!   include_ignored: true / false
 //!   # also look for snapshots in hidden folders
 //!   include_hidden: true / false
+//!   # show a warning if undiscovered (ignored or hidden) snapshots are found.
+//!   # defaults to true but creates a performance hit.
+//!   warn_undiscovered: true / false
 //! ```
 //!
 //! # Optional: Faster Runs