-
Notifications
You must be signed in to change notification settings - Fork 0
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
SarifFixAdapter #4
Changes from 2 commits
15c5697
fbd7661
de88318
cd1d693
3dce90e
2c582be
37c24f9
a18d09c
a1175ee
8aaac64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
package com.saveourtool.sarifutils.cli.adapter | ||
|
||
import com.saveourtool.sarifutils.cli.config.FileReplacements | ||
import com.saveourtool.sarifutils.cli.config.RuleReplacements | ||
import com.saveourtool.sarifutils.cli.files.copyFileContent | ||
import com.saveourtool.sarifutils.cli.files.createTempDir | ||
import com.saveourtool.sarifutils.cli.files.fs | ||
import com.saveourtool.sarifutils.cli.files.readFile | ||
import com.saveourtool.sarifutils.cli.files.readLines | ||
import io.github.detekt.sarif4k.Replacement | ||
|
||
import io.github.detekt.sarif4k.Run | ||
import io.github.detekt.sarif4k.SarifSchema210 | ||
import okio.Path | ||
import okio.Path.Companion.toPath | ||
|
||
import kotlinx.serialization.decodeFromString | ||
import kotlinx.serialization.json.Json | ||
|
||
/** | ||
* Adapter for applying sarif fix object replacements to the corresponding test files | ||
* | ||
* @param sarifFile path to the sarif file with fix object replacements | ||
* @param testFiles list of the test file, to which above fixes need to be applied | ||
*/ | ||
class SarifFixAdapter( | ||
private val sarifFile: Path, | ||
private val testFiles: List<Path> | ||
) { | ||
private val tmpDir = fs.createTempDir(SarifFixAdapter::class.simpleName!!) | ||
|
||
/** | ||
* Main entry for processing and applying fixes from sarif file into the test files | ||
* | ||
* @return list of processed files | ||
*/ | ||
fun process(): List<Path?> { | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
val sarifSchema210: SarifSchema210 = Json.decodeFromString( | ||
fs.readFile(sarifFile) | ||
) | ||
// A run object describes a single run of an analysis tool and contains the output of that run. | ||
val processedFiles = sarifSchema210.runs.flatMapIndexed { index, run -> | ||
val runReplacements: List<RuleReplacements?> = extractFixObjects(run) | ||
if (runReplacements.isEmpty()) { | ||
// TODO: Use logging library | ||
println("Run #$index have no fix object section!") | ||
emptyList() | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
applyReplacementsToFiles(runReplacements, testFiles) | ||
} | ||
} | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return processedFiles | ||
} | ||
|
||
/** | ||
* @param run describes a single run of an analysis tool, and contains the reported output of that run | ||
* @return list of replacements for all files from single [run] | ||
*/ | ||
fun extractFixObjects(run: Run): List<RuleReplacements?> { | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!run.isFixObjectExist()) { | ||
nulls marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return emptyList() | ||
} | ||
// A result object describes a single result detected by an analysis tool. | ||
// Each result is produced by the evaluation of a rule. | ||
return run.results?.map { result -> | ||
// A fix object represents a proposed fix for the problem indicated by the result. | ||
// It specifies a set of artifacts to modify. | ||
// For each artifact, it specifies regions to remove, and provides new content to insert. | ||
result.fixes?.flatMap { fix -> | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
fix.artifactChanges.map { artifactChange -> | ||
// TODO: What if uri is not provided? Could it be? | ||
val filePath = artifactChange.artifactLocation.uri!!.toPath() | ||
val replacements = artifactChange.replacements | ||
FileReplacements(filePath, replacements) | ||
} | ||
} ?: emptyList() | ||
} ?: emptyList() | ||
} | ||
|
||
private fun Run.isFixObjectExist(): Boolean = this.results?.any { result -> | ||
result.fixes != null | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} ?: false | ||
|
||
@Suppress("UnusedPrivateMember") | ||
private fun applyReplacementsToFiles(runReplacements: List<RuleReplacements?>?, testFiles: List<Path>): List<Path?> = runReplacements?.flatMap { ruleReplacements -> | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ruleReplacements?.mapNotNull { fileReplacements -> | ||
val testFile = testFiles.find { | ||
it.toString().contains(fileReplacements.filePath.toString()) | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
if (testFile == null) { | ||
println("Couldn't find appropriate test file on the path ${fileReplacements.filePath}, which provided in Sarif!") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test files? what is the relation of test files to a separate sarif library? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, changed to |
||
null | ||
} else { | ||
applyChangesToFile(testFile, fileReplacements.replacements) | ||
} | ||
} ?: emptyList() | ||
} ?: emptyList() | ||
|
||
private fun applyChangesToFile(testFile: Path, replacements: List<Replacement>): Path { | ||
val testFileCopy = tmpDir.resolve(testFile.name) | ||
fs.copyFileContent(testFile, testFileCopy) | ||
val fileContent = fs.readLines(testFileCopy).toMutableList() | ||
|
||
replacements.forEach { replacement -> | ||
val startLine = replacement.deletedRegion.startLine!!.toInt() - 1 | ||
val startColumn = replacement.deletedRegion.startColumn!!.toInt() - 1 | ||
val endColumn = replacement.deletedRegion.endColumn!!.toInt() - 1 | ||
val insertedContent = replacement.insertedContent?.text | ||
|
||
insertedContent?.let { | ||
fileContent[startLine] = fileContent[startLine].replaceRange(startColumn, endColumn, it) | ||
petertrr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} ?: run { | ||
fileContent[startLine] = fileContent[startLine].removeRange(startColumn, endColumn) | ||
} | ||
} | ||
fs.write(testFileCopy) { | ||
fileContent.forEach { | ||
write((it + "\n").encodeToByteArray()) | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
return testFileCopy | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package com.saveourtool.sarifutils.cli.config | ||
|
||
import io.github.detekt.sarif4k.Replacement | ||
import okio.Path | ||
|
||
typealias RuleReplacements = List<FileReplacements> | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* @property filePath path to the file | ||
* @property replacements list of artifact changes for this [file] | ||
*/ | ||
data class FileReplacements( | ||
val filePath: Path, | ||
val replacements: List<Replacement> | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
/** | ||
* Utility methods to work with file system | ||
*/ | ||
|
||
package com.saveourtool.sarifutils.cli.files | ||
|
||
import okio.FileSystem | ||
import okio.Path | ||
import kotlin.random.Random | ||
|
||
expect val fs: FileSystem | ||
|
||
/** | ||
* @param path a path to a file | ||
* @return list of strings from the file | ||
*/ | ||
fun FileSystem.readLines(path: Path): List<String> = this.read(path) { | ||
generateSequence { readUtf8Line() }.toList() | ||
} | ||
|
||
/** | ||
* @param path a path to a file | ||
* @return string from the file | ||
*/ | ||
fun FileSystem.readFile(path: Path): String = this.read(path) { | ||
this.readUtf8() | ||
} | ||
|
||
/** | ||
* Create file in [this] [FileSystem], denoted by [Path] [path] | ||
* | ||
* @param path path to a new file | ||
* @return [path] | ||
*/ | ||
fun FileSystem.createFile(path: Path): Path { | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sink(path).close() | ||
return path | ||
} | ||
|
||
/** | ||
* Create a temporary directory | ||
* | ||
* @param prefix will be prepended to directory name | ||
* @return a [Path] representing the created directory | ||
*/ | ||
fun FileSystem.createTempDir(prefix: String = "sarifutils-tmp"): Path { | ||
nulls marked this conversation as resolved.
Show resolved
Hide resolved
|
||
val dirName = "$prefix-${Random.nextInt()}" | ||
return (FileSystem.SYSTEM_TEMPORARY_DIRECTORY / dirName).also { | ||
createDirectory(it) | ||
nulls marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
/** | ||
* Copy file content from [from] into [into] | ||
* | ||
* @param [from] path to the file, from which content need to be copied | ||
* @param [into] path to the file, into which content need to be copied | ||
* @param mustCreate whether to create file on [into] path | ||
*/ | ||
fun FileSystem.copyFileContent( | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
from: Path, | ||
into: Path, | ||
mustCreate: Boolean = false | ||
) { | ||
fs.write(into, mustCreate) { | ||
fs.readLines(from).forEach { | ||
0x6675636b796f75676974687562 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
write( | ||
(it + "\n").encodeToByteArray() | ||
) | ||
} | ||
} | ||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it an adapter?
And also why do we need "testFiles"?
Forget word 'test'. It is
targetFile
or something like thisThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I name this by analogy from
save-cli
, like we haveSarifWarnAdapter
Can't imagine better name for now, so leave as is. Will change in future PR
test
->target
changed