Correctly determine nullability of members in IDLv2 models

smithy-lang · Sep 15, 2022 · 4839471 · 4839471
1 parent e6177b3
commit 4839471
Show file tree

Hide file tree

Showing 13 changed files with 62 additions and 97 deletions.
diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml
@@ -236,3 +236,9 @@ renamed to `with_retry_classifier` and `retry_classifier` respectively. Public m
 references = ["smithy-rs#1715", "smithy-rs#1717"]
 meta = { "breaking" = true, "tada" = false, "bug" = false }
 author = "jdisanti"
+
+[[smithy-rs]]
+message = "Correctly determine nullability of members in IDLv2 models"
+references = ["smithy-rs#1725"]
+meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "all"}
+author = "sugmanue"
diff --git a/...odegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/ec2/BoxPrimitiveShapes.kt b/...odegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/ec2/BoxPrimitiveShapes.kt
diff --git a/...src/main/kotlin/software/amazon/smithy/rustsdk/customize/ec2/EC2MakePrimitivesOptional.kt b/...src/main/kotlin/software/amazon/smithy/rustsdk/customize/ec2/EC2MakePrimitivesOptional.kt
@@ -0,0 +1,24 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.rustsdk.customize.ec2
+
+import software.amazon.smithy.model.Model
+import software.amazon.smithy.model.shapes.Shape
+import software.amazon.smithy.model.traits.ClientOptionalTrait
+import software.amazon.smithy.model.transform.ModelTransformer
+
+object EC2MakePrimitivesOptional {
+    fun processModel(model: Model): Model {
+        val updates = arrayListOf<Shape>()
+        for (struct in model.structureShapes) {
+            for (member in struct.allMembers.values) {
+                updates.add(member.toBuilder().addTrait(ClientOptionalTrait()).build())
+            }
+
+        }
+        return ModelTransformer.create().replaceShapes(model, updates)
+    }
+}
diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/ec2/Ec2Decorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/customize/ec2/Ec2Decorator.kt
@@ -8,10 +8,10 @@ package software.amazon.smithy.rustsdk.customize.ec2
 import software.amazon.smithy.model.Model
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.ShapeId
-import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
-import software.amazon.smithy.rust.codegen.client.smithy.CoreCodegenContext
-import software.amazon.smithy.rust.codegen.client.smithy.customize.RustCodegenDecorator
-import software.amazon.smithy.rust.codegen.client.smithy.letIf
+import software.amazon.smithy.rust.codegen.smithy.ClientCodegenContext
+import software.amazon.smithy.rust.codegen.smithy.CoreCodegenContext
+import software.amazon.smithy.rust.codegen.smithy.customize.RustCodegenDecorator
+import software.amazon.smithy.rust.codegen.smithy.letIf
 
 class Ec2Decorator : RustCodegenDecorator<ClientCodegenContext> {
     override val name: String = "Ec2"
@@ -26,7 +26,7 @@ class Ec2Decorator : RustCodegenDecorator<ClientCodegenContext> {
         // need to be boxed for the API to work properly
         return model.letIf(
             applies(service),
-            BoxPrimitiveShapes::processModel,
+            EC2MakePrimitivesOptional::processModel,
         )
     }
 

diff --git a/...k/customize/ec2/BoxPrimitiveShapesTest.kt → ...mize/ec2/EC2MakePrimitivesOptionalTest.kt b/...k/customize/ec2/BoxPrimitiveShapesTest.kt → ...mize/ec2/EC2MakePrimitivesOptionalTest.kt
@@ -7,13 +7,12 @@ package software.amazon.smithy.rustsdk.customize.ec2
 
 import io.kotest.matchers.shouldBe
 import org.junit.jupiter.api.Test
+import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.shapes.StructureShape
-import software.amazon.smithy.model.traits.BoxTrait
-import software.amazon.smithy.rust.codegen.client.testutil.asSmithyModel
-import software.amazon.smithy.rust.codegen.client.util.hasTrait
-import software.amazon.smithy.rust.codegen.client.util.lookup
+import software.amazon.smithy.rust.codegen.testutil.asSmithyModel
+import software.amazon.smithy.rust.codegen.util.lookup
 
-internal class BoxPrimitiveShapesTest {
+internal class AddClientOptionalToShapesTest {
     @Test
     fun `primitive shapes are boxed`() {
         val baseModel = """
@@ -33,15 +32,11 @@ internal class BoxPrimitiveShapesTest {
             structure Other {}
 
         """.asSmithyModel()
-        val model = BoxPrimitiveShapes.processModel(baseModel)
-
+        val model = EC2MakePrimitivesOptional.processModel(baseModel)
+        val nullableIndex = NullableIndex(model)
         val struct = model.lookup<StructureShape>("test#Primitives")
         struct.members().forEach {
-            val target = model.expectShape(it.target)
-            when (target) {
-                is StructureShape -> target.hasTrait<BoxTrait>() shouldBe false
-                else -> target.hasTrait<BoxTrait>() shouldBe true
-            }
+            nullableIndex.isMemberNullable(it, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1) shouldBe true
         }
     }
 }
diff --git a/...lient/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenVisitor.kt b/...lient/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/CodegenVisitor.kt
@@ -7,6 +7,7 @@ package software.amazon.smithy.rust.codegen.client.smithy
 
 import software.amazon.smithy.build.PluginContext
 import software.amazon.smithy.model.Model
+import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.neighbor.Walker
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.Shape
@@ -60,8 +61,8 @@ class CodegenVisitor(context: PluginContext, private val codegenDecorator: RustC
             SymbolVisitorConfig(
                 runtimeConfig = settings.runtimeConfig,
                 renameExceptions = settings.codegenConfig.renameExceptions,
-                handleRequired = false,
                 handleRustBoxing = true,
+                nullabilityCheckMode = NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1,
             )
         val baseModel = baselineTransform(context.model)
         val service = settings.getService(baseModel)

diff --git a/...client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/SymbolVisitor.kt b/...client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/SymbolVisitor.kt
@@ -64,7 +64,7 @@ data class SymbolVisitorConfig(
     val runtimeConfig: RuntimeConfig,
     val renameExceptions: Boolean,
     val handleRustBoxing: Boolean,
-    val handleRequired: Boolean,
+    val nullabilityCheckMode: NullableIndex.CheckMode,
 )
 
 /**
@@ -209,13 +209,7 @@ open class SymbolVisitor(
     }
 
     private fun handleOptionality(symbol: Symbol, member: MemberShape): Symbol =
-        if (config.handleRequired && member.isRequired) {
-            symbol
-        } else if (nullableIndex.isNullable(member)) {
-            symbol.makeOptional()
-        } else {
-            symbol
-        }
+        symbol.letIf(nullableIndex.isMemberNullable(member, config.nullabilityCheckMode)) { symbol.makeOptional() }
 
     /**
      * Produce `Box<T>` when the shape has the `RustBoxTrait`

diff --git a/...client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/testutil/TestHelpers.kt b/...client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/testutil/TestHelpers.kt
@@ -6,6 +6,7 @@
 package software.amazon.smithy.rust.codegen.client.testutil
 
 import software.amazon.smithy.model.Model
+import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.ShapeId
 import software.amazon.smithy.model.shapes.StructureShape
@@ -37,7 +38,7 @@ val TestSymbolVisitorConfig = SymbolVisitorConfig(
     runtimeConfig = TestRuntimeConfig,
     renameExceptions = true,
     handleRustBoxing = true,
-    handleRequired = false,
+    nullabilityCheckMode = NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1,
 )
 
 fun testRustSettings(

diff --git a/...en-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/SymbolBuilderTest.kt b/...en-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/SymbolBuilderTest.kt
@@ -125,17 +125,13 @@ class SymbolBuilderTest {
         "PrimitiveBoolean, false, bool",
     )
     fun `creates primitives`(primitiveType: String, optional: Boolean, rustName: String) {
-        val memberBuilder = MemberShape.builder().id("foo.bar#MyStruct\$quux").target("smithy.api#$primitiveType")
-        val member = memberBuilder.build()
-        val struct = StructureShape.builder()
-            .id("foo.bar#MyStruct")
-            .addMember(member)
-            .build()
-        val model = Model.assembler()
-            .addShapes(struct, member)
-            .assemble()
-            .unwrap()
-
+        val model = """
+    namespace foo.bar
+    structure MyStruct {
+        quux: $primitiveType
+    }
+""".asSmithyModel()
+        val member = model.expectShape(ShapeId.from("foo.bar#MyStruct\$quux"))
         val provider: SymbolProvider = testSymbolProvider(model)
         val memberSymbol = provider.toSymbol(member)
         // builtins should not have a namespace set

diff --git a/...in/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerCodegenVisitor.kt b/...in/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerCodegenVisitor.kt
@@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.server.python.smithy
 
 import software.amazon.smithy.build.PluginContext
 import software.amazon.smithy.codegen.core.CodegenException
+import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.StringShape
 import software.amazon.smithy.model.shapes.StructureShape
@@ -46,8 +47,8 @@ class PythonServerCodegenVisitor(
             SymbolVisitorConfig(
                 runtimeConfig = settings.runtimeConfig,
                 renameExceptions = false,
-                handleRequired = true,
                 handleRustBoxing = true,
+                nullabilityCheckMode = NullableIndex.CheckMode.SERVER,
             )
         val baseModel = baselineTransform(context.model)
         val service = settings.getService(baseModel)

diff --git a/...src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt b/...src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCodegenVisitor.kt
@@ -7,6 +7,7 @@ package software.amazon.smithy.rust.codegen.server.smithy
 
 import software.amazon.smithy.build.PluginContext
 import software.amazon.smithy.model.Model
+import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.neighbor.Walker
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.Shape
@@ -69,8 +70,8 @@ open class ServerCodegenVisitor(
             SymbolVisitorConfig(
                 runtimeConfig = settings.runtimeConfig,
                 renameExceptions = false,
-                handleRequired = true,
                 handleRustBoxing = true,
+                nullabilityCheckMode = NullableIndex.CheckMode.SERVER,
             )
         val baseModel = baselineTransform(context.model)
         val service = settings.getService(baseModel)

diff --git a/...in/kotlin/software/amazon/smithy/rust/codegen/server/smithy/testutil/ServerTestHelpers.kt b/...in/kotlin/software/amazon/smithy/rust/codegen/server/smithy/testutil/ServerTestHelpers.kt
@@ -6,6 +6,7 @@
 package software.amazon.smithy.rust.codegen.server.smithy.testutil
 
 import software.amazon.smithy.model.Model
+import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.node.ObjectNode
 import software.amazon.smithy.model.shapes.ServiceShape
 import software.amazon.smithy.model.shapes.ShapeId
@@ -24,7 +25,7 @@ val ServerTestSymbolVisitorConfig = SymbolVisitorConfig(
     runtimeConfig = TestRuntimeConfig,
     renameExceptions = false,
     handleRustBoxing = true,
-    handleRequired = true,
+    nullabilityCheckMode = NullableIndex.CheckMode.SERVER,
 )
 
 fun serverTestSymbolProvider(

diff --git a/gradle.properties b/gradle.properties
@@ -15,7 +15,7 @@ kotlin.code.style=official
 
 # codegen
 smithyGradlePluginVersion=0.6.0
-smithyVersion=1.23.1
+smithyVersion=1.25.0
 
 # kotlin
 kotlinVersion=1.6.21