Skip to content

Commit

Permalink
Correct path manipulation on Windows and UNIX (#37)
Browse files Browse the repository at this point in the history
This fixes: 

- [X] `Path` to `URI` and `URI` to `Path` conversions
- [X] Path manipulation issues (comparision, relative to absolute conversion, etc.)
- [X] Path relativization issues (w.r.t. the test root)
- [X] Attempts to copy a file onto itself
- [X] Issues when line numbers from SARIF exceed the actual line count
  • Loading branch information
0x6675636b796f75676974687562 authored Feb 22, 2023
1 parent 7cf2ba4 commit 40e16d6
Show file tree
Hide file tree
Showing 8 changed files with 701 additions and 24 deletions.
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")
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
)

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
*/
@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()) {
log.warn { "The list of replacements is empty." }
}
if (targetFiles.isEmpty()) {
log.warn { "The list of target files 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)
}
}

/**
* 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) {
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 =
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

0 comments on commit 40e16d6

Please sign in to comment.