fix: use snakeyaml-engine-kmp for validation

Author: Vampire

build.gradle.kts

@@ -81,6 +81,7 @@ kotlin {
         jvmMain {
             dependencies {
                 implementation("com.charleskorn.kaml:kaml:0.72.0")
+                implementation("it.krzeminski:snakeyaml-engine-kmp:3.1.1")
             }
         }
 

src/jvmMain/kotlin/it/krzeminski/githubactionstyping/ManifestsToReport.kt

@@ -1,6 +1,6 @@
 package it.krzeminski.githubactionstyping
 
-import it.krzeminski.githubactionstyping.parsing.TypesManifest
+import it.krzeminski.githubactionstyping.parsing.ValidationException
 import it.krzeminski.githubactionstyping.parsing.parseManifest
 import it.krzeminski.githubactionstyping.parsing.parseTypesManifest
 import it.krzeminski.githubactionstyping.reporting.appendStatus
@@ -25,11 +25,14 @@ fun manifestsToReport(manifestAndPath: Pair<String, Path>?, typesManifest: Strin
         })
     }
 
-    val parsedTypesManifest = if (typesManifest.isNotBlank()) {
+    val parsedTypesManifest =
         parseTypesManifest(typesManifest)
-    } else {
-        TypesManifest()
-    }
+            .onFailure {
+                if (it is ValidationException) {
+                    return false to it.message
+                }
+                throw it
+            }.getOrThrow()
     val parsedManifest = parseManifest(manifestAndPath.first)
 
     val inputsInTypesManifest = parsedTypesManifest.inputs?.keys ?: emptySet()

src/jvmMain/kotlin/it/krzeminski/githubactionstyping/parsing/TypesManifestParsing.kt

@@ -1,44 +1,124 @@
 package it.krzeminski.githubactionstyping.parsing
 
-import com.charleskorn.kaml.AnchorsAndAliases
-import com.charleskorn.kaml.EmptyYamlDocumentException
-import com.charleskorn.kaml.Yaml
-import kotlinx.serialization.SerialName
-import kotlinx.serialization.Serializable
-import kotlinx.serialization.decodeFromString
-
-@Serializable
+import it.krzeminski.snakeyaml.engine.kmp.api.Load
+import it.krzeminski.snakeyaml.engine.kmp.api.LoadSettings
+import it.krzeminski.snakeyaml.engine.kmp.schema.CoreSchema
+
 data class TypesManifest(
     val inputs: Map<String, ApiItem>? = null,
     val outputs: Map<String, ApiItem>? = null,
 )
 
-@Serializable
 data class ApiItem(
     val type: String? = null,
     val name: String? = null,
-    @SerialName("allowed-values")
     val allowedValues: List<String>? = null,
     val separator: String? = null,
-    @SerialName("named-values")
-    val namedValues: Map<String, String>? = null,
-    @SerialName("list-item")
+    val namedValues: Map<String, Int>? = null,
     val listItem: ApiItem? = null,
 )
 
