Fix removing symbolic links to directories on Windows

Update the implementation of `rm::remove` to, conditionally on the
target platform, remove symbolic files either:
1. On non-Windows systems always as files (unchanged), or
2. On Windows systems as either a directory or file depending on what
   the link is pointing to.

The latter is necessary for removal of symbolic links on Windows because
otherwise users will get a `PermissionDenied` error.

All tests that were disabled on Windows for this reason previously now
run conditionally on the `test-symlink` test feature, just like all
other symlink-related tests. Except for the unit test named `symlink_to_
dir_at_ location_of_a_dir_toctou`. With the current implementation of
the `remove` function this TOCTOU scenario is handled differently on
Windows from non-Windows systems, but both outcomes are considered okay
(both versions of the test are kept to notice changes/regressions on any
platform).

src/main.rs

@@ -2594,8 +2594,8 @@ mod rm {
 
         #[test]
         #[cfg_attr(
-            all(windows, not(feature = "test-symlink")),
-            ignore = "Only run with the test-symlink feature"
+            any(not(feature = "test-trash"), all(windows, not(feature = "test-symlink"))),
+            ignore = "Only run with the test-trash (and test-symlink on Windows) feature"
         )]
         fn symlink_to_empty_dir() -> TestResult {
             with_test_dir(|test_dir| {
@@ -2693,8 +2693,8 @@ mod rm {
 
         #[test]
         #[cfg_attr(
-            all(windows, not(feature = "test-symlink")),
-            ignore = "Only run with the test-symlink feature"
+            any(not(feature = "test-trash"), all(windows, not(feature = "test-symlink"))),
+            ignore = "Only run with the test-trash (and test-symlink on Windows) feature"
         )]
         fn symlink_to_dir_at_location_of_a_dir_toctou() -> TestResult {
             with_test_dir(|test_dir| {
@@ -2738,7 +2738,16 @@ mod rm {
         let path = entry.path();
         let result = match entry.kind() {
             fs::EntryKind::Dir => remove_dir(path),
-            fs::EntryKind::File | fs::EntryKind::Symlink => remove_file(path),
+            fs::EntryKind::File => remove_file(path),
+            #[cfg(not(windows))]
+            fs::EntryKind::Symlink => remove_file(path),
+            #[cfg(windows)]
+            fs::EntryKind::Symlink => match std::fs::metadata(&path) {
+                Ok(metadata) if metadata.is_dir() => remove_dir(path),
+                Ok(metadata) if metadata.is_file() => remove_file(path),
+                Ok(_) => unreachable!(),
+                Err(err) => Err(err),
+            },
         };
 
         match result {
@@ -2881,7 +2890,10 @@ mod rm {
         }
 
         #[test]
-        #[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+        #[cfg_attr(
+            all(windows, not(feature = "test-symlink")),
+            ignore = "Only run with the test-symlink feature"
+        )]
         fn symlink_to_empty_dir() -> TestResult {
             with_test_dir(|test_dir| {
                 let dir = test_dir.child("dir");
@@ -2903,7 +2915,10 @@ mod rm {
         }
 
         #[test]
-        #[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+        #[cfg_attr(
+            all(windows, not(feature = "test-symlink")),
+            ignore = "Only run with the test-symlink feature"
+        )]
         fn symlink_to_filled_dir() -> TestResult {
             with_test_dir(|test_dir| {
                 let dir = test_dir.child("dir");
@@ -2972,7 +2987,7 @@ mod rm {
         }
 
         #[test]
-        #[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+        #[cfg(not(windows))]
         fn symlink_to_dir_at_location_of_a_dir_toctou() -> TestResult {
             with_test_dir(|test_dir| {
                 let dir = test_dir.child("dir");
@@ -2992,6 +3007,32 @@ mod rm {
                 Ok(())
             })
         }
+
+        #[test]
+        #[cfg(windows)]
+        #[cfg_attr(
+            not(feature = "test-symlink"),
+            ignore = "Only run with the test-symlink feature"
+        )]
+        fn symlink_to_dir_at_location_of_a_dir_toctou() -> TestResult {
+            with_test_dir(|test_dir| {
+                let dir = test_dir.child("dir");
+                dir.create_dir_all()?;
+                let link = test_dir.child("link");
+                link.symlink_to_dir(&dir)?;
+
+                let path = link.path();
+                let entry = fs::test_helpers::new_dir(path);
+
+                let out = remove(entry);
+                assert_eq!(out, Ok(format!("Removed {}", path.display().bold())));
+
+                dir.assert(predicate::path::exists());
+                link.assert(predicate::path::missing());
+
+                Ok(())
+            })
+        }
     }
 
     /// Pretend to dispose of the [`fs::Entry`].

tests/dir_test.rs

@@ -147,7 +147,10 @@ fn symlink_to_file() -> TestResult {
 }
 
 #[test]
-#[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+#[cfg_attr(
+    all(windows, not(feature = "test-symlink")),
+    ignore = "Only run with the test-symlink feature"
+)]
 fn symlink_to_empty_dir() -> TestResult {
     let linkname = "link";
 
@@ -186,7 +189,10 @@ fn symlink_to_empty_dir() -> TestResult {
 }
 
 #[test]
-#[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+#[cfg_attr(
+    all(windows, not(feature = "test-symlink")),
+    ignore = "Only run with the test-symlink feature"
+)]
 fn symlink_to_filled_dir() -> TestResult {
     let linkname = "link";
 

tests/link_test.rs

@@ -96,7 +96,10 @@ fn symlink_to_a_file_remove_file() -> TestResult {
 }
 
 #[test]
-#[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+#[cfg_attr(
+    all(windows, not(feature = "test-symlink")),
+    ignore = "Only run with the test-symlink feature"
+)]
 fn symlink_to_an_empty_dir_remove_link() -> TestResult {
     let linkname = "link";
 
@@ -177,7 +180,10 @@ fn symlink_to_an_empty_dir_remove_dir() -> TestResult {
 }
 
 #[test]
-#[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+#[cfg_attr(
+    all(windows, not(feature = "test-symlink")),
+    ignore = "Only run with the test-symlink feature"
+)]
 fn symlink_to_a_filled_dir_remove_link() -> TestResult {
     let linkname = "link";
 

tests/recursive_test.rs

@@ -214,7 +214,10 @@ fn symlink_to_file() -> TestResult {
 }
 
 #[test]
-#[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+#[cfg_attr(
+    all(windows, not(feature = "test-symlink")),
+    ignore = "Only run with the test-symlink feature"
+)]
 fn symlink_to_empty_dir() -> TestResult {
     let linkname = "link";
 
@@ -253,7 +256,10 @@ fn symlink_to_empty_dir() -> TestResult {
 }
 
 #[test]
-#[cfg_attr(windows, ignore = "TODO: investigate symlink test errors on Windows")]
+#[cfg_attr(
+    all(windows, not(feature = "test-symlink")),
+    ignore = "Only run with the test-symlink feature"
+)]
 fn symlink_to_filled_dir() -> TestResult {
     let linkname = "link";