Skip to content

Commit

Permalink
Support multiline fixes (#16)
Browse files Browse the repository at this point in the history 
### What's done:
* Support multiline fixes
* Add more tests
  • Loading branch information
@kgevorkyan
kgevorkyan authored Dec 23, 2022
1 parent 55bdb37 commit 26796fb
Show file tree
Hide file tree
Showing 6 changed files with 516 additions and 63 deletions.
243 changes: 184 additions & 59 deletions fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/cli/adapter/SarifFixAdapter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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 com.saveourtool.sarifutils.cli.files.writeContentWithNewLinesToFile
import com.saveourtool.sarifutils.cli.utils.adaptedIsAbsolute
import com.saveourtool.sarifutils.cli.utils.getUriBaseIdForArtifactLocation
import com.saveourtool.sarifutils.cli.utils.resolveUriBaseId
Expand Down Expand Up @@ -44,11 +45,14 @@ class SarifFixAdapter(
val processedFiles = sarifSchema210.runs.asSequence().flatMapIndexed { index, run ->
val runReplacements: List<RuleReplacements> = extractFixObjects(run)
if (runReplacements.isEmpty()) {
println("Run #$index have no any `fix object` section!")
println("The run #$index doesn't have any `fix object` section!")
emptyList()
} else {
val groupedReplacements = groupReplacementsByFiles(runReplacements)
applyReplacementsToFiles(groupedReplacements, targetFiles)
// if there are several fixes by different rules for the same region within one file, take only first of them
// and reverse the order of replacements for each file
val filteredRuleReplacements = filterRuleReplacements(groupedReplacements)
applyReplacementsToFiles(filteredRuleReplacements, targetFiles)
}
}
return processedFiles.toList()
Expand Down Expand Up @@ -113,36 +117,83 @@ class SarifFixAdapter(
}

/**
* If there are several fixes in one file on the same line by different rules, take only the first one
* If there are several fixes in one file for the same region by different rules, take only the first one,
* also return filtered replacements in reverse order, in aim to provide fixes on the next stages without invalidation of indexes,
* i.e. in case of multiline fixes, which will add new lines, or remove regions, that is, shift all the lines below,
* apply all fixes, starting from the last fix, thus it won't break startLine's and endLine's for other fixes
*
* @param fileReplacementsList list of replacements by all rules
* @return filtered list of replacements by all rules
*/
private fun filterRuleReplacements(fileReplacementsList: List<FileReplacements>): List<FileReplacements> {
// distinct replacements for each file by `startLine`,
// i.e., take only first of possible fixes for each line
return fileReplacementsList.map { fileReplacementsListForSingleFile ->
val filePath = fileReplacementsListForSingleFile.filePath
val distinctReplacements = fileReplacementsListForSingleFile.replacements.groupBy {
// group all fixes for current file by startLine
it.deletedRegion.startLine
}.flatMap { entry ->
val startLine = entry.key
val replacementsList = entry.value

if (replacementsList.size > 1) {
println("Some of fixes for $filePath were ignored, due they refer to the same line: $startLine." +
" Only the first fix will be applied")
}
replacementsList
}.distinctBy {
it.deletedRegion.startLine
private fun filterRuleReplacements(fileReplacementsList: List<FileReplacements>): List<FileReplacements> = fileReplacementsList.map { fileReplacementsListForSingleFile ->
val filePath = fileReplacementsListForSingleFile.filePath
// sort replacements by startLine
val sortedReplacements = sortReplacementsByStartLine(fileReplacementsListForSingleFile)
// now, from overlapping fixes, take only the first one
val nonOverlappingFixes = getNonOverlappingReplacements(fileReplacementsListForSingleFile.filePath, sortedReplacements)
// save replacements in reverse order
FileReplacements(
filePath,
nonOverlappingFixes.reversed()
)
}

/**
* Sort provided list of replacements by [startLine]
*
* @param fileReplacementsListForSingleFile list of replacements for target file
* @return sorted list of replacements by [startLine]
*/
private fun sortReplacementsByStartLine(
fileReplacementsListForSingleFile: FileReplacements
): List<Replacement> = fileReplacementsListForSingleFile.replacements.map { replacement ->
val updatedReplacement = recoverEndLine(replacement)
updatedReplacement
}.sortedWith(
compareBy({ it.deletedRegion.startLine }, { it.deletedRegion.endLine })
)

/**
* It's not require to present endLine, if fix represents the single line changes,
* so, for consistency we will set `endLine` by ourselves, if it absent
*
* @param replacement replacement instance, with probably missing [endLine] field
* @return updated replacement, with filled [endLine], if it was absent
*/
private fun recoverEndLine(replacement: Replacement): Replacement = if (replacement.deletedRegion.endLine == null) {
val deletedRegion = replacement.deletedRegion.copy(
endLine = replacement.deletedRegion.startLine
)
replacement.copy(
deletedRegion = deletedRegion
)
} else {
replacement
}

/**
* For the [sortedReplacements] list take only non overlapping replacements.
* Replacement overlaps, if they are fix the same region, e.g: max(fix1.startLine, fix2.startLine) <= min(fix1.endLine, fix2.endLine),
* or, for sorted intervals by startLine it's equals to (fix2.startLine <= fix1.endLine)
*
* @param filePath file path
* @param sortedReplacements list of replacements, sorted by [startLine]
* @return list of non overlapping replacements
*/
private fun getNonOverlappingReplacements(filePath: Path, sortedReplacements: List<Replacement>): MutableList<Replacement> {
val nonOverlappingFixes: MutableList<Replacement> = mutableListOf(sortedReplacements[0])
var currentEndLine = sortedReplacements[0].deletedRegion.endLine!!

for (i in 1 until sortedReplacements.size) {
if (sortedReplacements[i].deletedRegion.startLine!! <= currentEndLine) {
println("Fix ${sortedReplacements[i].prettyString()} for $filePath was ignored, due it overlaps with others." +
" Only the first fix for this region will be applied.")
} else {
nonOverlappingFixes.add(sortedReplacements[i])
currentEndLine = sortedReplacements[i].deletedRegion.endLine!!
}
FileReplacements(
filePath,
distinctReplacements
)
}
return nonOverlappingFixes
}

/**
Expand All @@ -151,24 +202,20 @@ 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> {
// if there are several fixes by different rules on the same line for any file, take only first of them
val filteredRuleReplacements = filterRuleReplacements(fileReplacementsList)
return filteredRuleReplacements.mapNotNull { fileReplacements ->
val targetFile = targetFiles.find {
val fullPathOfFileFromSarif = if (!fileReplacements.filePath.adaptedIsAbsolute()) {
fs.canonicalize(sarifFile.parent!! / fileReplacements.filePath)
} else {
fileReplacements.filePath
}
fs.canonicalize(it) == fullPathOfFileFromSarif
}
if (targetFile == null) {
println("Couldn't find appropriate target file on the path ${fileReplacements.filePath}, which provided in Sarif!")
null
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)
} else {
applyReplacementsToSingleFile(targetFile, fileReplacements.replacements)
fileReplacements.filePath
}
fs.canonicalize(it) == fullPathOfFileFromSarif
}
if (targetFile == null) {
println("Couldn't find appropriate target file on the path ${fileReplacements.filePath}, which provided in Sarif!")
null
} else {
applyReplacementsToSingleFile(targetFile, fileReplacements.replacements)
}
}

Expand All @@ -188,6 +235,7 @@ class SarifFixAdapter(

replacements.forEach { replacement ->
val startLine = replacement.deletedRegion.startLine!!.toInt() - 1
val endLine = replacement.deletedRegion.endLine!!.toInt() - 1
val startColumn = replacement.deletedRegion.startColumn?.let {
it.toInt() - 1
}
Expand All @@ -196,45 +244,122 @@ class SarifFixAdapter(
}
val insertedContent = replacement.insertedContent?.text

applyFixToLine(fileContent, insertedContent, startLine, startColumn, endColumn)
}
fs.write(targetFileCopy) {
fileContent.forEach { line ->
writeUtf8(line + '\n')
}
applyFix(fileContent, insertedContent, startLine, endLine, startColumn, endColumn)
}
writeContentWithNewLinesToFile(targetFileCopy, fileContent)
return targetFileCopy
}

/**
* Apply single line fixes into [fileContent]
* Apply fixes into [fileContent]
*
* @param fileContent file data, where need to change content
* @param insertedContent represents inserted content into the line from [fileContent] with index [startLine], or null if fix represent the deletion of region
* @param startLine index of line, which need to be changed
* @param startColumn index of column, starting from which content should be changed, or null if [startLine] will be completely replaced
* @param endColumn index of column, ending with which content should be changed, or null if [startLine] will be completely replaced
* @param insertedContent represents inserted content for the [fileContent] starting with line [startLine] and ending with [endLine],
* or null if fix represent the deletion of region
* @param startLine start index of line, which need to be changed
* @param endLine end index of line, which need to be changed
* @param startColumn index of column, starting from which content should be changed, or null if range ([startLine], [endLine]) will be completely replaced
* @param endColumn index of column, ending with which content should be changed, or null if range ([startLine], [endLine]) will be completely replaced
*/
private fun applyFixToLine(
@Suppress("TOO_MANY_PARAMETERS")
private fun applyFix(
fileContent: MutableList<String>,
insertedContent: String?,
startLine: Int,
endLine: Int,
startColumn: Int?,
endColumn: Int?
) {
if (startLine != endLine) {
// multiline fix
applyMultiLineFix(fileContent, insertedContent, startLine, endLine, startColumn, endColumn)
} else {
// single line fix
applySingleLineFix(fileContent, insertedContent, startLine, startColumn, endColumn)
}
}

@Suppress("TOO_MANY_PARAMETERS")
private fun applyMultiLineFix(
fileContent: MutableList<String>,
insertedContent: String?,
startLine: Int,
endLine: Int,
startColumn: Int?,
endColumn: Int?
) {
insertedContent?.let { content ->
if (startColumn != null && endColumn != null) {
// first, remove changeable region
removeMultiLineRange(fileContent, startLine, endLine, startColumn, endColumn)
// insertedContent already contains newlines, so could be inserted simply starting from startLine
fileContent[startLine] = StringBuilder(fileContent[startLine]).apply { insert(startColumn, content) }.toString()
} else {
// remove whole changeable region
removeMultiLines(fileContent, startLine, endLine)
// insertedContent already contains newlines, so could be inserted simply starting from startLine
fileContent.add(startLine, content)
}
} ?: run {
// just remove changeable region
if (startColumn != null && endColumn != null) {
removeMultiLineRange(fileContent, startLine, endLine, startColumn, endColumn)
} else {
removeMultiLines(fileContent, startLine, endLine)
}
}
}

private fun removeMultiLineRange(
fileContent: MutableList<String>,
startLine: Int,
endLine: Int,
startColumn: Int,
endColumn: Int
) {
// remove characters in startLine after startColumn
fileContent[startLine] = fileContent[startLine].removeRange(startColumn, fileContent[startLine].length)
// remove lines between startLine and endLine
fileContent.subList(startLine + 1, endLine).clear()
// remove characters in endLine before endColumn
fileContent[endLine] = fileContent[endLine].removeRange(0, endColumn)
}

private fun removeMultiLines(
fileContent: MutableList<String>,
startLine: Int,
endLine: Int,
) {
// remove whole range (startLine, endLine)
fileContent.subList(startLine, endLine + 1).clear()
}

private fun applySingleLineFix(
fileContent: MutableList<String>,
insertedContent: String?,
startLine: Int,
startColumn: Int?,
endColumn: Int?
) {
insertedContent?.let {
insertedContent?.let { content ->
if (startColumn != null && endColumn != null) {
fileContent[startLine] = fileContent[startLine].replaceRange(startColumn, endColumn, it)
// replace range
fileContent[startLine] = fileContent[startLine].replaceRange(startColumn, endColumn, content)
} else {
fileContent[startLine] = it
// replace whole line
fileContent[startLine] = content
}
} ?: run {
if (startColumn != null && endColumn != null) {
// remove range
fileContent[startLine] = fileContent[startLine].removeRange(startColumn, endColumn)
} else {
// remove all content but leave empty line, until we support https://github.com/saveourtool/sarif-utils/issues/13
fileContent[startLine] = "\n"
// remove whole line
fileContent.removeAt(startLine)
}
}
}

private fun Replacement.prettyString(): String = "(startLine: ${this.deletedRegion.startLine}, endLine: ${this.deletedRegion.endLine}, " +
"startColumn: ${this.deletedRegion.startColumn}, endColumn: ${this.deletedRegion.endColumn}, insertedContent: ${this.insertedContent})"
}
18 changes: 18 additions & 0 deletions fixpatches/src/commonMain/kotlin/com/saveourtool/sarifutils/cli/files/FileUtils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,24 @@ 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<String>) = fs.write(targetFile) {
content.forEach { line ->
if (!line.contains('\n')) {
writeUtf8(line + '\n')
} else {
writeUtf8(line)
}
}
}

/**
* Create a temporary directory
*
Expand Down
Loading

0 comments on commit 26796fb

Please sign in to comment.