Rm error priority

src/uu/rm/src/platform/linux.rs

@@ -239,7 +239,7 @@ pub fn safe_remove_dir_recursive(
 
         // Before trying to remove the directory, check if it's actually empty
         // This handles the case where some children weren't removed due to user "no" responses
-        if !is_dir_empty(path) {
+        if !is_dir_empty(path).unwrap_or(false) {
             // Directory is not empty, so we can't/shouldn't remove it
             // In interactive mode, this might be expected if user said "no" to some children
             // In non-interactive mode, this indicates an error (some children couldn't be removed)
@@ -292,7 +292,7 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt
         if is_dir {
             // Ask user if they want to descend into this directory
             if options.interactive == InteractiveMode::Always
-                && !is_dir_empty(&entry_path)
+                && !is_dir_empty(&entry_path).unwrap_or(false)
                 && !prompt_descend(&entry_path)
             {
                 continue;

src/uu/rm/src/rm.rs

@@ -527,8 +527,8 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool {
 ///
 /// `path` must be a directory. If there is an error reading the
 /// contents of the directory, this returns `false`.
-fn is_dir_empty(path: &Path) -> bool {
-    fs::read_dir(path).is_ok_and(|mut iter| iter.next().is_none())
+fn is_dir_empty(path: &Path) -> Result<bool, io::Error> {
+    fs::read_dir(path).map(|mut iter| iter.next().is_none())
 }
 
 #[cfg(unix)]
@@ -599,7 +599,7 @@ fn remove_dir_recursive(
     // Base case 2: this is a non-empty directory, but the user
     // doesn't want to descend into it.
     if options.interactive == InteractiveMode::Always
-        && !is_dir_empty(path)
+        && !is_dir_empty(path).unwrap_or(false)
         && !prompt_descend(path)
     {
         return false;
@@ -685,35 +685,52 @@ fn remove_dir_recursive(
 }
 
 fn handle_dir(path: &Path, options: &Options, progress_bar: Option<&ProgressBar>) -> bool {
-    let mut had_err = false;
-
     let path = clean_trailing_slashes(path);
-    if path_is_current_or_parent_directory(path) {
+
+    // Error priority: Is directory > Dir not empty > preserve-root > current/parent directory
+
+    // Refuse to remove a directory without -r/-R or -d
+    if !options.recursive && !options.dir {
         show_error!(
             "{}",
-            RmError::RefusingToRemoveDirectory(path.as_os_str().to_os_string())
+            RmError::CannotRemoveIsDirectory(path.as_os_str().to_os_string())
+        );
+        return true;
+    }
+
+    // Refuse to remove non-empty directory without -r/-R
+    // If we can't read the directory, fall through to let remove_dir handle the error
+    if !options.recursive && !is_dir_empty(path).unwrap_or(true) {
+        show_error!(
+            "{}: Directory not empty",
+            translate!("rm-error-cannot-remove", "file" => path.quote())
         );
         return true;
     }
 
+    // Preserve root directory
     let is_root = path.has_root() && path.parent().is_none();
-    if options.recursive && (!is_root || !options.preserve_root) {
-        had_err = remove_dir_recursive(path, options, progress_bar);
-    } else if options.dir && (!is_root || !options.preserve_root) {
-        had_err = remove_dir(path, options, progress_bar).bitor(had_err);
-    } else if options.recursive {
+    if is_root && options.preserve_root {
         show_error!("{}", RmError::DangerousRecursiveOperation);
         show_error!("{}", RmError::UseNoPreserveRoot);
-        had_err = true;
-    } else {
+        return true;
+    }
+
+    // Refuse to remove current or parent directories
+    if path_is_current_or_parent_directory(path) {
         show_error!(
             "{}",
-            RmError::CannotRemoveIsDirectory(path.as_os_str().to_os_string())
+            RmError::RefusingToRemoveDirectory(path.as_os_str().to_os_string())
         );
-        had_err = true;
+        return true;
     }
 
-    had_err
+    // Now we can safely remove the directory
+    if options.recursive {
+        remove_dir_recursive(path, options, progress_bar)
+    } else {
+        remove_dir(path, options, progress_bar)
+    }
 }
 
 /// Remove the given directory, asking the user for permission if necessary.

tests/by-util/test_rm.rs

@@ -1217,3 +1217,33 @@ fn test_progress_no_output_on_error() {
         .stderr_contains("cannot remove")
         .stderr_contains("No such file or directory");
 }
+
+/// Test error 'Is directory' comes before 'Dir not empty'
+#[test]
+fn test_error_message_priority1() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.mkdir("d");
+    at.touch("d/f");
+
+    ts.ucmd()
+        .arg("d")
+        .fails()
+        .stderr_is("rm: cannot remove 'd': Is a directory\n");
+}
+
+/// Test error 'Dir not empty' comes before 'Refusing to remove . or ..'
+#[test]
+fn test_error_message_priority2() {
+    let ts = TestScenario::new(util_name!());
+    let at = &ts.fixtures;
+
+    at.touch("f");
+
+    ts.ucmd()
+        .arg("-d")
+        .arg(".")
+        .fails()
+        .stderr_is("rm: cannot remove '.': Directory not empty\n");
+}