From 0b437a9c2139d14f34f6da0efdde7586881464c5 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Sun, 3 Dec 2023 22:36:31 -0500 Subject: [PATCH 01/12] Extract levelOption() util --- .../slack/cli/lint/LintBaselineMergerCli.kt | 14 ++------ src/main/kotlin/slack/cli/sarif/SarifUtil.kt | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 src/main/kotlin/slack/cli/sarif/SarifUtil.kt diff --git a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt index 63975cb..bdef9dd 100644 --- a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt +++ b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt @@ -20,7 +20,6 @@ import com.github.ajalt.clikt.parameters.options.default import com.github.ajalt.clikt.parameters.options.flag import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.options.required -import com.github.ajalt.clikt.parameters.types.enum import com.github.ajalt.clikt.parameters.types.path import com.google.auto.service.AutoService import com.tickaroo.tikxml.converter.htmlescape.StringEscapeUtils @@ -62,19 +61,13 @@ import nl.adaptivity.xmlutil.serialization.XML import nl.adaptivity.xmlutil.serialization.XmlSerialName import slack.cli.CommandFactory import slack.cli.projectDirOption +import slack.cli.sarif.levelOption import slack.cli.skipBuildAndCacheDirs /** A CLI that merges lint baseline xml files into one. */ public class LintBaselineMergerCli : CliktCommand(DESCRIPTION) { private companion object { const val DESCRIPTION = "Merges multiple lint baselines into one" - private val LEVEL_NAMES = - Level.entries.joinToString( - separator = ", ", - prefix = "[", - postfix = "]", - transform = Level::name - ) } @AutoService(CommandFactory::class) @@ -102,10 +95,7 @@ public class LintBaselineMergerCli : CliktCommand(DESCRIPTION) { ) .default("{message}") - private val level by - option("--level", "-l", help = "Priority level. Defaults to Error. Options are $LEVEL_NAMES") - .enum() - .default(Level.Error) + private val level by levelOption().default(Level.Error) private val verbose by option("--verbose", "-v").flag() diff --git a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt new file mode 100644 index 0000000..074af0f --- /dev/null +++ b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package slack.cli.sarif + +import com.github.ajalt.clikt.core.CliktCommand +import com.github.ajalt.clikt.parameters.options.NullableOption +import com.github.ajalt.clikt.parameters.options.option +import com.github.ajalt.clikt.parameters.types.enum +import io.github.detekt.sarif4k.Level + +private val LEVEL_NAMES = + Level.entries.joinToString(separator = ", ", prefix = "[", postfix = "]", transform = Level::name) + +internal fun CliktCommand.levelOption(): NullableOption { + return option( + "--level", + "-l", + help = "Priority level. Defaults to Error. Options are $LEVEL_NAMES" + ) + .enum() +} From 171776f574a937753e118d5adb749e3718a75e07 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Sun, 3 Dec 2023 22:36:46 -0500 Subject: [PATCH 02/12] Add level option to merging --- .../kotlin/slack/cli/sarif/MergeSarifReports.kt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt index 2e44691..ce73631 100644 --- a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt +++ b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt @@ -78,6 +78,8 @@ public class MergeSarifReports : CliktCommand(help = DESCRIPTION) { ) .flag() + private val level by levelOption() + private fun log(message: String) { if (verbose) { echo(message) @@ -117,9 +119,8 @@ public class MergeSarifReports : CliktCommand(help = DESCRIPTION) { filePrefix?.let { prefix -> // Find build files first, this gives us an easy hook to then go looking in build/reports - // dirs. - // Otherwise we don't have a way to easily exclude populated build dirs that would take - // forever. + // dirs. Otherwise we don't have a way to easily exclude populated build dirs that would + // take forever. val buildFiles = findBuildFiles() log("Finding sarif files") @@ -309,6 +310,13 @@ public class MergeSarifReports : CliktCommand(help = DESCRIPTION) { val ruleIndex = ruleIndicesById.getValue(ruleId) result.copy(ruleIndex = ruleIndex.toLong()) } + .map { + if (level != null) { + it.copy(level = this.level) + } else { + it + } + } .sortedWith( compareBy( { it.ruleIndex }, From ec9581e46bcf61175cf7630c133763700395afba Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Sun, 3 Dec 2023 22:41:26 -0500 Subject: [PATCH 03/12] Extract RESULT_SORT_COMPARATOR --- .../slack/cli/sarif/MergeSarifReports.kt | 14 ++--------- src/main/kotlin/slack/cli/sarif/SarifUtil.kt | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt index ce73631..a9764ac 100644 --- a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt +++ b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt @@ -23,6 +23,7 @@ import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.options.required import com.github.ajalt.clikt.parameters.types.path import com.google.auto.service.AutoService +import io.github.detekt.sarif4k.Result import io.github.detekt.sarif4k.SarifSchema210 import io.github.detekt.sarif4k.SarifSerializer import java.nio.file.Path @@ -317,18 +318,7 @@ public class MergeSarifReports : CliktCommand(help = DESCRIPTION) { it } } - .sortedWith( - compareBy( - { it.ruleIndex }, - { it.ruleID }, - { it.locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri }, - { it.locations?.firstOrNull()?.physicalLocation?.region?.startLine }, - { it.locations?.firstOrNull()?.physicalLocation?.region?.startColumn }, - { it.locations?.firstOrNull()?.physicalLocation?.region?.endLine }, - { it.locations?.firstOrNull()?.physicalLocation?.region?.endColumn }, - { it.message.text }, - ) - ) + .sortedWith(RESULT_SORT_COMPARATOR) val sarifToUse = if (removeUriPrefixes) { diff --git a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt index 074af0f..46d2bd9 100644 --- a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt +++ b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt @@ -20,6 +20,31 @@ import com.github.ajalt.clikt.parameters.options.NullableOption import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.types.enum import io.github.detekt.sarif4k.Level +import io.github.detekt.sarif4k.Result + +/** + * A comparator used to sort instances of the Result class. + * + * The comparison is done based on the following properties in the given order: + * - ruleIndex + * - ruleID + * - uri of the first physical location's artifact location + * - startLine of the first physical location's region + * - startColumn of the first physical location's region + * - endLine of the first physical location's region + * - endColumn of the first physical location's region + * - text of the message + */ +internal val RESULT_SORT_COMPARATOR = compareBy( + { it.ruleIndex }, + { it.ruleID }, + { it.locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri }, + { it.locations?.firstOrNull()?.physicalLocation?.region?.startLine }, + { it.locations?.firstOrNull()?.physicalLocation?.region?.startColumn }, + { it.locations?.firstOrNull()?.physicalLocation?.region?.endLine }, + { it.locations?.firstOrNull()?.physicalLocation?.region?.endColumn }, + { it.message.text }, +) private val LEVEL_NAMES = Level.entries.joinToString(separator = ", ", prefix = "[", postfix = "]", transform = Level::name) From 8dd111262e4e29d9ef8a5c2c2be6ad20f4edc336 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Sun, 3 Dec 2023 22:51:05 -0500 Subject: [PATCH 04/12] Add identity and shallow hash --- src/main/kotlin/slack/cli/sarif/SarifUtil.kt | 29 ++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt index 46d2bd9..3321427 100644 --- a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt +++ b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt @@ -21,6 +21,7 @@ import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.types.enum import io.github.detekt.sarif4k.Level import io.github.detekt.sarif4k.Result +import java.util.Objects /** * A comparator used to sort instances of the Result class. @@ -46,6 +47,34 @@ internal val RESULT_SORT_COMPARATOR = compareBy( { it.message.text }, ) +/** + * Returns the identity hash code for the [Result] object. This seeks to create a hash code for results that point to + * the same issue+location, but not necessarily the same [Result.level]/[Result.message]. + */ +internal val Result.identityHash: Int + get() = Objects.hash( + ruleID, + locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri, + locations?.firstOrNull()?.physicalLocation?.region?.startLine, + locations?.firstOrNull()?.physicalLocation?.region?.startColumn, + locations?.firstOrNull()?.physicalLocation?.region?.endLine, + locations?.firstOrNull()?.physicalLocation?.region?.endColumn, + ) + +/** + * Returns the shallow hash code for the [Result] object. This seeks to create a hash code for results that include the + * [identityHash] but also differentiate if the [Result.level]/[Result.message] are different. + */ +internal val Result.shallowHash: Int + get() = Objects.hash( + ruleID, + locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri, + locations?.firstOrNull()?.physicalLocation?.region?.startLine, + locations?.firstOrNull()?.physicalLocation?.region?.startColumn, + locations?.firstOrNull()?.physicalLocation?.region?.endLine, + locations?.firstOrNull()?.physicalLocation?.region?.endColumn, + ) + private val LEVEL_NAMES = Level.entries.joinToString(separator = ", ", prefix = "[", postfix = "]", transform = Level::name) From b593fc95d60d8bee3dea9336faf9f2d355159e7e Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Sun, 3 Dec 2023 23:04:07 -0500 Subject: [PATCH 05/12] Mark merged lint baselines as suppressed --- src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt index bdef9dd..8e05ac4 100644 --- a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt +++ b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt @@ -36,6 +36,8 @@ import io.github.detekt.sarif4k.ReportingDescriptor import io.github.detekt.sarif4k.Result import io.github.detekt.sarif4k.Run import io.github.detekt.sarif4k.SarifSchema210 +import io.github.detekt.sarif4k.Suppression +import io.github.detekt.sarif4k.SuppressionKind import io.github.detekt.sarif4k.Tool import io.github.detekt.sarif4k.ToolComponent import io.github.detekt.sarif4k.Version @@ -156,6 +158,12 @@ public class LintBaselineMergerCli : CliktCommand(DESCRIPTION) { level = level, ruleIndex = ruleIndices.getValue(id), locations = listOf(issue.toLocation(projectPath)), + suppressions = listOf( + Suppression( + kind = SuppressionKind.External, + justification = "This issue was suppressed by the baseline" + ) + ), message = Message( text = From 87773fc63afe29be2af3f038e7e2984b20ccd9a0 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Sun, 3 Dec 2023 23:04:46 -0500 Subject: [PATCH 06/12] Spotless and API dump --- api/kotlin-cli-util.api | 8 +-- .../slack/cli/lint/LintBaselineMergerCli.kt | 13 ++-- .../slack/cli/sarif/MergeSarifReports.kt | 1 - src/main/kotlin/slack/cli/sarif/SarifUtil.kt | 65 ++++++++++--------- 4 files changed, 46 insertions(+), 41 deletions(-) diff --git a/api/kotlin-cli-util.api b/api/kotlin-cli-util.api index af4ec36..31eb40d 100644 --- a/api/kotlin-cli-util.api +++ b/api/kotlin-cli-util.api @@ -80,10 +80,6 @@ public final class slack/cli/lint/LintBaselineMergerCli : com/github/ajalt/clikt public fun run ()V } -public synthetic class slack/cli/lint/LintBaselineMergerCli$EntriesMappings { - public static final synthetic field entries$0 Lkotlin/enums/EnumEntries; -} - public final class slack/cli/lint/LintBaselineMergerCli$Factory : slack/cli/CommandFactory { public fun ()V public fun create ()Lcom/github/ajalt/clikt/core/CliktCommand; @@ -104,6 +100,10 @@ public final class slack/cli/sarif/MergeSarifReports$Factory : slack/cli/Command public fun getKey ()Ljava/lang/String; } +public synthetic class slack/cli/sarif/SarifUtilKt$EntriesMappings { + public static final synthetic field entries$0 Lkotlin/enums/EnumEntries; +} + public final class slack/cli/shellsentry/AnalysisResult { public fun (Ljava/lang/String;Ljava/lang/String;Lslack/cli/shellsentry/RetrySignal;ILkotlin/jvm/functions/Function1;)V public final fun component1 ()Ljava/lang/String; diff --git a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt index 8e05ac4..8b10caf 100644 --- a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt +++ b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt @@ -158,12 +158,13 @@ public class LintBaselineMergerCli : CliktCommand(DESCRIPTION) { level = level, ruleIndex = ruleIndices.getValue(id), locations = listOf(issue.toLocation(projectPath)), - suppressions = listOf( - Suppression( - kind = SuppressionKind.External, - justification = "This issue was suppressed by the baseline" - ) - ), + suppressions = + listOf( + Suppression( + kind = SuppressionKind.External, + justification = "This issue was suppressed by the baseline" + ) + ), message = Message( text = diff --git a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt index a9764ac..58bebf4 100644 --- a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt +++ b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt @@ -23,7 +23,6 @@ import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.options.required import com.github.ajalt.clikt.parameters.types.path import com.google.auto.service.AutoService -import io.github.detekt.sarif4k.Result import io.github.detekt.sarif4k.SarifSchema210 import io.github.detekt.sarif4k.SarifSerializer import java.nio.file.Path diff --git a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt index 3321427..4bb8d56 100644 --- a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt +++ b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt @@ -36,44 +36,49 @@ import java.util.Objects * - endColumn of the first physical location's region * - text of the message */ -internal val RESULT_SORT_COMPARATOR = compareBy( - { it.ruleIndex }, - { it.ruleID }, - { it.locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri }, - { it.locations?.firstOrNull()?.physicalLocation?.region?.startLine }, - { it.locations?.firstOrNull()?.physicalLocation?.region?.startColumn }, - { it.locations?.firstOrNull()?.physicalLocation?.region?.endLine }, - { it.locations?.firstOrNull()?.physicalLocation?.region?.endColumn }, - { it.message.text }, -) +internal val RESULT_SORT_COMPARATOR = + compareBy( + { it.ruleIndex }, + { it.ruleID }, + { it.locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri }, + { it.locations?.firstOrNull()?.physicalLocation?.region?.startLine }, + { it.locations?.firstOrNull()?.physicalLocation?.region?.startColumn }, + { it.locations?.firstOrNull()?.physicalLocation?.region?.endLine }, + { it.locations?.firstOrNull()?.physicalLocation?.region?.endColumn }, + { it.message.text }, + ) /** - * Returns the identity hash code for the [Result] object. This seeks to create a hash code for results that point to - * the same issue+location, but not necessarily the same [Result.level]/[Result.message]. + * Returns the identity hash code for the [Result] object. This seeks to create a hash code for + * results that point to the same issue+location, but not necessarily the same + * [Result.level]/[Result.message]. */ internal val Result.identityHash: Int - get() = Objects.hash( - ruleID, - locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri, - locations?.firstOrNull()?.physicalLocation?.region?.startLine, - locations?.firstOrNull()?.physicalLocation?.region?.startColumn, - locations?.firstOrNull()?.physicalLocation?.region?.endLine, - locations?.firstOrNull()?.physicalLocation?.region?.endColumn, - ) + get() = + Objects.hash( + ruleID, + locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri, + locations?.firstOrNull()?.physicalLocation?.region?.startLine, + locations?.firstOrNull()?.physicalLocation?.region?.startColumn, + locations?.firstOrNull()?.physicalLocation?.region?.endLine, + locations?.firstOrNull()?.physicalLocation?.region?.endColumn, + ) /** - * Returns the shallow hash code for the [Result] object. This seeks to create a hash code for results that include the - * [identityHash] but also differentiate if the [Result.level]/[Result.message] are different. + * Returns the shallow hash code for the [Result] object. This seeks to create a hash code for + * results that include the [identityHash] but also differentiate if the + * [Result.level]/[Result.message] are different. */ internal val Result.shallowHash: Int - get() = Objects.hash( - ruleID, - locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri, - locations?.firstOrNull()?.physicalLocation?.region?.startLine, - locations?.firstOrNull()?.physicalLocation?.region?.startColumn, - locations?.firstOrNull()?.physicalLocation?.region?.endLine, - locations?.firstOrNull()?.physicalLocation?.region?.endColumn, - ) + get() = + Objects.hash( + ruleID, + locations?.firstOrNull()?.physicalLocation?.artifactLocation?.uri, + locations?.firstOrNull()?.physicalLocation?.region?.startLine, + locations?.firstOrNull()?.physicalLocation?.region?.startColumn, + locations?.firstOrNull()?.physicalLocation?.region?.endLine, + locations?.firstOrNull()?.physicalLocation?.region?.endColumn, + ) private val LEVEL_NAMES = Level.entries.joinToString(separator = ", ", prefix = "[", postfix = "]", transform = Level::name) From 1e29eab162cc14906bca9142c16238866dc17e17 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Mon, 4 Dec 2023 01:22:27 -0500 Subject: [PATCH 07/12] Extract util --- .../kotlin/slack/cli/lint/LintBaselineMergerCli.kt | 11 ++--------- src/main/kotlin/slack/cli/sarif/SarifUtil.kt | 7 +++++++ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt index 8b10caf..bfefe46 100644 --- a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt +++ b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt @@ -36,8 +36,6 @@ import io.github.detekt.sarif4k.ReportingDescriptor import io.github.detekt.sarif4k.Result import io.github.detekt.sarif4k.Run import io.github.detekt.sarif4k.SarifSchema210 -import io.github.detekt.sarif4k.Suppression -import io.github.detekt.sarif4k.SuppressionKind import io.github.detekt.sarif4k.Tool import io.github.detekt.sarif4k.ToolComponent import io.github.detekt.sarif4k.Version @@ -63,6 +61,7 @@ import nl.adaptivity.xmlutil.serialization.XML import nl.adaptivity.xmlutil.serialization.XmlSerialName import slack.cli.CommandFactory import slack.cli.projectDirOption +import slack.cli.sarif.BASELINE_SUPPRESSION import slack.cli.sarif.levelOption import slack.cli.skipBuildAndCacheDirs @@ -158,13 +157,7 @@ public class LintBaselineMergerCli : CliktCommand(DESCRIPTION) { level = level, ruleIndex = ruleIndices.getValue(id), locations = listOf(issue.toLocation(projectPath)), - suppressions = - listOf( - Suppression( - kind = SuppressionKind.External, - justification = "This issue was suppressed by the baseline" - ) - ), + suppressions = listOf(BASELINE_SUPPRESSION), message = Message( text = diff --git a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt index 4bb8d56..b7bad03 100644 --- a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt +++ b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt @@ -21,8 +21,15 @@ import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.types.enum import io.github.detekt.sarif4k.Level import io.github.detekt.sarif4k.Result +import io.github.detekt.sarif4k.Suppression +import io.github.detekt.sarif4k.SuppressionKind import java.util.Objects +internal val BASELINE_SUPPRESSION: Suppression = Suppression( + kind = SuppressionKind.External, + justification = "This issue was suppressed by the baseline" +) + /** * A comparator used to sort instances of the Result class. * From 6e3b826884e98b5d5724c4866e0078988b07d0cb Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Mon, 4 Dec 2023 01:44:08 -0500 Subject: [PATCH 08/12] Reuse SarifSerializer --- src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt index bfefe46..12b94fb 100644 --- a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt +++ b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt @@ -36,6 +36,7 @@ import io.github.detekt.sarif4k.ReportingDescriptor import io.github.detekt.sarif4k.Result import io.github.detekt.sarif4k.Run import io.github.detekt.sarif4k.SarifSchema210 +import io.github.detekt.sarif4k.SarifSerializer import io.github.detekt.sarif4k.Tool import io.github.detekt.sarif4k.ToolComponent import io.github.detekt.sarif4k.Version @@ -100,12 +101,6 @@ public class LintBaselineMergerCli : CliktCommand(DESCRIPTION) { private val verbose by option("--verbose", "-v").flag() - @OptIn(ExperimentalSerializationApi::class) - private val json = Json { - prettyPrint = true - prettyPrintIndent = " " - } - private val xml = XML { defaultPolicy { ignoreUnknownChildren() } } override fun run() { @@ -169,7 +164,7 @@ public class LintBaselineMergerCli : CliktCommand(DESCRIPTION) { ) ) - json.encodeToString(SarifSchema210.serializer(), outputSarif).let { outputFile.writeText(it) } + SarifSerializer.toJson(outputSarif).let { outputFile.writeText(it) } } private fun parseIssues(): Map { From 6a392a72041e2af0d1e5bfb6ea4a75bed1209a0a Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Mon, 4 Dec 2023 01:45:32 -0500 Subject: [PATCH 09/12] Extract merger --- .../slack/cli/sarif/MergeSarifReports.kt | 58 +----------------- src/main/kotlin/slack/cli/sarif/SarifUtil.kt | 59 +++++++++++++++++++ 2 files changed, 60 insertions(+), 57 deletions(-) diff --git a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt index 58bebf4..c67a456 100644 --- a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt +++ b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt @@ -284,63 +284,7 @@ public class MergeSarifReports : CliktCommand(help = DESCRIPTION) { private fun merge(inputs: List) { log("Parsing ${inputs.size} sarif files") - val sarifs = loadSarifs(inputs) - - log("Merging ${inputs.size} sarif files") - val sortedMergedRules = - sarifs - .flatMap { it.runs.single().tool.driver.rules.orEmpty() } - .associateBy { it.id } - .toSortedMap() - val mergedResults = - sarifs - .flatMap { it.runs.single().results.orEmpty() } - // Some projects produce multiple reports for different variants, so we need to - // de-dupe. - .distinct() - .also { echo("Merged ${it.size} results") } - - // Update rule.ruleIndex to match the index in rulesToAdd - val ruleIndicesById = - sortedMergedRules.entries.withIndex().associate { (index, entry) -> entry.key to index } - val correctedResults = - mergedResults - .map { result -> - val ruleId = result.ruleID - val ruleIndex = ruleIndicesById.getValue(ruleId) - result.copy(ruleIndex = ruleIndex.toLong()) - } - .map { - if (level != null) { - it.copy(level = this.level) - } else { - it - } - } - .sortedWith(RESULT_SORT_COMPARATOR) - - val sarifToUse = - if (removeUriPrefixes) { - // Just use the first if we don't care about originalUriBaseIDs - sarifs.first() - } else { - // Pick a sarif file to use as the base for the merged sarif file. We want one that has an - // `originalURIBaseIDS` too since parsing possibly uses this. - sarifs.find { it.runs.firstOrNull()?.originalURIBaseIDS?.isNotEmpty() == true } - ?: error("No sarif files had originalURIBaseIDS set, can't merge") - } - - // Note: we don't sort these results by anything currently (location, etc), but maybe some day - // we should if it matters for caching - val runToCopy = sarifToUse.runs.single() - val mergedTool = - runToCopy.tool.copy( - driver = runToCopy.tool.driver.copy(rules = sortedMergedRules.values.toList()) - ) - - val mergedSarif = - sarifToUse.copy(runs = listOf(runToCopy.copy(tool = mergedTool, results = correctedResults))) - + val mergedSarif = loadSarifs(inputs).merge(levelOverride = level, log = ::log) log("Writing merged sarif to $outputFile") prepareOutput() outputFile.writeText(SarifSerializer.toJson(mergedSarif)) diff --git a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt index b7bad03..346bc1e 100644 --- a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt +++ b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt @@ -21,6 +21,7 @@ import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.types.enum import io.github.detekt.sarif4k.Level import io.github.detekt.sarif4k.Result +import io.github.detekt.sarif4k.SarifSchema210 import io.github.detekt.sarif4k.Suppression import io.github.detekt.sarif4k.SuppressionKind import java.util.Objects @@ -98,3 +99,61 @@ internal fun CliktCommand.levelOption(): NullableOption { ) .enum() } + +internal fun List.merge( + levelOverride: Level? = null, + removeUriPrefixes: Boolean = false, + log: (String) -> Unit, +): SarifSchema210 { + log("Merging $size sarif files") + val sortedMergedRules = + flatMap { it.runs.single().tool.driver.rules.orEmpty() } + .associateBy { it.id } + .toSortedMap() + val mergedResults = + flatMap { it.runs.single().results.orEmpty() } + // Some projects produce multiple reports for different variants, so we need to + // de-dupe. + .distinct() + .also { log("Merged ${it.size} results") } + + // Update rule.ruleIndex to match the index in rulesToAdd + val ruleIndicesById = + sortedMergedRules.entries.withIndex().associate { (index, entry) -> entry.key to index } + val correctedResults = + mergedResults + .map { result -> + val ruleId = result.ruleID + val ruleIndex = ruleIndicesById.getValue(ruleId) + result.copy(ruleIndex = ruleIndex.toLong()) + } + .map { + if (levelOverride != null) { + it.copy(level = levelOverride) + } else { + it + } + } + .sortedWith(RESULT_SORT_COMPARATOR) + + val sarifToUse = + if (removeUriPrefixes) { + // Just use the first if we don't care about originalUriBaseIDs + first() + } else { + // Pick a sarif file to use as the base for the merged sarif file. We want one that has an + // `originalURIBaseIDS` too since parsing possibly uses this. + find { it.runs.firstOrNull()?.originalURIBaseIDS?.isNotEmpty() == true } + ?: error("No sarif files had originalURIBaseIDS set, can't merge") + } + + // Note: we don't sort these results by anything currently (location, etc), but maybe some day + // we should if it matters for caching + val runToCopy = sarifToUse.runs.single() + val mergedTool = + runToCopy.tool.copy( + driver = runToCopy.tool.driver.copy(rules = sortedMergedRules.values.toList()) + ) + + return sarifToUse.copy(runs = listOf(runToCopy.copy(tool = mergedTool, results = correctedResults))) +} From 868cef3c694a4ed46e0766bd10a80a688be74a67 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Tue, 5 Dec 2023 00:07:29 -0500 Subject: [PATCH 10/12] Spotless --- src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt index c67a456..72a4946 100644 --- a/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt +++ b/src/main/kotlin/slack/cli/sarif/MergeSarifReports.kt @@ -61,6 +61,7 @@ public class MergeSarifReports : CliktCommand(help = DESCRIPTION) { "When enabled, remaps uri roots to include the subproject path (relative to the root project)." ) .flag() + private val removeUriPrefixes by option( "--remove-uri-prefixes", @@ -284,7 +285,9 @@ public class MergeSarifReports : CliktCommand(help = DESCRIPTION) { private fun merge(inputs: List) { log("Parsing ${inputs.size} sarif files") - val mergedSarif = loadSarifs(inputs).merge(levelOverride = level, log = ::log) + val mergedSarif = + loadSarifs(inputs) + .merge(levelOverride = level, removeUriPrefixes = removeUriPrefixes, log = ::log) log("Writing merged sarif to $outputFile") prepareOutput() outputFile.writeText(SarifSerializer.toJson(mergedSarif)) From c7ad7e8780725353413127be1352cce64c7cfc96 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Tue, 5 Dec 2023 17:30:20 -0500 Subject: [PATCH 11/12] Implement ApplyBaselinesToSarifs --- api/kotlin-cli-util.api | 13 ++ .../slack/cli/lint/LintBaselineMergerCli.kt | 2 - .../slack/cli/sarif/ApplyBaselinesToSarifs.kt | 198 ++++++++++++++++++ src/main/kotlin/slack/cli/sarif/SarifUtil.kt | 26 ++- 4 files changed, 229 insertions(+), 10 deletions(-) create mode 100644 src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt diff --git a/api/kotlin-cli-util.api b/api/kotlin-cli-util.api index 31eb40d..ce26c6b 100644 --- a/api/kotlin-cli-util.api +++ b/api/kotlin-cli-util.api @@ -87,6 +87,19 @@ public final class slack/cli/lint/LintBaselineMergerCli$Factory : slack/cli/Comm public fun getKey ()Ljava/lang/String; } +public final class slack/cli/sarif/ApplyBaselinesToSarifs : com/github/ajalt/clikt/core/CliktCommand { + public static final field DESCRIPTION Ljava/lang/String; + public fun ()V + public fun run ()V +} + +public final class slack/cli/sarif/ApplyBaselinesToSarifs$Factory : slack/cli/CommandFactory { + public fun ()V + public fun create ()Lcom/github/ajalt/clikt/core/CliktCommand; + public fun getDescription ()Ljava/lang/String; + public fun getKey ()Ljava/lang/String; +} + public final class slack/cli/sarif/MergeSarifReports : com/github/ajalt/clikt/core/CliktCommand { public static final field DESCRIPTION Ljava/lang/String; public fun ()V diff --git a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt index 12b94fb..94906da 100644 --- a/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt +++ b/src/main/kotlin/slack/cli/lint/LintBaselineMergerCli.kt @@ -48,7 +48,6 @@ import kotlin.io.path.name import kotlin.io.path.readText import kotlin.io.path.relativeTo import kotlin.io.path.writeText -import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.KSerializer import kotlinx.serialization.Serializable import kotlinx.serialization.descriptors.PrimitiveKind @@ -56,7 +55,6 @@ import kotlinx.serialization.descriptors.PrimitiveSerialDescriptor import kotlinx.serialization.descriptors.SerialDescriptor import kotlinx.serialization.encoding.Decoder import kotlinx.serialization.encoding.Encoder -import kotlinx.serialization.json.Json import kotlinx.serialization.serializer import nl.adaptivity.xmlutil.serialization.XML import nl.adaptivity.xmlutil.serialization.XmlSerialName diff --git a/src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt b/src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt new file mode 100644 index 0000000..c504069 --- /dev/null +++ b/src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt @@ -0,0 +1,198 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package slack.cli.sarif + +import com.github.ajalt.clikt.core.CliktCommand +import com.github.ajalt.clikt.parameters.options.flag +import com.github.ajalt.clikt.parameters.options.option +import com.github.ajalt.clikt.parameters.options.required +import com.github.ajalt.clikt.parameters.types.enum +import com.github.ajalt.clikt.parameters.types.path +import com.google.auto.service.AutoService +import io.github.detekt.sarif4k.BaselineState +import io.github.detekt.sarif4k.SarifSchema210 +import io.github.detekt.sarif4k.SarifSerializer +import kotlin.io.path.readText +import kotlin.io.path.writeText +import kotlin.system.exitProcess +import slack.cli.CommandFactory + +/** A CLI that applies baselines data to a SARIF file. See the docs on [Mode] for more details. */ +public class ApplyBaselinesToSarifs : CliktCommand(help = DESCRIPTION) { + + @AutoService(CommandFactory::class) + public class Factory : CommandFactory { + override val key: String = "apply-baselines-to-sarifs" + override val description: String = DESCRIPTION + + override fun create(): CliktCommand = ApplyBaselinesToSarifs() + } + + private companion object { + const val DESCRIPTION = "A CLI that applies baselines data to a SARIF file." + } + + private val baseline by + option("--baseline", "-b", help = "The baseline SARIF file to use.") + .path(mustExist = true, canBeDir = false) + .required() + + private val current by + option("--current", "-c", help = "The SARIF file to apply the baseline to.") + .path(mustExist = true, canBeDir = false) + .required() + + private val output by + option("--output", "-o", help = "The output SARIF file to write.") + .path(canBeDir = false) + .required() + + private val removeUriPrefixes by + option( + "--remove-uri-prefixes", + help = + "When enabled, removes the root project directory from location uris such that they are only " + + "relative to the root project dir." + ) + .flag() + + private val mode by + option("--mode", "-m", help = "The mode to run in.").enum(ignoreCase = true).required() + + private val includeAbsent by + option("--include-absent", "-a", help = "Include absent results in updating.").flag() + + override fun run() { + if (includeAbsent && mode != Mode.UPDATE) { + echo("--include-absent can only be used with --mode=update", err = true) + exitProcess(1) + } + val baseline = SarifSerializer.fromJson(baseline.readText()) + val sarifToUpdate = SarifSerializer.fromJson(current.readText()) + + val updatedSarif = sarifToUpdate.applyBaseline(baseline) + + output.writeText(SarifSerializer.toJson(updatedSarif)) + } + + private fun SarifSchema210.applyBaseline(baseline: SarifSchema210): SarifSchema210 { + // Assume a single run for now + val results = runs.first().results!! + val baselineResults = baseline.runs.first().results!! + + val suppressions = listOf(BASELINE_SUPPRESSION) + + return when (mode) { + Mode.MERGE -> { + // Mark baselines as suppressed and no baseline state + val suppressedBaselineSchema = + baseline.copy( + runs = + baseline.runs.map { run -> + run.copy( + results = + baselineResults.map { + it.copy(baselineState = null, suppressions = suppressions) + } + ) + } + ) + // Mark new results as new and not suppressed + val newSchema = + copy( + runs = + runs.map { run -> + run.copy( + results = + results.map { + it.copy(baselineState = BaselineState.New, suppressions = emptyList()) + } + ) + } + ) + // Merge the two + listOf(suppressedBaselineSchema, newSchema) + .merge(removeUriPrefixes = removeUriPrefixes, log = ::echo) + } + Mode.UPDATE -> { + val baselineResultsByHash = baselineResults.associateBy { it.identityHash } + val resultsByHash = results.associateBy { it.identityHash } + // New -> No match in the baseline + // Unchanged -> Exact match in the baseline. + // Updated -> Partial match is found. Not sure if we could realistically detect this well + // based on just ID and location though. May be that the only change we could + // match here would be if the severity changes + // Absent -> Nothing to report, means this issue was fixed presumably. Not sure how this + // would show up in a baseline state tbh + val baselinedResults = + results.map { result -> + val baselineResult = baselineResultsByHash[result.identityHash] + when { + baselineResult == null -> { + // No baseline result, so it's new! + result.copy(baselineState = BaselineState.New) + } + baselineResult.shallowHash == result.shallowHash -> { + // They're they same, so it's unchanged + result.copy(baselineState = BaselineState.Unchanged, suppressions = suppressions) + } + else -> { + // They're different, so it's updated + result.copy(baselineState = BaselineState.Updated, suppressions = suppressions) + } + } + } + val absentResults = + if (includeAbsent) { + // Create a copy of the baseline results that are absent with a suppression + baselineResults + .filter { result -> result.identityHash !in resultsByHash } + .map { it.copy(baselineState = BaselineState.Absent, suppressions = suppressions) } + } else { + emptyList() + } + val absentResultsSchema = + baseline.copy(runs = runs.map { run -> run.copy(results = absentResults) }) + val newCurrentSchema = copy(runs = runs.map { run -> run.copy(results = baselinedResults) }) + + newCurrentSchema.mergeWith( + absentResultsSchema, + removeUriPrefixes = removeUriPrefixes, + log = ::echo + ) + } + } + } + + internal enum class Mode { + /** + * Merge two SARIFs, this does the following: + * - Marks the baseline results as "suppressed". + * - Marks the new results as "new". + * + * The two SARIFs are deemed to be distinct results and have no overlaps. + */ + MERGE, + /** + * Update the input SARIF based on a previous baseline: + * - Marks the new results as "new". + * - Marks the absent results as "absent" (aka "fixed"). + * - Mark remaining as updated or unchanged. + * - No changes are made to suppressions. + */ + UPDATE, + } +} diff --git a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt index 346bc1e..fadbc64 100644 --- a/src/main/kotlin/slack/cli/sarif/SarifUtil.kt +++ b/src/main/kotlin/slack/cli/sarif/SarifUtil.kt @@ -26,10 +26,11 @@ import io.github.detekt.sarif4k.Suppression import io.github.detekt.sarif4k.SuppressionKind import java.util.Objects -internal val BASELINE_SUPPRESSION: Suppression = Suppression( - kind = SuppressionKind.External, - justification = "This issue was suppressed by the baseline" -) +internal val BASELINE_SUPPRESSION: Suppression = + Suppression( + kind = SuppressionKind.External, + justification = "This issue was suppressed by the baseline" + ) /** * A comparator used to sort instances of the Result class. @@ -100,6 +101,15 @@ internal fun CliktCommand.levelOption(): NullableOption { .enum() } +internal fun SarifSchema210.mergeWith( + other: SarifSchema210, + levelOverride: Level? = null, + removeUriPrefixes: Boolean = false, + log: (String) -> Unit, +): SarifSchema210 { + return listOf(this, other).merge(levelOverride, removeUriPrefixes, log) +} + internal fun List.merge( levelOverride: Level? = null, removeUriPrefixes: Boolean = false, @@ -107,9 +117,7 @@ internal fun List.merge( ): SarifSchema210 { log("Merging $size sarif files") val sortedMergedRules = - flatMap { it.runs.single().tool.driver.rules.orEmpty() } - .associateBy { it.id } - .toSortedMap() + flatMap { it.runs.single().tool.driver.rules.orEmpty() }.associateBy { it.id }.toSortedMap() val mergedResults = flatMap { it.runs.single().results.orEmpty() } // Some projects produce multiple reports for different variants, so we need to @@ -155,5 +163,7 @@ internal fun List.merge( driver = runToCopy.tool.driver.copy(rules = sortedMergedRules.values.toList()) ) - return sarifToUse.copy(runs = listOf(runToCopy.copy(tool = mergedTool, results = correctedResults))) + return sarifToUse.copy( + runs = listOf(runToCopy.copy(tool = mergedTool, results = correctedResults)) + ) } From 80541de3ad64e9fda849ec861af2764d647ec1f4 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Tue, 5 Dec 2023 17:30:55 -0500 Subject: [PATCH 12/12] Detekt --- src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt b/src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt index c504069..b7282cf 100644 --- a/src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt +++ b/src/main/kotlin/slack/cli/sarif/ApplyBaselinesToSarifs.kt @@ -88,6 +88,7 @@ public class ApplyBaselinesToSarifs : CliktCommand(help = DESCRIPTION) { output.writeText(SarifSerializer.toJson(updatedSarif)) } + @Suppress("LongMethod") private fun SarifSchema210.applyBaseline(baseline: SarifSchema210): SarifSchema210 { // Assume a single run for now val results = runs.first().results!!