-private val myYaml = Yaml(
-    configuration = Yaml.default.configuration.copy(
-        strictMode = false,
-        anchorsAndAliases = AnchorsAndAliases.Permitted(),
-    )
-)
-
-fun parseTypesManifest(manifestString: String): TypesManifest =
+fun parseTypesManifest(manifestString: String): Result<TypesManifest> =
     runCatching {
-        myYaml.decodeFromString<TypesManifest>(manifestString)
-    }.getOrElse {
-        if (it is EmptyYamlDocumentException) {
-            return TypesManifest()
+        val loadedTypesManifest = Load(
+            // work-around for https://github.com/krzema12/snakeyaml-engine-kmp/pull/390
+            LoadSettings.builder().setSchema(CoreSchema()).build()
+        ).loadOne(manifestString)
+
+        when (loadedTypesManifest) {
+            null -> TypesManifest()
+
+            is Map<*, *> -> {
+                val excessKeys = loadedTypesManifest.keys - listOf("inputs", "outputs")
+                if (excessKeys.isNotEmpty()) {
+                    throw ValidationException(excessKeys.joinToString(prefix = "Excess keys: ", postfix = "."))
+                }
+                TypesManifest(
+                    inputs = loadedTypesManifest.toInputsOrOutputs("inputs"),
+                    outputs = loadedTypesManifest.toInputsOrOutputs("outputs"),
+                )
+            }
+
+            else -> throw ValidationException("Types file must be a mapping.")
         }
-        throw it
     }
+
+private fun Map<*, *>.toInputsOrOutputs(key: String): Map<String, ApiItem>? =
+    when (val inputsOrOutputs = this[key]) {
+        null -> null
+
+        is Map<*, *> -> {
+            inputsOrOutputs.entries.associate { (key, value) -> key as String to value.toApiItem(key) }
+        }
+
+        else -> throw ValidationException("$key must be a mapping.")
+    }
+
+private fun Any?.toApiItem(key: String): ApiItem =
+    when (this) {
+        null -> ApiItem()
+
+        is Map<*, *> -> {
+            val excessKeys = keys - listOf(
+                "type",
+                "name",
+                "allowed-values",
+                "separator",
+                "named-values",
+                "list-item",
+            )
+            if (excessKeys.isNotEmpty()) {
+                throw ValidationException(excessKeys.joinToString(prefix = "Excess keys: ", postfix = "."))
+            }
+
+            ApiItem(
+                type = get("type")?.let {
+                    if (it !is String) {
+                        throw ValidationException("type must be a string.")
+                    }
+                    it
+                },
+                name = get("name")?.let {
+                    if (it !is String) {
+                        throw ValidationException("name must be a string.")
+                    }
+                    it
+                },
+                allowedValues = get("allowed-values")?.let {
+                    if (it !is List<*>) {
+                        throw ValidationException("allowed-values must be a sequence.")
+                    }
+                    it.map {
+                        if (it !is String) {
+                            throw ValidationException("Allowed Value must be string.")
+                        }
+                        it
+                    }
+                },
+                separator = get("separator")?.let {
+                    if (it !is String) {
+                        throw ValidationException("separator must be string.")
+                    }
+                    it
+                },
+                namedValues = get("named-values")?.let {
+                    if (it !is Map<*, *>) {
+                        throw ValidationException("named-values must be a mapping.")
+                    }
+                    it.mapKeys { (key, _) -> key as String }.mapValues { (_, value) ->
+                        if (value !is Int) {
+                            throw ValidationException("Named values must be integer.")
+                        }
+                        value
+                    }
+                },
+                listItem = toApiItem("list-item"),
+            )
+        }
+
+        else -> throw ValidationException("$key must be a mapping.")
+    }
+
+private fun Map<*, *>.toApiItem(key: String): ApiItem? = get(key)?.toApiItem(key)
+
+internal class ValidationException(override val message: String) : Exception(message)

src/jvmMain/kotlin/it/krzeminski/githubactionstyping/validation/types/Integer.kt

@@ -13,9 +13,6 @@ fun ApiItem.validateInteger(): ItemValidationResult {
     if (this.namedValues?.keys?.any { it.isBlank() } == true) {
         return ItemValidationResult.Invalid("Named value names must not be empty.")
     }
-    if (this.namedValues?.values?.any { it.toIntOrNull() == null } == true) {
-        return ItemValidationResult.Invalid("Named values must be integer.")
-    }
     if (this.name != null && this.name.isBlank()) {
         return ItemValidationResult.Invalid("Name must not be empty.")
     }

src/jvmTest/kotlin/it/krzeminski/githubactionstyping/LogicConsistencyTest.kt

@@ -5,6 +5,7 @@ import io.kotest.assertions.withClue
 import io.kotest.matchers.should
 import io.kotest.matchers.shouldBe
 import io.kotest.matchers.types.beOfType
+import it.krzeminski.githubactionstyping.parsing.ValidationException
 import it.krzeminski.githubactionstyping.parsing.parseTypesManifest
 import it.krzeminski.githubactionstyping.reporting.toPlaintextReport
 import it.krzeminski.githubactionstyping.validation.ItemValidationResult
@@ -16,7 +17,7 @@ import java.io.File
  */
 class LogicConsistencyTest : UseCaseTest() {
     override suspend fun testValid(typing: File) {
-        val validationResult = parseTypesManifest(typing.readText()).validate(typing.toPath().fileName)
+        val validationResult = parseTypesManifest(typing.readText()).getOrThrow().validate(typing.toPath().fileName)
         withClue(validationResult.toPlaintextReport()) {
             validationResult.overallResult shouldBe ItemValidationResult.Valid
         }
@@ -29,14 +30,19 @@ class LogicConsistencyTest : UseCaseTest() {
             .trimMargin("#")
             .trim()
 
-        val validationResult = parseTypesManifest(typesManifest).validate(typing.toPath().fileName)
-        assertSoftly {
-            validationResult.overallResult should beOfType<ItemValidationResult.Invalid>()
-            validationResult
-                .toPlaintextReport()
-                .trim()
-                .replace("\u001B", "\\x1B")
-                .shouldBe(expectedValidationError)
+        val parsedTypesManifest = parseTypesManifest(typesManifest)
+        if (parsedTypesManifest.isFailure && parsedTypesManifest.exceptionOrNull() is ValidationException) {
+            parsedTypesManifest.exceptionOrNull()!!.message shouldBe expectedValidationError
+        } else {
+            val validationResult = parsedTypesManifest.getOrThrow().validate(typing.toPath().fileName)
+            assertSoftly {
+                validationResult.overallResult should beOfType<ItemValidationResult.Invalid>()
+                validationResult
+                    .toPlaintextReport()
+                    .trim()
+                    .replace("\u001B", "\\x1B")
+                    .shouldBe(expectedValidationError)
+            }
         }
     }
 }

src/jvmTest/kotlin/it/krzeminski/githubactionstyping/validation/ManifestValidationTest.kt

@@ -17,12 +17,12 @@ class ManifestValidationTest : FunSpec({
                     "integer-input" to ApiItem(type = "integer"),
                     "integer-with-named-values-input" to ApiItem(
                         type = "integer",
-                        namedValues = mapOf("foo" to "1", "bar" to "2")
+                        namedValues = mapOf("foo" to 1, "bar" to 2)
                     ),
                     "integer-with-named-values-and-custom-item-name-input" to ApiItem(
                         type = "integer",
                         name = "SomeItemName",
-                        namedValues = mapOf("foo" to "1", "bar" to "2")
+                        namedValues = mapOf("foo" to 1, "bar" to 2)
                     ),
                     "float-input" to ApiItem(type = "float"),
                 ),
@@ -481,11 +481,11 @@ class ManifestValidationTest : FunSpec({
             // given
             val manifest = TypesManifest(
                 inputs = mapOf(
-                    "string-input" to ApiItem(type = "string", namedValues = mapOf("foo" to "1")),
-                    "boolean-input" to ApiItem(type = "boolean", namedValues = mapOf("foo" to "1")),
-                    "float-input" to ApiItem(type = "float", namedValues = mapOf("foo" to "1")),
-                    "list-input" to ApiItem(type = "list", separator = ",", listItem = ApiItem(type = "string"), namedValues = mapOf("foo" to "1")),
-                    "enum-input" to ApiItem(type = "enum", allowedValues = listOf("foo", "bar"), namedValues = mapOf("foo" to "1")),
+                    "string-input" to ApiItem(type = "string", namedValues = mapOf("foo" to 1)),
+                    "boolean-input" to ApiItem(type = "boolean", namedValues = mapOf("foo" to 1)),
+                    "float-input" to ApiItem(type = "float", namedValues = mapOf("foo" to 1)),
+                    "list-input" to ApiItem(type = "list", separator = ",", listItem = ApiItem(type = "string"), namedValues = mapOf("foo" to 1)),
+                    "enum-input" to ApiItem(type = "enum", allowedValues = listOf("foo", "bar"), namedValues = mapOf("foo" to 1)),
                 ),
             )
 

src/test/resources/bad-typings/excess_input_keys.yml

@@ -0,0 +1,12 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+inputs:
+  foo:
+    type: string
+    bar:
+    baz:
+
+# Expected validation error
+#
+#Excess keys: bar, baz.
+#

src/test/resources/bad-typings/excess_output_keys.yml

@@ -0,0 +1,12 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+outputs:
+  foo:
+    type: string
+    bar:
+    baz:
+
+# Expected validation error
+#
+#Excess keys: bar, baz.
+#

src/test/resources/bad-typings/excess_top_level_keys.yml

@@ -0,0 +1,9 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+foo:
+bar:
+
+# Expected validation error
+#
+#Excess keys: foo, bar.
+#

src/test/resources/bad-typings/inputs_enum_list_item_with_non_sequence_allowed_values.yml

@@ -0,0 +1,14 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+inputs:
+  granted-scopes:
+    type: list
+    separator: ','
+    list-item:
+      type: enum
+      allowed-values: foo
+
+# Expected validation error
+#
+#allowed-values must be a sequence.
+#

src/test/resources/bad-typings/inputs_enum_list_item_with_non_string_allowed_value.yml

@@ -0,0 +1,15 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+inputs:
+  granted-scopes:
+    type: list
+    separator: ','
+    list-item:
+      type: enum
+      allowed-values:
+        - 0x0
+
+# Expected validation error
+#
+#Allowed Value must be string.
+#

src/test/resources/bad-typings/inputs_enum_list_item_with_non_string_name.yml

@@ -0,0 +1,17 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+inputs:
+  granted-scopes:
+    type: list
+    separator: ','
+    list-item:
+      type: enum
+      name: 0x0
+      allowed-values:
+        - read
+        - write
+
+# Expected validation error
+#
+#name must be a string.
+#

src/test/resources/bad-typings/inputs_enum_with_non_sequence_allowed_values.yml

@@ -0,0 +1,11 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+inputs:
+  permissions:
+    type: enum
+    allowed-values: foo
+
+# Expected validation error
+#
+#allowed-values must be a sequence.
+#

src/test/resources/bad-typings/inputs_enum_with_non_string_allowed_value.yml

@@ -0,0 +1,12 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+inputs:
+  permissions:
+    type: enum
+    allowed-values:
+      - 0o0
+
+# Expected validation error
+#
+#Allowed Value must be string.
+#

src/test/resources/bad-typings/inputs_enum_with_non_string_name.yml

@@ -0,0 +1,15 @@
+# yaml-language-server: $schema=../../../../github-actions-typing.schema.json
+# See https://github.com/typesafegithub/github-actions-typing
+inputs:
+  permissions:
+    type: enum
+    name: 0o0
+    allowed-values:
+      - user
+      - admin
+      - guest
+
+# Expected validation error
+#
+#name must be a string.
+#