Skip to content

Fix #11644: Fix broken dotty.tools.io.Path#parent #11662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions compiler/src/dotty/tools/io/Path.scala
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +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 => Directory(jpath)
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
Expand Down
49 changes: 49 additions & 0 deletions compiler/test/dotty/tools/io/PathTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package dotty.tools.io

import org.junit.Test

class PathTest {
// Ref https://github.com/lampepfl/dotty/issues/11644#issuecomment-792457275
@Test def parent(): Unit = {
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) {
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>""")
}
}
7 changes: 7 additions & 0 deletions project/scripts/bootstrapCmdTests
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ clear_out "$OUT"
# check that `scalac -from-tasty` compiles and `scala` runs it
echo "testing ./bin/scalac -from-tasty and scala -classpath"
clear_out "$OUT1"
./bin/scalac "$SOURCE" -d "$OUT"
./bin/scalac -from-tasty -d "$OUT1" "$OUT/$TASTY"
./bin/scala -classpath "$OUT1" "$MAIN" > "$tmp"
test "$EXPECTED_OUTPUT" = "$(cat "$tmp")"
Expand All @@ -57,3 +58,9 @@ grep -qe "def main(args: scala.Array\[scala.Predef.String\]): scala.Unit =" "$tm
echo "testing ./bin/scaladoc"
clear_out "$OUT1"
./bin/scaladoc -project Hello -d "$OUT1" "$OUT/out.jar"

# check compilation when the class/tasty files already exist in the current directory
echo "testing i11644"
cwd=$(pwd)
clear_out "$OUT"
(cd "$OUT" && "$cwd/bin/scalac" "$cwd/tests/pos/i11644.scala" && "$cwd/bin/scalac" "$cwd/tests/pos/i11644.scala")
2 changes: 1 addition & 1 deletion project/scripts/cmdTestsCommon.inc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ tmp=$(mktemp)
clear_out()
{
local out="$1"
rm -rf "$out/*"
rm -rf "$out"/*
}
1 change: 1 addition & 0 deletions tests/pos/i11644.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@main def hello = println("hello, world")