Fix --unreferenced flag (#634)

A previous fix of mine broke this, because it wasn't consistent on
whether an extension was `.snap` or `snap`.

As part of this fix, it also now works for non-standard extensions. And
we have an integration test.

cargo-insta/src/cli.rs

@@ -215,7 +215,7 @@ struct TestCommand {
     test_runner: TestRunner,
     #[arg(long)]
     test_runner_fallback: Option<bool>,
-    /// Delete unreferenced snapshots after a successful test run.
+    /// Delete unreferenced snapshots after a successful test run (deprecated)
     #[arg(long, hide = true)]
     delete_unreferenced_snapshots: bool,
     /// Disable force-passing of snapshot tests (deprecated)
@@ -769,7 +769,7 @@ fn handle_unreferenced_snapshots(
     for package in loc.packages.clone() {
         let unreferenced_snapshots = make_snapshot_walker(
             package.manifest_path.parent().unwrap().as_std_path(),
-            &[".snap"],
+            &["snap"],
             FindFlags {
                 include_ignored: true,
                 include_hidden: true,
@@ -780,7 +780,11 @@ fn handle_unreferenced_snapshots(
         .filter(|e| {
             e.file_name()
                 .to_str()
-                .map(|name| name.ends_with(".snap"))
+                .map(|name| {
+                    loc.exts
+                        .iter()
+                        .any(|ext| name.ends_with(&format!(".{}", ext)))
+                })
                 .unwrap_or(false)
         })
         .filter_map(|e| e.path().canonicalize().ok())
@@ -1131,6 +1135,7 @@ fn show_undiscovered_hint(
         .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()) {

cargo-insta/src/walk.rs

@@ -55,7 +55,9 @@ pub(crate) fn find_pending_snapshots<'a>(
         })
 }
 
-/// Creates a walker for snapshots & pending snapshots within a package.
+/// Creates a walker for snapshots & pending snapshots within a package. The
+/// walker returns snapshots ending in any of the supplied extensions, any of
+/// the supplied extensions with a `.new` suffix, and `.pending-snap` files.
 pub(crate) fn make_snapshot_walker(
     package_root: &Path,
     extensions: &[&str],
@@ -71,30 +73,26 @@ pub(crate) fn make_snapshot_walker(
     }
 
     let mut override_builder = OverrideBuilder::new(package_root);
+
     extensions
         .iter()
-        .map(|ext| format!("*.{}.new", ext))
-        .chain(
-            ["*.pending-snap", "*.snap.new"]
-                .iter()
-                .map(ToString::to_string),
-        )
+        .flat_map(|ext| [format!("*.{}", ext), format!("*.{}.new", ext)])
+        .chain(std::iter::once("*.pending-snap".to_string()))
         .for_each(|pattern| {
             override_builder.add(&pattern).unwrap();
         });
-
     builder.overrides(override_builder.build().unwrap());
-    let root_path = package_root.to_path_buf();
 
+    let root_path = package_root.to_path_buf();
     // Add a custom filter to skip interior crates; otherwise we get duplicate
     // snapshots (https://github.com/mitsuhiko/insta/issues/396)
     builder.filter_entry(move |entry| {
-        if entry.file_type().map_or(false, |ft| ft.is_dir()) {
-            let cargo_toml_path = entry.path().join("Cargo.toml");
-            if cargo_toml_path.exists() && entry.path() != root_path {
-                // Skip this directory if it contains a Cargo.toml and is not the root
-                return false;
-            }
+        if entry.file_type().map_or(false, |ft| ft.is_dir())
+            && entry.path().join("Cargo.toml").exists()
+            && entry.path() != root_path
+        {
+            // Skip this directory if it contains a Cargo.toml and is not the root
+            return false;
         }
         // We always want to skip `target` even if it was not excluded by
         // ignore files.

cargo-insta/tests/main.rs

@@ -1454,3 +1454,106 @@ fn test_hello() {
     Hello, world!
     "#);
 }
+
+#[test]
+fn test_unreferenced_delete() {
+    let test_project = TestFiles::new()
+        .add_file(
+            "Cargo.toml",
+            r#"
+[package]
+name = "test_unreferenced_delete"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+doctest = false
+
+[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(),
+        )
+        .create_project();
+
+    // Run tests to create snapshots
+    let output = test_project
+        .insta_cmd()
+        .args(["test", "--accept"])
+        .output()
+        .unwrap();
+
+    assert!(&output.status.success());
+
+    // Manually add an unreferenced snapshot
+    let unreferenced_snapshot_path = test_project
+        .workspace_dir
+        .join("src/snapshots/test_unreferenced_delete__tests__unused_snapshot.snap");
+    std::fs::create_dir_all(unreferenced_snapshot_path.parent().unwrap()).unwrap();
+    std::fs::write(
+        &unreferenced_snapshot_path,
+        r#"---
+source: src/lib.rs
+expression: "Unused snapshot"
+---
+Unused snapshot
+"#,
+    )
+    .unwrap();
+
+    assert_snapshot!(test_project.file_tree_diff(), @r"
+    --- Original file tree
+    +++ Updated file tree
+    @@ -1,4 +1,8 @@
+     
+    +  Cargo.lock
+       Cargo.toml
+       src
+         src/lib.rs
+    +    src/snapshots
+    +      src/snapshots/test_unreferenced_delete__tests__snapshot.snap
+    +      src/snapshots/test_unreferenced_delete__tests__unused_snapshot.snap
+    ");
+
+    // Run cargo insta test with --unreferenced=delete
+    let output = test_project
+        .insta_cmd()
+        .args([
+            "test",
+            "--unreferenced=delete",
+            "--accept",
+            "--",
+            "--nocapture",
+        ])
+        .output()
+        .unwrap();
+
+    assert!(&output.status.success());
+
+    // We should now see the unreferenced snapshot deleted
+    assert_snapshot!(test_project.file_tree_diff(), @r"
+    --- Original file tree
+    +++ Updated file tree
+    @@ -1,4 +1,7 @@
+     
+    +  Cargo.lock
+       Cargo.toml
+       src
+         src/lib.rs
+    +    src/snapshots
+    +      src/snapshots/test_unreferenced_delete__tests__snapshot.snap
+    ");
+}