rm: add verbose output and trim multiple slashes (#1988)

* rm: add verbose output and trim multiple slashes

Uses the normalize_path used in cargo to strip duplicate slashes
With a link to a std rfc https://github.com/rust-lang/rfcs/issues/2208

This fixes https://github.com/uutils/coreutils/issues/1829

This also touches https://github.com/uutils/coreutils/issues/1768 
but does not attempt to fully solve it

src/uu/rm/Cargo.toml

@@ -18,7 +18,8 @@ path = "src/rm.rs"
 clap = "2.33"
 walkdir = "2.2"
 remove_dir_all = "0.5.1"
-uucore = { version=">=0.0.8", package="uucore", path="../../uucore" }
+
+uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["fs"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 
 [[bin]]

src/uu/rm/src/rm.rs

@@ -16,7 +16,7 @@ use std::collections::VecDeque;
 use std::fs;
 use std::io::{stderr, stdin, BufRead, Write};
 use std::ops::BitOr;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use walkdir::{DirEntry, WalkDir};
 
 #[derive(Eq, PartialEq, Clone, Copy)]
@@ -251,7 +251,7 @@ fn handle_dir(path: &Path, options: &Options) -> bool {
 
     let is_root = path.has_root() && path.parent().is_none();
     if options.recursive && (!is_root || !options.preserve_root) {
-        if options.interactive != InteractiveMode::Always {
+        if options.interactive != InteractiveMode::Always && !options.verbose {
             // we need the extra crate because apparently fs::remove_dir_all() does not function
             // correctly on Windows
             if let Err(e) = remove_dir_all(path) {
@@ -311,7 +311,7 @@ fn remove_dir(path: &Path, options: &Options) -> bool {
                     match fs::remove_dir(path) {
                         Ok(_) => {
                             if options.verbose {
-                                println!("removed directory '{}'", path.display());
+                                println!("removed directory '{}'", normalize(path).display());
                             }
                         }
                         Err(e) => {
@@ -349,7 +349,7 @@ fn remove_file(path: &Path, options: &Options) -> bool {
         match fs::remove_file(path) {
             Ok(_) => {
                 if options.verbose {
-                    println!("removed '{}'", path.display());
+                    println!("removed '{}'", normalize(path).display());
                 }
             }
             Err(e) => {
@@ -370,6 +370,14 @@ fn prompt_file(path: &Path, is_dir: bool) -> bool {
     }
 }
 
+fn normalize(path: &Path) -> PathBuf {
+    // copied from https://github.com/rust-lang/cargo/blob/2e4cfc2b7d43328b207879228a2ca7d427d188bb/src/cargo/util/paths.rs#L65-L90
+    // both projects are MIT https://github.com/rust-lang/cargo/blob/master/LICENSE-MIT
+    // for std impl progress see rfc https://github.com/rust-lang/rfcs/issues/2208
+    // TODO: replace this once that lands
+    uucore::fs::normalize_path(path)
+}
+
 fn prompt(msg: &str) -> bool {
     let _ = stderr().write_all(msg.as_bytes());
     let _ = stderr().flush();

src/uucore/src/lib/features/fs.rs

@@ -60,6 +60,37 @@ pub enum CanonicalizeMode {
     Missing,
 }
 
+// copied from https://github.com/rust-lang/cargo/blob/2e4cfc2b7d43328b207879228a2ca7d427d188bb/src/cargo/util/paths.rs#L65-L90
+// both projects are MIT https://github.com/rust-lang/cargo/blob/master/LICENSE-MIT
+// for std impl progress see rfc https://github.com/rust-lang/rfcs/issues/2208
+// replace this once that lands
+pub fn normalize_path(path: &Path) -> PathBuf {
+    let mut components = path.components().peekable();
+    let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() {
+        components.next();
+        PathBuf::from(c.as_os_str())
+    } else {
+        PathBuf::new()
+    };
+
+    for component in components {
+        match component {
+            Component::Prefix(..) => unreachable!(),
+            Component::RootDir => {
+                ret.push(component.as_os_str());
+            }
+            Component::CurDir => {}
+            Component::ParentDir => {
+                ret.pop();
+            }
+            Component::Normal(c) => {
+                ret.push(c);
+            }
+        }
+    }
+    ret
+}
+
 fn resolve<P: AsRef<Path>>(original: P) -> IOResult<PathBuf> {
     const MAX_LINKS_FOLLOWED: u32 = 255;
     let mut followed = 0;
@@ -266,3 +297,62 @@ pub fn display_permissions_unix(mode: u32) -> String {
 
     result
 }
+
+#[cfg(test)]
+mod tests {
+    // Note this useful idiom: importing names from outer (for mod tests) scope.
+    use super::*;
+
+    struct NormalizePathTestCase<'a> {
+        path: &'a str,
+        test: &'a str,
+    }
+
+    const NORMALIZE_PATH_TESTS: [NormalizePathTestCase; 8] = [
+        NormalizePathTestCase {
+            path: "./foo/bar.txt",
+            test: "foo/bar.txt",
+        },
+        NormalizePathTestCase {
+            path: "bar/../foo/bar.txt",
+            test: "foo/bar.txt",
+        },
+        NormalizePathTestCase {
+            path: "foo///bar.txt",
+            test: "foo/bar.txt",
+        },
+        NormalizePathTestCase {
+            path: "foo///bar",
+            test: "foo/bar",
+        },
+        NormalizePathTestCase {
+            path: "foo//./bar",
+            test: "foo/bar",
+        },
+        NormalizePathTestCase {
+            path: "/foo//./bar",
+            test: "/foo/bar",
+        },
+        NormalizePathTestCase {
+            path: r"C:/you/later/",
+            test: "C:/you/later",
+        },
+        NormalizePathTestCase {
+            path: "\\networkshare/a//foo//./bar",
+            test: "\\networkshare/a/foo/bar",
+        },
+    ];
+
+    #[test]
+    fn test_normalize_path() {
+        for test in NORMALIZE_PATH_TESTS.iter() {
+            let path = Path::new(test.path);
+            let normalized = normalize_path(path);
+            assert_eq!(
+                test.test
+                    .replace("/", std::path::MAIN_SEPARATOR.to_string().as_str()),
+                normalized.to_str().expect("Path is not valid utf-8!")
+            );
+        }
+    }
+}

tests/by-util/test_rm.rs

@@ -263,3 +263,32 @@ fn test_rm_no_operand() {
     ucmd.fails()
         .stderr_is("rm: error: missing an argument\nrm: error: for help, try 'rm --help'\n");
 }
+
+#[test]
+fn test_rm_verbose_slash() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let dir = "test_rm_verbose_slash_directory";
+    let file_a = &format!("{}/test_rm_verbose_slash_file_a", dir);
+
+    at.mkdir(dir);
+    at.touch(file_a);
+
+    let file_a_normalized = &format!(
+        "{}{}test_rm_verbose_slash_file_a",
+        dir,
+        std::path::MAIN_SEPARATOR
+    );
+
+    ucmd.arg("-r")
+        .arg("-f")
+        .arg("-v")
+        .arg(&format!("{}///", dir))
+        .succeeds()
+        .stdout_only(format!(
+            "removed '{}'\nremoved directory '{}'\n",
+            file_a_normalized, dir
+        ));
+
+    assert!(!at.dir_exists(dir));
+    assert!(!at.file_exists(file_a));
+}