Fix mv <symlink>/ target for various situations. Align error message with GNU.

Author: Vitamin9C

src/uu/mv/locales/en-US.ftl

@@ -37,6 +37,7 @@ mv-error-dangling-symlink = can't determine symlink type, since it is dangling
 mv-error-no-symlink-support = your operating system does not support symlinks
 mv-error-permission-denied = Permission denied
 mv-error-inter-device-move-failed = inter-device move failed: {$from} to {$to}; unable to remove target: {$err}
+mv-error-cannot-move-not-directory = cannot move {$source} to {$target}: Not a directory
 
 # Help messages
 mv-help-force = do not prompt before overwriting

src/uu/mv/locales/fr-FR.ftl

@@ -37,6 +37,7 @@ mv-error-dangling-symlink = impossible de déterminer le type de lien symbolique
 mv-error-no-symlink-support = votre système d'exploitation ne prend pas en charge les liens symboliques
 mv-error-permission-denied = Permission refusée
 mv-error-inter-device-move-failed = échec du déplacement inter-périphérique : {$from} vers {$to} ; impossible de supprimer la cible : {$err}
+mv-error-cannot-move-not-directory = impossible de déplacer {$source} vers {$target} : N'est pas un répertoire
 
 # Messages d'aide
 mv-help-force = ne pas demander avant d'écraser

src/uu/mv/src/error.rs

@@ -26,6 +26,8 @@ pub enum MvError {
     TargetNotADirectory(String),
     #[error("{}", translate!("mv-error-failed-access-not-directory", "path" => .0.clone()))]
     FailedToAccessNotADirectory(String),
+    #[error("{}", translate!("mv-error-cannot-move-not-directory", "source" => .0.clone(), "target" => .1.clone()))]
+    CannotMoveNotADirectory(String, String),
 }
 
 impl UError for MvError {}

src/uu/mv/src/mv.rs

@@ -43,7 +43,7 @@ use uucore::fs::display_permissions_unix;
 use uucore::fs::make_fifo;
 use uucore::fs::{
     MissingHandling, ResolveMode, are_hardlinks_or_one_way_symlink_to_same_file,
-    are_hardlinks_to_same_file, canonicalize, path_ends_with_terminator,
+    are_hardlinks_to_same_file, canonicalize, is_symlink_with_trailing, path_ends_with_terminator,
 };
 #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
 use uucore::fsxattr;
@@ -372,6 +372,8 @@ fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()>
         )
         .into());
     }
+
+    // Path("symlink/").symlink_metadata() will resolve to destination of symlink
     if source.symlink_metadata().is_err() {
         return Err(if path_ends_with_terminator(source) {
             MvError::CannotStatNotADirectory(source.quote().to_string()).into()
@@ -387,6 +389,23 @@ fn handle_two_paths(source: &Path, target: &Path, opts: &Options) -> UResult<()>
         target.is_dir()
     };
 
+    if is_symlink_with_trailing(source) {
+        if !source_is_dir {
+            return Err(MvError::CannotStatNotADirectory(source.quote().to_string()).into());
+        } else if target_is_dir {
+            let target_with_source_filename = match source.file_name() {
+                Some(name) => target.join(name),
+                None => target.to_path_buf(),
+            };
+
+            return Err(MvError::CannotMoveNotADirectory(
+                source.quote().to_string().into(),
+                target_with_source_filename.quote().to_string().into(),
+            )
+            .into());
+        }
+    }
+
     if path_ends_with_terminator(target)
         && (!target_is_dir && !source_is_dir)
         && !opts.no_target_dir

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

@@ -630,6 +630,46 @@ pub fn is_symlink_loop(path: &Path) -> bool {
     false
 }
 
+#[cfg(unix)]
+pub fn is_symlink_with_trailing(path: &Path) -> bool {
+    use std::os::unix::prelude::OsStrExt;
+
+    let bytes = path.as_os_str().as_bytes();
+    // Not reusing path_ends_with_terminator for best performance on unix
+    if bytes.last().is_some_and(|&last| last == b'/') {
+        let len = bytes.len();
+        let stripped = &bytes[..len - 1];
+        return Path::new(OsStr::from_bytes(stripped)).is_symlink();
+    } else {
+        false
+    }
+}
+
+#[cfg(windows)]
+pub fn is_symlink_with_trailing(path: &Path) -> bool {
+    if !path_ends_with_terminator(path) {
+        return false;
+    }
+
+    use std::ffi::OsString;
+    use std::os::windows::ffi::OsStrExt;
+    use std::os::windows::ffi::OsStringExt;
+
+    let mut wides: Vec<u16> = path.as_os_str().encode_wide().collect();
+    // Handle multiple trailing separators on Windows
+    while wides
+        .last()
+        .is_some_and(|&last| last == b'/'.into() || last == b'\\'.into())
+    {
+        wides.pop();
+    }
+    if wides.is_empty() {
+        return false;
+    }
+    let stripped = OsString::from_wide(&wides);
+    std::path::Path::new(&stripped).is_symlink()
+}
+
 #[cfg(not(unix))]
 // Hard link comparison is not supported on non-Unix platforms
 pub fn are_hardlinks_to_same_file(_source: &Path, _target: &Path) -> bool {

tests/by-util/test_mv.rs

@@ -2856,3 +2856,82 @@ fn test_mv_no_prompt_unwriteable_file_with_no_tty() {
     assert!(!at.file_exists("source_notty"));
     assert!(at.file_exists("target_notty"));
 }
+
+#[test]
+fn test_mv_dir_symlink_slash_to_dest_dir() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir("foo");
+    at.symlink_dir("foo", "symlink");
+
+    ucmd.arg("symlink/")
+        .arg("foo")
+        .fails()
+        .stderr_contains("cannot move 'symlink/' to 'foo/symlink': Not a directory");
+}
+
+#[test]
+fn test_mv_dir_symlink_slash_to_another_dir() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir("foo");
+    at.mkdir("target");
+    at.symlink_dir("foo", "symlink_foo");
+
+    ucmd.arg("symlink_foo/")
+        .arg("target")
+        .fails()
+        .stderr_contains("cannot move 'symlink_foo/' to 'target/symlink_foo': Not a directory");
+}
+
+#[test]
+fn test_mv_file_symlink_slash_to_dir() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.touch("a");
+    at.mkdir("target");
+    at.symlink_file("a", "symlink_a");
+
+    ucmd.arg("symlink/")
+        .arg("target")
+        .fails()
+        .stderr_contains("cannot stat 'symlink/': Not a directory");
+}
+
+#[test]
+fn test_mv_dir_symlink_slash_to_file() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.touch("a");
+    at.mkdir("foo");
+    at.symlink_dir("foo", "symlink_foo");
+
+    ucmd.arg("symlink_foo/")
+        .arg("a")
+        .fails()
+        .stderr_contains("cannot overwrite non-directory 'a' with directory 'symlink_foo/'");
+}
+
+#[test]
+fn test_mv_file_symlink_slash_to_dest_file() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.touch("a");
+    at.symlink_dir("a", "symlink_a");
+
+    ucmd.arg("symlink_a/")
+        .arg("a")
+        .fails()
+        .stderr_contains("cannot stat 'symlink_a/': Not a directory");
+}
+#[test]
+fn test_mv_file_symlink_slash_to_another_file() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.touch("a");
+    at.touch("b");
+    at.symlink_dir("a", "symlink_a");
+
+    ucmd.arg("symlink_a/")
+        .arg("b")
+        .fails()
+        .stderr_contains("cannot stat 'symlink_a/': Not a directory");
+}