From c0598b3c3871c1e501e479db27e740900816a94d Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Tue, 27 Feb 2024 17:21:43 +0300 Subject: [PATCH 1/4] Supported ExpectedWarningsFormat.PLAIN ### What's done: - supported reading expected results from external plain file --- .../save/core/config/WarningFormats.kt | 1 + .../saveourtool/save/core/files/FileUtils.kt | 13 +++++ .../save/core/utils/SarifFileUtils.kt | 10 ++-- .../save/plugin/warn/WarnPlugin.kt | 54 ++++++++++++++----- .../plugin/warn/utils/WarningsExtraction.kt | 7 +-- 5 files changed, 62 insertions(+), 23 deletions(-) diff --git a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/config/WarningFormats.kt b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/config/WarningFormats.kt index 9f6e09311..522e5313f 100644 --- a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/config/WarningFormats.kt +++ b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/config/WarningFormats.kt @@ -18,6 +18,7 @@ enum class ActualWarningsFormat { */ enum class ExpectedWarningsFormat { IN_PLACE, + PLAIN, SARIF, ; } diff --git a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/files/FileUtils.kt b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/files/FileUtils.kt index 6f11cc420..ed8637208 100644 --- a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/files/FileUtils.kt +++ b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/files/FileUtils.kt @@ -230,6 +230,19 @@ fun FileSystem.findAncestorDirContainingFile(path: Path, fileName: String): Path } } +/** + * Find a file in any of parent directories and return this file + * + * @param path path for which ancestors should be checked + * @param fileName a name of the file that will be searched for + * @return a path to a file with name [fileName] or null + */ +fun FileSystem.findFileInAncestorDir(path: Path, fileName: String): Path? = findAncestorDirContainingFile( + path, fileName +)?.let { + it / fileName +} + /** * @return current working directory */ diff --git a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/utils/SarifFileUtils.kt b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/utils/SarifFileUtils.kt index cfc9e9f84..23f697237 100644 --- a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/utils/SarifFileUtils.kt +++ b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/utils/SarifFileUtils.kt @@ -4,7 +4,7 @@ package com.saveourtool.save.core.utils -import com.saveourtool.save.core.files.findAncestorDirContainingFile +import com.saveourtool.save.core.files.findFileInAncestorDir import com.saveourtool.save.core.files.fs import com.saveourtool.save.core.files.parents import com.saveourtool.save.core.plugin.PluginException @@ -51,11 +51,9 @@ fun FileSystem.topmostTestDirectory(path: Path): Path = path.parents().last { pa * @return path to sarif * @throws PluginException in case of absence of sarif file */ -fun calculatePathToSarifFile(sarifFileName: String, anchorTestFilePath: Path): Path = fs.findAncestorDirContainingFile( +fun calculatePathToSarifFile(sarifFileName: String, anchorTestFilePath: Path): Path = fs.findFileInAncestorDir( anchorTestFilePath, sarifFileName -)?.let { - it / sarifFileName -} ?: throw PluginException( +) ?: throw PluginException( "Could not find SARIF file with expected warnings/fixes for file $anchorTestFilePath. " + - "Please check if correct `FarningsFormat`/`FixFormat` is set (should be SARIF) and if the file is present and called `$sarifFileName`." + "Please check if correct `WarningsFormat`/`FixFormat` is set (should be SARIF) and if the file is present and called `$sarifFileName`." ) diff --git a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt index 7d2bfb744..88b1c87a6 100644 --- a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt +++ b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt @@ -4,6 +4,7 @@ import com.saveourtool.save.core.config.ActualWarningsFormat import com.saveourtool.save.core.config.ExpectedWarningsFormat import com.saveourtool.save.core.config.TestConfig import com.saveourtool.save.core.files.createFile +import com.saveourtool.save.core.files.findFileInAncestorDir import com.saveourtool.save.core.files.getWorkingDirectory import com.saveourtool.save.core.files.readLines import com.saveourtool.save.core.logging.describe @@ -13,6 +14,7 @@ import com.saveourtool.save.core.logging.logWarn import com.saveourtool.save.core.plugin.ExtraFlagsExtractor import com.saveourtool.save.core.plugin.GeneralConfig import com.saveourtool.save.core.plugin.Plugin +import com.saveourtool.save.core.plugin.PluginException import com.saveourtool.save.core.result.DebugInfo import com.saveourtool.save.core.result.Fail import com.saveourtool.save.core.result.TestResult @@ -265,23 +267,51 @@ class WarnPlugin( originalPaths: List, copyPaths: List, workingDirectory: Path, - ): WarningMap = if (warnPluginConfig.expectedWarningsFormat == ExpectedWarningsFormat.SARIF) { - val warningsFromSarif = try { - collectWarningsFromSarif(warnPluginConfig, originalPaths, fs, workingDirectory) - } catch (e: Exception) { - throw SarifParsingException("We failed to parse sarif. Check the your tool generation of sarif report, cause: ${e.message}", e.cause) - } - copyPaths.associate { copyPath -> - copyPath.name to warningsFromSarif.filter { it.fileName == copyPath.name } + ): WarningMap = when { + warnPluginConfig.expectedWarningsFileName != null -> { + val expectedWarningsFileName: String = warnPluginConfig.expectedWarningsFileName + when (warnPluginConfig.expectedWarningsFormat) { + ExpectedWarningsFormat.IN_PLACE, null -> + throw IllegalArgumentException(" cannot be provided for expectedWarningsFormat=${ExpectedWarningsFormat.IN_PLACE}") + ExpectedWarningsFormat.PLAIN -> { + val anchorTestFilePath = originalPaths.first() + val plainFile = fs.findFileInAncestorDir(anchorTestFilePath, expectedWarningsFileName) ?: throw PluginException( + "Could not find PLAIN file with expected warnings/fixes for file $anchorTestFilePath. " + + "Please check if correct `WarningsFormat`/`FixFormat` is set (should be PLAIN) and if the file is present and called `$expectedWarningsFileName`." + ) + val warningsFromPlain = plainFile.collectExpectedWarningsWithLineNumbers( + warnPluginConfig, + generalConfig + ) + copyPaths.associate { copyPath -> + copyPath.name to warningsFromPlain.filter { it.fileName == copyPath.name } + } + } + ExpectedWarningsFormat.SARIF -> { + val warningsFromSarif = try { + collectWarningsFromSarif(expectedWarningsFileName, originalPaths, fs, workingDirectory) + } catch (e: Exception) { + throw SarifParsingException("We failed to parse sarif. Check the your tool generation of sarif report, cause: ${e.message}", e.cause) + } + copyPaths.associate { copyPath -> + copyPath.name to warningsFromSarif.filter { it.fileName == copyPath.name } + } + } + } + } - } else { - copyPaths.associate { copyPath -> - val warningsForCurrentPath = + else -> { + require(warnPluginConfig.expectedWarningsFormat != ExpectedWarningsFormat.SARIF) { + " should be provided for expectedWarningsFormat=${ExpectedWarningsFormat.SARIF}" + } + copyPaths.associate { copyPath -> + val warningsForCurrentPath = copyPath.collectExpectedWarningsWithLineNumbers( warnPluginConfig, generalConfig ) - copyPath.name to warningsForCurrentPath + copyPath.name to warningsForCurrentPath + } } } diff --git a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/utils/WarningsExtraction.kt b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/utils/WarningsExtraction.kt index 3130f3a76..931ef8c04 100644 --- a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/utils/WarningsExtraction.kt +++ b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/utils/WarningsExtraction.kt @@ -18,7 +18,6 @@ import io.github.detekt.sarif4k.SarifSchema210 import okio.FileSystem import okio.Path -import kotlinx.serialization.decodeFromString import kotlinx.serialization.json.Json /** @@ -97,7 +96,7 @@ internal fun collectionSingleWarnings( .sortedBy { warn -> warn.message } /** - * @param warnPluginConfig + * @param sarifFileName * @param originalPaths * @param fs * @param workingDirectory initial working directory, when SAVE started @@ -105,13 +104,11 @@ internal fun collectionSingleWarnings( * @throws PluginException */ internal fun collectWarningsFromSarif( - warnPluginConfig: WarnPluginConfig, + sarifFileName: String, originalPaths: List, fs: FileSystem, workingDirectory: Path, ): List { - val sarifFileName = warnPluginConfig.expectedWarningsFileName!! - // Since we have one .sarif file for all tests, just take the first of them as anchor for calculation of paths val anchorTestFilePath = originalPaths.first() val sarif = calculatePathToSarifFile( From 83f1ba450deb9a8b6445184bf34ddd63e6e4dbee Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Tue, 27 Feb 2024 17:44:43 +0300 Subject: [PATCH 2/4] fixed else branch --- .../kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt index 88b1c87a6..bc8221de6 100644 --- a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt +++ b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt @@ -271,8 +271,6 @@ class WarnPlugin( warnPluginConfig.expectedWarningsFileName != null -> { val expectedWarningsFileName: String = warnPluginConfig.expectedWarningsFileName when (warnPluginConfig.expectedWarningsFormat) { - ExpectedWarningsFormat.IN_PLACE, null -> - throw IllegalArgumentException(" cannot be provided for expectedWarningsFormat=${ExpectedWarningsFormat.IN_PLACE}") ExpectedWarningsFormat.PLAIN -> { val anchorTestFilePath = originalPaths.first() val plainFile = fs.findFileInAncestorDir(anchorTestFilePath, expectedWarningsFileName) ?: throw PluginException( @@ -297,8 +295,9 @@ class WarnPlugin( copyPath.name to warningsFromSarif.filter { it.fileName == copyPath.name } } } + else -> + throw IllegalArgumentException(" cannot be provided for expectedWarningsFormat=${ExpectedWarningsFormat.IN_PLACE}") } - } else -> { require(warnPluginConfig.expectedWarningsFormat != ExpectedWarningsFormat.SARIF) { From 9c5fd57622062f762f6b7f182814cbd3bebf13d4 Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Tue, 27 Feb 2024 17:58:16 +0300 Subject: [PATCH 3/4] self review --- .../save/plugin/warn/WarnPlugin.kt | 63 +++++++++---------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt index bc8221de6..6f5a3688f 100644 --- a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt +++ b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt @@ -39,7 +39,6 @@ import okio.FileSystem import okio.Path import kotlin.random.Random -import kotlinx.serialization.decodeFromString import kotlinx.serialization.json.Json private typealias WarningMap = Map> @@ -267,43 +266,37 @@ class WarnPlugin( originalPaths: List, copyPaths: List, workingDirectory: Path, - ): WarningMap = when { - warnPluginConfig.expectedWarningsFileName != null -> { - val expectedWarningsFileName: String = warnPluginConfig.expectedWarningsFileName - when (warnPluginConfig.expectedWarningsFormat) { - ExpectedWarningsFormat.PLAIN -> { - val anchorTestFilePath = originalPaths.first() - val plainFile = fs.findFileInAncestorDir(anchorTestFilePath, expectedWarningsFileName) ?: throw PluginException( - "Could not find PLAIN file with expected warnings/fixes for file $anchorTestFilePath. " + - "Please check if correct `WarningsFormat`/`FixFormat` is set (should be PLAIN) and if the file is present and called `$expectedWarningsFileName`." - ) - val warningsFromPlain = plainFile.collectExpectedWarningsWithLineNumbers( - warnPluginConfig, - generalConfig - ) - copyPaths.associate { copyPath -> - copyPath.name to warningsFromPlain.filter { it.fileName == copyPath.name } - } - } - ExpectedWarningsFormat.SARIF -> { - val warningsFromSarif = try { - collectWarningsFromSarif(expectedWarningsFileName, originalPaths, fs, workingDirectory) - } catch (e: Exception) { - throw SarifParsingException("We failed to parse sarif. Check the your tool generation of sarif report, cause: ${e.message}", e.cause) - } - copyPaths.associate { copyPath -> - copyPath.name to warningsFromSarif.filter { it.fileName == copyPath.name } - } + ): WarningMap { + val expectedWarningsFileName: String by lazy { + warnPluginConfig.expectedWarningsFileName + ?: throw IllegalArgumentException(" is not provided for expectedWarningsFormat=${warnPluginConfig.expectedWarningsFormat}") + } + return when (warnPluginConfig.expectedWarningsFormat) { + ExpectedWarningsFormat.PLAIN -> { + val anchorTestFilePath = originalPaths.first() + val plainFile = fs.findFileInAncestorDir(anchorTestFilePath, expectedWarningsFileName) ?: throw PluginException( + "Could not find PLAIN file with expected warnings/fixes for file $anchorTestFilePath. " + + "Please check if correct `WarningsFormat`/`FixFormat` is set (should be PLAIN) and if the file is present and called `$expectedWarningsFileName`." + ) + val warningsFromPlain = plainFile.collectExpectedWarningsWithLineNumbers( + warnPluginConfig, + generalConfig + ) + copyPaths.associate { copyPath -> + copyPath.name to warningsFromPlain.filter { it.fileName == copyPath.name } } - else -> - throw IllegalArgumentException(" cannot be provided for expectedWarningsFormat=${ExpectedWarningsFormat.IN_PLACE}") } - } - else -> { - require(warnPluginConfig.expectedWarningsFormat != ExpectedWarningsFormat.SARIF) { - " should be provided for expectedWarningsFormat=${ExpectedWarningsFormat.SARIF}" + ExpectedWarningsFormat.SARIF -> { + val warningsFromSarif = try { + collectWarningsFromSarif(expectedWarningsFileName, originalPaths, fs, workingDirectory) + } catch (e: Exception) { + throw SarifParsingException("We failed to parse sarif. Check the your tool generation of sarif report, cause: ${e.message}", e.cause) + } + copyPaths.associate { copyPath -> + copyPath.name to warningsFromSarif.filter { it.fileName == copyPath.name } + } } - copyPaths.associate { copyPath -> + else -> copyPaths.associate { copyPath -> val warningsForCurrentPath = copyPath.collectExpectedWarningsWithLineNumbers( warnPluginConfig, From f64f5e7690af5284c0906f8104d8304907cd1e4d Mon Sep 17 00:00:00 2001 From: Nariman Abdullin Date: Tue, 27 Feb 2024 18:12:56 +0300 Subject: [PATCH 4/4] self review 2 --- .../save/plugin/warn/WarnPlugin.kt | 32 +++++++++---------- .../plugin/warn/utils/WarningsExtraction.kt | 24 ++++++++++++++ 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt index 6f5a3688f..3cbdb96e4 100644 --- a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt +++ b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/WarnPlugin.kt @@ -4,7 +4,6 @@ import com.saveourtool.save.core.config.ActualWarningsFormat import com.saveourtool.save.core.config.ExpectedWarningsFormat import com.saveourtool.save.core.config.TestConfig import com.saveourtool.save.core.files.createFile -import com.saveourtool.save.core.files.findFileInAncestorDir import com.saveourtool.save.core.files.getWorkingDirectory import com.saveourtool.save.core.files.readLines import com.saveourtool.save.core.logging.describe @@ -14,7 +13,6 @@ import com.saveourtool.save.core.logging.logWarn import com.saveourtool.save.core.plugin.ExtraFlagsExtractor import com.saveourtool.save.core.plugin.GeneralConfig import com.saveourtool.save.core.plugin.Plugin -import com.saveourtool.save.core.plugin.PluginException import com.saveourtool.save.core.result.DebugInfo import com.saveourtool.save.core.result.Fail import com.saveourtool.save.core.result.TestResult @@ -28,6 +26,7 @@ import com.saveourtool.save.plugin.warn.sarif.toWarnings import com.saveourtool.save.plugin.warn.utils.CmdExecutorWarn import com.saveourtool.save.plugin.warn.utils.ResultsChecker import com.saveourtool.save.plugin.warn.utils.Warning +import com.saveourtool.save.plugin.warn.utils.collectWarningsFromPlain import com.saveourtool.save.plugin.warn.utils.collectWarningsFromSarif import com.saveourtool.save.plugin.warn.utils.collectionMultilineWarnings import com.saveourtool.save.plugin.warn.utils.collectionSingleWarnings @@ -259,7 +258,11 @@ class WarnPlugin( ) }.asSequence() - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress( + "TooGenericExceptionCaught", + "SwallowedException", + "TOO_LONG_FUNCTION" + ) private fun collectExpectedWarnings( generalConfig: GeneralConfig, warnPluginConfig: WarnPluginConfig, @@ -273,15 +276,12 @@ class WarnPlugin( } return when (warnPluginConfig.expectedWarningsFormat) { ExpectedWarningsFormat.PLAIN -> { - val anchorTestFilePath = originalPaths.first() - val plainFile = fs.findFileInAncestorDir(anchorTestFilePath, expectedWarningsFileName) ?: throw PluginException( - "Could not find PLAIN file with expected warnings/fixes for file $anchorTestFilePath. " + - "Please check if correct `WarningsFormat`/`FixFormat` is set (should be PLAIN) and if the file is present and called `$expectedWarningsFileName`." - ) - val warningsFromPlain = plainFile.collectExpectedWarningsWithLineNumbers( - warnPluginConfig, - generalConfig - ) + val warningsFromPlain = collectWarningsFromPlain(expectedWarningsFileName, originalPaths, fs) { plainFile -> + plainFile.collectExpectedWarningsWithLineNumbers( + warnPluginConfig, + generalConfig + ) + } copyPaths.associate { copyPath -> copyPath.name to warningsFromPlain.filter { it.fileName == copyPath.name } } @@ -298,10 +298,10 @@ class WarnPlugin( } else -> copyPaths.associate { copyPath -> val warningsForCurrentPath = - copyPath.collectExpectedWarningsWithLineNumbers( - warnPluginConfig, - generalConfig - ) + copyPath.collectExpectedWarningsWithLineNumbers( + warnPluginConfig, + generalConfig + ) copyPath.name to warningsForCurrentPath } } diff --git a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/utils/WarningsExtraction.kt b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/utils/WarningsExtraction.kt index 931ef8c04..f3809daca 100644 --- a/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/utils/WarningsExtraction.kt +++ b/save-plugins/warn-plugin/src/commonMain/kotlin/com/saveourtool/save/plugin/warn/utils/WarningsExtraction.kt @@ -5,6 +5,7 @@ package com.saveourtool.save.plugin.warn.utils +import com.saveourtool.save.core.files.findFileInAncestorDir import com.saveourtool.save.core.files.readFile import com.saveourtool.save.core.plugin.GeneralConfig import com.saveourtool.save.core.plugin.PluginException @@ -95,6 +96,29 @@ internal fun collectionSingleWarnings( .filterNotNull() .sortedBy { warn -> warn.message } +/** + * @param plainFileName + * @param originalPaths + * @param fs + * @param warningExtractor extractor of warning from [Path] + * @return a list of warnings extracted from PLAIN file for test [file] + * @throws PluginException + */ +internal fun collectWarningsFromPlain( + plainFileName: String, + originalPaths: List, + fs: FileSystem, + warningExtractor: (Path) -> List, +): List { + // Since we have one file for all tests, just take the first of them as anchor for calculation of paths + val anchorTestFilePath = originalPaths.first() + val plainFile = fs.findFileInAncestorDir(anchorTestFilePath, plainFileName) ?: throw PluginException( + "Could not find PLAIN file with expected warnings/fixes for file $anchorTestFilePath. " + + "Please check if correct `WarningsFormat`/`FixFormat` is set (should be PLAIN) and if the file is present and called `$plainFileName`." + ) + return warningExtractor(plainFile) +} + /** * @param sarifFileName * @param originalPaths