Fix "mv allows moving a directory into itself" (#2619)

* make sort-by fail gracefully if mismatched types are compared

* Added a test to check if sorted-by with invalid types exists gracefully

* Linter changes

* removed redundant pattern matching

* Changed the error message

* Added a comma after every argument

* Changed the test to accomodate the new err messages

* Err message for sort-by invalid types now shows the mismatched types

* Lints problems

* Changed unwrap to expect

* Added the -f flag to rm command

Now when you a use rm -f there will be no error message, even if the
file doesnt actually exist

* Lint problems

* Fixed the wrong line

* Removed println

* Spelling mistake

* Fix problems when you mv a file into itself

* Lint mistakes

* Remove unecessary filtering in most cases
This commit is contained in:
Luccas Mateus 2020-09-30 22:01:05 -03:00 committed by GitHub
parent b7bc4c1f80
commit 66061192f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 1 deletions

View File

@ -446,7 +446,7 @@ impl Shell for FilesystemShell {
let source = path.join(&src.item);
let destination = path.join(&dst.item);
let sources =
let mut sources =
glob::glob(&source.to_string_lossy()).map_or_else(|_| Vec::new(), Iterator::collect);
if sources.is_empty() {
@ -477,6 +477,29 @@ impl Shell for FilesystemShell {
));
}
let some_if_source_is_destination = sources
.iter()
.find(|f| matches!(f, Ok(f) if destination.starts_with(f)));
if destination.exists() && destination.is_dir() && sources.len() == 1 {
if let Some(Ok(filename)) = some_if_source_is_destination {
return Err(ShellError::labeled_error(
format!(
"Not possible to move {:?} to itself",
filename.file_name().expect("Invalid file name")
),
"cannot move to itself",
dst.tag,
));
}
}
if let Some(Ok(_filename)) = some_if_source_is_destination {
sources = sources
.into_iter()
.filter(|f| matches!(f, Ok(f) if !destination.starts_with(f)))
.collect();
}
for entry in sources {
if let Ok(entry) = entry {
move_file(

View File

@ -257,6 +257,20 @@ fn errors_if_renaming_directory_to_an_existing_file() {
})
}
#[test]
fn errors_if_moving_to_itself() {
Playground::setup("mv_test_10_4", |dirs, sandbox| {
sandbox.mkdir("mydir").mkdir("mydir/mydir_2");
let actual = nu!(
cwd: dirs.test(),
"mv mydir mydir/mydir_2/"
);
assert!(actual.err.contains("cannot move to itself"));
})
}
#[test]
fn does_not_error_on_relative_parent_path() {
Playground::setup("mv_test_11", |dirs, sandbox| {
@ -331,3 +345,17 @@ fn move_file_from_two_parents_up_using_multiple_dots_to_current_dir() {
assert!(!original.exists());
})
}
#[test]
fn does_not_error_when_some_file_is_moving_into_itself() {
Playground::setup("mv_test_13", |dirs, sandbox| {
sandbox.mkdir("11").mkdir("12");
let original_dir = dirs.test().join("11");
let expected = dirs.test().join("12/11");
nu!(cwd: dirs.test(), "mv 1* 12");
assert!(!original_dir.exists());
assert!(expected.exists());
})
}