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

Allow to add arbitrary values to PropertyBag #101

Merged
merged 25 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2a908c0
Implement PropertyBag properly
BraisGabin Feb 13, 2024
ad749e9
Don't expose JsonElement
BraisGabin Feb 13, 2024
2485362
Update src/commonMain/kotlin/io/github/detekt/sarif4k/SarifSerializer.kt
BraisGabin Feb 13, 2024
0da43f8
Update src/commonMain/kotlin/io/github/detekt/sarif4k/SarifSerializer.kt
BraisGabin Feb 14, 2024
3acbeca
Update src/commonMain/kotlin/io/github/detekt/sarif4k/SarifSerializer.kt
BraisGabin Feb 14, 2024
aa8184f
Move code to its own file
BraisGabin Feb 22, 2024
8d60533
Improve error messages
BraisGabin Feb 22, 2024
62a7ea8
Use companion object to follow the convention in the rest of the file
BraisGabin Feb 22, 2024
99f1664
move Arrays down
BraisGabin Feb 22, 2024
a263b46
WIP
BraisGabin Feb 22, 2024
fa1b557
Add more tests
BraisGabin Feb 22, 2024
9b5f8b2
Merge branch 'main' into fix-78
BraisGabin Feb 22, 2024
400e08c
Reduce visibility
BraisGabin Feb 23, 2024
a94dcef
Rename
BraisGabin Feb 23, 2024
589a2a2
format
BraisGabin Feb 23, 2024
274ab50
Add missing test
BraisGabin Feb 23, 2024
05701ea
format
BraisGabin Feb 23, 2024
9775c15
format
BraisGabin Feb 23, 2024
9245538
Simplify code
BraisGabin Feb 23, 2024
dd3a05f
Improve exception messages
BraisGabin Feb 23, 2024
0bec44d
Update src/commonMain/kotlin/io/github/detekt/sarif4k/JsonElementExte…
BraisGabin Feb 26, 2024
da90097
Update src/commonMain/kotlin/io/github/detekt/sarif4k/SarifDataBindin…
BraisGabin Feb 26, 2024
577564f
Update src/commonMain/kotlin/io/github/detekt/sarif4k/SarifDataBindin…
BraisGabin Feb 26, 2024
e6d1b83
Add corrections
BraisGabin Feb 26, 2024
534d89e
Add comment
BraisGabin Feb 27, 2024
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
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ kotlin {
implementation(kotlin("stdlib"))
implementation(kotlin("test"))
implementation("org.junit.jupiter:junit-jupiter-api:5.10.2")
implementation("org.junit.jupiter:junit-jupiter-params:5.10.2")
runtimeOnly("org.junit.jupiter:junit-jupiter-engine:5.10.2")
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package io.github.detekt.sarif4k

import kotlinx.serialization.json.JsonArray
import kotlinx.serialization.json.JsonElement
import kotlinx.serialization.json.JsonNull
import kotlinx.serialization.json.JsonObject
import kotlinx.serialization.json.JsonPrimitive

// Extracted from https://github.com/Kotlin/kotlinx.serialization/issues/296#issuecomment-1859572541
private fun Any?.toJsonElement(): JsonElement = when (this) {
null -> JsonNull
is JsonElement -> this
is Map<*, *> -> toJsonElement()
is Collection<*> -> toJsonElement()
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
is Boolean -> JsonPrimitive(this)
is Number -> JsonPrimitive(this)
is String -> JsonPrimitive(this)
is Enum<*> -> JsonPrimitive(this.toString())
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
is ByteArray -> this.toList().toJsonElement()
is CharArray -> this.toList().toJsonElement()
is ShortArray -> this.toList().toJsonElement()
is IntArray -> this.toList().toJsonElement()
is LongArray -> this.toList().toJsonElement()
is FloatArray -> this.toList().toJsonElement()
is DoubleArray -> this.toList().toJsonElement()
is BooleanArray -> this.toList().toJsonElement()
is Array<*> -> toJsonElement()
else -> error("Can't serialize unknown type: $this")
}

internal fun Map<*, *>.toJsonElement(): JsonObject {
val map = this.map { (key, value) ->
key as? String ?: error("$key is not allowed as JSON key, please use a String!")
key to value.toJsonElement()
}
return JsonObject(map.toMap())
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
}

private fun Collection<*>.toJsonElement(): JsonArray {
return JsonArray(this.map { it.toJsonElement() })
}

private fun Array<*>.toJsonElement(): JsonArray {
return JsonArray(this.map { it.toJsonElement() })
}

private fun JsonElement.toNativeObject(): Any? = when (this) {
JsonNull -> null
is JsonArray -> this.map { it.toNativeObject() }
is JsonObject -> this.toNativeObject()
is JsonPrimitive -> if (isString) content else {
content
.toBooleanStrictOrNull()
?: content.toLongOrNull()
?: content.toDoubleOrNull()
?: error("Unknown primitive: $content")
}
}

internal fun JsonObject.toNativeObject(): Map<String, Any?> {
return this.map { (key, value) -> key to value.toNativeObject() }.toMap()
}
25 changes: 18 additions & 7 deletions src/commonMain/kotlin/io/github/detekt/sarif4k/Merging.kt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@file:JvmName("Merging")

package io.github.detekt.sarif4k

import kotlin.jvm.JvmName
Expand All @@ -10,7 +11,12 @@ fun SarifSchema210.merge(other: SarifSchema210): SarifSchema210 {
val mergedExternalProperties = (inlineExternalProperties.orEmpty() + other.inlineExternalProperties.orEmpty())
.takeIf { it.isNotEmpty() }

val mergedProperties = properties.merge(other.properties)
val mergedProperties = when {
properties != null && other.properties != null -> properties.merge(other.properties)
properties != null -> properties
other.properties != null -> other.properties
else -> null
}

val mergedRuns = runs.merge(other.runs)

Expand All @@ -23,14 +29,19 @@ fun SarifSchema210.merge(other: SarifSchema210): SarifSchema210 {
)
}

fun PropertyBag?.merge(other: PropertyBag?): PropertyBag? {
return (this?.tags.orEmpty() + other?.tags.orEmpty())
.distinct()
.takeIf { it.isNotEmpty() }
?.let { PropertyBag(it) }
private fun PropertyBag.merge(other: PropertyBag): PropertyBag {
val aTags = this["tags"] as? Collection<*>
val bTags = other["tags"] as? Collection<*>
val tags = if (aTags != null && bTags != null) {
mapOf("tags" to (aTags + bTags).distinct())
} else {
emptyMap()
}

return PropertyBag(this + other + tags)
}

fun List<Run>.merge(other: List<Run>): List<Run> {
private fun List<Run>.merge(other: List<Run>): List<Run> {
val runsByTool = (this + other).groupBy { it.tool.driver.fullName }

return runsByTool.mapValues { (_, runs) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import kotlinx.serialization.descriptors.PrimitiveSerialDescriptor
import kotlinx.serialization.descriptors.SerialDescriptor
import kotlinx.serialization.encoding.Decoder
import kotlinx.serialization.encoding.Encoder
import kotlinx.serialization.json.JsonObject
import kotlin.jvm.JvmInline

/*
* Using https://app.quicktype.io/ to generate from schema
Expand Down Expand Up @@ -339,13 +341,41 @@ data class Address (
*
* Key/value pairs that provide additional information about the version control details.
*/
@Serializable
data class PropertyBag (
@Serializable(with = PropertyBag.Companion::class)
@JvmInline
value class PropertyBag(
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
private val value: Map<String, Any?>,
) : Map<String, Any?> by value {
/**
* A set of distinct strings that provide additional information.
*/
val tags: List<String>? = null
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
)
val tags: List<String>?
get() = try {
val tagList = value["tags"] as Collection<*>?
tagList?.mapIndexed { position, it ->
try {
it as String
} catch (e: ClassCastException) {
throw IllegalStateException("the tag \"$it\" at the position $position is not a String", e)
}
}
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
} catch (e: ClassCastException) {
throw IllegalStateException("tags should be a collection", e)
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
}

companion object : KSerializer<PropertyBag> {
override val descriptor: SerialDescriptor = JsonObject.serializer().descriptor

override fun deserialize(decoder: Decoder): PropertyBag {
val jsonObject = decoder.decodeSerializableValue(JsonObject.serializer())
return PropertyBag(jsonObject.toNativeObject())
}

override fun serialize(encoder: Encoder, value: PropertyBag) {
encoder.encodeSerializableValue(JsonObject.serializer(), value.toJsonElement())
}
}
}

/**
* A single artifact. In some cases, this artifact might be nested within another artifact.
Expand Down
80 changes: 80 additions & 0 deletions src/jvmTest/kotlin/io/github/detekt/sarif4k/PropertyBagTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package io.github.detekt.sarif4k

import kotlinx.serialization.json.Json
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.Arguments
import org.junit.jupiter.params.provider.MethodSource
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.fail

class PropertyBagTest {

@ParameterizedTest
@MethodSource("data")
fun encode(value: Map<String, Any?>, json: String) {
assertEquals(json, Json.encodeToString(PropertyBag.serializer(), PropertyBag(value)))
}

@ParameterizedTest
@MethodSource("data")
fun decode(value: Map<String, Any?>, json: String) {
assertEquals(PropertyBag(value), Json.decodeFromString(PropertyBag.serializer(), json))
}

@Test
fun noTags() {
assertEquals(null, PropertyBag(mapOf()).tags)
}

@Test
fun nullTags() {
assertEquals(null, PropertyBag(mapOf("tags" to null)).tags)
}

@Test
fun emptyTags() {
assertEquals(emptyList(), PropertyBag(mapOf("tags" to emptyList<Nothing>())).tags)
}

@Test
fun tags() {
assertEquals(listOf("foo", "bar"), PropertyBag(mapOf("tags" to listOf("foo", "bar"))).tags)
}

@Test
fun incorrectTags_noList() {
try {
PropertyBag(mapOf("tags" to 1)).tags
fail()
} catch (e: IllegalStateException) {
assertEquals("tags should be a collection", e.message)
assertNotNull(e.cause)
}
}

@Test
fun incorrectTags_noString() {
try {
PropertyBag(mapOf("tags" to listOf("foo", true, "bar"))).tags
fail()
} catch (e: IllegalStateException) {
assertEquals("the tag \"true\" at the position 1 is not a String", e.message)
assertNotNull(e.cause)
}
}

companion object {
@JvmStatic
fun data(): List<Arguments> {
return listOf(
mapOf<String, Any?>() to """{}""",
mapOf("foo" to null) to """{"foo":null}""",
mapOf("foo" to mapOf("bar" to listOf("1", 2L, false, 3.5))) to """{"foo":{"bar":["1",2,false,3.5]}}""",
mapOf("tags" to null) to """{"tags":null}""",
mapOf("tags" to emptyList<Nothing>()) to """{"tags":[]}""",
).map { (first, second) -> Arguments.of(first, second) }
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package io.github.detekt.sarif4k

import io.github.detekt.sarif4k.util.resourceAsTextContent
import org.junit.jupiter.api.Test
import java.io.File
import kotlin.test.assertEquals

class SarifMergingTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ class SarifSerializerTest {
)
),
properties = PropertyBag(
tags = listOf("tag")
)
mapOf(
"tags" to listOf("tag"),
"foo" to mapOf("bar" to "buz")
)
),
)
)
),
Expand Down
5 changes: 4 additions & 1 deletion src/jvmTest/resources/input_1.sarif.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@
"tags": [
"tag",
"tag1"
]
],
"foo": {
"bar": "buz"
}
}
}
3 changes: 2 additions & 1 deletion src/jvmTest/resources/input_2.sarif.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"tags": [
"tag",
"tag2"
]
],
"bar": 5
}
}
6 changes: 5 additions & 1 deletion src/jvmTest/resources/output.sarif.json
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@
"tag",
"tag1",
"tag2"
]
],
"foo": {
"bar": "buz"
},
"bar": 5
}
}
5 changes: 4 additions & 1 deletion src/jvmTest/resources/output_different_tool.sarif.json
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@
"tags": [
"tag",
"tag1"
]
],
"foo": {
"bar": "buz"
}
}
}
5 changes: 4 additions & 1 deletion src/jvmTest/resources/test.sarif.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@
"properties": {
TWiStErRob marked this conversation as resolved.
Show resolved Hide resolved
BraisGabin marked this conversation as resolved.
Show resolved Hide resolved
"tags": [
"tag"
]
],
"foo": {
"bar": "buz"
}
}
}
],
Expand Down