Skip to content

Commit

Permalink
Execute tool in sarif mode before report checking (#498)
Browse files Browse the repository at this point in the history
### What's done:
* Execute tool before checking the sarif report, this logic allows:
  1) Support checking `.sarif` from stdout
  2) Rewrite existing sarif report, if `actual[Warnings|Fix]SarifFileName` was provided and something were changed in report

* Minor refactoring
* Add possible config for one of the test (however it's not required, since `save-cli` will look into stdout by default)
  • Loading branch information
kgevorkyan authored Feb 16, 2023
1 parent daf00e5 commit c520f6b
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 50 deletions.
155 changes: 155 additions & 0 deletions examples/kotlin-diktat/sarif-actual/save-warnings-actual.sarif
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
{
"$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
"version": "2.1.0",
"runs": [
{
"originalUriBaseIds": {
"%SRCROOT%": {
"uri": "file://D:/projects/"
}
},
"results": [
{
"level": "error",
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "src\\kotlin\\EnumValueSnakeCaseTest.kt",
"uriBaseId": "%SRCROOT%"
},
"region": {
"startColumn": 35,
"startLine": 6
}
}
}
],
"message": {
"text": "[WRONG_DECLARATIONS_ORDER] declarations of constants and enum members should be sorted alphabetically: enum entries order is incorrect (diktat-ruleset:abp-sort-rule)"
},
"ruleId": "diktat-ruleset:abp-sort-rule"
},
{
"level": "error",
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "src\\kotlin\\EnumValueSnakeCaseTest.kt",
"uriBaseId": "%SRCROOT%"
},
"region": {
"startColumn": 5,
"startLine": 10
}
}
}
],
"message": {
"text": "[ENUMS_SEPARATED] enum is incorrectly formatted: enums must end with semicolon (diktat-ruleset:abq-enum-separated)"
},
"ruleId": "diktat-ruleset:abq-enum-separated"
},
{
"level": "error",
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "src\\kotlin\\EnumValueSnakeCaseTest.kt",
"uriBaseId": "%SRCROOT%"
},
"region": {
"startColumn": 5,
"startLine": 8
}
}
}
],
"message": {
"text": "[ENUM_VALUE] enum values should be in selected UPPER_CASE snake/PascalCase format: paSC_SAl_l (diktat-ruleset:aai-identifier-naming)"
},
"ruleId": "diktat-ruleset:aai-identifier-naming"
},
{
"level": "error",
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "src\\kotlin\\EnumValueSnakeCaseTest.kt",
"uriBaseId": "%SRCROOT%"
},
"region": {
"startColumn": 5,
"startLine": 11
}
}
}
],
"message": {
"text": "[ENUM_VALUE] enum values should be in selected UPPER_CASE snake/PascalCase format: PascAsl_f (diktat-ruleset:aai-identifier-naming)"
},
"ruleId": "diktat-ruleset:aai-identifier-naming"
},
{
"level": "error",
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "src\\kotlin\\EnumValueSnakeCaseTest.kt",
"uriBaseId": "%SRCROOT%"
},
"region": {
"startColumn": 5,
"startLine": 10
}
}
}
],
"message": {
"text": "[ENUMS_SEPARATED] enum is incorrectly formatted: last enum entry must end with a comma (diktat-ruleset:abq-enum-separated)"
},
"ruleId": "diktat-ruleset:abq-enum-separated"
},
{
"level": "error",
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "src\\kotlin\\EnumValueSnakeCaseTest.kt",
"uriBaseId": "%SRCROOT%"
},
"region": {
"startColumn": 9,
"startLine": 1
}
}
}
],
"message": {
"text": "[PACKAGE_NAME_INCORRECT_PREFIX] package name should start from company's domain: com.saveourtool.save (diktat-ruleset:aah-package-naming)"
},
"ruleId": "diktat-ruleset:aah-package-naming"
}
],
"tool": {
"driver": {
"downloadUri": "https://github.com/pinterest/ktlint/releases/tag/0.42.0",
"fullName": "ktlint",
"informationUri": "https://github.com/pinterest/ktlint/",
"language": "en",
"name": "ktlint",
"organization": "pinterest",
"rules": [
],
"semanticVersion": "0.42.0",
"version": "0.42.0"
}
}
}
]
}
3 changes: 3 additions & 0 deletions examples/kotlin-diktat/sarif-actual/save.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
# regular expression to detect tests
testNameRegex = ".*Test.kt"
actualWarningsFormat = "SARIF"
# by default, in SARIF mode, it's supposed, that tool will print sarif report into the stdout
# however, it also could be provided via file
# actualWarningsFileName = "save-warnings-actual.sarif"
fileNameCaptureGroupOut = 1
lineCaptureGroupOut = 2
columnCaptureGroupOut = 3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import com.saveourtool.save.core.result.DebugInfo
import com.saveourtool.save.core.result.Fail
import com.saveourtool.save.core.result.Pass
import com.saveourtool.save.core.result.TestResult
import com.saveourtool.save.core.utils.ExecutionResult
import com.saveourtool.save.core.utils.PathSerializer
import com.saveourtool.save.core.utils.ProcessExecutionException
import com.saveourtool.save.core.utils.ProcessTimeoutException
Expand Down Expand Up @@ -104,21 +103,22 @@ class FixPlugin(

logDebug("Executing fix plugin in ${fixPluginConfig.actualFixFormat?.name} mode")

val (executionResult, adjustedTestCopyToExpectedFilesMap) = if (fixPluginConfig.actualFixFormat == ActualFixFormat.IN_PLACE) {
try {
// hold testCopyToExpectedFilesMap as is
pb.exec(execCmd, testConfig.getRootConfig().directory.toString(), redirectTo, time) to testCopyToExpectedFilesMap
} catch (ex: ProcessTimeoutException) {
logWarn("The following tests took too long to run and were stopped: ${chunk.map { it.test }}, timeout for single test: ${ex.timeoutMillis}")
return@map failTestResult(chunk, ex, execCmd)
} catch (ex: ProcessExecutionException) {
return@map failTestResult(chunk, ex, execCmd)
}
val executionResult = try {
pb.exec(execCmd, testConfig.getRootConfig().directory.toString(), redirectTo, time)
} catch (ex: ProcessTimeoutException) {
logWarn("The following tests took too long to run and were stopped: ${chunk.map { it.test }}, timeout for single test: ${ex.timeoutMillis}")
return@map failTestResult(chunk, ex, execCmd)
} catch (ex: ProcessExecutionException) {
return@map failTestResult(chunk, ex, execCmd)
}

val adjustedTestCopyToExpectedFilesMap = if (fixPluginConfig.actualFixFormat == ActualFixFormat.IN_PLACE) {
// hold testCopyToExpectedFilesMap as is
testCopyToExpectedFilesMap
} else {
// replace test files by modificated copies, obtained from sarif lib
// replace test files with modified copies, obtained from sarif lib
val fixedTestCopyToExpectedFilesMap = applyFixesFromSarif(fixPluginConfig, testsPaths, testCopyToExpectedFilesMap)
val dbgMsg = "Fixes were obtained from SARIF file, no debug info is available"
ExecutionResult(0, listOf(dbgMsg), listOf(dbgMsg)) to fixedTestCopyToExpectedFilesMap
fixedTestCopyToExpectedFilesMap
}

val stdout = executionResult.stdout
Expand Down Expand Up @@ -188,6 +188,7 @@ class FixPlugin(
* @param testCopyToExpectedFilesMap list of paths to the copy of tests files, which will be replaced by modificated files
* @return updated list of test files copies
*/
// TODO: support case, when sarif report is provided via stdout
private fun applyFixesFromSarif(
fixPluginConfig: FixPluginConfig,
testsPaths: List<Path>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,29 +143,24 @@ class WarnPlugin(
val execCmd = cmdExecutor.constructExecCmd(tmpDirName)

val expectedWarningsMap = try {
collectExpectedWarnings(generalConfig, warnPluginConfig, originalPaths, copyPaths, workingDirectory)
processExpectedWarnings(generalConfig, warnPluginConfig, originalPaths, copyPaths, workingDirectory)
} catch (ex: SarifParsingException) {
return failTestResult(originalPaths, ex, execCmd)
}

if (expectedWarningsMap.isEmpty()) {
warnMissingExpectedWarnings(warnPluginConfig, generalConfig, originalPaths)
val executionResult = try {
cmdExecutor.execCmdAndGetExecutionResults(redirectTo)
} catch (ex: ProcessTimeoutException) {
logWarn("The following tests took too long to run and were stopped: $originalPaths, timeout for single test: ${ex.timeoutMillis}")
return failTestResult(originalPaths, ex, execCmd)
} catch (ex: ProcessExecutionException) {
return failTestResult(originalPaths, ex, execCmd)
}

val (actualWarningsMap, result) = try {
actualWarningsIfExistActualWarningsFile(warnPluginConfig, originalPaths, workingDirectory)
val actualWarningsMap = try {
processActualWarnings(executionResult, warnPluginConfig, originalPaths, workingDirectory)
} catch (ex: SarifParsingException) {
return failTestResult(originalPaths, ex, execCmd)
} ?: run {
val result = try {
cmdExecutor.execCmdAndGetExecutionResults(redirectTo)
} catch (ex: ProcessTimeoutException) {
logWarn("The following tests took too long to run and were stopped: $originalPaths, timeout for single test: ${ex.timeoutMillis}")
return failTestResult(originalPaths, ex, execCmd)
} catch (ex: ProcessExecutionException) {
return failTestResult(originalPaths, ex, execCmd)
}
collectActualWarningsWithLineNumbers(result, warnPluginConfig, workingDirectory) to result
}

val resultsChecker = ResultsChecker(
Expand All @@ -181,32 +176,15 @@ class WarnPlugin(
resultsStatus.first,
DebugInfo(
execCmd,
result.stdout.filter { it.contains(path.name) }.joinToString("\n"),
result.stderr.filter { it.contains(path.name) }.joinToString("\n"),
executionResult.stdout.filter { it.contains(path.name) }.joinToString("\n"),
executionResult.stderr.filter { it.contains(path.name) }.joinToString("\n"),
null,
resultsStatus.second,
),
)
}.asSequence()
}

private fun actualWarningsIfExistActualWarningsFile(
warnPluginConfig: WarnPluginConfig,
originalPaths: List<Path>,
workingDirectory: Path,
) = warnPluginConfig.actualWarningsFileName?.let {
val sarif = calculatePathToSarifFile(
sarifFileName = warnPluginConfig.actualWarningsFileName,
anchorTestFilePath = originalPaths.first()
)
val execResult = ExecutionResult(
0,
fs.readLines(sarif),
listOf("Warnings were obtained from SARIF file, no debug info is available")
)
collectActualWarningsWithLineNumbers(execResult, warnPluginConfig, workingDirectory) to execResult
}

private fun createTestFiles(paths: List<Path>, warnPluginConfig: WarnPluginConfig): List<Path> {
logDebug("Creating temp copy files of resources for WarnPlugin...")
logTrace("Trying to create temp files for: $paths")
Expand All @@ -230,6 +208,44 @@ class WarnPlugin(
}
}

private fun processExpectedWarnings(
generalConfig: GeneralConfig,
warnPluginConfig: WarnPluginConfig,
originalPaths: List<Path>,
copyPaths: List<Path>,
workingDirectory: Path,
): WarningMap {
val expectedWarningsMap = collectExpectedWarnings(generalConfig, warnPluginConfig, originalPaths, copyPaths, workingDirectory)

if (expectedWarningsMap.isEmpty()) {
warnMissingExpectedWarnings(warnPluginConfig, generalConfig, originalPaths)
}
return expectedWarningsMap
}

private fun processActualWarnings(
executionResult: ExecutionResult,
warnPluginConfig: WarnPluginConfig,
originalPaths: List<Path>,
workingDirectory: Path,
): WarningMap {
val actualResult = if (warnPluginConfig.actualWarningsFormat == ActualWarningsFormat.SARIF &&
warnPluginConfig.actualWarningsFileName != null) {
// in this case, after tool execution, there was created sarif report, extract warnings from it,
// not from stdout
val sarif = calculatePathToSarifFile(
sarifFileName = warnPluginConfig.actualWarningsFileName,
anchorTestFilePath = originalPaths.first()
)
ExecutionResult(executionResult.code, fs.readLines(sarif), executionResult.stderr)
} else {
// Note: this case is applicable both for PLAIN and SARIF mode, but in the latter case, we suppose,
// that sarif report is provided in stdout of tool
executionResult
}
return collectActualWarningsWithLineNumbers(actualResult, warnPluginConfig, workingDirectory)
}

private fun failTestResult(
paths: List<Path>,
ex: Exception,
Expand Down Expand Up @@ -317,7 +333,7 @@ class WarnPlugin(
.groupBy { it.fileName }
.mapValues { (_, warning) -> warning.sortedBy { it.message } }
} catch (e: Exception) {
throw SarifParsingException("We failed to parse sarif. Check the your tool generation of sarif report, cause: ${e.message}", e.cause)
throw SarifParsingException("Failed to parse sarif. Check the your tool generation of sarif report, cause: ${e.message}", e.cause)
}

else -> result.stdout.mapNotNull {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,12 @@ data class WarnPluginConfig(
requireValidPatternForRegexInWarning()

val expectedWarningsFormat = expectedWarningsFormat ?: ExpectedWarningsFormat.IN_PLACE
val expectedWarningsFileName = expectedWarningsFileName ?: "save-warnings-expected.sarif"

val actualWarningsFormat = actualWarningsFormat ?: ActualWarningsFormat.PLAIN
val expectedWarningsFileName = expectedWarningsFileName ?: "save-warnings.sarif"
// it could be null even, if actualWarningsFormat = sarif: in this case, we suppose, that tool will print
// sarif report into stdout
val actualWarningsFileName = actualWarningsFileName

val newWarningTextHasLine = warningTextHasLine ?: true
val newWarningTextHasColumn = warningTextHasColumn ?: true
Expand Down

0 comments on commit c520f6b

Please sign in to comment.