From 42bb42a2e1718469133fe3a71ac76c40389bbb30 Mon Sep 17 00:00:00 2001
From: tomoda <40169443+yukitomoda@users.noreply.github.com>
Date: Tue, 2 Jan 2024 22:27:03 +0900
Subject: [PATCH] Fix rm for symlinks pointing to directory on windows (issue
 #11461) (#11463)

<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

- this PR closes #11461

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

Using `std::fs::remove_dir` instead of `std::fs::remove_file` when try
remove symlinks pointing to a directory on Windows.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

none

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

- [x] `cargo fmt --all -- --check` to check standard code formatting
(`cargo fmt --all` applies these changes)
- [x] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used`
to check that you're using the standard code style
- [x] `cargo test --workspace` to check that all tests pass (on Windows
make sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- I got 2 test fails on my Windows devenv; these fails in main branch
too
- `commands::complete::basic` : passed on Ubuntu, failed on Windows (a
bug?)
- `commands::cp::copy_file_with_read_permission`: failed on Windows with
Japanese environment (This test refers error message, so that fails on
environments using a language except for english.)
- [x] `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->

This fix has no changes to user-facing interface.
---
 crates/nu-command/src/filesystem/rm.rs | 22 +++++++++++++++++-----
 crates/nu-command/tests/commands/rm.rs | 13 +++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs
index a05668a43..17ecb6578 100644
--- a/crates/nu-command/src/filesystem/rm.rs
+++ b/crates/nu-command/src/filesystem/rm.rs
@@ -381,11 +381,23 @@ fn rm(
                         {
                             unreachable!()
                         }
-                    } else if metadata.is_file()
-                        || is_socket
-                        || is_fifo
-                        || metadata.file_type().is_symlink()
-                    {
+                    } else if metadata.is_symlink() {
+                        // In Windows, symlink pointing to a directory can be removed using
+                        // std::fs::remove_dir instead of std::fs::remove_file.
+                        #[cfg(windows)]
+                        {
+                            f.metadata().and_then(|metadata| {
+                                if metadata.is_dir() {
+                                    std::fs::remove_dir(&f)
+                                } else {
+                                    std::fs::remove_file(&f)
+                                }
+                            })
+                        }
+
+                        #[cfg(not(windows))]
+                        std::fs::remove_file(&f)
+                    } else if metadata.is_file() || is_socket || is_fifo {
                         std::fs::remove_file(&f)
                     } else {
                         std::fs::remove_dir_all(&f)
diff --git a/crates/nu-command/tests/commands/rm.rs b/crates/nu-command/tests/commands/rm.rs
index 90e697c31..2b9345e2f 100644
--- a/crates/nu-command/tests/commands/rm.rs
+++ b/crates/nu-command/tests/commands/rm.rs
@@ -375,6 +375,19 @@ fn removes_symlink() {
     });
 }
 
+#[test]
+fn removes_symlink_pointing_to_directory() {
+    Playground::setup("rm_symlink_to_directory", |dirs, sandbox| {
+        sandbox.mkdir("test").symlink("test", "test_link");
+
+        nu!(cwd: sandbox.cwd(), "rm test_link");
+
+        assert!(!dirs.test().join("test_link").exists());
+        // The pointed directory should not be deleted.
+        assert!(dirs.test().join("test").exists());
+    });
+}
+
 #[test]
 fn removes_file_after_cd() {
     Playground::setup("rm_after_cd", |dirs, sandbox| {