Merge branch 'main' of github.com:awslabs/smithy-kotlin into feat-clo…

…ck-skew

.github/workflows/continuous-integration.yml

@@ -4,87 +4,104 @@ on:
   push:
     branches: [ main ]
   pull_request:
-    branches:
-      - main
-      - 'feat-*'
   workflow_dispatch:
 
+# Allow one instance of this workflow per pull request, and cancel older runs when new changes are pushed
+concurrency:
+  group: ci-pr-${{ github.ref }}
+  cancel-in-progress: true
+
 env:
-  BUILDER_VERSION: v0.8.22
-  BUILDER_SOURCE: releases
-  # host owned by CRT team to host aws-crt-builder releases. Contact their on-call with any issues
-  BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net
-  PACKAGE_NAME: smithy-kotlin
-  LINUX_BASE_IMAGE: ubuntu-16-x64
   RUN: ${{ github.run_id }}-${{ github.run_number }}
+  GRADLE_OPTS: "-Dorg.gradle.daemon=false -Dkotlin.incremental=false"
 
 jobs:
-  linux-compat:
+  jvm:
     runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        # we build with a specific JDK version but source/target compatibility should ensure the jar is usable by
+        # the target versions we want to support
+        java-version:
+          - 8
+          - 11
+          - 17
+          - 21
     steps:
-    - name: Checkout sources
-      uses: actions/checkout@v2
-    - uses: actions/cache@v2
-      with:
-        path: |
-          ~/.gradle/caches
-          ~/.gradle/wrapper
-        key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*') }}
-        restore-keys: |
-          ${{ runner.os }}-gradle-
-    - name: Build and Test ${{ env.PACKAGE_NAME }}
-      run: |
-        python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')"
-        chmod a+x builder.pyz
-        echo "kotlinWarningsAsErrors=true" >> $GITHUB_WORKSPACE/local.properties
-        ./builder.pyz build -p ${{ env.PACKAGE_NAME }}
-
-  macos-compat:
-    runs-on: macos-latest
-    steps:
-    - name: Checkout sources
-      uses: actions/checkout@v2
-    - uses: actions/cache@v2
-      with:
-        path: |
-          ~/.gradle/caches
-          ~/.gradle/wrapper
-        key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*') }}
-        restore-keys: |
-          ${{ runner.os }}-gradle-
-    - name: Build and Test ${{ env.PACKAGE_NAME }}
-      run: |
-        python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')"
-        chmod a+x builder.pyz
-        echo "kotlinWarningsAsErrors=true" >> $GITHUB_WORKSPACE/local.properties
-        ./builder.pyz build -p ${{ env.PACKAGE_NAME }}
+      - name: Checkout sources
+        uses: actions/checkout@v4
+      - name: Configure JDK
+        uses: actions/setup-java@v3
+        with:
+          distribution: 'corretto'
+          java-version: 17
+          cache: 'gradle'
+      - name: Test
+        shell: bash
+        run: |
+          ./gradlew -Ptest.java.version=${{ matrix.java-version }} jvmTest --stacktrace
 
-  windows-compat:
-    runs-on: windows-latest
+  all-platforms:
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ ubuntu-latest, macos-latest, windows-latest ]
     steps:
       - name: Checkout sources
-        uses: actions/checkout@v2
-      - name: Build and Test ${{ env.PACKAGE_NAME }}
+        uses: actions/checkout@v4
+      - name: Configure JDK
+        uses: actions/setup-java@v3
+        with:
+          distribution: 'corretto'
+          java-version: 17
+          cache: 'gradle'
+      - name: Test
+        shell: bash
         run: |
-          python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder.pyz')"
-          python3 builder.pyz build -p ${{ env.PACKAGE_NAME }}
+          echo "kotlinWarningsAsErrors=true" >> $GITHUB_WORKSPACE/local.properties
+          ./gradlew apiCheck
+          ./gradlew test allTests
+      - name: Save Test Reports
+        if: failure()
+        uses: actions/upload-artifact@v3
+        with:
+          name: test-reports
+          path: '**/build/reports'
 
   downstream:
     runs-on: ubuntu-latest
     steps:
     - name: Checkout sources
