Skip to content

Commit

Permalink
Correctly determine nullability of members in IDLv2 models
Browse files Browse the repository at this point in the history
  • Loading branch information
sugmanue committed Sep 12, 2022
1 parent e008387 commit b853aca
Show file tree
Hide file tree
Showing 18 changed files with 86 additions and 79 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,15 @@ message = "Smithy IDL v2 mixins are now supported"
references = ["smithy-rs#1680"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all"}
author = "ogudavid"

[[smithy-rs]]
message = "Upgrade Smithy to 1.25.0"
references = ["smithy-rs#1725"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all"}
author = "sugmanue"

[[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"
6 changes: 6 additions & 0 deletions aws/sdk-adhoc-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ configure<software.amazon.smithy.gradle.SmithyExtension> {
}

buildscript {
repositories {
mavenLocal()
mavenCentral()
google()
}

val smithyVersion: String by project
dependencies {
classpath("software.amazon.smithy:smithy-cli:$smithyVersion")
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.util.hasTrait
import software.amazon.smithy.rust.codegen.util.lookup

internal class BoxPrimitiveShapesTest {
internal class AddClientOptionalToShapesTest {
@Test
fun `primitive shapes are boxed`() {
val baseModel = """
Expand All @@ -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
}
}
}
6 changes: 6 additions & 0 deletions aws/sdk/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ val sdkOutputDir = outputDir.resolve("sdk")
val examplesOutputDir = outputDir.resolve("examples")

buildscript {
repositories {
mavenLocal()
mavenCentral()
google()
}

val smithyVersion: String by project
dependencies {
classpath("software.amazon.smithy:smithy-aws-traits:$smithyVersion")
Expand Down
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
buildscript {
repositories {
mavenLocal()
mavenCentral()
google()
}
Expand Down
1 change: 1 addition & 0 deletions buildSrc/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ plugins {
jacoco
}
repositories {
mavenLocal()
mavenCentral()
google()
}
Expand Down
5 changes: 5 additions & 0 deletions codegen-client-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ val pluginName = "rust-codegen"
val workingDirUnderBuildDir = "smithyprojections/codegen-client-test/"

buildscript {
repositories {
mavenLocal()
mavenCentral()
google()
}
val smithyVersion: String by project
dependencies {
classpath("software.amazon.smithy:smithy-cli:$smithyVersion")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package software.amazon.smithy.rust.codegen.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
Expand Down Expand Up @@ -62,6 +63,7 @@ class CodegenVisitor(context: PluginContext, private val codegenDecorator: RustC
renameExceptions = settings.codegenConfig.renameExceptions,
handleRequired = false,
handleRustBoxing = true,
nullabilityCheckMode = NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1_NO_INPUT,
)
val baseModel = baselineTransform(context.model)
val service = settings.getService(baseModel)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ data class SymbolVisitorConfig(
val renameExceptions: Boolean,
val handleRustBoxing: Boolean,
val handleRequired: Boolean,
val nullabilityCheckMode: NullableIndex.CheckMode,
)

/**
Expand Down Expand Up @@ -211,7 +212,7 @@ open class SymbolVisitor(
private fun handleOptionality(symbol: Symbol, member: MemberShape): Symbol =
if (config.handleRequired && member.isRequired) {
symbol
} else if (nullableIndex.isNullable(member)) {
} else if (nullableIndex.isMemberNullable(member, config.nullabilityCheckMode)) {
symbol.makeOptional()
} else {
symbol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ fun String.asSmithyModel(sourceLocation: String? = null, smithyVersion: String =
.unwrap()
}


/**
* In tests, we frequently need to generate a struct, a builder, and an impl block to access said builder.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
var 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
Expand Down
5 changes: 5 additions & 0 deletions codegen-server-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ val pluginName = "rust-server-codegen"
val workingDirUnderBuildDir = "smithyprojections/codegen-server-test/"

buildscript {
repositories {
mavenLocal()
mavenCentral()
google()
}
val smithyVersion: String by project
dependencies {
classpath("software.amazon.smithy:smithy-cli:$smithyVersion")
Expand Down
5 changes: 5 additions & 0 deletions codegen-server-test/python/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ configure<software.amazon.smithy.gradle.SmithyExtension> {
}

buildscript {
repositories {
mavenLocal()
mavenCentral()
google()
}
val smithyVersion: String by project
dependencies {
classpath("software.amazon.smithy:smithy-cli:$smithyVersion")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -70,6 +71,7 @@ open class ServerCodegenVisitor(
renameExceptions = false,
handleRequired = true,
handleRustBoxing = true,
nullabilityCheckMode = NullableIndex.CheckMode.SERVER,
)
val baseModel = baselineTransform(context.model)
val service = settings.getService(baseModel)
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b853aca

Please sign in to comment.