From dcd1121b80713950af6005867ce2094b4a89f2c8 Mon Sep 17 00:00:00 2001 From: "Konstantin.Nisht" Date: Thu, 2 Jan 2025 19:07:06 +0100 Subject: [PATCH] [eel] IJ-CR-151436: Address review feedback GitOrigin-RevId: cbbb0406ddbcdcbe59813cc831af79a368c08ab1 --- .../intellij/platform/eel/fs/EelFileInfo.kt | 21 +++++++++++-- .../platform/eel/fs/EelPosixFileInfoImpl.kt | 2 +- .../eel/path/ArrayListEelAbsolutePath.kt | 13 +++++++- .../com/intellij/platform/eel/path/EelPath.kt | 31 +++++++++++++++---- .../platform/eel/path/EelAbsolutePathTest.kt | 26 ++++++++++++++++ .../community/impl/nio/IjentNioFileSystem.kt | 3 +- .../impl/nio/IjentNioFileSystemProvider.kt | 6 +++- .../ijent/community/impl/nio/IjentNioPath.kt | 13 +++++--- 8 files changed, 99 insertions(+), 16 deletions(-) diff --git a/platform/eel/src/com/intellij/platform/eel/fs/EelFileInfo.kt b/platform/eel/src/com/intellij/platform/eel/fs/EelFileInfo.kt index 26c26b7c9d2a..1269d8f834ed 100644 --- a/platform/eel/src/com/intellij/platform/eel/fs/EelFileInfo.kt +++ b/platform/eel/src/com/intellij/platform/eel/fs/EelFileInfo.kt @@ -53,19 +53,36 @@ interface EelPosixFileInfo : EelFileInfo { interface Unresolved : Symlink sealed interface Resolved : Symlink { + /** + * This instance is returned in the following scenario: + * ```sh + * /tmp/d$ ls -l + * lrwxrwxrwx 1 knisht knisht 3 Dec 24 18:43 link -> /tmp/p1/p2/d1/.././d5 + * ``` + */ interface Absolute : Resolved { val result: EelPath } + + /** + * This instance is returned in each of these scenarios (for `link`, `link2`, `link3`): + * ```sh + * /tmp/d$ ls -l + * drwxr-xr-x 2 knisht knisht 4096 Dec 24 18:45 d1 + * lrwxrwxrwx 1 knisht knisht 3 Dec 24 18:43 link -> ../ # result == ".." + * lrwxrwxrwx 1 knisht knisht 3 Dec 24 18:43 link2 -> ./ # result == "." + * lrwxrwxrwx 1 knisht knisht 3 Dec 24 18:43 link3 -> d1/d3 # result == "d1/d3" + * ``` + */ interface Relative : Resolved { - val result: List + val result: String } } } } - interface Permissions : EelFileInfo.Permissions { /** TODO */ val owner: Int diff --git a/platform/eel/src/com/intellij/platform/eel/fs/EelPosixFileInfoImpl.kt b/platform/eel/src/com/intellij/platform/eel/fs/EelPosixFileInfoImpl.kt index 76edf2234963..6a5c5e9e828e 100644 --- a/platform/eel/src/com/intellij/platform/eel/fs/EelPosixFileInfoImpl.kt +++ b/platform/eel/src/com/intellij/platform/eel/fs/EelPosixFileInfoImpl.kt @@ -16,7 +16,7 @@ data class EelPosixFileInfoImpl( data class Directory(override val sensitivity: EelFileInfo.CaseSensitivity) : EelFileInfo.Type.Directory data class Regular(override val size: Long) : EelFileInfo.Type.Regular data class SymlinkResolvedAbsolute(override val result: EelPath) : EelPosixFileInfo.Type.Symlink.Resolved.Absolute - data class SymlinkResolvedRelative(override val result: List) : EelPosixFileInfo.Type.Symlink.Resolved.Relative + data class SymlinkResolvedRelative(override val result: String) : EelPosixFileInfo.Type.Symlink.Resolved.Relative data object SymlinkUnresolved : EelPosixFileInfo.Type.Symlink.Unresolved data object Other : EelFileInfo.Type.Other diff --git a/platform/eel/src/com/intellij/platform/eel/path/ArrayListEelAbsolutePath.kt b/platform/eel/src/com/intellij/platform/eel/path/ArrayListEelAbsolutePath.kt index 1c39cd120d60..95568edf8308 100644 --- a/platform/eel/src/com/intellij/platform/eel/path/ArrayListEelAbsolutePath.kt +++ b/platform/eel/src/com/intellij/platform/eel/path/ArrayListEelAbsolutePath.kt @@ -25,6 +25,10 @@ internal class ArrayListEelAbsolutePath private constructor( root.fileName == other.root.fileName && (0..): Boolean { + return nameCount >= suffix.size && this.parts.takeLast(suffix.size) == suffix + } + override fun normalize(): EelPath { val result = mutableListOf() for (part in parts) { @@ -44,7 +48,8 @@ internal class ArrayListEelAbsolutePath private constructor( } override fun resolve(other: String): EelPath { - val otherParts = other.split('/', '\\') + val delimiters = this.os.directorySeparators + val otherParts = other.split(*delimiters).filter(String::isNotEmpty) for (name in otherParts) { if (name.isNotEmpty()) { val error = checkFileName(name) @@ -107,6 +112,12 @@ internal class ArrayListEelAbsolutePath private constructor( return nameCount - other.nameCount } + override val os: EelPath.OS + get() = when (this._root) { + Root.Unix -> EelPath.OS.UNIX + is Root.Windows -> EelPath.OS.WINDOWS + } + override fun equals(other: Any?): Boolean = other is EelPath && nameCount == other.nameCount && diff --git a/platform/eel/src/com/intellij/platform/eel/path/EelPath.kt b/platform/eel/src/com/intellij/platform/eel/path/EelPath.kt index 05a1a2e02b19..414cdb6d9508 100644 --- a/platform/eel/src/com/intellij/platform/eel/path/EelPath.kt +++ b/platform/eel/src/com/intellij/platform/eel/path/EelPath.kt @@ -19,7 +19,7 @@ sealed interface EelPath { @Throws(EelPathException::class) @JvmStatic fun parse(raw: String, os: OS?): EelPath { - return ArrayListEelAbsolutePath.parseOrNull(raw, os) ?: throw EelPathException(raw, "Invalid absolute path") + return ArrayListEelAbsolutePath.parseOrNull(raw, os) ?: throw EelPathException(raw, "Not a valid absolute path") } @Throws(EelPathException::class) @@ -42,10 +42,11 @@ sealed interface EelPath { /** - * Returns parts of a path composed as a list. + * Returns a path that corresponds to the root. * * ```kotlin * EelPath.parse("C:\\a\\b\\c").root == EelPath.parse("C:\\") + * EelPath.parse("/a/b/c").root == EelPath.parse("/") * ``` */ val root: EelPath @@ -56,6 +57,7 @@ sealed interface EelPath { * * ```kotlin * EelPath.parse("/abc/def/ghi", OS.UNIX).parts == listOf("abc", "def", "ghi") + * EelPath.parse("C:\\abc\\def\\ghi", OS.WINDOWS).parts == listOf("abc", "def", "ghi") * ``` */ val parts: List @@ -119,6 +121,19 @@ sealed interface EelPath { /** See [java.nio.file.Path.startsWith] */ fun startsWith(other: EelPath): Boolean + /** See [java.nio.file.Path.endsWith] */ + fun endsWith(suffix: List): Boolean + + /** + * Returns [EelPath.OS] that corresponds to this path. + * + * ```kotlin + * EelPath.parse("/abc/").os == EelPath.OS.UNIX + * EelPath.parse("C:\\abc\").os == EelPath.OS.WINDOWS + * ``` + */ + val os: OS + /** * ```kotlin * EelPath.parse("/abc", OS.UNIX).getChild("..") == EelPath.parse("abc/..", false) @@ -154,10 +169,14 @@ val EelPlatform.pathOs: OS is EelPlatform.Windows -> OS.WINDOWS } +private val UNIX_DIRECTORY_SEPARATORS = charArrayOf('/') +private val WINDOWS_DIRECTORY_SEPARATORS = charArrayOf('/', '\\') -interface EelPathError { - val raw: String - val reason: String +val OS.directorySeparators: CharArray + get() = when (this) { + OS.UNIX -> UNIX_DIRECTORY_SEPARATORS + OS.WINDOWS -> WINDOWS_DIRECTORY_SEPARATORS } -class EelPathException(override val raw: String, override val reason: String) : RuntimeException("`$raw`: $reason"), EelPathError + +class EelPathException(val raw: String, val reason: String) : RuntimeException("`$raw`: $reason") diff --git a/platform/eel/tests/com/intellij/platform/eel/path/EelAbsolutePathTest.kt b/platform/eel/tests/com/intellij/platform/eel/path/EelAbsolutePathTest.kt index 50193b344f8b..bb05030f181e 100644 --- a/platform/eel/tests/com/intellij/platform/eel/path/EelAbsolutePathTest.kt +++ b/platform/eel/tests/com/intellij/platform/eel/path/EelAbsolutePathTest.kt @@ -4,6 +4,7 @@ package com.intellij.platform.eel.path import io.kotest.matchers.shouldBe import org.junit.jupiter.api.DynamicTest import org.junit.jupiter.api.DynamicTest.dynamicTest +import org.junit.jupiter.api.Test import org.junit.jupiter.api.TestFactory class EelAbsolutePathTest { @@ -38,4 +39,29 @@ class EelAbsolutePathTest { }) } } + + @TestFactory + fun `os-dependent separators`(): List = buildList { + val unixPath = EelPath.parse("/", null) + val windowsPath = EelPath.parse("C:\\", null) + val parts = listOf(Triple("a/b/c/d", listOf("a", "b", "c", "d"), listOf("a", "b", "c", "d")), + Triple("a\\b\\c\\d", listOf("a\\b\\c\\d"), listOf("a", "b", "c", "d")), + Triple("a\\b/c\\d", listOf("a\\b", "c\\d"), listOf("a", "b", "c", "d"))) + for ((resolvable, unixAnswer, windowsAnswer) in parts) { + add(dynamicTest("unix: $resolvable") { + unixPath.resolve(resolvable).parts shouldBe unixAnswer + }) + add(dynamicTest("windows: $resolvable") { + windowsPath.resolve(resolvable).parts shouldBe windowsAnswer + }) + } + } + + @Test + fun endsWith() { + val path = EelPath.parse("C:\\foo\\bar\\baz", null) + path.endsWith(listOf("bar", "baz")) shouldBe true + path.endsWith(listOf("bar", "baz", "qux")) shouldBe false + path.endsWith(listOf("C:", "foo", "bar", "bax")) shouldBe false + } } \ No newline at end of file diff --git a/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileSystem.kt b/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileSystem.kt index 56493d356c82..b6eeee06d9fe 100644 --- a/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileSystem.kt +++ b/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileSystem.kt @@ -3,6 +3,7 @@ package com.intellij.platform.ijent.community.impl.nio import com.intellij.platform.eel.path.EelPath import com.intellij.platform.eel.path.EelPathException +import com.intellij.platform.eel.path.directorySeparators import com.intellij.platform.ijent.fs.IjentFileSystemApi import com.intellij.platform.ijent.fs.IjentFileSystemPosixApi import com.intellij.platform.ijent.fs.IjentFileSystemWindowsApi @@ -74,7 +75,7 @@ class IjentNioFileSystem internal constructor( more.fold(EelPath.parse(first, os)) { path, newPart -> path.resolve(newPart) }.toNioPath() } catch (_: EelPathException) { - RelativeIjentNioPath(first.split('/', '\\') + more, this) + RelativeIjentNioPath(first.split(*os.directorySeparators) + more, this) } } diff --git a/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileSystemProvider.kt b/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileSystemProvider.kt index b8af909aceec..ad1a76b29bed 100644 --- a/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileSystemProvider.kt +++ b/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioFileSystemProvider.kt @@ -8,6 +8,7 @@ import com.intellij.platform.eel.fs.EelFileInfo.Type.* import com.intellij.platform.eel.fs.EelFileSystemApi.ReplaceExistingDuringMove.* import com.intellij.platform.eel.fs.EelPosixFileInfo.Type.Symlink import com.intellij.platform.eel.impl.fs.EelFsResultImpl +import com.intellij.platform.eel.path.directorySeparators import com.intellij.platform.eel.provider.utils.getOrThrowFileSystemException import com.intellij.platform.eel.provider.utils.throwFileSystemException import com.intellij.platform.ijent.community.impl.nio.IjentNioFileSystemProvider.Companion.newFileSystemMap @@ -572,11 +573,14 @@ class IjentNioFileSystemProvider : FileSystemProvider() { override fun readSymbolicLink(link: Path): Path { val fs = ensureAbsoluteIjentNioPath(link).nioFs val absolutePath = link.eelPath + val os = fs.ijentFs.pathOs return fsBlocking { when (val ijentFs = fs.ijentFs) { is IjentFileSystemPosixApi -> when (val type = ijentFs.stat(absolutePath, EelFileSystemApi.SymlinkPolicy.JUST_RESOLVE).getOrThrowFileSystemException().type) { is Symlink.Resolved.Absolute -> AbsoluteIjentNioPath(type.result, link.nioFs, null) - is Symlink.Resolved.Relative -> RelativeIjentNioPath(type.result, link.nioFs) + is Symlink.Resolved.Relative -> { + RelativeIjentNioPath(type.result.split(*os.directorySeparators), link.nioFs) + } is Directory, is Regular, is Other -> throw NotLinkException(link.toString()) is Symlink.Unresolved -> error("Impossible, the link should be resolved") } diff --git a/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioPath.kt b/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioPath.kt index 7083af2fd434..e523d89dccbf 100644 --- a/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioPath.kt +++ b/platform/ijent/impl/src/com/intellij/platform/ijent/community/impl/nio/IjentNioPath.kt @@ -88,7 +88,7 @@ internal class AbsoluteIjentNioPath(val eelPath: EelPath, nioFs: IjentNioFileSys createRelativePath(eelPath.getName(index)) override fun subpath(beginIndex: Int, endIndex: Int): IjentNioPath { - TODO("Not yet implemented") + return RelativeIjentNioPath(eelPath.parts.subList(beginIndex, endIndex), nioFs) } override fun startsWith(other: Path): Boolean { @@ -107,7 +107,7 @@ internal class AbsoluteIjentNioPath(val eelPath: EelPath, nioFs: IjentNioFileSys } when (other) { is AbsoluteIjentNioPath -> return eelPath == other.eelPath - is RelativeIjentNioPath -> return eelPath.parts.run { subList(size - other.segments.size, size) } == other.segments + is RelativeIjentNioPath -> return eelPath.endsWith(other.segments) } } @@ -121,7 +121,7 @@ internal class AbsoluteIjentNioPath(val eelPath: EelPath, nioFs: IjentNioFileSys is AbsoluteIjentNioPath -> other is RelativeIjentNioPath -> { val curatedSegments = other.segments.filter { it != "." && it != "" } - curatedSegments.fold(eelPath) { acc, part -> acc.resolve(part) }.toNioPath(curatedSegments.isNotEmpty()) + other.segments.fold(eelPath) { acc, part -> acc.resolve(part) }.toNioPath(curatedSegments.isNotEmpty()) } } } @@ -153,7 +153,12 @@ internal class AbsoluteIjentNioPath(val eelPath: EelPath, nioFs: IjentNioFileSys } override fun toUri(): URI { - return nioFs.uri.resolve(eelPath.toString()) + val prefix = when (eelPath.os) { + EelPath.OS.WINDOWS -> "/" + eelPath.root.toString().replace('\\', '/') + EelPath.OS.UNIX -> null + } + val allParts = listOfNotNull(prefix) + eelPath.parts + return allParts.fold(nioFs.uri, URI::resolve) } override fun toAbsolutePath(): IjentNioPath {