From 866a142409941451940c21fd50537588a6cc6e4c Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Thu, 16 Feb 2023 20:32:57 +0300 Subject: [PATCH 01/10] Ignore run configurations by default --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7c54fe0..2fd4066 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ target .gradle build /.idea +/.run/ *.iml out .DS_Store From 99fb0af15ac9a5021d0623b131982b664ede7513 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 22 Feb 2023 16:31:37 +0300 Subject: [PATCH 02/10] Add the dependency on okio-extras --- fixpatches/build.gradle.kts | 13 +++++++++++++ gradle/libs.versions.toml | 2 ++ 2 files changed, 15 insertions(+) diff --git a/fixpatches/build.gradle.kts b/fixpatches/build.gradle.kts index 5b235ec..396c287 100644 --- a/fixpatches/build.gradle.kts +++ b/fixpatches/build.gradle.kts @@ -5,6 +5,18 @@ plugins { `maven-publish` } +repositories { + mavenCentral() + maven { + name = "saveourtool/okio-extras" + url = uri("https://maven.pkg.github.com/saveourtool/okio-extras") + credentials { + username = project.findProperty("gprUser") as String? ?: System.getenv("GITHUB_ACTOR") + password = project.findProperty("gprKey") as String? ?: System.getenv("GITHUB_TOKEN") + } + } +} + kotlin { jvm() @@ -12,6 +24,7 @@ kotlin { val commonMain by getting { dependencies { api(libs.okio) + implementation(libs.okio.extras) implementation(libs.kotlinx.serialization.json) implementation(libs.sarif4k) implementation(libs.multiplatform.diff) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 19dfb19..69b61af 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,6 +1,7 @@ [versions] kotlin = "1.8.10" okio = "3.3.0" +okio-extras = "1.0" serialization = "1.4.1" diktat = "1.2.4.1" kotlinx-cli = "0.3.5" @@ -28,6 +29,7 @@ kotlinx-datetime = { module = "org.jetbrains.kotlinx:kotlinx-datetime", version. kotlinx-coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlinx-coroutines" } okio = { module = "com.squareup.okio:okio", version.ref = "okio" } okio-fakefilesystem = { module = "com.squareup.okio:okio-fakefilesystem", version.ref = "okio" } +okio-extras = { module = "com.saveourtool:okio-extras", version.ref = "okio-extras" } kotlinx-cli = { module = "org.jetbrains.kotlinx:kotlinx-cli", version.ref = "kotlinx-cli" } junit-jupiter-engine = { module = "org.junit.jupiter:junit-jupiter-engine", version.ref = "junit" } multiplatform-diff = { module = "io.github.petertrr:kotlin-multiplatform-diff", version.ref = "multiplatform-diff" } From 760c9d80ec4473ddb5a5f6ed3500185c7c25625d Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 22 Feb 2023 16:42:28 +0300 Subject: [PATCH 03/10] Implement `Uri.toLocalPathExt()` --- .../saveourtool/sarifutils/net/UrilUtils.kt | 176 +++++++++++ .../sarifutils/net/UriUtilsTest.kt | 287 ++++++++++++++++++ 2 files changed, 463 insertions(+) create mode 100644 fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/net/UrilUtils.kt create mode 100644 fixpatches/src/commonTest/kotlin/com/saveourtool/sarifutils/net/UriUtilsTest.kt diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/net/UrilUtils.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/net/UrilUtils.kt new file mode 100644 index 0000000..f9edd60 --- /dev/null +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/net/UrilUtils.kt @@ -0,0 +1,176 @@ +@file:JvmName("UriUtils") +@file:Suppress("HEADER_MISSING_IN_NON_SINGLE_CLASS_FILE") + +package com.saveourtool.sarifutils.net + +import com.saveourtool.okio.BACKSLASH +import com.saveourtool.okio.SLASH +import com.saveourtool.okio.URI_UNC_PATH_PREFIX +import com.saveourtool.okio.Uri +import com.saveourtool.okio.backslashify +import com.saveourtool.okio.slashify +import com.saveourtool.okio.toLocalPath +import com.saveourtool.system.OsFamily +import okio.Path +import okio.Path.Companion.toPath +import kotlin.jvm.JvmName + +private typealias UriPath = CharSequence + +/** + * Converts this URI to a local path. + * + * Puts more effort into the conversion than [Uri.toLocalPath], + * covering some extra corner cases. + * + * @return the local path created from this `file://` or UNC URI. + * @throws IllegalArgumentException if the URI contains an absolute path from + * the different platform (e.g.: a Windows path while the current OS is UNIX). + * @see Uri.toLocalPath + */ +@Suppress( + "CyclomaticComplexMethod", + "NestedBlockDepth", + "TOO_LONG_FUNCTION", +) +internal fun Uri.toLocalPathExt(): Path = + when { + isAbsoluteWindowsPath() -> { + val localPath = "$scheme:${path ?: schemeSpecificPart}" + + localPath.requireOsIsWindows() + + localPath.backslashify().toPath() + } + + isAbsolute -> when (val path = path) { + /* + * When a URI is opaque, its path is `null`. + */ + null -> schemeSpecificPart.toPath() + + else -> { + @Suppress("WHEN_WITHOUT_ELSE") + when { + /* + * This is not 100% correct, as a + * normalized UNC URI is indistinguishable + * from a URI that holds an absolute + * UNIX path, e.g.: + * `file:/WSL$/Debian/etc/passwd`. + */ + path.isAbsoluteUnixPath() && authority == null -> path.requireOsIsUnix() + path.isAbsoluteWindowsPath() -> path.requireOsIsWindows() + path.isUncPath() -> path.requireOsIsWindows() + } + + toLocalPath() + } + } + + else -> { + val path = path + + check(path != null) { + "The `path` part of the URI is null: $this" + } + + path.run { + when { + OsFamily.isWindows() -> { + if (isAbsoluteUnixPath()) { + requireOsIsUnix() + } + + backslashify() + } + + else -> { + if (isAbsoluteWindowsPath()) { + requireOsIsWindows() + } + + slashify() + } + }.toPath() + } + } + } + +/** + * Despite this is a misuse of the URI, it may well contain an absolute + * _Windows_ path in the form of `C:/path/to/file.ext`. + * + * @return `true` if this URI holds an absolute _Windows_ path, false otherwise. + */ +private fun Uri.isAbsoluteWindowsPath(): Boolean { + val scheme = scheme + val uriPath = path + + return scheme != null && + scheme.length == 1 && + scheme[0].isWindowsDriveLetter() && + when (uriPath) { + null -> schemeSpecificPart.startsWith(BACKSLASH) + else -> uriPath.startsWith(SLASH) + } +} + +/** + * Applied to the [path][Uri.path] fragment of a URI. Returns `true` if the + * _path_ is an absolute Windows path (e.g.: `C:/autoexec.bat` or + * `/C:/autoexec.bat`). + * + * @return `true` if this is an absolute Windows path, `false` otherwise. + */ +@Suppress( + "MagicNumber", + "MAGIC_NUMBER", + "WRONG_NEWLINES", +) +private fun UriPath.isAbsoluteWindowsPath(): Boolean { + return when { + startsWith(SLASH) && length >= 4 -> subSequence(1..3) + length >= 3 -> subSequence(0..2) + else -> return false + }.let { prefix -> + prefix[0].isWindowsDriveLetter() && + prefix[1] == ':' && + prefix[2] in sequenceOf(SLASH, BACKSLASH) + } +} + +/** + * Applied to the [path][Uri.path] fragment of a URI. Returns `true` if the + * _path_ is a UNC path (e.g.: `//host/share`). + * + * @return `true` if this is a UNC path, `false` otherwise. + */ +private fun UriPath.isUncPath(): Boolean = + startsWith(URI_UNC_PATH_PREFIX) + +/** + * Applied to the [path][Uri.path] fragment of a URI. Returns `true` if the + * _path_ is an absolute UNIX path (e.g.: `/etc/passwd`) and, at the same time, + * is not a slash followed by an absolute Windows path (e.g.: `/C:/autoexec.bat`). + * + * @return `true` if this is an absolute UNIX path, `false` otherwise. + */ +private fun UriPath.isAbsoluteUnixPath(): Boolean = + startsWith(SLASH) && + !isUncPath() && + !isAbsoluteWindowsPath() + +private fun UriPath.requireOsIsWindows() = + require(OsFamily.isWindows()) { + "Current OS is not a Windows; unable to construct an absolute Windows path from \"$this\"" + } + +private fun UriPath.requireOsIsUnix() = + require(OsFamily.isUnix()) { + "Current OS is not a UNIX; unable to construct an absolute UNIX path from \"$this\"" + } + +private fun Char.isWindowsDriveLetter(): Boolean = + this in 'A'..'Z' || + this in 'a'..'z' diff --git a/fixpatches/src/commonTest/kotlin/com/saveourtool/sarifutils/net/UriUtilsTest.kt b/fixpatches/src/commonTest/kotlin/com/saveourtool/sarifutils/net/UriUtilsTest.kt new file mode 100644 index 0000000..2388a2d --- /dev/null +++ b/fixpatches/src/commonTest/kotlin/com/saveourtool/sarifutils/net/UriUtilsTest.kt @@ -0,0 +1,287 @@ +package com.saveourtool.sarifutils.net + +import com.saveourtool.okio.BACKSLASH +import com.saveourtool.okio.SLASH +import com.saveourtool.okio.Uri +import com.saveourtool.okio.absolute +import com.saveourtool.okio.pathString +import com.saveourtool.okio.toFileUri +import com.saveourtool.system.OsFamily +import okio.Path +import okio.Path.Companion.toPath +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class UriUtilsTest { + @BeforeTest + fun before() { + assertFalse(OsFamily.isUnknown(), OsFamily.osName()) + } + + @Test + fun `absolute URIs from absolute paths - platform-independent`() { + sequenceOf( + "path/to/file.ext", + "./path/to/file.ext", + ) + .map { it.toPath() } + .map(Path::toFileUri) + .flatMap { uri -> + sequenceOf( + uri, + uri.normalize(), + ) + } + .distinctBy(Uri::toString) + .map(Uri::toLocalPathExt) + .map(Path::normalized) + .forEach { path -> + assertEquals( + expected = "".toPath().absolute() / "path" / "to" / "file.ext", + actual = path.normalized(), + ) + } + } + + @Test + fun `absolute URIs from relative paths`() { + sequenceOf( + "file:file.ext", + "file:./file.ext", + "file:./path/to/../../file.ext", + ) + .map { Uri(it) } + .forEach { uri -> + assertTrue(uri.isAbsolute) + assertTrue(uri.isOpaque) + assertEquals(expected = "file", actual = uri.scheme) + assertEquals( + expected = "file.ext".toPath(), + actual = uri.toLocalPathExt().normalized() + ) + } + } + + @Test + @Suppress("MaxLineLength") + fun `absolute URIs from absolute Windows paths`() { + sequenceOf( + "file:///C:/autoexec.bat", + "file:/C:/autoexec.bat", + ) + .map { Uri(it) } + .forEach { uri -> + assertTrue(uri.isAbsolute) + assertFalse(uri.isOpaque) + assertEquals(expected = "file", actual = uri.scheme) + + when { + OsFamily.isWindows() -> assertEquals(expected = "C:\\autoexec.bat".toPath(), actual = uri.toLocalPathExt().normalized()) + + else -> assertEquals( + expected = "Current OS is not a Windows; unable to construct an absolute Windows path from \"/C:/autoexec.bat\"", + actual = assertFailsWith { + uri.toLocalPathExt() + }.message, + ) + } + } + } + + @Test + @Suppress("MaxLineLength") + fun `absolute URIs from absolute UNIX paths`() { + sequenceOf( + "file:///etc/passwd", + "file:/etc/passwd", + ) + .map { Uri(it) } + .forEach { uri -> + assertTrue(uri.isAbsolute) + assertFalse(uri.isOpaque) + assertEquals(expected = "file", actual = uri.scheme) + + when { + OsFamily.isUnix() -> assertEquals( + expected = "/etc/passwd".toPath(), + actual = uri.toLocalPathExt().normalized() + ) + + else -> assertEquals( + expected = "Current OS is not a UNIX; unable to construct an absolute UNIX path from \"/etc/passwd\"", + actual = assertFailsWith { + uri.toLocalPathExt() + }.message, + ) + } + } + } + + @Test + fun `relative URIs from relative paths`() { + sequenceOf( + "file.ext", + "./file.ext", + "./path/to/../../file.ext", + ) + .map { Uri(it) } + .forEach { uri -> + assertFalse(uri.isAbsolute) + assertFalse(uri.isOpaque) + assertNull(uri.scheme) + assertEquals( + expected = "file.ext".toPath(), + actual = uri.toLocalPathExt().normalized() + ) + } + } + + @Test + @Suppress("MaxLineLength") + fun `relative URIs from absolute Windows paths`() { + @Suppress("COMMENT_WHITE_SPACE") + sequenceOf( + "C:/autoexec.bat", // forward slash + "C:%5Cautoexec.bat", // backslash + "C%3A%2Fautoexec.bat", // forward slash + "C%3A%5Cautoexec.bat", // backslash + ) + .map { Uri(it) } + .forEach { uri -> + when { + OsFamily.isWindows() -> assertEquals(expected = "C:\\autoexec.bat".toPath(), actual = uri.toLocalPathExt().normalized()) + + else -> assertEquals( + expected = "Current OS is not a Windows; unable to construct an absolute Windows path from \"C:\\autoexec.bat\"", + actual = assertFailsWith { + uri.toLocalPathExt() + }.message?.replace(SLASH, BACKSLASH), + ) + } + } + } + + @Test + @Suppress("MaxLineLength") + fun `relative URIs from absolute UNIX paths`() { + sequenceOf( + "/etc/passwd", + "%2Fetc%2Fpasswd", + ) + .map { Uri(it) } + .forEach { uri -> + when { + OsFamily.isUnix() -> assertEquals( + expected = "/etc/passwd".toPath(), + actual = uri.toLocalPathExt().normalized() + ) + + else -> assertEquals( + expected = "Current OS is not a UNIX; unable to construct an absolute UNIX path from \"/etc/passwd\"", + actual = assertFailsWith { + uri.toLocalPathExt() + }.message, + ) + } + } + } + + @Test + fun `paths with spaces`() { + sequenceOf( + "file:Program%20Files", + "file:./Program%20Files", + "Program%20Files", + "./Program%20Files", + ) + .map { Uri(it) } + .forEach { uri -> + assertEquals( + expected = "Program Files".toPath(), + actual = uri.toLocalPathExt().normalized() + ) + } + } + + @Test + @Suppress("MaxLineLength") + fun `UNC paths with reserved characters`() { + uncPathUrisWithReservedCharacters().forEach { uri -> + when { + OsFamily.isWindows() -> { + val path = uri.toLocalPathExt().normalized() + assertNull(uri.authority) + assertEquals( + expected = "\\\\WSL$\\Debian\\etc\\passwd", + actual = path.pathString + ) + assertNull(path.toFileUri().authority) + } + + else -> assertEquals( + expected = "Current OS is not a Windows; unable to construct an absolute Windows path from \"//WSL$/Debian/etc/passwd\"", + actual = assertFailsWith { + uri.toLocalPathExt() + }.message, + ) + } + } + } + + @Test + @Suppress("MaxLineLength") + fun `UNC paths`() { + uncPathUris().forEach { uri -> + when { + OsFamily.isWindows() -> { + val path = uri.toLocalPathExt().normalized() + assertEquals("127.0.0.1", uri.authority) + assertEquals( + expected = "\\\\127.0.0.1\\share\\file", + actual = path.pathString + ) + assertEquals("127.0.0.1", path.toFileUri().authority) + } + + else -> assertEquals( + expected = "Current OS is not a Windows; unable to construct an absolute Windows path from \"//127.0.0.1/share/file\"", + actual = assertFailsWith { + uri.toLocalPathExt() + }.message, + ) + } + } + } + + private companion object { + private fun uncPathUrisWithReservedCharacters(): Sequence = + when { + OsFamily.isWindows() -> sequenceOf( + "\\\\WSL$\\Debian\\etc\\passwd", + "//WSL$/Debian/etc/passwd", + ) + .map { it.toPath() } + .map(Path::toFileUri) + .distinctBy(Uri::toString) + + else -> sequenceOf(Uri("file:////WSL$/Debian/etc/passwd")) + } + + private fun uncPathUris(): Sequence = + when { + OsFamily.isWindows() -> sequenceOf( + "\\\\127.0.0.1\\share\\file", + ) + .map { it.toPath() } + .map(Path::toFileUri) + .distinctBy(Uri::toString) + + else -> sequenceOf(Uri("file:////127.0.0.1/share/file")) + } + } +} From ccd04e595030831f999d167a1440f44b78463d0a Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 22 Feb 2023 18:02:44 +0300 Subject: [PATCH 04/10] Bump okio-extras version to 1.1 --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 69b61af..c56dfad 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,7 +1,7 @@ [versions] kotlin = "1.8.10" okio = "3.3.0" -okio-extras = "1.0" +okio-extras = "1.1" serialization = "1.4.1" diktat = "1.2.4.1" kotlinx-cli = "0.3.5" From 9241925ae20c209e1346c9bb26d529f2d1daa298 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 22 Feb 2023 19:46:25 +0300 Subject: [PATCH 05/10] Use the library function --- .../kotlin/com/saveourtool/sarifutils/utils/SarifUtils.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/utils/SarifUtils.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/utils/SarifUtils.kt index bb4c499..bb23521 100644 --- a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/utils/SarifUtils.kt +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/utils/SarifUtils.kt @@ -4,6 +4,7 @@ package com.saveourtool.sarifutils.utils +import com.saveourtool.okio.backslashify import io.github.detekt.sarif4k.ArtifactLocation import io.github.detekt.sarif4k.Result import io.github.detekt.sarif4k.Run @@ -32,7 +33,7 @@ internal fun Path.adaptedIsAbsolute(): Boolean { (stringRepresentation.first() in 'a'..'z' || stringRepresentation.first() in 'A'..'Z') && (stringRepresentation[1] == ':') ) { - return stringRepresentation.replace('/', '\\').toPath().isAbsolute + return stringRepresentation.backslashify().toPath().isAbsolute } return this.isAbsolute } From af00b34b3b2d5d8e1c2a6328c2af3183a6fcbe03 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 22 Feb 2023 19:46:53 +0300 Subject: [PATCH 06/10] Add `Path` extensions --- .../saveourtool/sarifutils/files/FileUtils.kt | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt index 8386590..139a660 100644 --- a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt @@ -4,7 +4,9 @@ package com.saveourtool.sarifutils.files +import com.saveourtool.okio.absolute import okio.FileSystem +import okio.IOException import okio.Path import kotlin.random.Random @@ -56,3 +58,62 @@ internal fun createTempDir(prefix: String = "sarifutils-tmp"): Path { fs.createDirectories(it) } } + +/** + * Returns the _real_ path of an existing file. + * + * If this path is relative then its absolute path is first obtained, as if by + * invoking the [Path.absolute] method. + * + * @return an absolute path represent the _real_ path of the file located by + * this object. + * @throws IOException if the file does not exist or an I/O error occurs. + * @see Path.toRealPathSafe + */ +@Throws(IOException::class) +internal fun Path.toRealPath(): Path = + fs.canonicalize(this) + +/** + * Same as [Path.toRealPath], but doesn't throw an exception if the path doesn't + * exist. + * + * @return an absolute path represent the _real_ path of the file located by + * this object, or an absolute normalized path if the file doesn't exist. + * @see Path.toRealPath + */ +internal fun Path.toRealPathSafe(): Path = + try { + toRealPath() + } catch (_: IOException) { + absolute().normalized() + } + +/** + * Checks if the file located by this path points to the same file or directory + * as [other]. + * + * @param other the other path. + * @return `true` if, and only if, the two paths locate the same file. + * @throws IOException if an I/O error occurs. + * @see Path.isSameFileAsSafe + */ +@Throws(IOException::class) +internal fun Path.isSameFileAs(other: Path): Boolean = + this.toRealPath() == other.toRealPath() + +/** + * Checks if the file located by this path points to the same file or directory + * as [other]. Same as [Path.isSameFileAs], but doesn't throw an exception if + * any of the paths doesn't exist. + * + * @param other the other path. + * @return `true` if the two paths locate the same file. + * @see Path.isSameFileAs + */ +internal fun Path.isSameFileAsSafe(other: Path): Boolean = + try { + this.isSameFileAs(other) + } catch (_: IOException) { + this.toRealPathSafe() == other.toRealPathSafe() + } From 2bfa54e509a4c15369102d71c30b82c76de3c6e9 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 22 Feb 2023 20:05:21 +0300 Subject: [PATCH 07/10] Fix path manipulation --- .../sarifutils/adapter/SarifFixAdapter.kt | 68 +++++++++++++++---- 1 file changed, 55 insertions(+), 13 deletions(-) diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt index da1d57d..9eb3cdb 100644 --- a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt @@ -1,13 +1,16 @@ package com.saveourtool.sarifutils.adapter +import com.saveourtool.okio.Uri +import com.saveourtool.okio.pathString import com.saveourtool.sarifutils.config.FileReplacements import com.saveourtool.sarifutils.config.RuleReplacements import com.saveourtool.sarifutils.files.createTempDir import com.saveourtool.sarifutils.files.fs +import com.saveourtool.sarifutils.files.isSameFileAsSafe import com.saveourtool.sarifutils.files.readFile import com.saveourtool.sarifutils.files.readLines import com.saveourtool.sarifutils.files.writeContentWithNewLinesToFile -import com.saveourtool.sarifutils.utils.adaptedIsAbsolute +import com.saveourtool.sarifutils.net.toLocalPathExt import com.saveourtool.sarifutils.utils.getUriBaseIdForArtifactLocation import com.saveourtool.sarifutils.utils.resolveUriBaseId import com.saveourtool.sarifutils.utils.setLoggingLevel @@ -215,20 +218,59 @@ class SarifFixAdapter( * @param fileReplacementsList list of replacements from all rules * @param targetFiles list of target files */ - private fun applyReplacementsToFiles(fileReplacementsList: List, targetFiles: List): List = fileReplacementsList.mapNotNull { fileReplacements -> - val targetFile = targetFiles.find { - val fullPathOfFileFromSarif = if (!fileReplacements.filePath.adaptedIsAbsolute()) { - fs.canonicalize(sarifFile.parent!! / fileReplacements.filePath) + @Suppress("MaxLineLength") + private fun applyReplacementsToFiles( + fileReplacementsList: List, + targetFiles: List, + ): List { + if (fileReplacementsList.isEmpty()) { + log.warn { "The list of replacements is empty." } + } + if (targetFiles.isEmpty()) { + log.warn { "The list of replacements is empty." } + } + + return fileReplacementsList.mapNotNull { fileReplacements -> + val fileUri = fileReplacements.filePath + log.info { "Processing file at URI: $fileUri" } + val localPath = try { + Uri(fileUri.pathString).toLocalPathExt() + } catch (_: IllegalArgumentException) { + /* + * `fileUri` is actually a path, most probably a Windows path. + */ + fileUri + } + + val absolute = localPath.isAbsolute + if (localPath != fileUri) { + log.info { "Resolved the URI to a local path: (absolute = $absolute): $localPath" } + } + + /* + * No need to check whether `localPath` is absolute: if it is, + * `resolve()` will ignore `sarifFile.parent` and return `localPath` + * intact. + */ + val absoluteLocalPath = (sarifFile.parent!! / localPath).normalized() + if (absoluteLocalPath != localPath) { + log.info { "Converted the path: $localPath -> $absoluteLocalPath" } + } + + val matchingFile = targetFiles.find { targetFile -> + targetFile.isSameFileAsSafe(absoluteLocalPath) + } + if (matchingFile == null) { + val targetFileCount = targetFiles.size + log.warn { "None of the $targetFileCount target file(s) matches the file from SARIF replacement: $localPath" } + targetFiles.forEachIndexed { index, targetFile -> + log.warn { "\t${index + 1} of $targetFileCount: $targetFile" } + } + + null } else { - fileReplacements.filePath + applyReplacementsToSingleFile(matchingFile, fileReplacements.replacements) } - fs.canonicalize(it) == fullPathOfFileFromSarif - } - if (targetFile == null) { - log.warn { "Couldn't find appropriate target file on the path ${fileReplacements.filePath}, which provided in Sarif!" } - null - } else { - applyReplacementsToSingleFile(targetFile, fileReplacements.replacements) } } From aaa4a3a41d27f687050060c812a152814bd4d703 Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 22 Feb 2023 22:07:59 +0300 Subject: [PATCH 08/10] Relativize file paths properly --- .../sarifutils/adapter/SarifFixAdapter.kt | 71 +++++++++++++++++-- .../saveourtool/sarifutils/files/FileUtils.kt | 36 +++++++++- 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt index 9eb3cdb..ee8abde 100644 --- a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt @@ -1,14 +1,17 @@ package com.saveourtool.sarifutils.adapter import com.saveourtool.okio.Uri +import com.saveourtool.okio.isDirectory import com.saveourtool.okio.pathString import com.saveourtool.sarifutils.config.FileReplacements import com.saveourtool.sarifutils.config.RuleReplacements +import com.saveourtool.sarifutils.files.createDirectories import com.saveourtool.sarifutils.files.createTempDir import com.saveourtool.sarifutils.files.fs import com.saveourtool.sarifutils.files.isSameFileAsSafe import com.saveourtool.sarifutils.files.readFile import com.saveourtool.sarifutils.files.readLines +import com.saveourtool.sarifutils.files.relativeToSafe import com.saveourtool.sarifutils.files.writeContentWithNewLinesToFile import com.saveourtool.sarifutils.net.toLocalPathExt import com.saveourtool.sarifutils.utils.getUriBaseIdForArtifactLocation @@ -30,11 +33,13 @@ import kotlinx.serialization.json.Json * * @param sarifFile path to the sarif file with fix object replacements * @param targetFiles list of the target files, to which above fixes need to be applied + * @param testRoot the root directory of the test suite. Should be set to a non-`null` value if */ @Suppress("TooManyFunctions") class SarifFixAdapter( private val sarifFile: Path, - private val targetFiles: List + private val targetFiles: List, + private val testRoot: Path? = null, ) { @Suppress("WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES") // https://github.com/saveourtool/diktat/issues/1602 private val classSimpleName = SarifFixAdapter::class.simpleName!! @@ -43,6 +48,10 @@ class SarifFixAdapter( private val log = KotlinLogging.logger(classSimpleName) private val tmpDir = createTempDir(classSimpleName) init { + check(testRoot == null || testRoot.isDirectory()) { + "Test root is not a directory: $testRoot" + } + setLoggingLevel() } @@ -277,22 +286,30 @@ class SarifFixAdapter( /** * Create copy of the target file and apply fixes from sarif * - * @param targetFile target file which need to be fixed + * @param targetFile target file which need to be fixed (may be an absolute + * or a relative path). * @param replacements corresponding replacements for [targetFile] * @return file with applied fixes */ @Suppress("TOO_LONG_FUNCTION") private fun applyReplacementsToSingleFile(targetFile: Path, replacements: List): Path { - val targetFileCopy = tmpDir.resolve(targetFile) + val relativeTargetFile = targetFile + .relativeToTestRoot() + .relativeToFileSystemRoot() + + val targetFileCopy = tmpDir.resolve(relativeTargetFile) // additionally create parent directories, before copy of content - targetFileCopy.parent?.let { - if (!fs.exists(it)) { - fs.createDirectories(it) - } + targetFileCopy.parent?.createDirectories() + + check(!targetFile.isSameFileAsSafe(targetFileCopy)) { + "Refusing to copy $targetFile onto itself." } + fs.copy(targetFile, targetFileCopy) + log.info { "Copied $targetFile -> $targetFileCopy" } val fileContent = readLines(targetFileCopy).toMutableList() + log.info { "Reading $targetFileCopy: ${fileContent.size} line(s) read." } replacements.forEach { replacement -> val startLine = replacement.deletedRegion.startLine!!.toInt() - 1 @@ -395,6 +412,9 @@ class SarifFixAdapter( fileContent.subList(startLine, endLine + 1).clear() } + /** + * @param startLine the 0-based line number, + */ private fun applySingleLineFix( fileContent: MutableList, insertedContent: String?, @@ -402,6 +422,19 @@ class SarifFixAdapter( startColumn: Int?, endColumn: Int? ) { + if (fileContent.isEmpty()) { + log.warn { "Unable to apply the fix at line ${startLine + 1}: the file is empty" } + return + } + + val lineCount = fileContent.size + + if (startLine >= lineCount) { + log.warn { "Unable to apply the fix at line ${startLine + 1}: the file only has $lineCount line(s)." } + return + } + + log.info { "Applying a single-line fix to line ${startLine + 1} out of $lineCount" } insertedContent?.let { content -> if (startColumn != null && endColumn != null) { // replace range @@ -423,4 +456,28 @@ class SarifFixAdapter( private fun Replacement.prettyString(): String = "(startLine: ${this.deletedRegion.startLine}, endLine: ${this.deletedRegion.endLine}, " + "startColumn: ${this.deletedRegion.startColumn}, endColumn: ${this.deletedRegion.endColumn}, insertedContent: ${this.insertedContent})" + + /** + * @return this path, relativized against the [test root][testRoot], + * assuming this path is absolute and [test root][testRoot] is non-`null`. + */ + private fun Path.relativeToTestRoot(): Path = + when (testRoot) { + null -> this + else -> relativeToSafe(testRoot) + } + + private fun Path.relativeToFileSystemRoot(): Path = + when (val root = root) { + null -> this + + /*- + * `root` is the file system root of the this path`, or `null` + * if the path is relative. + * + * On UNIX, this will always be `/`. + * On Windows, this may be `C:\`, `D:\`, etc. + */ + else -> relativeTo(root) + } } diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt index 139a660..bc5f5dc 100644 --- a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt @@ -54,9 +54,7 @@ internal fun writeContentWithNewLinesToFile(targetFile: Path, content: List Date: Wed, 22 Feb 2023 22:22:36 +0300 Subject: [PATCH 09/10] Make Diktat happy --- .../sarifutils/adapter/SarifFixAdapter.kt | 10 +- .../saveourtool/sarifutils/files/FileUtils.kt | 99 +++++++++---------- 2 files changed, 58 insertions(+), 51 deletions(-) diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt index ee8abde..bc334c0 100644 --- a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt @@ -1,3 +1,8 @@ +@file:Suppress( + "FILE_IS_TOO_LONG", + "FILE_UNORDERED_IMPORTS", // false positive +) + package com.saveourtool.sarifutils.adapter import com.saveourtool.okio.Uri @@ -227,7 +232,10 @@ class SarifFixAdapter( * @param fileReplacementsList list of replacements from all rules * @param targetFiles list of target files */ - @Suppress("MaxLineLength") + @Suppress( + "MaxLineLength", + "TOO_LONG_FUNCTION", + ) private fun applyReplacementsToFiles( fileReplacementsList: List, targetFiles: List, diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt index bc5f5dc..12445f9 100644 --- a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/files/FileUtils.kt @@ -12,51 +12,6 @@ import kotlin.random.Random expect val fs: FileSystem -/** - * @param path a path to a file - * @return list of strings from the file - */ -internal fun readLines(path: Path): List = fs.read(path) { - generateSequence { readUtf8Line() }.toList() -} - -/** - * @param path a path to a file - * @return string from the file - */ -internal fun readFile(path: Path): String = fs.read(path) { - this.readUtf8() -} - -/** - * Write [content] to the [targetFile], some of the elements in [content] could represent the multiline strings, - * which already contain all necessary escape characters, in this case, write them as-is, otherwise add newline at the end - * - * @param targetFile file whether to write [content] - * @param content data to be written - * @return [Unit] - */ -internal fun writeContentWithNewLinesToFile(targetFile: Path, content: List) = fs.write(targetFile) { - content.forEach { line -> - if (!line.contains('\n')) { - writeUtf8(line + '\n') - } else { - writeUtf8(line) - } - } -} - -/** - * Create a temporary directory - * - * @param prefix will be prepended to directory name - * @return a [Path] representing the created directory - */ -internal fun createTempDir(prefix: String = "sarifutils-tmp"): Path { - val dirName = "$prefix-${Random.nextInt()}" - return (FileSystem.SYSTEM_TEMPORARY_DIRECTORY / dirName).createDirectories() -} - /** * Returns the _real_ path of an existing file. * @@ -140,10 +95,54 @@ internal fun Path.createDirectories(): Path { * @return this path relativized against [other], * or `this` if this and other have different file system roots. */ -internal fun Path.relativeToSafe(other: Path): Path { - return try { - relativeTo(other) - } catch (_: IllegalArgumentException) { - this +internal fun Path.relativeToSafe(other: Path): Path = + try { + relativeTo(other) + } catch (_: IllegalArgumentException) { + this + } + +/** + * @param path a path to a file + * @return list of strings from the file + */ +internal fun readLines(path: Path): List = fs.read(path) { + generateSequence { readUtf8Line() }.toList() +} + +/** + * @param path a path to a file + * @return string from the file + */ +internal fun readFile(path: Path): String = fs.read(path) { + this.readUtf8() +} + +/** + * Write [content] to the [targetFile], some of the elements in [content] could represent the multiline strings, + * which already contain all necessary escape characters, in this case, write them as-is, otherwise add newline at the end + * + * @param targetFile file whether to write [content] + * @param content data to be written + * @return [Unit] + */ +internal fun writeContentWithNewLinesToFile(targetFile: Path, content: List) = fs.write(targetFile) { + content.forEach { line -> + if (!line.contains('\n')) { + writeUtf8(line + '\n') + } else { + writeUtf8(line) + } } } + +/** + * Create a temporary directory + * + * @param prefix will be prepended to directory name + * @return a [Path] representing the created directory + */ +internal fun createTempDir(prefix: String = "sarifutils-tmp"): Path { + val dirName = "$prefix-${Random.nextInt()}" + return (FileSystem.SYSTEM_TEMPORARY_DIRECTORY / dirName).createDirectories() +} From 64af9402efb34d4e2754b300553ddbc85d38aeba Mon Sep 17 00:00:00 2001 From: Andrey Shcheglov Date: Wed, 22 Feb 2023 22:40:39 +0300 Subject: [PATCH 10/10] Correct the message --- .../com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt index bc334c0..6ee9282 100644 --- a/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt +++ b/fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/adapter/SarifFixAdapter.kt @@ -244,7 +244,7 @@ class SarifFixAdapter( log.warn { "The list of replacements is empty." } } if (targetFiles.isEmpty()) { - log.warn { "The list of replacements is empty." } + log.warn { "The list of target files is empty." } } return fileReplacementsList.mapNotNull { fileReplacements ->