-      uses: actions/checkout@v2
-    - uses: actions/cache@v2
+      uses: actions/checkout@v4
+      with:
+        path: 'smithy-kotlin'
+    - name: Checkout tools
+      uses: actions/checkout@v4
+      with:
+        path: 'aws-kotlin-repo-tools'
+        repository: 'awslabs/aws-kotlin-repo-tools'
+        ref: '0.2.3'
+        sparse-checkout: |
+          .github
+    - name: Checkout aws-sdk-kotlin
+      uses: ./aws-kotlin-repo-tools/.github/actions/checkout-head
+      with:
+        # smithy-kotlin is checked out as a sibling dir which will automatically make it an included build
+        path: 'aws-sdk-kotlin'
+        repository: 'awslabs/aws-sdk-kotlin'
+    - name: Configure JDK
+      uses: actions/setup-java@v3
       with:
-        path: |
-          ~/.gradle/caches
-          ~/.gradle/wrapper
-        key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*') }}
-        restore-keys: |
-          ${{ runner.os }}-gradle-
-    - name: Build and Test ${{ env.PACKAGE_NAME }} Downstream Consumers
+        distribution: 'corretto'
+        java-version: 17
+        cache: 'gradle'
+    - name: Build and Test aws-sdk-kotlin downstream
       run: |
-        python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
-        chmod a+x builder
-        echo "kotlinWarningsAsErrors=true" >> $GITHUB_WORKSPACE/local.properties
-        ./builder build -p ${{ env.PACKAGE_NAME }} --spec downstream
+        # TODO - JVM only
+        cd $GITHUB_WORKSPACE/smithy-kotlin
+        ./gradlew --parallel publishToMavenLocal
+        cd $GITHUB_WORKSPACE/aws-sdk-kotlin
+        ./gradlew --parallel publishToMavenLocal
+        ./gradlew testAllProtocols

build.gradle.kts

