Fix cd'ing to symlinked directories (#1651)

* fix: absolutize path against its parent if it was a symlink.

On Linux this happens because Rust calls readlink but doesn't canonicalize the resultant path.

* feat: playground function to create symlinks

* fix: use playground dirs

* feat: test for #1631, shift tests names

crates/nu-cli/src/path.rs

@@ -49,12 +49,16 @@ where
     P: AsRef<Path>,
     Q: AsRef<Path>,
 {
-    let canonicalized = absolutize(relative_to, path);
+    let absolutized = absolutize(&relative_to, path);
-    let path = match std::fs::read_link(&canonicalized) {
+    let path = match std::fs::read_link(&absolutized) {
-        Ok(resolved) => resolved,
+        Ok(resolved) => {
+            let parent = absolutized.parent().unwrap_or(&absolutized);
+            absolutize(parent, resolved)
+        }
+
         Err(e) => {
-            if canonicalized.exists() {
+            if absolutized.exists() {
-                canonicalized
+                absolutized
             } else {
                 return Err(e);
             }

crates/nu-cli/tests/commands/cd.rs

@@ -176,9 +176,11 @@ fn filesystem_not_a_directory() {
 
 #[test]
 fn filesystem_directory_not_found() {
+    Playground::setup("cd_test_11", |dirs, _| {
         let actual = nu_error!(
-        cwd: "tests/fixtures",
+            cwd: dirs.test(),
             "cd dir_that_does_not_exist"
+
         );
 
         assert!(
@@ -191,11 +193,31 @@ fn filesystem_directory_not_found() {
             "actual={:?}",
             actual
         );
+    })
+}
+
+#[test]
+fn filesystem_change_directory_to_symlink_relative() {
+    Playground::setup("cd_test_12", |dirs, sandbox| {
+        sandbox.mkdir("foo");
+        sandbox.mkdir("boo");
+        sandbox.symlink("foo", "foo_link");
+
+        let actual = nu!(
+            cwd: dirs.test().join("boo"),
+            r#"
+                cd ../foo_link
+                pwd | echo $it
+            "#
+        );
+
+        assert_eq!(PathBuf::from(actual), dirs.test().join("foo"));
+    })
 }
 
 #[test]
 fn valuesystem_change_from_current_path_using_relative_path() {
-    Playground::setup("cd_test_11", |dirs, sandbox| {
+    Playground::setup("cd_test_13", |dirs, sandbox| {
         sandbox.with_files(vec![FileWithContent(
             "sample.toml",
             r#"
@@ -226,7 +248,7 @@ fn valuesystem_change_from_current_path_using_relative_path() {
 
 #[test]
 fn valuesystem_change_from_current_path_using_absolute_path() {
-    Playground::setup("cd_test_12", |dirs, sandbox| {
+    Playground::setup("cd_test_14", |dirs, sandbox| {
         sandbox.with_files(vec![FileWithContent(
             "sample.toml",
             r#"
@@ -260,7 +282,7 @@ fn valuesystem_change_from_current_path_using_absolute_path() {
 
 #[test]
 fn valuesystem_switch_back_to_previous_working_path() {
-    Playground::setup("cd_test_13", |dirs, sandbox| {
+    Playground::setup("cd_test_15", |dirs, sandbox| {
         sandbox.with_files(vec![FileWithContent(
             "sample.toml",
             r#"
@@ -296,7 +318,7 @@ fn valuesystem_switch_back_to_previous_working_path() {
 
 #[test]
 fn valuesystem_change_from_current_path_using_relative_path_and_dash() {
-    Playground::setup("cd_test_14", |dirs, sandbox| {
+    Playground::setup("cd_test_16", |dirs, sandbox| {
         sandbox
             .with_files(vec![FileWithContent(
                 "sample.toml",
@@ -330,7 +352,7 @@ fn valuesystem_change_from_current_path_using_relative_path_and_dash() {
 
 #[test]
 fn valuesystem_change_current_path_to_parent_path() {
-    Playground::setup("cd_test_15", |dirs, sandbox| {
+    Playground::setup("cd_test_17", |dirs, sandbox| {
         sandbox
             .with_files(vec![FileWithContent(
                 "sample.toml",
@@ -357,7 +379,7 @@ fn valuesystem_change_current_path_to_parent_path() {
 
 #[test]
 fn valuesystem_change_to_a_path_containing_spaces() {
-    Playground::setup("cd_test_17", |dirs, sandbox| {
+    Playground::setup("cd_test_18", |dirs, sandbox| {
         sandbox.with_files(vec![FileWithContent(
             "sample.toml",
             r#"
@@ -382,8 +404,9 @@ fn valuesystem_change_to_a_path_containing_spaces() {
 
 #[test]
 fn valuesystem_path_not_found() {
+    Playground::setup("cd_test_19", |dirs, _| {
         let actual = nu_error!(
-        cwd: "tests/fixtures/formats",
+            cwd: dirs.formats(),
             r#"
             enter cargo_sample.toml
             cd im_a_path_that_does_not_exist
@@ -393,4 +416,5 @@ fn valuesystem_path_not_found() {
 
         assert!(actual.contains("Can not change to path inside"));
         assert!(actual.contains("No such path exists"));
+    })
 }

crates/nu-test-support/src/playground.rs

@@ -95,6 +95,33 @@ impl Playground {
         self
     }
 
+    pub fn symlink(&mut self, from: impl AsRef<Path>, to: impl AsRef<Path>) -> &mut Self {
+        let from = self.cwd.join(from);
+        let to = self.cwd.join(to);
+
+        let create_symlink = {
+            #[cfg(unix)]
+            {
+                std::os::unix::fs::symlink
+            }
+
+            #[cfg(windows)]
+            {
+                if from.is_file() {
+                    std::os::windows::fs::symlink_file
+                } else if from.is_dir() {
+                    std::os::windows::fs::symlink_dir
+                } else {
+                    panic!("symlink from must be a file or dir")
+                }
+            }
+        };
+
+        create_symlink(from, to).expect("can not create symlink");
+        self.back_to_playground();
+        self
+    }
+
     pub fn with_files(&mut self, files: Vec<Stub>) -> &mut Self {
         let endl = fs::line_ending();