rm: massive refactor

Cargo.lock

@@ -914,12 +914,23 @@ dependencies = [
  "uucore 0.0.1",
 ]
 
+[[package]]
+name = "remove_dir_all"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "rm"
 version = "0.0.1"
 dependencies = [
  "getopts 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)",
+ "remove_dir_all 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "uucore 0.0.1",
+ "walkdir 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -1490,6 +1501,7 @@ dependencies = [
 "checksum regex 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1731164734096285ec2a5ec7fea5248ae2f5485b3feeb0115af4fda2183b2d1b"
 "checksum regex-syntax 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "f9ec002c35e86791825ed294b50008eea9ddfc8def4420124fbc6b08db834957"
 "checksum regex-syntax 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ad890a5eef7953f55427c50575c680c42841653abd2b028b68cd223d157f62db"
+"checksum remove_dir_all 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "0882bc41b0ba6131c7f0ce97233b62d8099e3f3abc60d4938185d3e35439c0cc"
 "checksum rust-crypto 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)" = "f76d05d3993fd5f4af9434e8e436db163a12a9d40e1a58a726f27a01dfd12a2a"
 "checksum rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)" = "dcf128d1287d2ea9d80910b5f1120d0b8eede3fbf1abe91c40d39ea7d51e6fda"
 "checksum rustc_version 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "c5f5376ea5e30ce23c03eb77cbe4962b988deead10910c372b226388b594c084"

src/rm/Cargo.toml

@@ -9,6 +9,8 @@ path = "rm.rs"
 
 [dependencies]
 getopts = "0.2.14"
+walkdir = "1.0.7"
+remove_dir_all = "0.2"
 uucore = { path="../uucore" }
 
 [[bin]]

src/rm/rm.rs

@@ -10,6 +10,8 @@
  */
 
 extern crate getopts;
+extern crate remove_dir_all;
+extern crate walkdir;
 
 #[macro_use]
 extern crate uucore;
@@ -18,7 +20,9 @@ use std::collections::VecDeque;
 use std::fs;
 use std::io::{stdin, stderr, BufRead, Write};
 use std::ops::BitOr;
-use std::path::{Path, PathBuf};
+use std::path::Path;
+use remove_dir_all::remove_dir_all;
+use walkdir::{DirEntry, WalkDir};
 
 #[derive(Eq, PartialEq, Clone, Copy)]
 enum InteractiveMode {
@@ -27,6 +31,17 @@ enum InteractiveMode {
     InteractiveAlways
 }
 
+struct Options {
+    force: bool,
+    interactive: InteractiveMode,
+    #[allow(dead_code)]
+    one_fs: bool,
+    preserve_root: bool,
+    recursive: bool,
+    dir: bool,
+    verbose: bool
+}
+
 static NAME: &'static str = "rm";
 static VERSION: &'static str = env!("CARGO_PKG_VERSION");
 
@@ -77,32 +92,36 @@ pub fn uumain(args: Vec<String>) -> i32 {
         show_error!("for help, try '{0} --help'", NAME);
         return 1;
     } else {
-        let force = matches.opt_present("force");
+        let options = Options {
-        let interactive =
+            force: matches.opt_present("force"),
-            if matches.opt_present("i") {
+            interactive: {
-                InteractiveMode::InteractiveAlways
+                if matches.opt_present("i") {
-            } else if matches.opt_present("I") {
+                    InteractiveMode::InteractiveAlways
-                InteractiveMode::InteractiveOnce
+                } else if matches.opt_present("I") {
-            } else if matches.opt_present("interactive") {
+                    InteractiveMode::InteractiveOnce
-                match &matches.opt_str("interactive").unwrap()[..] {
+                } else if matches.opt_present("interactive") {
-                    "none" => InteractiveMode::InteractiveNone,
+                    match &matches.opt_str("interactive").unwrap()[..] {
-                    "once" => InteractiveMode::InteractiveOnce,
+                        "none" => InteractiveMode::InteractiveNone,
-                    "always" => InteractiveMode::InteractiveAlways,
+                        "once" => InteractiveMode::InteractiveOnce,
-                    val => {
+                        "always" => InteractiveMode::InteractiveAlways,
-                        crash!(1, "Invalid argument to interactive ({})", val)
+                        val => {
+                            crash!(1, "Invalid argument to interactive ({})", val)
+                        }
                     }
+                } else {
+                    InteractiveMode::InteractiveNone
                 }
-            } else {
+            },
-                InteractiveMode::InteractiveNone
+            one_fs: matches.opt_present("one-file-system"),
-            };
+            preserve_root: !matches.opt_present("no-preserve-root"),
-        let one_fs = matches.opt_present("one-file-system");
+            recursive: matches.opt_present("recursive"),
-        let preserve_root = !matches.opt_present("no-preserve-root");
+            dir: matches.opt_present("dir"),
-        let recursive = matches.opt_present("recursive");
+            verbose: matches.opt_present("verbose")
-        let dir = matches.opt_present("dir");
+        };
-        let verbose = matches.opt_present("verbose");
+        if options.interactive == InteractiveMode::InteractiveOnce
-        if interactive == InteractiveMode::InteractiveOnce && (recursive || matches.free.len() > 3) {
+                && (options.recursive || matches.free.len() > 3) {
             let msg =
-                if recursive {
+                if options.recursive {
                     "Remove all arguments recursively? "
                 } else {
                     "Remove all arguments? "
@@ -112,7 +131,7 @@ pub fn uumain(args: Vec<String>) -> i32 {
             }
         }
 
-        if remove(matches.free, force, interactive, one_fs, preserve_root, recursive, dir, verbose) {
+        if remove(matches.free, options) {
             return 1;
         }
     }
@@ -120,118 +139,100 @@ pub fn uumain(args: Vec<String>) -> i32 {
     0
 }
 
-// TODO: implement one-file-system
+// TODO: implement one-file-system (this may get partially implemented in walkdir)
-#[allow(unused_variables)]
+fn remove(files: Vec<String>, options: Options) -> bool {
-fn remove(files: Vec<String>, force: bool, interactive: InteractiveMode, one_fs: bool, preserve_root: bool, recursive: bool, dir: bool, verbose: bool) -> bool {
     let mut had_err = false;
 
     for filename in &files {
-        let filename = &filename[..];
         let file = Path::new(filename);
-        match file.symlink_metadata() {
+        had_err = match file.symlink_metadata() {
-            Ok(metadata) => if metadata.is_dir() {
+            Ok(metadata) => {
-                if recursive && (filename != "/" || !preserve_root) {
+                if metadata.is_dir() {
-                    if interactive != InteractiveMode::InteractiveAlways {
+                    handle_dir(file, &options)
-                        if let Err(e) = fs::remove_dir_all(file) {
-                            had_err = true;
-                            show_error!("could not remove '{}': {}", filename, e);
-                        };
-                    } else {
-                        let mut dirs: VecDeque<PathBuf> = VecDeque::new();
-                        let mut files: Vec<PathBuf> = Vec::new();
-                        let mut rmdirstack: Vec<PathBuf> = Vec::new();
-                        dirs.push_back(file.to_path_buf());
-
-                        while !dirs.is_empty() {
-                            let dir = dirs.pop_front().unwrap();
-                            if !prompt(&(format!("rm: descend into directory '{}'? ", dir.display()))[..]) {
-                                continue;
-                            }
-
-                            // iterate over items in this directory, adding to either file or
-                            // directory queue
-                            match fs::read_dir(dir.as_path()) {
-                                Ok(rdir) => {
-                                    for ent in rdir {
-                                        match ent {
-                                            Ok(ref f) => match f.file_type() {
-                                                Ok(t) => {
-                                                    if t.is_dir() {
-                                                        dirs.push_back(f.path());
-                                                    } else {
-                                                        files.push(f.path());
-                                                    }
-                                                },
-                                                Err(e) => {
-                                                    had_err = true;
-                                                    show_error!("reading '{}': {}", f.path().display(), e);
-                                                },
-                                            },
-                                            Err(ref e) => {
-                                                had_err = true;
-                                                show_error!("recursing into '{}': {}", filename, e);
-                                            },
-                                        };
-                                    }
-                                },
-                                Err(e) => {
-                                    had_err = true;
-                                    show_error!("could not recurse into '{}': {}", dir.display(), e);
-                                    continue;
-                                },
-                            };
-
-                            for f in &files {
-                                had_err = remove_file(f.as_path(), interactive, verbose).bitor(had_err);
-                            }
-
-                            files.clear();
-                            rmdirstack.push(dir);
-                        }
-
-                        for d in rmdirstack.iter().rev() {
-                            had_err = remove_dir(d.as_path(), interactive, verbose).bitor(had_err);
-                        }
-                    }
-                } else if dir && (filename != "/" || !preserve_root) {
-                    had_err = remove_dir(&file, interactive, verbose).bitor(had_err);
                 } else {
-                    if recursive {
+                    remove_file(file, &options)
-                        show_error!("could not remove directory '{}'", filename);
-                        had_err = true;
-                    } else {
-                        show_error!("could not remove directory '{}' (did you mean to pass '-r'?)", filename);
-                        had_err = true;
-                    }
                 }
-            } else {
+            }
-                had_err = remove_file(&file, interactive, verbose).bitor(had_err);
+            Err(_e) => {
-            },
+                // TODO: actually print out the specific error
-            Err(e) => {
                 // TODO: When the error is not about missing files
                 // (e.g., permission), even rm -f should fail with
                 // outputting the error, but there's no easy eay.
-                if !force {
+                if !options.force {
-                    had_err = true;
                     show_error!("no such file or directory '{}'", filename);
+                    true
+                } else {
+                    false
                 }
             }
+        }.bitor(had_err);
+    }
+
+    had_err
+}
+
+fn handle_dir(path: &Path, options: &Options) -> bool {
+    let mut had_err = false;
+
+    let is_root = path.has_root() && path.parent().is_none();
+    if options.recursive && (!is_root || !options.preserve_root) {
+        if options.interactive != InteractiveMode::InteractiveAlways {
+            // 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) {
+                had_err = true;
+                show_error!("could not remove '{}': {}", path.display(), e);
+            }
+        } else {
+            let mut dirs: VecDeque<DirEntry> = VecDeque::new();
+
+            for entry in WalkDir::new(path) {
+                match entry {
+                    Ok(entry) => {
+                        let file_type = entry.file_type();
+                        if file_type.is_dir() {
+                            dirs.push_back(entry);
+                        } else {
+                            had_err = remove_file(entry.path(), options).bitor(had_err);
+                        }
+                    }
+                    Err(e) => {
+                        had_err = true;
+                        show_error!("recursing in '{}': {}", path.display(), e);
+                    }
+                }
+            }
+
+            for dir in dirs.iter().rev() {
+                had_err = remove_dir(dir.path(), options).bitor(had_err);
+            }
+        }
+    } else if options.dir && (!is_root || !options.preserve_root) {
+        had_err = remove_dir(path, options).bitor(had_err);
+    } else {
+        if options.recursive {
+            show_error!("could not remove directory '{}'", path.display());
+            had_err = true;
+        } else {
+            show_error!("could not remove directory '{}' (did you mean to pass '-r'?)",
+                        path.display());
+            had_err = true;
         }
     }
 
     had_err
 }
 
-fn remove_dir(path: &Path, interactive: InteractiveMode, verbose: bool) -> bool {
+fn remove_dir(path: &Path, options: &Options) -> bool {
     let response =
-        if interactive == InteractiveMode::InteractiveAlways {
+        if options.interactive == InteractiveMode::InteractiveAlways {
             prompt_file(path, true)
         } else {
             true
         };
     if response {
         match fs::remove_dir(path) {
-            Ok(_) => if verbose { println!("removed '{}'", path.display()); },
+            Ok(_) => if options.verbose { println!("removed '{}'", path.display()); },
             Err(e) => {
                 show_error!("removing '{}': {}", path.display(), e);
                 return true;
@@ -242,16 +243,16 @@ fn remove_dir(path: &Path, interactive: InteractiveMode, verbose: bool) -> bool
     false
 }
 
-fn remove_file(path: &Path, interactive: InteractiveMode, verbose: bool) -> bool {
+fn remove_file(path: &Path, options: &Options) -> bool {
     let response =
-        if interactive == InteractiveMode::InteractiveAlways {
+        if options.interactive == InteractiveMode::InteractiveAlways {
             prompt_file(path, false)
         } else {
             true
         };
     if response {
         match fs::remove_file(path) {
-            Ok(_) => if verbose { println!("removed '{}'", path.display()); },
+            Ok(_) => if options.verbose { println!("removed '{}'", path.display()); },
             Err(e) => {
                 show_error!("removing '{}': {}", path.display(), e);
                 return true;
@@ -264,22 +265,24 @@ fn remove_file(path: &Path, interactive: InteractiveMode, verbose: bool) -> bool
 
 fn prompt_file(path: &Path, is_dir: bool) -> bool {
     if is_dir {
-        prompt(&(format!("rm: remove directory '{}'? ", path.display()))[..])
+        prompt(&(format!("rm: remove directory '{}'? ", path.display())))
     } else {
-        prompt(&(format!("rm: remove file '{}'? ", path.display()))[..])
+        prompt(&(format!("rm: remove file '{}'? ", path.display())))
     }
 }
 
 fn prompt(msg: &str) -> bool {
-    stderr().write_all(msg.as_bytes()).unwrap_or(());
+    let _ = stderr().write_all(msg.as_bytes());
-    stderr().flush().unwrap_or(());
+    let _ = stderr().flush();
+
     let mut buf = Vec::new();
     let stdin = stdin();
     let mut stdin = stdin.lock();
+
     match stdin.read_until('\n' as u8, &mut buf) {
         Ok(x) if x > 0 => {
             match buf[0] {
-                0x59 | 0x79 => true,
+                b'y' | b'Y' => true,
                 _ => false,
             }
         }