From 66061192f84610b756185bead313289bc3ff6d53 Mon Sep 17 00:00:00 2001
From: Luccas Mateus <Luccasmmg@gmail.com>
Date: Wed, 30 Sep 2020 22:01:05 -0300
Subject: [PATCH] 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
---
 crates/nu-cli/src/shell/filesystem_shell.rs | 25 +++++++++++++++++-
 crates/nu-cli/tests/commands/move_/mv.rs    | 28 +++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/crates/nu-cli/src/shell/filesystem_shell.rs b/crates/nu-cli/src/shell/filesystem_shell.rs
index ebae43532b..76cb0ac230 100644
--- a/crates/nu-cli/src/shell/filesystem_shell.rs
+++ b/crates/nu-cli/src/shell/filesystem_shell.rs
@@ -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(
diff --git a/crates/nu-cli/tests/commands/move_/mv.rs b/crates/nu-cli/tests/commands/move_/mv.rs
index 41c0e99c43..5debb64b38 100644
--- a/crates/nu-cli/tests/commands/move_/mv.rs
+++ b/crates/nu-cli/tests/commands/move_/mv.rs
@@ -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());
+    })
+}