diff --git a/src/commands/rm.rs b/src/commands/rm.rs index c3a13700d..714fb4799 100644 --- a/src/commands/rm.rs +++ b/src/commands/rm.rs @@ -1,12 +1,19 @@ +use crate::commands::command::RunnablePerItemContext; use crate::errors::ShellError; use crate::parser::hir::SyntaxType; +use crate::parser::registry::{CommandRegistry, Signature}; use crate::prelude::*; - -use glob::glob; +use crate::utils::FileStructure; use std::path::PathBuf; pub struct Remove; +#[derive(Deserialize)] +pub struct RemoveArgs { + path: Tagged, + recursive: Tagged, +} + impl PerItemCommand for Remove { fn name(&self) -> &str { "rm" @@ -25,52 +32,113 @@ impl PerItemCommand for Remove { shell_manager: &ShellManager, _input: Tagged, ) -> Result, ShellError> { - rm(call_info, shell_manager) + call_info.process(shell_manager, rm)?.run() } } pub fn rm( - call_info: &CallInfo, - shell_manager: &ShellManager, + args: RemoveArgs, + context: &RunnablePerItemContext, ) -> Result, ShellError> { - let mut full_path = PathBuf::from(shell_manager.path()); + let mut path = PathBuf::from(context.shell_manager.path()); + let name_span = context.name; - match call_info - .args - .nth(0) - .ok_or_else(|| ShellError::string(&format!("No file or directory specified")))? - .as_string()? - .as_str() - { - "." | ".." => return Err(ShellError::string("\".\" and \"..\" may not be removed")), - file => full_path.push(file), + let file = &args.path.item.to_string_lossy(); + + if file == "." || file == ".." { + return Err(ShellError::labeled_error( + "Remove aborted. \".\" or \"..\" may not be removed.", + "Remove aborted. \".\" or \"..\" may not be removed.", + args.path.span(), + )); } - let entries = glob(&full_path.to_string_lossy()); + path.push(&args.path.item); - if entries.is_err() { - return Err(ShellError::string("Invalid pattern.")); + let entries: Vec<_> = match glob::glob(&path.to_string_lossy()) { + Ok(files) => files.collect(), + Err(_) => { + return Err(ShellError::labeled_error( + "Invalid pattern.", + "Invalid pattern.", + args.path.tag, + )) + } + }; + + if entries.len() == 1 { + if let Ok(entry) = &entries[0] { + if entry.is_dir() { + let mut source_dir: FileStructure = FileStructure::new(); + + source_dir.walk_decorate(&entry)?; + + if source_dir.contains_files() && !args.recursive.item { + return Err(ShellError::labeled_error( + format!( + "{:?} is a directory. Try using \"--recursive\".", + &args.path.item.to_string_lossy() + ), + format!( + "{:?} is a directory. Try using \"--recursive\".", + &args.path.item.to_string_lossy() + ), + args.path.span(), + )); + } + } + } } - let entries = entries.unwrap(); - for entry in entries { match entry { Ok(path) => { - if path.is_dir() { - if !call_info.args.has("recursive") { - return Err(ShellError::labeled_error( - "is a directory", - "is a directory", - call_info.name_span, - )); + let path_file_name = { + let p = &path; + + match p.file_name() { + Some(name) => PathBuf::from(name), + None => { + return Err(ShellError::labeled_error( + "Remove aborted. Not a valid path", + "Remove aborted. Not a valid path", + name_span, + )) + } } - std::fs::remove_dir_all(&path).expect("can not remove directory"); + }; + + let mut source_dir: FileStructure = FileStructure::new(); + + source_dir.walk_decorate(&path)?; + + if source_dir.contains_more_than_one_file() && !args.recursive.item { + return Err(ShellError::labeled_error( + format!( + "Directory {:?} found somewhere inside. Try using \"--recursive\".", + path_file_name + ), + format!( + "Directory {:?} found somewhere inside. Try using \"--recursive\".", + path_file_name + ), + args.path.span(), + )); + } + + if path.is_dir() { + std::fs::remove_dir_all(&path)?; } else if path.is_file() { - std::fs::remove_file(&path).expect("can not remove file"); + std::fs::remove_file(&path)?; } } - Err(e) => return Err(ShellError::string(&format!("{:?}", e))), + Err(e) => { + return Err(ShellError::labeled_error( + format!("Remove aborted. {:}", e.to_string()), + format!("Remove aborted. {:}", e.to_string()), + name_span, + )) + } } } diff --git a/src/utils.rs b/src/utils.rs index ac17462f3..464cfb665 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -93,6 +93,14 @@ impl FileStructure { } } + pub fn contains_more_than_one_file(&self) -> bool { + self.resources.len() > 1 + } + + pub fn contains_files(&self) -> bool { + self.resources.len() > 0 + } + pub fn set_root(&mut self, path: &Path) { self.root = path.to_path_buf(); } diff --git a/tests/command_rm_tests.rs b/tests/command_rm_tests.rs new file mode 100644 index 000000000..f53510083 --- /dev/null +++ b/tests/command_rm_tests.rs @@ -0,0 +1,171 @@ +mod helpers; + +use h::{in_directory as cwd, Playground, Stub::*}; +use helpers as h; +use std::path::{Path, PathBuf}; + +#[test] +fn rm_removes_a_file() { + let sandbox = Playground::setup_for("rm_regular_file_test") + .with_files(vec![EmptyFile("i_will_be_deleted.txt")]) + .test_dir_name(); + + nu!( + _output, + cwd(&Playground::root()), + "rm rm_regular_file_test/i_will_be_deleted.txt" + ); + + let path = &format!( + "{}/{}/{}", + Playground::root(), + sandbox, + "i_will_be_deleted.txt" + ); + + assert!(!h::file_exists_at(PathBuf::from(path))); +} + +#[test] +fn rm_removes_files_with_wildcard() { + let sandbox = Playground::setup_for("rm_wildcard_test_1") + .within("src") + .with_files(vec![ + EmptyFile("cli.rs"), + EmptyFile("lib.rs"), + EmptyFile("prelude.rs"), + ]) + .within("src/parser") + .with_files(vec![EmptyFile("parse.rs"), EmptyFile("parser.rs")]) + .within("src/parser/parse") + .with_files(vec![EmptyFile("token_tree.rs")]) + .within("src/parser/hir") + .with_files(vec![ + EmptyFile("baseline_parse.rs"), + EmptyFile("baseline_parse_tokens.rs"), + ]) + .test_dir_name(); + + let full_path = format!("{}/{}", Playground::root(), sandbox); + + nu!( + _output, + cwd("tests/fixtures/nuplayground/rm_wildcard_test_1"), + "rm \"src/*/*/*.rs\"" + ); + + assert!(!h::files_exist_at( + vec![ + Path::new("src/parser/parse/token_tree.rs"), + Path::new("src/parser/hir/baseline_parse.rs"), + Path::new("src/parser/hir/baseline_parse_tokens.rs") + ], + PathBuf::from(&full_path) + )); + + assert_eq!( + Playground::glob_vec(&format!("{}/src/*/*/*.rs", &full_path)), + Vec::::new() + ); +} + +#[test] +fn rm_removes_deeply_nested_directories_with_wildcard_and_recursive_flag() { + let sandbox = Playground::setup_for("rm_wildcard_test_2") + .within("src") + .with_files(vec![ + EmptyFile("cli.rs"), + EmptyFile("lib.rs"), + EmptyFile("prelude.rs"), + ]) + .within("src/parser") + .with_files(vec![EmptyFile("parse.rs"), EmptyFile("parser.rs")]) + .within("src/parser/parse") + .with_files(vec![EmptyFile("token_tree.rs")]) + .within("src/parser/hir") + .with_files(vec![ + EmptyFile("baseline_parse.rs"), + EmptyFile("baseline_parse_tokens.rs"), + ]) + .test_dir_name(); + + let full_path = format!("{}/{}", Playground::root(), sandbox); + + nu!( + _output, + cwd("tests/fixtures/nuplayground/rm_wildcard_test_2"), + "rm src/* --recursive" + ); + + assert!(!h::files_exist_at( + vec![ + Path::new("src/parser/parse"), + Path::new("src/parser/hir"), + ], + PathBuf::from(&full_path) + )); +} + +#[test] +fn rm_removes_directory_contents_without_recursive_flag_if_empty() { + let sandbox = Playground::setup_for("rm_directory_removal_recursively_test_1").test_dir_name(); + + nu!( + _output, + cwd("tests/fixtures/nuplayground"), + "rm rm_directory_removal_recursively_test_1" + ); + + let expected = format!("{}/{}", Playground::root(), sandbox); + + assert!(!h::file_exists_at(PathBuf::from(expected))); +} + +#[test] +fn rm_removes_directory_contents_with_recursive_flag() { + let sandbox = Playground::setup_for("rm_directory_removal_recursively_test_2") + .with_files(vec![ + EmptyFile("yehuda.txt"), + EmptyFile("jonathan.txt"), + EmptyFile("andres.txt"), + ]) + .test_dir_name(); + + nu!( + _output, + cwd("tests/fixtures/nuplayground"), + "rm rm_directory_removal_recursively_test_2 --recursive" + ); + + let expected = format!("{}/{}", Playground::root(), sandbox); + + assert!(!h::file_exists_at(PathBuf::from(expected))); +} + +#[test] +fn rm_errors_if_attempting_to_delete_a_directory_with_content_without_recursive_flag() { + let sandbox = Playground::setup_for("rm_prevent_directory_removal_without_flag_test") + .with_files(vec![EmptyFile("some_empty_file.txt")]) + .test_dir_name(); + + let full_path = format!("{}/{}", Playground::root(), sandbox); + + nu_error!(output, cwd(&Playground::root()), "rm rm_prevent_directory_removal_without_flag_test"); + + assert!(h::file_exists_at(PathBuf::from(full_path))); + assert!(output.contains("is a directory")); +} + +#[test] +fn rm_errors_if_attempting_to_delete_single_dot_as_argument() { + nu_error!(output, cwd(&Playground::root()), "rm ."); + + assert!(output.contains("may not be removed")); +} + +#[test] +fn rm_errors_if_attempting_to_delete_two_dot_as_argument() { + nu_error!(output, cwd(&Playground::root()), "rm .."); + + assert!(output.contains("may not be removed")); +} \ No newline at end of file diff --git a/tests/commands_test.rs b/tests/commands_test.rs index 748214c83..faced2631 100644 --- a/tests/commands_test.rs +++ b/tests/commands_test.rs @@ -2,7 +2,6 @@ mod helpers; use h::{in_directory as cwd, Playground, Stub::*}; use helpers as h; -use std::path::{Path, PathBuf}; #[test] fn lines() { @@ -139,138 +138,3 @@ fn save_can_write_out_csv() { let actual = h::file_contents(&expected_file); assert!(actual.contains("[list list],A shell for the GitHub era,2018,ISC,nu,0.2.0")); } - -#[test] -fn rm_removes_a_file() { - let sandbox = Playground::setup_for("rm_regular_file_test") - .with_files(vec![EmptyFile("i_will_be_deleted.txt")]) - .test_dir_name(); - - nu!( - _output, - cwd(&Playground::root()), - "rm rm_regular_file_test/i_will_be_deleted.txt" - ); - - let path = &format!( - "{}/{}/{}", - Playground::root(), - sandbox, - "i_will_be_deleted.txt" - ); - - assert!(!h::file_exists_at(PathBuf::from(path))); -} - -#[test] -fn rm_removes_files_with_wildcard() { - r#" - Given these files and directories - src - src/cli.rs - src/lib.rs - src/prelude.rs - src/parser - src/parser/parse.rs - src/parser/parser.rs - src/parser/parse - src/parser/hir - src/parser/parse/token_tree.rs - src/parser/hir/baseline_parse.rs - src/parser/hir/baseline_parse_tokens.rs - "#; - - let sandbox = Playground::setup_for("rm_wildcard_test") - .within("src") - .with_files(vec![ - EmptyFile("cli.rs"), - EmptyFile("lib.rs"), - EmptyFile("prelude.rs"), - ]) - .within("src/parser") - .with_files(vec![EmptyFile("parse.rs"), EmptyFile("parser.rs")]) - .within("src/parser/parse") - .with_files(vec![EmptyFile("token_tree.rs")]) - .within("src/parser/hir") - .with_files(vec![ - EmptyFile("baseline_parse.rs"), - EmptyFile("baseline_parse_tokens.rs"), - ]) - .test_dir_name(); - - let full_path = format!("{}/{}", Playground::root(), sandbox); - - r#" The pattern - src/*/*/*.rs - matches - src/parser/parse/token_tree.rs - src/parser/hir/baseline_parse.rs - src/parser/hir/baseline_parse_tokens.rs - "#; - - nu!( - _output, - cwd("tests/fixtures/nuplayground/rm_wildcard_test"), - "rm \"src/*/*/*.rs\"" - ); - - assert!(!h::files_exist_at( - vec![ - Path::new("src/parser/parse/token_tree.rs"), - Path::new("src/parser/hir/baseline_parse.rs"), - Path::new("src/parser/hir/baseline_parse_tokens.rs") - ], - PathBuf::from(&full_path) - )); - - assert_eq!( - Playground::glob_vec(&format!("{}/src/*/*/*.rs", &full_path)), - Vec::::new() - ); -} - -#[test] -fn rm_removes_directory_contents_with_recursive_flag() { - let sandbox = Playground::setup_for("rm_directory_removal_recursively_test") - .with_files(vec![ - EmptyFile("yehuda.txt"), - EmptyFile("jonathan.txt"), - EmptyFile("andres.txt"), - ]) - .test_dir_name(); - - nu!( - _output, - cwd("tests/fixtures/nuplayground"), - "rm rm_directory_removal_recursively_test --recursive" - ); - - let expected = format!("{}/{}", Playground::root(), sandbox); - - assert!(!h::file_exists_at(PathBuf::from(expected))); -} - -#[test] -fn rm_errors_if_attempting_to_delete_a_directory_without_recursive_flag() { - let sandbox = Playground::setup_for("rm_prevent_directory_removal_without_flag_test").test_dir_name(); - let full_path = format!("{}/{}", Playground::root(), sandbox); - - nu_error!(output, cwd(&Playground::root()), "rm rm_prevent_directory_removal_without_flag_test"); - - assert!(h::file_exists_at(PathBuf::from(full_path))); - assert!(output.contains("is a directory")); -} - -#[test] -fn rm_errors_if_attempting_to_delete_single_dot_as_argument() { - nu_error!(output, cwd(&Playground::root()), "rm ."); - - assert!(output.contains("may not be removed")); -} - -#[test] -fn rm_errors_if_attempting_to_delete_two_dot_as_argument() { - nu_error!(output, cwd(&Playground::root()), "rm .."); - - assert!(output.contains("may not be removed")); -}