Skip to content

Commit

Permalink
Fix handling of Path.relativze on Windows (#3299)
Browse files Browse the repository at this point in the history
* WindowsPath: Correct detection of the path type and the extraction of a root for DriveRelative and DirectoryRelative paths

* WindowsPath: Correct parent extraction, consider DirectoryRelative

* WindowsPath: Correct path creation in `relativize`

* WindowsPath: Empty path's type should be Relative

* [WIP] `relativize` should throw exceptions on invalid input

* WindowsPath: add unit tests

Tests are based on UnixPathTest with modifications and additions to
for Windows paths.
Tests were verified by running them vs JVM on Windows.
Some additional tests are commented out to mark known issues and help
with future development.

* WindowsPath: better switching of tests on JVM 8; add assertion IDs

Added IDs, so it is possible to identify which assertion is failing
just from the test output.
  • Loading branch information
jpsacha authored and WojciechMazur committed Jun 1, 2023
1 parent 957eaa1 commit daf0e78
Show file tree
Hide file tree
Showing 3 changed files with 728 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ class WindowsPath private[windows] (
case Array(host, share) => share + "\\"
case _ => ""
}
case (PathType.Absolute, Some(root)) => root
case (PathType.DirectoryRelative, Some(root)) => root + "\\"
case (PathType.DriveRelative, _) => "\\"
case _ => ""
case (PathType.Absolute, Some(root)) => root
case (PathType.DirectoryRelative, _) => "\\"
case (PathType.DriveRelative, Some(root)) => root
case _ => ""
}
drivePrefix + segments.mkString(seperator)
}
Expand All @@ -69,7 +69,9 @@ class WindowsPath private[windows] (

override def getParent(): Path = {
val nameCount = getNameCount()
if (nameCount == 0 || (nameCount == 1 && !isAbsolute()))
if (nameCount == 0)
null
else if (nameCount == 1 && pathType != PathType.Absolute && pathType != PathType.DirectoryRelative)
null
else if (root.isDefined)
new WindowsPath(pathType, root, segments.init)
Expand Down Expand Up @@ -164,20 +166,26 @@ class WindowsPath private[windows] (
resolveSibling(WindowsPathParser(other))

override def relativize(other: Path): Path = {
if (isAbsolute() ^ other.isAbsolute()) {
val otherType = other match {
case null => throw new NullPointerException()
case p: WindowsPath => p.pathType
case _ =>
throw new IllegalArgumentException("'other' is different Path class")
}
if (pathType != otherType) {
throw new IllegalArgumentException("'other' is different type of Path")
} else {
val normThis = new WindowsPath(WindowsPath.normalized(this))
val normThis = WindowsPathParser(WindowsPath.normalized(this))
if (normThis.toString.isEmpty()) {
other
} else if (other.startsWith(normThis)) {
other.subpath(getNameCount(), other.getNameCount())
} else if (normThis.getParent() == null) {
new WindowsPath("../" + other.toString())
WindowsPathParser("../" + other.toString())
} else {
val next = normThis.getParent().relativize(other).toString()
if (next.isEmpty()) new WindowsPath("..")
else new WindowsPath("../" + next)
else WindowsPathParser("../" + next)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,23 @@ object WindowsPathParser {
rawPath.size > n && pred(rawPath.charAt(n))
}

val (tpe, root) = if (charAtIdx(0, isSlash)) {
if (charAtIdx(1, isSlash))
UNC -> Some(getUNCRoot(rawPath))
else if (charAtIdx(1, isASCIILetter) && charAtIdx(2, _ == ':'))
// URI specific, absolute path starts with / followed by absolute path
Absolute -> Some(rawPath.substring(1, 4))
else
DriveRelative -> None
} else if (charAtIdx(0, isASCIILetter) && charAtIdx(1, _ == ':')) {
if (charAtIdx(2, isSlash))
Absolute -> Some(rawPath.substring(0, 3))
else
DirectoryRelative -> Some(rawPath.substring(0, 2))
} else Relative -> None
val (tpe, root) =
if (rawPath.isEmpty)
Relative -> None
else if (charAtIdx(0, isSlash)) {
if (charAtIdx(1, isSlash))
UNC -> Some(getUNCRoot(rawPath))
else if (charAtIdx(1, isASCIILetter) && charAtIdx(2, _ == ':'))
// URI specific, absolute path starts with / followed by absolute path
Absolute -> Some(rawPath.substring(1, 4))
else
DirectoryRelative -> Some(rawPath.substring(0, 1))
} else if (charAtIdx(0, isASCIILetter) && charAtIdx(1, _ == ':')) {
if (charAtIdx(2, isSlash))
Absolute -> Some(rawPath.substring(0, 3))
else
DriveRelative -> Some(rawPath.substring(0, 2))
} else Relative -> None

val relativePath = root
.map(r => rawPath.substring(r.length))
Expand Down

0 comments on commit daf0e78

Please sign in to comment.