Separately track files and directories removed.

The previous status line was a little awkward in the way it combined
both counts. I don't think showing the directories is particularly
interesting, so they are only displayed when no files are deleted.

Author: ehuss

src/cargo/ops/cargo_clean.rs

@@ -31,7 +31,8 @@ pub struct CleanContext<'cfg> {
     pub config: &'cfg Config,
     progress: Box<dyn CleaningProgressBar + 'cfg>,
     pub dry_run: bool,
-    num_files_folders_cleaned: u64,
+    num_files_removed: u64,
+    num_dirs_removed: u64,
     total_bytes_removed: u64,
 }
 
@@ -270,7 +271,8 @@ impl<'cfg> CleanContext<'cfg> {
             config,
             progress: Box::new(progress),
             dry_run: false,
-            num_files_folders_cleaned: 0,
+            num_files_removed: 0,
+            num_dirs_removed: 0,
             total_bytes_removed: 0,
         }
     }
@@ -352,27 +354,27 @@ impl<'cfg> CleanContext<'cfg> {
                 // byte sizes and not the block sizes.
                 self.total_bytes_removed += meta.len();
             }
+            self.num_files_removed += 1;
             if !self.dry_run {
                 paths::remove_file(path)?;
             }
             Ok(())
         };
 
         if !meta.is_dir() {
-            self.num_files_folders_cleaned += 1;
             return rm_file(path, Ok(meta));
         }
 
         for entry in walkdir::WalkDir::new(path).contents_first(true) {
             let entry = entry?;
             self.progress.on_clean()?;
-            self.num_files_folders_cleaned += 1;
             if self.dry_run {
                 self.config
                     .shell()
                     .verbose(|shell| Ok(writeln!(shell.out(), "{}", entry.path().display())?))?;
             }
             if entry.file_type().is_dir() {
+                self.num_dirs_removed += 1;
                 // The contents should have been removed by now, but sometimes a race condition is hit
                 // where other files have been added by the OS. `paths::remove_dir_all` also falls back
                 // to `std::fs::remove_dir_all`, which may be more reliable than a simple walk in
@@ -401,13 +403,21 @@ impl<'cfg> CleanContext<'cfg> {
                 format!(", {bytes:.1}{unit} total")
             }
         };
-        self.config.shell().status(
-            status,
-            format!(
-                "{} files/directories{byte_count}",
-                self.num_files_folders_cleaned
-            ),
-        )
+        // I think displaying the number of directories removed isn't
+        // particularly interesting to the user. However, if there are 0
+        // files, and a nonzero number of directories, cargo should indicate
+        // that it did *something*, so directory counts are only shown in that
+        // case.
+        let file_count = match (self.num_files_removed, self.num_dirs_removed) {
+            (0, 0) => format!("0 files"),
+            (0, 1) => format!("1 directory"),
+            (0, 2..) => format!("{} directories", self.num_dirs_removed),
+            (1, _) => format!("1 file"),
+            (2.., _) => format!("{} files", self.num_files_removed),
+        };
+        self.config
+            .shell()
+            .status(status, format!("{file_count}{byte_count}"))
     }
 
     /// Deletes all of the given paths, showing a progress bar as it proceeds.

tests/testsuite/clean.rs

@@ -417,7 +417,7 @@ fn clean_verbose() {
             expected.push_str(&format!("[REMOVING] [..]{}\n", obj.unwrap().display()));
         }
     }
-    expected.push_str("[REMOVED] [..] files/directories, [..] total\n");
+    expected.push_str("[REMOVED] [..] files, [..] total\n");
     p.cargo("clean -p bar --verbose")
         .with_stderr_unordered(&expected)
         .run();
@@ -609,7 +609,7 @@ error: package ID specification `baz` did not match any packages
         .with_stderr(
             "warning: version qualifier in `-p bar:1.0.0` is ignored, \
             cleaning all versions of `bar` found\n\
-            [REMOVED] [..] files/directories, [..] total",
+            [REMOVED] [..] files, [..] total",
         )
         .run();
     let mut walker = walkdir::WalkDir::new(p.build_dir())
@@ -707,13 +707,13 @@ fn clean_dry_run() {
     // Start with no files.
     p.cargo("clean --dry-run")
         .with_stdout("")
-        .with_stderr("[SUMMARY] 0 files/directories")
+        .with_stderr("[SUMMARY] 0 files")
         .run();
     p.cargo("check").run();
     let before = ls_r();
     p.cargo("clean --dry-run")
         .with_stdout("[CWD]/target")
-        .with_stderr("[SUMMARY] [..] files/directories, [..] total")
+        .with_stderr("[SUMMARY] [..] files, [..] total")
         .run();
     // Verify it didn't delete anything.
     let after = ls_r();
@@ -723,7 +723,7 @@ fn clean_dry_run() {
     // Verify the verbose output.
     p.cargo("clean --dry-run -v")
         .with_stdout_unordered(expected)
-        .with_stderr("[SUMMARY] [..] files/directories, [..] total")
+        .with_stderr("[SUMMARY] [..] files, [..] total")
         .run();
 }
 

tests/testsuite/profile_custom.rs

@@ -544,7 +544,7 @@ fn clean_custom_dirname() {
 
     // This should clean 'other'
     p.cargo("clean --profile=other")
-        .with_stderr("[REMOVED] [..] files/directories, [..] total")
+        .with_stderr("[REMOVED] [..] files, [..] total")
         .run();
     assert!(p.build_dir().join("debug").is_dir());
     assert!(!p.build_dir().join("other").is_dir());

tests/testsuite/script.rs

@@ -980,7 +980,7 @@ fn cmd_clean_with_embedded() {
         .with_stderr(
             "\
 [WARNING] `package.edition` is unspecified, defaulting to `2021`
-[REMOVED] [..] files/directories, [..] total
+[REMOVED] [..] files, [..] total
 ",
         )
         .run();