feat(dsl): accept maps and sets instead of linked versions (#1428)

krzema12 · web-flow · commit 0d125b399cba · 2024-05-04T09:04:32.000+02:00
It turned out that iterating over a map/set returned by a common map
builder `mapOf`/`setOf` corresponds to the order given to this function.
It wasn't obvious since the builder returns a regular `Map`/`Set` where
the order is not specified. Thanks to this observation, we can simplify
the API and not require `linkedMapOf`/`linkedSetOf` which is more
verbose.

Action bindings still operate on `LinkedHashMap` because making this
change would break the Maven-based bindings server. Leaving this type
there is not that painful, it's only visible if someone decides to
create a local action binding. Otherwise, the binding code is
auto-generated.
diff --git a/.github/workflows/end-to-end-tests.main.kts b/.github/workflows/end-to-end-tests.main.kts
@@ -49,7 +49,7 @@ workflow(
         Push(branches = listOf("main")),
         PullRequest(),
     ),
-    yamlConsistencyJobEnv = linkedMapOf(
+    yamlConsistencyJobEnv = mapOf(
         "GITHUB_TOKEN" to expr("secrets.GITHUB_TOKEN")
     ),
     yamlConsistencyJobAdditionalSteps = {
@@ -67,7 +67,7 @@ workflow(
         job(
             id = "test_job_1",
             runsOn = RunnerType.UbuntuLatest,
-            env = linkedMapOf(
+            env = mapOf(
                 GREETING to "World",
             ),
             permissions = mapOf(
@@ -180,14 +180,14 @@ workflow(
 
             run(
                 name = "Custom environment variable",
-                env = linkedMapOf(
+                env = mapOf(
                     FIRST_NAME to "Patrick",
                 ),
                 command = "echo $GREETING $FIRST_NAME",
             )
             run(
                 name = "Encrypted secret",
-                env = linkedMapOf(
+                env = mapOf(
                     SECRET to expr { SUPER_SECRET },
                     TOKEN to expr { secrets.GITHUB_TOKEN },
                 ),
diff --git a/github-workflows-kt/api/github-workflows-kt.api b/github-workflows-kt/api/github-workflows-kt.api
diff --git a/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/Container.kt b/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/Container.kt
@@ -11,7 +11,7 @@ import kotlinx.serialization.Contextual
 public data class Container(
     val image: String,
     val credentials: Credentials? = null,
-    val env: LinkedHashMap<String, String> = linkedMapOf(),
+    val env: Map<String, String> = mapOf(),
     val ports: List<PortMapping> = emptyList(),
     val volumes: List<VolumeMapping> = emptyList(),
     val options: List<String> = emptyList(),
diff --git a/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/Job.kt b/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/Job.kt
@@ -11,7 +11,7 @@ public data class Job<OUTPUT : JobOutputs>(
     val steps: List<Step>,
     val needs: List<Job<*>> = emptyList(),
     val outputs: OUTPUT,
-    val env: LinkedHashMap<String, String> = linkedMapOf(),
+    val env: Map<String, String> = mapOf(),
     val condition: String? = null,
     val strategyMatrix: Map<String, List<String>>? = null,
     val permissions: Map<Permission, Mode>? = null,
diff --git a/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/RunnerType.kt b/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/RunnerType.kt
@@ -8,7 +8,7 @@ public sealed interface RunnerType {
     // Custom runner. Could be an expression `runsOn = expr("github.event.inputs.run-on")`
     public data class Custom(val runsOn: String) : RunnerType
 
-    public data class Labelled(val labels: LinkedHashSet<String>) : RunnerType {
+    public data class Labelled(val labels: Set<String>) : RunnerType {
         public constructor(vararg labels: String) : this(LinkedHashSet(labels.toList()))
 
         init {
@@ -20,7 +20,7 @@ public sealed interface RunnerType {
      * @see <a href="https://docs.github.com/en/actions/using-jobs/choosing-the-runner-for-a-job#choosing-runners-in-a-group">Choosing runners in a group</a>
      */
     @Suppress("MaxLineLength")
-    public data class Group(val name: String, val labels: LinkedHashSet<String>? = null) : RunnerType {
+    public data class Group(val name: String, val labels: Set<String>? = null) : RunnerType {
         public constructor(name: String, vararg labels: String) : this(name, LinkedHashSet(labels.toList()))
     }
 
diff --git a/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/Step.kt b/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/Step.kt
@@ -8,7 +8,7 @@ import kotlinx.serialization.Contextual
 
 public sealed class Step(
     public open val id: String,
-    public open val env: LinkedHashMap<String, String> = linkedMapOf(),
+    public open val env: Map<String, String> = mapOf(),
     public open val condition: String? = null,
     public open val continueOnError: Boolean? = null,
     public open val timeoutMinutes: Int? = null,
@@ -30,7 +30,7 @@ public data class CommandStep(
     override val id: String,
     val name: String? = null,
     val command: String,
-    override val env: LinkedHashMap<String, String> = linkedMapOf(),
+    override val env: Map<String, String> = mapOf(),
     override val condition: String? = null,
     override val continueOnError: Boolean? = null,
     override val timeoutMinutes: Int? = null,
@@ -50,7 +50,7 @@ public data class KotlinLogicStep(
     override val id: String,
     val name: String? = null,
     val command: String,
-    override val env: LinkedHashMap<String, String> = linkedMapOf(),
+    override val env: Map<String, String> = mapOf(),
     override val condition: String? = null,
     override val continueOnError: Boolean? = null,
     override val timeoutMinutes: Int? = null,
@@ -72,7 +72,7 @@ public open class ActionStep<out OUTPUTS : Outputs>(
     override val id: String,
     public open val name: String? = null,
     public open val action: Action<OUTPUTS>,
-    override val env: LinkedHashMap<String, String> = linkedMapOf(),
+    override val env: Map<String, String> = mapOf(),
     override val condition: String? = null,
     override val continueOnError: Boolean? = null,
     override val timeoutMinutes: Int? = null,
diff --git a/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/Workflow.kt b/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/domain/Workflow.kt
@@ -9,13 +9,13 @@ import java.nio.file.Path
 public data class Workflow(
     val name: String,
     val on: List<Trigger>,
-    val env: LinkedHashMap<String, String>,
+    val env: Map<String, String>,
     val sourceFile: Path?,
     val targetFileName: String?,
     val concurrency: Concurrency? = null,
     val permissions: Map<Permission, Mode>? = null,
     val yamlConsistencyJobCondition: String? = null,
-    val yamlConsistencyJobEnv: LinkedHashMap<String, String> = linkedMapOf(),
+    val yamlConsistencyJobEnv: Map<String, String> = mapOf(),
     val yamlConsistencyJobAdditionalSteps: (JobBuilder<JobOutputs.EMPTY>.() -> Unit)? = null,
     val jobs: List<Job<*>>,
     override val _customArguments: Map<String, @Contextual Any?> = mapOf(),
diff --git a/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/dsl/JobBuilder.kt b/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/dsl/JobBuilder.kt
@@ -26,7 +26,7 @@ public class JobBuilder<OUTPUT : JobOutputs>(
     public val name: String?,
     public val runsOn: RunnerType,
     public val needs: List<Job<*>>,
-    public val env: LinkedHashMap<String, String>,
+    public val env: Map<String, String>,
     public val condition: String?,
     public val strategyMatrix: Map<String, List<String>>?,
     public val permissions: Map<Permission, Mode>? = null,
@@ -64,7 +64,7 @@ public class JobBuilder<OUTPUT : JobOutputs>(
         vararg pleaseUseNamedArguments: Unit,
         command: String,
         name: String? = null,
-        env: LinkedHashMap<String, String> = linkedMapOf(),
+        env: Map<String, String> = mapOf(),
         @SuppressWarnings("FunctionParameterNaming")
         `if`: String? = null,
         condition: String? = null,
@@ -101,7 +101,7 @@ public class JobBuilder<OUTPUT : JobOutputs>(
         @Suppress("UNUSED_PARAMETER")
         vararg pleaseUseNamedArguments: Unit,
         name: String? = null,
-        env: LinkedHashMap<String, String> = linkedMapOf(),
+        env: Map<String, String> = mapOf(),
         @SuppressWarnings("FunctionParameterNaming")
         `if`: String? = null,
         condition: String? = null,
@@ -142,7 +142,7 @@ public class JobBuilder<OUTPUT : JobOutputs>(
                 // simplified implementation is used.
                 command = "GHWKT_RUN_STEP='${this.id}:$id' '.github/workflows/${sourceFile.name}'",
                 logic = logic,
-                env = LinkedHashMap(env.toMap() + mapOf("GHWKT_GITHUB_CONTEXT_JSON" to "${'$'}{{ toJSON(github) }}")),
+                env = env + mapOf("GHWKT_GITHUB_CONTEXT_JSON" to "${'$'}{{ toJSON(github) }}"),
                 condition =
                     conditionCheckingStep?.let {
                         expr { "steps.${conditionCheckingStep.id}.outputs.$evaluationResultOutput" }
@@ -162,7 +162,7 @@ public class JobBuilder<OUTPUT : JobOutputs>(
         vararg pleaseUseNamedArguments: Unit,
         action: Action<T>,
         name: String? = null,
-        env: LinkedHashMap<String, String> = linkedMapOf(),
+        env: Map<String, String> = mapOf(),
         @SuppressWarnings("FunctionParameterNaming")
         `if`: String? = null,
         condition: String? = null,
diff --git a/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/dsl/WorkflowBuilder.kt b/github-workflows-kt/src/main/kotlin/io/github/typesafegithub/workflows/dsl/WorkflowBuilder.kt
@@ -18,12 +18,12 @@ import java.nio.file.Path
 public class WorkflowBuilder(
     name: String,
     on: List<Trigger>,
-    env: LinkedHashMap<String, String> = linkedMapOf(),
+    env: Map<String, String> = mapOf(),
     sourceFile: Path?,
     targetFileName: String?,
     concurrency: Concurrency? = null,
     yamlConsistencyJobCondition: String? = null,
-    yamlConsistencyJobEnv: LinkedHashMap<String, String> = linkedMapOf(),
+    yamlConsistencyJobEnv: Map<String, String> = mapOf(),
     yamlConsistencyJobAdditionalSteps: (JobBuilder<JobOutputs.EMPTY>.() -> Unit)? = null,
     jobs: List<Job<*>> = emptyList(),
     permissions: Map<Permission, Mode>? = null,
@@ -55,7 +55,7 @@ public class WorkflowBuilder(
         needs: List<Job<*>> = emptyList(),
         `if`: String? = null,
         condition: String? = null,
-        env: LinkedHashMap<String, String> = linkedMapOf(),
+        env: Map<String, String> = mapOf(),
         strategyMatrix: Map<String, List<String>>? = null,
         permissions: Map<Permission, Mode>? = null,
         _customArguments: Map<String, @Contextual Any?> = mapOf(),
@@ -109,7 +109,7 @@ public class WorkflowBuilder(
         needs: List<Job<*>> = emptyList(),
         `if`: String? = null,
         condition: String? = null,
-        env: LinkedHashMap<String, String> = linkedMapOf(),
+        env: Map<String, String> = mapOf(),
         strategyMatrix: Map<String, List<String>>? = null,
         permissions: Map<Permission, Mode>? = null,
         _customArguments: Map<String, @Contextual Any?> = mapOf(),
@@ -163,12 +163,12 @@ public fun workflow(
     vararg pleaseUseNamedArguments: Unit,
     name: String,
     on: List<Trigger>,
-    env: LinkedHashMap<String, String> = linkedMapOf(),
+    env: Map<String, String> = mapOf(),
     sourceFile: Path? = null,
     targetFileName: String? = sourceFile?.fileName?.let { it.toString().substringBeforeLast(".main.kts") + ".yaml" },
     concurrency: Concurrency? = null,
     yamlConsistencyJobCondition: String? = null,
-    yamlConsistencyJobEnv: LinkedHashMap<String, String> = linkedMapOf(),
+    yamlConsistencyJobEnv: Map<String, String> = mapOf(),
     yamlConsistencyJobAdditionalSteps: (JobBuilder<JobOutputs.EMPTY>.() -> Unit)? = null,
     permissions: Map<Permission, Mode>? = null,
     _customArguments: Map<String, @Contextual Any> = mapOf(),
diff --git a/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/IntegrationTest.kt b/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/IntegrationTest.kt
@@ -118,7 +118,7 @@ class IntegrationTest : FunSpec({
                 name = "Test workflow",
                 on = listOf(Push()),
                 sourceFile = sourceTempFile.toPath(),
-                yamlConsistencyJobEnv = linkedMapOf("GITHUB_TOKEN" to expr("secrets.GITHUB_TOKEN")),
+                yamlConsistencyJobEnv = mapOf("GITHUB_TOKEN" to expr("secrets.GITHUB_TOKEN")),
             ) {
                 job(
                     id = "test_job",
@@ -187,7 +187,7 @@ class IntegrationTest : FunSpec({
                 name = "Test workflow",
                 on = listOf(Push()),
                 env =
-                    linkedMapOf(
+                    mapOf(
                         "SIMPLE_VAR" to "simple-value-workflow",
                         "MULTILINE_VAR" to
                             """
@@ -208,7 +208,7 @@ class IntegrationTest : FunSpec({
                     runsOn = RunnerType.UbuntuLatest,
                     condition = "\${{ always() }}",
                     env =
-                        linkedMapOf(
+                        mapOf(
                             "SIMPLE_VAR" to "simple-value-job",
                             "MULTILINE_VAR" to
                                 """
@@ -229,7 +229,7 @@ class IntegrationTest : FunSpec({
                         name = "Check out",
                         action = CheckoutV4(),
                         env =
-                            linkedMapOf(
+                            mapOf(
                                 "SIMPLE_VAR" to "simple-value-uses",
                                 "MULTILINE_VAR" to
                                     """
@@ -245,7 +245,7 @@ class IntegrationTest : FunSpec({
                         name = "Hello world!",
                         command = "echo 'hello!'",
                         env =
-                            linkedMapOf(
+                            mapOf(
                                 "SIMPLE_VAR" to "simple-value-run",
                                 "MULTILINE_VAR" to
                                     """
diff --git a/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/actions/CustomActionTest.kt b/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/actions/CustomActionTest.kt
@@ -14,7 +14,7 @@ class CustomActionTest : FunSpec({
                 actionName = "latex-action",
                 actionVersion = "v2",
                 inputs =
-                    linkedMapOf(
+                    mapOf(
                         "root_file" to "report.tex",
                         "compiler" to "latexmk",
                     ),
diff --git a/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/docsnippets/TypeSafeExpressionsSnippets.kt b/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/docsnippets/TypeSafeExpressionsSnippets.kt
@@ -49,14 +49,14 @@ class TypeSafeExpressionsSnippets : FunSpec({
                 runsOn = RunnerType.UbuntuLatest,
                 // --8<-- [start:custom-environment-variables-2]
                 env =
-                    linkedMapOf(
+                    mapOf(
                         GREETING to "World",
                     ),
             ) {
                 run(
                     name = "Custom environment variable",
                     env =
-                        linkedMapOf(
+                        mapOf(
                             FIRST_NAME to "Patrick",
                         ),
                     command = "echo $GREETING $FIRST_NAME",
@@ -81,7 +81,7 @@ class TypeSafeExpressionsSnippets : FunSpec({
                 run(
                     name = "Encrypted secret",
                     env =
-                        linkedMapOf(
+                        mapOf(
                             SECRET to expr { SUPER_SECRET },
                             TOKEN to expr { secrets.GITHUB_TOKEN },
                         ),
diff --git a/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/docsnippets/UsingActionsSnippets.kt b/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/docsnippets/UsingActionsSnippets.kt
@@ -103,7 +103,7 @@ class UsingActionsSnippets : FunSpec({
                 actionName = "latex-action",
                 actionVersion = "v2",
                 inputs =
-                    linkedMapOf(
+                    mapOf(
                         "root_file" to "report.tex",
                         "compiler" to "latexmk",
                     ),
diff --git a/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/domain/RunnerTypeTest.kt b/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/domain/RunnerTypeTest.kt
@@ -10,7 +10,7 @@ class RunnerTypeTest : FunSpec({
                 RunnerType.Labelled()
             }
             shouldThrow<IllegalArgumentException> {
-                RunnerType.Labelled(linkedSetOf())
+                RunnerType.Labelled(emptySet())
             }
         }
     }
diff --git a/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/yaml/ContainerToYamlTest.kt b/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/yaml/ContainerToYamlTest.kt
@@ -37,7 +37,7 @@ class ContainerToYamlTest : DescribeSpec({
                         volume("/host/path" to "/container/path"),
                         volume("/host/path2" to "/container/path2"),
                     ),
-                env = linkedMapOf("test-env-key" to "test-env-value"),
+                env = mapOf("test-env-key" to "test-env-value"),
                 options = listOf("test-option1", "test-option2"),
                 credentials =
                     Credentials(
@@ -83,7 +83,7 @@ class ContainerToYamlTest : DescribeSpec({
                     timeoutSeconds = 5,
                     retries = 5,
                 ),
-            env = linkedMapOf("POSTGRES_PASSWORD" to "postgres"),
+            env = mapOf("POSTGRES_PASSWORD" to "postgres"),
         ).toYaml() shouldBe
             mapOf(
                 "image" to "postgres",
@@ -113,7 +113,7 @@ class ContainerToYamlTest : DescribeSpec({
         // https://docs.github.com/en/actions/using-jobs/running-jobs-in-a-container
         Container(
             image = "node:14.16",
-            env = linkedMapOf("NODE_ENV" to "development"),
+            env = mapOf("NODE_ENV" to "development"),
             ports = listOf(port(80)),
             volumes = listOf(volume("my_docker_volume" to "/volume_mount")),
             options = listOf("--cpus 1"),
diff --git a/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/yaml/JobsToYamlTest.kt b/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/yaml/JobsToYamlTest.kt
@@ -130,7 +130,7 @@ class JobsToYamlTest : DescribeSpec({
                     runsOn = UbuntuLatest,
                     outputs = JobOutputs.EMPTY,
                     env =
-                        linkedMapOf(
+                        mapOf(
                             "FOO" to "bar",
                             "BAZ" to
                                 """
diff --git a/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/yaml/StepsToYamlTest.kt b/github-workflows-kt/src/test/kotlin/io/github/typesafegithub/workflows/yaml/StepsToYamlTest.kt
@@ -82,7 +82,7 @@ class StepsToYamlTest : DescribeSpec({
                         id = "someId",
                         name = "Some command",
                         env =
-                            linkedMapOf(
+                            mapOf(
                                 "FOO" to "bar",
                                 "BAZ" to
                                     """
@@ -266,7 +266,7 @@ class StepsToYamlTest : DescribeSpec({
                         timeoutMinutes = 123,
                         action = CheckoutV4(fetchDepth = FetchDepth.Infinite),
                         env =
-                            linkedMapOf(
+                            mapOf(
                                 "FOO" to "bar",
                                 "BAZ" to
                                     """
@@ -333,7 +333,7 @@ class StepsToYamlTest : DescribeSpec({
                                 actionName = "latex-action",
                                 actionVersion = "v2",
                                 inputs =
-                                    linkedMapOf(
+                                    mapOf(
                                         "root_file" to "report.tex",
                                         "compiler" to "latexmk",
                                     ),
