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

Adapt FixPlugin for library for applying of SARIF fix patches #464

Merged
merged 7 commits into from
Dec 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
Fixed Show fixed Hide fixed
* 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 {
kgevorkyan marked this conversation as resolved.
Show resolved Hide resolved
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
Fixed Show fixed Hide fixed
* @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