Skip to content

Commit

Permalink
Adapt FixPlugin for library for applying of SARIF fix patches (#464)
Browse files Browse the repository at this point in the history
### What's done:
* Adapt FixPlugin for library for applying of SARIF fix patches
  • Loading branch information
kgevorkyan authored Dec 2, 2022
1 parent e557722 commit cf1d71d
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Classes that describe format of expected and actual fixes
*/

@file:Suppress("FILE_NAME_MATCH_CLASS", "MatchingDeclarationName")

package com.saveourtool.save.core.config

/**
* Possible formats of actual set of fixes, provided by user
*/
enum class ActualFixFormat {
IN_PLACE,
SARIF,
;
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,7 @@ class FixAndWarnPlugin(
}

// Fill back original data with warnings
filesAndTheirWarningsMap.forEach { (filePath, warningsList) ->
val fileData = fs.readLines(filePath) as MutableList
// Append warnings into appropriate place
warningsList.forEach { (line, warningMsg) ->
fileData.add(line, warningMsg)
}
fs.write(filePath) {
fileData.forEach {
write((it + "\n").encodeToByteArray())
}
}
}
restoreWarningsIntoExpectedFiles(filesAndTheirWarningsMap)

// TODO: If we receive just one command for execution, and want to avoid extra executions
// TODO: then warn plugin should look at the fix plugin output for actual warnings, and not execute command one more time.
Expand Down Expand Up @@ -136,8 +125,7 @@ class FixAndWarnPlugin(
/**
* Remove warnings from the given files, which satisfy pattern from <warn> plugin and save data about warnings, which were deleted
*
* @files files to be modified
*
* @param files files to be modified
* @return map of files and theirs list of warnings
*/
private fun removeWarningsFromExpectedFiles(files: Sequence<Path>): MutableMap<Path, WarningsList> {
Expand All @@ -157,6 +145,26 @@ class FixAndWarnPlugin(
return filesAndTheirWarningsMap
}

/**
* Remove warnings into the given files, which were deleted in aim to provide clear fix changes by FixPlugin
*
* @param filesAndTheirWarningsMap map of files with corresponding warnings list, which need to be restored
*/
private fun restoreWarningsIntoExpectedFiles(filesAndTheirWarningsMap: MutableMap<Path, WarningsList>) {
filesAndTheirWarningsMap.forEach { (filePath, warningsList) ->
val fileData = fs.readLines(filePath) as MutableList
// Append warnings into appropriate place
warningsList.forEach { (line, warningMsg) ->
fileData.add(line, warningMsg)
}
fs.write(filePath) {
fileData.forEach {
write((it + "\n").encodeToByteArray())
}
}
}
}

private fun writeDataWithoutWarnings(
file: Path,
filesAndTheirWarningsMap: MutableMap<Path, WarningsList>,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.saveourtool.save.plugins.fix

import com.saveourtool.save.core.config.ActualFixFormat
import com.saveourtool.save.core.config.TestConfig
import com.saveourtool.save.core.files.createFile
import com.saveourtool.save.core.files.createRelativePathToTheRoot
Expand Down Expand Up @@ -62,8 +63,8 @@ class FixPlugin(
newTag = { _, start -> if (start) "<" else ">" },
)

// fixme: consider refactoring under https://github.com/saveourtool/save/issues/156
// fixme: should not be common for a class instance during https://github.com/saveourtool/save/issues/28
// fixme: consider refactoring under https://github.com/saveourtool/save-cli/issues/156
// fixme: should not be common for a class instance during https://github.com/saveourtool/save-cli/issues/28
private var tmpDirectory: Path? = null
private lateinit var extraFlagsExtractor: ExtraFlagsExtractor

Expand Down Expand Up @@ -94,7 +95,7 @@ class FixPlugin(

val pathMap = chunk.map { it.test to it.expected }
val pathCopyMap = pathMap.map { (test, expected) ->
createTestFile(test, generalConfig, fixPluginConfig) to expected
createCopyOfTestFile(test, generalConfig, fixPluginConfig) to expected
}
val testCopyNames =
pathCopyMap.joinToString(separator = batchSeparator) { (testCopy, _) -> testCopy.toString() }
Expand All @@ -117,6 +118,13 @@ class FixPlugin(
val stdout = executionResult.stdout
val stderr = executionResult.stderr

// In this case fixes weren't performed by tool into the test files directly,
// instead, there was created sarif file with list of fixes, which we will apply ourselves
if (fixPluginConfig.actualFixFormat == ActualFixFormat.SARIF) {
// TODO: Apply fixes from sarif file on `testCopyNames` here
// applySarifFixesToFiles(fixPluginConfig.actualFixSarifFileName, testCopyNames)
}

pathCopyMap.map { (testCopy, expected) ->
val fixedLines = fs.readLines(testCopy)
val expectedLines = fs.readLines(expected)
Expand Down Expand Up @@ -160,7 +168,7 @@ class FixPlugin(
}
}

private fun createTestFile(
private fun createCopyOfTestFile(
path: Path,
generalConfig: GeneralConfig,
fixPluginConfig: FixPluginConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

package com.saveourtool.save.plugins.fix

import com.saveourtool.save.core.config.ActualFixFormat
import com.saveourtool.save.core.config.TestConfigSections
import com.saveourtool.save.core.plugin.PluginConfig
import com.saveourtool.save.core.utils.RegexSerializer
Expand All @@ -22,13 +23,17 @@ import kotlinx.serialization.UseSerializers
* @property resourceNameTestSuffix suffix name of the test file.
* @property resourceNameExpectedSuffix suffix name of the expected file.
* @property ignoreLines mutable list of patterns that later will be used to filter lines in test file
* @property actualFixFormat format for type for fixes: they could be done in place or provided via Sarif file
* @property actualFixSarifFileName name of sarif file with list of fixes, that were made by tool
*/
@Serializable
data class FixPluginConfig(
val execFlags: String? = null,
val resourceNameTestSuffix: String? = null,
val resourceNameExpectedSuffix: String? = null,
val ignoreLines: MutableList<String>? = null
val ignoreLines: MutableList<String>? = null,
val actualFixFormat: ActualFixFormat? = null,
val actualFixSarifFileName: String? = null,
) : PluginConfig {
override val type = TestConfigSections.FIX

Expand Down Expand Up @@ -62,7 +67,9 @@ data class FixPluginConfig(
this.resourceNameExpectedSuffix ?: other.resourceNameExpectedSuffix,
other.ignoreLines?.let {
this.ignoreLines?.let { other.ignoreLines.union(this.ignoreLines) } ?: other.ignoreLines
}?.toMutableList() ?: this.ignoreLines
}?.toMutableList() ?: this.ignoreLines,
this.actualFixFormat ?: other.actualFixFormat,
this.actualFixSarifFileName ?: other.actualFixSarifFileName
).also {
it.configLocation = this.configLocation
}
Expand All @@ -73,7 +80,9 @@ data class FixPluginConfig(
execFlags ?: "",
resourceNameTest,
resourceNameExpected,
ignoreLines
ignoreLines,
actualFixFormat ?: ActualFixFormat.IN_PLACE,
actualFixSarifFileName ?: "save-fixes.sarif",
).also {
it.configLocation = this.configLocation
}
Expand Down

0 comments on commit cf1d71d

Please sign in to comment.