mkdir: implement all review feedback

- Fix critical "." component handling bug in recursive directory creation
- Optimize path processing to avoid O(C²) complexity
- Implement matches! macro pattern for idiomatic Rust
- Convert test to use PathBuf instead of String
- Move Windows-specific comment to correct function location
- Add comprehensive test coverage for "." and ".." path components
- Remove redundant test comments per code review feedback

Author: naoNao89

src/uu/mkdir/src/mkdir.rs

@@ -8,6 +8,7 @@
 use clap::builder::ValueParser;
 use clap::parser::ValuesRef;
 use clap::{Arg, ArgAction, ArgMatches, Command};
+use std::collections::VecDeque;
 use std::ffi::OsString;
 use std::path::{Path, PathBuf};
 #[cfg(not(windows))]
@@ -218,7 +219,6 @@ fn chmod(_path: &Path, _mode: u32) -> UResult<()> {
 
 // Create a directory at the given path.
 // Uses iterative approach instead of recursion to avoid stack overflow with deep nesting.
-// `is_parent` argument is not used on windows
 #[allow(unused_variables)]
 fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
     let path_exists = path.exists();
@@ -235,24 +235,22 @@ fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
     // Iterative implementation: collect all directories to create, then create them
     // This avoids stack overflow with deeply nested directories
     if config.recursive {
-        // Collect all parent directories that need to be created
-        let mut dirs_to_create = Vec::new();
+        // Collect all parent directories to process
+        let mut dirs_to_process = VecDeque::new();
         let mut current = path;
 
-        // Walk up the tree collecting non-existent directories
+        // Walk up the tree collecting all parent directories
         while let Some(parent) = current.parent() {
-            if parent == Path::new("") || parent.exists() {
+            if parent == Path::new("") {
                 break;
             }
-            dirs_to_create.push(parent);
+
+            dirs_to_process.push_front(parent);
             current = parent;
         }
 
-        // Reverse to create from root to leaf
-        dirs_to_create.reverse();
-
-        // Create each parent directory
-        for dir in dirs_to_create {
+        // Process each parent directory (already in correct order)
+        for dir in dirs_to_process {
             create_single_dir(dir, true, config)?;
         }
     }
@@ -262,6 +260,7 @@ fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
 }
 
 // Helper function to create a single directory with appropriate permissions
+// `is_parent` argument is not used on windows
 #[allow(unused_variables)]
 fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
     let path_exists = path.exists();
@@ -313,7 +312,24 @@ fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<(
             Ok(())
         }
 
-        Err(_) if path.is_dir() => Ok(()),
+        Err(_) if path.is_dir() => {
+            // Directory already exists - check if this is a logical directory creation
+            // (i.e., not just a parent reference like "test_dir/..")
+            let ends_with_dotdot = matches!(
+                path.components().next_back(),
+                Some(std::path::Component::ParentDir)
+            );
+
+            // Print verbose message for logical directories, even if they exist
+            // This matches GNU behavior for paths like "test_dir/../test_dir_a"
+            if config.verbose && is_parent && config.recursive && !ends_with_dotdot {
+                println!(
+                    "{}",
+                    translate!("mkdir-verbose-created-directory", "util_name" => uucore::util_name(), "path" => path.quote())
+                );
+            }
+            Ok(())
+        }
         Err(e) => Err(e.into()),
     }
 }

tests/by-util/test_mkdir.rs

@@ -441,17 +441,94 @@ fn test_mkdir_deep_nesting() {
     // Create a path with 350 levels of nesting
     let depth = 350;
     let dir_name = "d";
-    let mut path = String::new();
-    for i in 0..depth {
-        if i > 0 {
-            path.push('/');
-        }
-        path.push_str(dir_name);
+    let mut path = std::path::PathBuf::new();
+    for _ in 0..depth {
+        path.push(dir_name);
     }
 
-    // This should succeed without stack overflow
     scene.ucmd().arg("-p").arg(&path).succeeds();
 
-    // Verify the deepest directory exists
     assert!(at.dir_exists(&path));
 }
+
+#[test]
+fn test_mkdir_dot_components() {
+    // Test handling of "." (current directory) components
+    // This addresses the critical bug identified in asder8215's review
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    // Test case from review comment: test/././test2/././test3/././test4
+    // GNU mkdir normalizes this path and creates test/test2/test3/test4
+    let path = "test/././test2/././test3/././test4";
+    scene.ucmd().arg("-p").arg(path).succeeds();
+
+    // Verify expected structure exists (GNU compatibility)
+    assert!(at.dir_exists("test/test2/test3/test4"));
+
+    // Test leading "." - should create test_dot/test_dot2
+    scene
+        .ucmd()
+        .arg("-p")
+        .arg("./test_dot/test_dot2")
+        .succeeds();
+    assert!(at.dir_exists("test_dot/test_dot2"));
+
+    // Test mixed "." and normal components
+    scene
+        .ucmd()
+        .arg("-p")
+        .arg("mixed/./normal/./path")
+        .succeeds();
+    assert!(at.dir_exists("mixed/normal/path"));
+
+    // Test that the command works without creating redundant directories
+    // The key test is that it doesn't fail or create incorrect structure
+    // The actual filesystem behavior (whether literal "." dirs exist)
+    // may vary, but the logical result should be correct
+}
+
+#[test]
+fn test_mkdir_dotdot_components() {
+    // Test handling of ".." (parent directory) components
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.mkdir("base");
+    at.mkdir("base/child");
+
+    scene
+        .ucmd()
+        .arg("-p")
+        .arg("base/child/../sibling")
+        .succeeds();
+    assert!(at.dir_exists("base/sibling"));
+
+    scene
+        .ucmd()
+        .arg("-p")
+        .arg("base/child/../../other")
+        .succeeds();
+    assert!(at.dir_exists("other"));
+
+    scene
+        .ucmd()
+        .arg("-p")
+        .arg("newbase/newchild/../newsibling")
+        .succeeds();
+    assert!(at.dir_exists("newbase/newsibling"));
+}
+
+#[test]
+fn test_mkdir_mixed_special_components() {
+    // Test complex paths with both "." and ".." components
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    scene
+        .ucmd()
+        .arg("-p")
+        .arg("./start/./middle/../end/./final")
+        .succeeds();
+    assert!(at.dir_exists("start/end/final"));
+}