@@ -30,13 +30,13 @@ plugins {
 // configures (KMP) subprojects with our own KMP conventions and some default dependencies
 apply(plugin = "aws.sdk.kotlin.kmp")
 
-allprojects {
-    repositories {
-        mavenLocal()
-        mavenCentral()
-        google()
-    }
+val testJavaVersion = typedProp<String>("test.java.version")?.let {
+    JavaLanguageVersion.of(it)
+}?.also {
+    println("configuring tests to run with jdk $it")
+}
 
+allprojects {
     tasks.withType<org.jetbrains.dokka.gradle.AbstractDokkaTask>().configureEach {
         val sdkVersion: String by project
         moduleVersion.set(sdkVersion)
@@ -61,14 +61,25 @@ allprojects {
         )
         pluginsMapConfiguration.set(pluginConfigMap)
     }
-}
 
-if (project.typedProp<Boolean>("kotlinWarningsAsErrors") == true) {
-    subprojects {
+    if (rootProject.typedProp<Boolean>("kotlinWarningsAsErrors") == true) {
         tasks.withType<org.jetbrains.kotlin.gradle.tasks.KotlinCompile> {
             kotlinOptions.allWarningsAsErrors = true
         }
     }
+
+    if (testJavaVersion != null) {
+        tasks.withType<Test> {
+            // JDK8 tests fail with out of memory sometimes, not sure why...
+            maxHeapSize = "2g"
+            val toolchains = project.extensions.getByType<JavaToolchainService>()
+            javaLauncher.set(
+                toolchains.launcherFor {
+                    languageVersion.set(testJavaVersion)
+                },
+            )
+        }
+    }
 }
 
 // configure the root multimodule docs

codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/waiters/KotlinJmespathExpressionVisitor.kt

@@ -26,8 +26,6 @@ import software.amazon.smithy.model.shapes.ListShape
 import software.amazon.smithy.model.shapes.MapShape
 import software.amazon.smithy.model.shapes.MemberShape
 import software.amazon.smithy.model.shapes.Shape
-import kotlin.contracts.ExperimentalContracts
-import kotlin.contracts.contract
 
 private val suffixSequence = sequenceOf("") + generateSequence(2) { it + 1 }.map(Int::toString) // "", "2", "3", etc.
 
@@ -76,14 +74,6 @@ class KotlinJmespathExpressionVisitor(
     private fun bestTempVarName(preferredName: String): String =
         suffixSequence.map { "$preferredName$it" }.first(tempVars::add)
 
-    @OptIn(ExperimentalContracts::class)
-    private fun codegenReq(condition: Boolean, lazyMessage: () -> String) {
-        contract {
-            returns() implies condition
-        }
-        if (!condition) throw CodegenException(lazyMessage())
-    }
-
     private fun flatMappingBlock(right: JmespathExpression, leftName: String, leftShape: Shape, innerShape: Shape?): VisitedExpression {
         if (right is CurrentExpression) return VisitedExpression(leftName, leftShape) // nothing to map
 
@@ -105,7 +95,9 @@ class KotlinJmespathExpressionVisitor(
         return VisitedExpression(outerName, leftShape, innerResult.shape)
     }
 
-    private fun subfield(expression: FieldExpression, parentName: String, isObject: Boolean = false): VisitedExpression {
+    private data class SubFieldData(val name: String, val codegen: String, val member: Shape?)
+
+    private fun subfieldLogic(expression: FieldExpression, parentName: String, isObject: Boolean = false): SubFieldData {
         val member = currentShape.targetOrSelf(ctx.model).getMember(expression.name).getOrNull()
 
         val name = expression.name.toCamelCase()
@@ -130,9 +122,17 @@ class KotlinJmespathExpressionVisitor(
         }
 
         member?.let { shapeCursor.addLast(it) }
-        return VisitedExpression(addTempVar(name, codegen), member)
+        return SubFieldData(name, codegen, member)
+    }
+
+    private fun subfield(expression: FieldExpression, parentName: String, isObject: Boolean = false): VisitedExpression {
+        val (name, codegen, member) = subfieldLogic(expression, parentName, isObject)
+        return VisitedExpression(addTempVar(name, codegen), member, nullable = currentShape.isNullable)
     }
 
+    private fun subfieldCodegen(expression: FieldExpression, parentName: String, isObject: Boolean = false): String =
+        subfieldLogic(expression, parentName, isObject).codegen
+
     override fun visitAnd(expression: AndExpression): VisitedExpression {
         writer.addImport(RuntimeTypes.Core.Utils.truthiness)
 
@@ -151,22 +151,21 @@ class KotlinJmespathExpressionVisitor(
 
         val codegen = buildString {
             val nullables = buildList {
-                if (left.shape?.isNullable == true) add("${left.identifier} == null")
-                if (right.shape?.isNullable == true) add("${right.identifier} == null")
+                if (left.shape?.isNullable == true || left.nullable) add("${left.identifier} == null")
+                if (right.shape?.isNullable == true || right.nullable) add("${right.identifier} == null")
             }
+
             if (nullables.isNotEmpty()) {
                 val isNullExpr = nullables.joinToString(" || ")
                 append("if ($isNullExpr) null else ")
             }
 
-            val unSafeComparatorExpr = "compareTo(${right.identifier}) ${expression.comparator} 0"
-            val comparatorExpr = if (left.nullable) "?.$unSafeComparatorExpr" else ".$unSafeComparatorExpr"
-
+            val comparatorExpr = ".compareTo(${right.identifier}) ${expression.comparator} 0"
             append("${left.identifier}$comparatorExpr")
         }
 
-        val ident = addTempVar("comparison", codegen)
-        return VisitedExpression(ident)
+        val identifier = addTempVar("comparison", codegen)
+        return VisitedExpression(identifier)
     }
 
     override fun visitCurrentNode(expression: CurrentExpression): VisitedExpression {
@@ -207,14 +206,11 @@ class KotlinJmespathExpressionVisitor(
         return VisitedExpression(ident, currentShape, inner.projected)
     }
 
-    private fun FunctionExpression.singleArg(): VisitedExpression {
-        codegenReq(arguments.size == 1) { "Unexpected number of arguments to $this" }
-        return acceptSubexpression(this.arguments[0])
-    }
-    private fun FunctionExpression.twoArgs(): Pair<VisitedExpression, VisitedExpression> {
-        codegenReq(arguments.size == 2) { "Unexpected number of arguments to $this" }
-        return acceptSubexpression(this.arguments[0]) to acceptSubexpression(this.arguments[1])
-    }
+    private fun FunctionExpression.singleArg(): VisitedExpression =
+        acceptSubexpression(this.arguments[0])
+
+    private fun FunctionExpression.twoArgs(): Pair<VisitedExpression, VisitedExpression> =
+        acceptSubexpression(this.arguments[0]) to acceptSubexpression(this.arguments[1])
 
     private fun FunctionExpression.args(): List<VisitedExpression> =
         this.arguments.map { acceptSubexpression(it) }
@@ -257,9 +253,7 @@ class KotlinJmespathExpressionVisitor(
 
         "avg" -> {
             val numbers = expression.singleArg()
-            val average = numbers.dotFunction(expression, "average()").identifier
-            val isNaN = ensureNullGuard(numbers.shape, "isNaN() == true")
-            VisitedExpression(addTempVar("averageOrNull", "if($average$isNaN) null else $average"), nullable = true)
+            numbers.dotFunction(expression, "average()")
         }
 
         "join" -> {
@@ -334,6 +328,35 @@ class KotlinJmespathExpressionVisitor(
             arg.dotFunction(expression, "type()", ensureNullGuard = false)
         }
 
+        "sort" -> {
+            val arg = expression.singleArg()
+            arg.dotFunction(expression, "sorted()")
+        }
+
+        "sort_by" -> {
+            val list = expression.arguments[0].accept(this)
+            val expressionValue = expression.arguments[1]
+            list.applyFunction(expression.name.toCamelCase(), "sortedBy", expressionValue)
+        }
+
+        "max_by" -> {
+            val list = expression.arguments[0].accept(this)
+            val expressionValue = expression.arguments[1]
+            list.applyFunction(expression.name.toCamelCase(), "maxBy", expressionValue)
+        }
+
+        "min_by" -> {
+            val list = expression.arguments[0].accept(this)
+            val expressionValue = expression.arguments[1]
+            list.applyFunction(expression.name.toCamelCase(), "minBy", expressionValue)
+        }
+
+        "map" -> {
+            val list = expression.arguments[1].accept(this)
+            val expressionValue = expression.arguments[0]
+            list.applyFunction(expression.name.toCamelCase(), "map", expressionValue)
+        }
+
         else -> throw CodegenException("Unknown function type in $expression")
     }
 
@@ -548,6 +571,21 @@ class KotlinJmespathExpressionVisitor(
         return notNull
     }
 
+    private fun VisitedExpression.applyFunction(
+        name: String,
+        operation: String,
+        expression: JmespathExpression,
+    ): VisitedExpression {
+        val result = bestTempVarName(name)
+
+        writer.withBlock("val $result = ${this.identifier}?.$operation {", "}") {
+            val expressionValue = subfieldCodegen((expression as ExpressionTypeExpression).expression as FieldExpression, "it")
+            write("$expressionValue!!")
+        }
+
+        return VisitedExpression(result)
+    }
+
     private val Shape.isNullable: Boolean
         get() = this is MemberShape &&
             ctx.model.expectShape(target).let { !it.hasTrait<OperationInput>() && !it.hasTrait<OperationOutput>() }