Skip to content
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

Correct path manipulation on Windows and UNIX #37

Merged
merged 10 commits into from
Feb 22, 2023
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ target
.gradle
build
/.idea
/.run/
*.iml
out
.DS_Store
Expand Down
13 changes: 13 additions & 0 deletions fixpatches/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,26 @@ 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")
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
password = project.findProperty("gprKey") as String? ?: System.getenv("GITHUB_TOKEN")
}
}
}

kotlin {
jvm()

sourceSets {
val commonMain by getting {
dependencies {
api(libs.okio)
implementation(libs.okio.extras)
implementation(libs.kotlinx.serialization.json)
implementation(libs.sarif4k)
implementation(libs.multiplatform.diff)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
@file:Suppress(
"FILE_IS_TOO_LONG",
"FILE_UNORDERED_IMPORTS", // false positive
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
)

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.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
Expand All @@ -27,11 +38,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
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
*/
@Suppress("TooManyFunctions")
class SarifFixAdapter(
private val sarifFile: Path,
private val targetFiles: List<Path>
private val targetFiles: List<Path>,
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!!
Expand All @@ -40,6 +53,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()
}

Expand Down Expand Up @@ -215,42 +232,92 @@ class SarifFixAdapter(
* @param fileReplacementsList list of replacements from all rules
* @param targetFiles list of target files
*/
private fun applyReplacementsToFiles(fileReplacementsList: List<FileReplacements>, targetFiles: List<Path>): List<Path> = fileReplacementsList.mapNotNull { fileReplacements ->
val targetFile = targetFiles.find {
val fullPathOfFileFromSarif = if (!fileReplacements.filePath.adaptedIsAbsolute()) {
fs.canonicalize(sarifFile.parent!! / fileReplacements.filePath)
@Suppress(
"MaxLineLength",
"TOO_LONG_FUNCTION",
)
private fun applyReplacementsToFiles(
fileReplacementsList: List<FileReplacements>,
targetFiles: List<Path>,
): List<Path> {
if (fileReplacementsList.isEmpty()) {
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
log.warn { "The list of replacements is empty." }
}
if (targetFiles.isEmpty()) {
log.warn { "The list of target files is empty." }
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
}

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" }
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
}

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" }
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}

/**
* 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<Replacement>): 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
Expand Down Expand Up @@ -353,13 +420,29 @@ class SarifFixAdapter(
fileContent.subList(startLine, endLine + 1).clear()
}

/**
* @param startLine the 0-based line number,
*/
private fun applySingleLineFix(
fileContent: MutableList<String>,
insertedContent: String?,
startLine: Int,
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
Expand All @@ -381,4 +464,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) {
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,104 @@

package com.saveourtool.sarifutils.files

import com.saveourtool.okio.absolute
import okio.FileSystem
import okio.IOException
import okio.Path
import kotlin.random.Random

expect val fs: FileSystem

/**
* 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()
}

/**
* Creates a directory, ensuring that all nonexistent parent directories exist
* by creating them first.
*
* If the directory already exists, this function does not throw an exception.
*
* @return this path.
* @throws IOException if an I/O error occurs.
*/
@Throws(IOException::class)
internal fun Path.createDirectories(): Path {
fs.createDirectories(this)
return this
}

/**
* Same as [Path.relativeTo], but doesn't throw an [IllegalArgumentException] if
* `this` and [other] are both absolute paths, but have different file system
* roots.
*
* @param other the other 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 =
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved Hide resolved
try {
relativeTo(other)
} catch (_: IllegalArgumentException) {
this
}

/**
* @param path a path to a file
* @return list of strings from the file
Expand Down Expand Up @@ -52,7 +144,5 @@ internal fun writeContentWithNewLinesToFile(targetFile: Path, content: List<Stri
*/
internal fun createTempDir(prefix: String = "sarifutils-tmp"): Path {
val dirName = "$prefix-${Random.nextInt()}"
return (FileSystem.SYSTEM_TEMPORARY_DIRECTORY / dirName).also {
fs.createDirectories(it)
}
return (FileSystem.SYSTEM_TEMPORARY_DIRECTORY / dirName).createDirectories()
}
Loading