Don't normalize paths in Path#parent

Calling java.nio.file.Path#normalize may result in resolving to a
different path than intended, as in the case where the given path
contains a `..` element and the preceding name is symbolic link.

Author: griggt

compiler/src/dotty/tools/io/Path.scala

@@ -126,17 +126,33 @@ class Path private[io] (val jpath: JPath) {
   /**
    * @return The path of the parent directory, or root if path is already root
    */
-  def parent: Directory = jpath.normalize.getParent match {
-    case null =>
-      if (isAbsolute)  // it should be a root
-        toDirectory
-      else if (segments.nonEmpty && segments.last == "..")
-        (this / "..").toDirectory
-      else path match {
-        case "" | "." => Directory("..")
-        case _        => Directory(".")
-      }
-    case parent => Directory(parent)
+  def parent: Directory = {
+    // We don't call JPath#normalize here because it may result in resolving
+    // to a different path than intended, such as when the given path contains
+    // a `..` component and the preceding name is a symbolic link.
+    // https://docs.oracle.com/javase/8/docs/api/java/nio/file/Path.html#normalize--
+    //
+    // Paths ending with `..` or `.` are handled specially here as
+    // JPath#getParent wants to simply strip away that last element.
+    // For the `..` case, we should take care not to fall into the above trap
+    // as the preceding name may be a symbolic link. This leaves little choice
+    // (since we don't want to access the file system) but to return the given
+    // path with `..` appended. By contrast, a `.` as the final path element
+    // is always redundant and should be removed before computing the parent.
+    if path.isEmpty then
+      Directory("..")
+    else if jpath.endsWith("..") then
+      (this / "..").toDirectory
+    else if jpath.endsWith(".") then
+      jpath.getParent match   // strip the trailing `.` element
+        case null => Directory("..")
+        case p    => new Path(p).parent
+    else jpath.getParent match
+      case null =>
+        if isAbsolute then toDirectory  // it should be a root
+        else Directory(".")
+      case x =>
+        Directory(x)
   }
   def parents: List[Directory] = {
     val p = parent

compiler/test/dotty/tools/io/PathTest.scala

@@ -1,25 +1,49 @@
 package dotty.tools.io
 
-import org.junit.Assert._
 import org.junit.Test
 
 class PathTest {
   // Ref https://github.com/lampepfl/dotty/issues/11644#issuecomment-792457275
   @Test def parent(): Unit = {
-    assertEquals(Directory(".."), Path("").parent)
-    assertEquals(Directory(".."), Path(".").parent)
-    assertEquals(Directory("..") / "..", Path("..").parent)
-    assertEquals(Directory("."), Path("foo.txt").parent)
-    assertEquals(Directory("."), (Path(".") / "foo.txt").parent)
-    assertEquals(Directory("."), (Path("bar") / ".").parent)
-    assertEquals(Directory("."), (Path("foo") / ".." / "." / "bar").parent)
-    assertEquals(Directory("baz") / "bar", (Path(".") / "baz" / "bar" / "foo.txt").parent)
+    testParent(Path(""), Directory(".."))
+    testParent(Path("."), Directory(".."))
+    testParent(Path(".") / ".", Directory(".."))
+    testParent(Path(".."), Directory("..") / "..")
+    testParent(Path("..") / ".", Directory("..") / "..")
+    testParent(Path("..") / "..", Directory("..") / ".." / "..")
+    testParent(Path(".") / "..",
+      Directory("..") / "..",
+      Directory(".") / ".." / "..")
+
+    testParent(Path("foo") / ".", Directory("."))
+    testParent(Path("foo") / ".." / "bar", Directory("foo") / "..")
+    testParent(Path("foo") / ".." / "." / "bar", Directory("foo") / ".." / ".")
+
+    testParent(Path("foo.txt"), Directory("."))
+    testParent(Path(".") / "foo.txt", Directory("."))
+    testParent(Path(".") / "baz" / "bar" / "foo.txt",
+      Directory(".") / "baz" / "bar",
+      Directory("baz") / "bar")
 
     for (root <- Path.roots) {
-      val rootDir = Directory(root.path)
-      assertEquals(rootDir, root.parent)
-      assertEquals(rootDir, (root / "foo.txt").parent)
-      assertEquals(rootDir / "baz" / "bar", (root / "baz" / "bar" / "foo.txt").parent)
+      testParent(root, root)
+      testParent(root / ".", root)
+      testParent(root / "..", root / ".." / "..")
+      testParent(root / "foo" / ".", root)
+      testParent(root / "foo.txt", root)
+      testParent(root / "baz" / "bar" / "foo.txt", root / "baz" / "bar")
+      testParent(root / "foo" / "bar" / "..",  root / "foo" / "bar" / ".." / "..")
     }
   }
+
+  /** The parent of a path may have multiple valid non-canonical representations.
+   *  Here we test that the parent of the specified path is among a curated list
+   *  of representations we consider to be valid.
+   */
+  private def testParent(path: Path, expected: Path*): Unit = {
+    val actual = path.parent
+    val some = if (expected.length > 1) " one of" else ""
+    assert(expected.contains(actual),
+      s"""expected$some: ${expected.mkString("<",">, <",">")} but was: <$actual>""")
+  }
 }