Skip to content

Commit 5b7b9bd

Browse files
sugmanueganeshnj
sugmanue
and
ganeshnj
authored
fix: properly check if a member can be nullable (#444)
Co-authored-by: Ganesh Jangir <[email protected]>
1 parent 2f74b74 commit 5b7b9bd

20 files changed

+127
-74
lines changed

‎gradle.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ kotlin.code.style=official
33
# config
44

55
# codegen
6-
smithyVersion=1.23.0
6+
smithyVersion=[1.25.0,1.26.0[
77
smithyGradleVersion=0.6.0
88

99
# kotlin

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/PackageManifestGenerator.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ fun writePackageManifest(settings: SwiftSettings, fileManifest: FileManifest, de
6363
}
6464
}
6565

66-
writer.write("let isUsingSPMLocal: Bool = FileManager.default.fileExists(atPath: \"${Resources.computeAbsolutePath("smithy-swift/Packages", "Packages", "SMITHY_SWIFT_CI_DIR")}/Packages/Package.swift\")")
66+
writer.write("let isUsingSPMLocal: Bool = FileManager.default.fileExists(atPath: \"${Resources.computeAbsolutePath("smithy-swift", "", "SMITHY_SWIFT_CI_DIR")}/Package.swift\")")
6767
writer.openBlock("if isUsingSPMLocal {", "}") {
6868
renderPackageDependenciesWithLocalPaths(writer, distinctDependencies)
6969
}

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/SwiftDelegator.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import software.amazon.smithy.codegen.core.SymbolReference
1313
import software.amazon.smithy.model.Model
1414
import software.amazon.smithy.model.shapes.Shape
1515
import software.amazon.smithy.swift.codegen.integration.SwiftIntegration
16+
import software.amazon.smithy.swift.codegen.model.SymbolProperty
1617
import software.amazon.smithy.swift.codegen.model.defaultValue
1718
import software.amazon.smithy.swift.codegen.model.isBoxed
1819
import software.amazon.smithy.utils.CodeWriter
@@ -100,7 +101,7 @@ class SwiftDelegator(
100101
val extensionSymbol = Symbol.builder()
101102
.name("${symbol.name}")
102103
.definitionFile("${symbol.definitionFile.replace(".swift", "+$extensionName.swift")}")
103-
.putProperty("boxed", symbol.isBoxed())
104+
.putProperty(SymbolProperty.BOXED_KEY, symbol.isBoxed())
104105
.putProperty("defaultValue", symbol.defaultValue())
105106
.build()
106107

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/SwiftDependency.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ enum class SwiftDependency(
2424
"main",
2525
"0.1.0",
2626
"https://github.com/awslabs/smithy-swift",
27-
Resources.computeAbsolutePath("smithy-swift/Packages", "Packages", "SMITHY_SWIFT_CI_DIR") + "/Packages",
27+
Resources.computeAbsolutePath("smithy-swift", "", "SMITHY_SWIFT_CI_DIR"),
2828
"ClientRuntime"
2929
),
3030
XCTest("XCTest", null, "", "", "", ""),
@@ -33,7 +33,7 @@ enum class SwiftDependency(
3333
"main",
3434
"0.1.0",
3535
"https://github.com/awslabs/smithy-swift",
36-
Resources.computeAbsolutePath("smithy-swift/Packages", "Packages", "SMITHY_SWIFT_CI_DIR") + "/Packages",
36+
Resources.computeAbsolutePath("smithy-swift", "", "SMITHY_SWIFT_CI_DIR"),
3737
"ClientRuntime"
3838
);
3939

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/SymbolVisitor.kt‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import software.amazon.smithy.codegen.core.Symbol
1212
import software.amazon.smithy.codegen.core.SymbolProvider
1313
import software.amazon.smithy.codegen.core.SymbolReference
1414
import software.amazon.smithy.model.Model
15+
import software.amazon.smithy.model.knowledge.NullableIndex
1516
import software.amazon.smithy.model.shapes.BigDecimalShape
1617
import software.amazon.smithy.model.shapes.BigIntegerShape
1718
import software.amazon.smithy.model.shapes.BlobShape
@@ -38,7 +39,6 @@ import software.amazon.smithy.model.shapes.StringShape
3839
import software.amazon.smithy.model.shapes.StructureShape
3940
import software.amazon.smithy.model.shapes.TimestampShape
4041
import software.amazon.smithy.model.shapes.UnionShape
41-
import software.amazon.smithy.model.traits.BoxTrait
4242
import software.amazon.smithy.model.traits.EnumTrait
4343
import software.amazon.smithy.model.traits.ErrorTrait
4444
import software.amazon.smithy.model.traits.SparseTrait
@@ -62,13 +62,15 @@ class SymbolVisitor(private val model: Model, swiftSettings: SwiftSettings) :
6262
private val sdkId = swiftSettings.sdkId
6363
private val service: ServiceShape? = try { swiftSettings.getService(model) } catch (e: CodegenException) { null }
6464
private val logger = Logger.getLogger(CodegenVisitor::class.java.name)
65-
private var escaper: Escaper
65+
private val escaper: Escaper
66+
private val nullableIndex: NullableIndex
6667
// model depth; some shapes use `toSymbol()` internally as they convert (e.g.) member shapes to symbols, this tracks
6768
// how deep in the model we have recursed
6869
private var depth = 0
6970

7071
init {
7172
val reservedWords = swiftReservedWords
73+
nullableIndex = NullableIndex.of(model)
7274
escaper = ReservedWordSymbolProvider
7375
.builder()
7476
.nameReservedWords(reservedWords) // Only escape words when the symbol has a definition file to
@@ -195,7 +197,11 @@ class SymbolVisitor(private val model: Model, swiftSettings: SwiftSettings) :
195197

196198
override fun memberShape(shape: MemberShape): Symbol {
197199
val targetShape = model.getShape(shape.target).orElseThrow { CodegenException("Shape not found: ${shape.target}") }
198-
return toSymbol(targetShape)
200+
var symbol = toSymbol(targetShape)
201+
if (nullableIndex.isMemberNullable(shape, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1)) {
202+
symbol = symbol.toBuilder().boxed().build()
203+
}
204+
return symbol
199205
}
200206

201207
override fun timestampShape(shape: TimestampShape): Symbol {
@@ -243,8 +249,7 @@ class SymbolVisitor(private val model: Model, swiftSettings: SwiftSettings) :
243249
*/
244250
private fun createSymbolBuilder(shape: Shape?, typeName: String, boxed: Boolean = false): Symbol.Builder {
245251
val builder = Symbol.builder().putProperty("shape", shape).name(typeName)
246-
val explicitlyBoxed = shape?.hasTrait<BoxTrait>() ?: false
247-
if (explicitlyBoxed || boxed) {
252+
if (boxed) {
248253
builder.boxed()
249254
}
250255
return builder

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/HttpProtocolUnitTestResponseGenerator.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ fun renderMemberAssertions(writer: SwiftWriter, test: HttpMessageTestCase, membe
147147
} else if ((shape.isDoubleShape || shape.isFloatShape)) {
148148
val stringNodes = test.params.stringMap.values.map { it.asStringNode().getOrNull() }
149149
if (stringNodes.isNotEmpty() && stringNodes.mapNotNull { it?.value }.contains("NaN")) {
150-
val suffix = if (symbolProvider.toSymbol(shape).isBoxed()) "?" else ""
150+
val suffix = if (symbolProvider.toSymbol(member).isBoxed()) "?" else ""
151151
writer.write("XCTAssertEqual(\$L$suffix.isNaN, \$L$suffix.isNaN)", expectedMemberName, actualMemberName)
152152
} else {
153153
writer.write("XCTAssertEqual(\$L, \$L)", expectedMemberName, actualMemberName)

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/httpResponse/HttpResponseHeaders.kt‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import software.amazon.smithy.model.knowledge.HttpBinding
1010
import software.amazon.smithy.model.knowledge.HttpBindingIndex
1111
import software.amazon.smithy.model.shapes.BlobShape
1212
import software.amazon.smithy.model.shapes.BooleanShape
13-
import software.amazon.smithy.model.shapes.CollectionShape
13+
import software.amazon.smithy.model.shapes.ListShape
1414
import software.amazon.smithy.model.shapes.NumberShape
1515
import software.amazon.smithy.model.shapes.ShapeType
1616
import software.amazon.smithy.model.shapes.StringShape
@@ -38,12 +38,12 @@ class HttpResponseHeaders(
3838
val memberName = ctx.symbolProvider.toMemberName(hdrBinding.member)
3939
val headerName = hdrBinding.locationName
4040
val headerDeclaration = "${memberName}HeaderValue"
41-
val isBoxed = ctx.symbolProvider.toSymbol(memberTarget).isBoxed()
41+
val isBoxed = ctx.symbolProvider.toSymbol(hdrBinding.member).isBoxed()
4242
writer.write("if let $headerDeclaration = httpResponse.headers.value(for: \$S) {", headerName)
4343
writer.indent()
4444
when (memberTarget) {
4545
is NumberShape -> {
46-
val memberValue = stringToNumber(memberTarget, headerDeclaration)
46+
val memberValue = stringToNumber(memberTarget, headerDeclaration, true)
4747
writer.write("self.\$L = $memberValue", memberName)
4848
}
4949
is BlobShape -> {
@@ -92,7 +92,7 @@ class HttpResponseHeaders(
9292
writer.write("self.\$L = $memberValue", memberName)
9393
}
9494
}
95-
is CollectionShape -> {
95+
is ListShape -> {
9696
// member > boolean, number, string, or timestamp
9797
// headers are List<String>, get the internal mapping function contents (if any) to convert
9898
// to the target symbol type
@@ -106,7 +106,7 @@ class HttpResponseHeaders(
106106
invalidHeaderListErrorName = "invalidBooleanHeaderList"
107107
"${SwiftTypes.Bool}(\$0)"
108108
}
109-
is NumberShape -> "(${stringToNumber(collectionMemberTarget, "\$0")} ?? 0)"
109+
is NumberShape -> "${stringToNumber(collectionMemberTarget, "\$0", false)}"
110110
is TimestampShape -> {
111111
val bindingIndex = HttpBindingIndex.of(ctx.model)
112112
val tsFormat = bindingIndex.determineTimestampFormat(
@@ -184,14 +184,17 @@ class HttpResponseHeaders(
184184
}
185185
}
186186

187-
private fun stringToNumber(shape: NumberShape, stringValue: String): String = when (shape.type) {
188-
ShapeType.BYTE -> "${SwiftTypes.Int8}($stringValue) ?? 0"
189-
ShapeType.SHORT -> "${SwiftTypes.Int16}($stringValue) ?? 0"
190-
ShapeType.INTEGER -> "${SwiftTypes.Int}($stringValue) ?? 0"
191-
ShapeType.LONG -> "${SwiftTypes.Int}($stringValue) ?? 0"
192-
ShapeType.FLOAT -> "${SwiftTypes.Float}($stringValue) ?? 0"
193-
ShapeType.DOUBLE -> "${SwiftTypes.Double}($stringValue) ?? 0"
194-
else -> throw CodegenException("unknown number shape: $shape")
187+
private fun stringToNumber(shape: NumberShape, stringValue: String, zeroDefaultValue: Boolean): String {
188+
val defaultValue = if (zeroDefaultValue) " ?? 0" else ""
189+
return when (shape.type) {
190+
ShapeType.BYTE -> "${SwiftTypes.Int8}($stringValue)$defaultValue"
191+
ShapeType.SHORT -> "${SwiftTypes.Int16}($stringValue)$defaultValue"
192+
ShapeType.INTEGER -> "${SwiftTypes.Int}($stringValue)$defaultValue"
193+
ShapeType.LONG -> "${SwiftTypes.Int}($stringValue)$defaultValue"
194+
ShapeType.FLOAT -> "${SwiftTypes.Float}($stringValue)$defaultValue"
195+
ShapeType.DOUBLE -> "${SwiftTypes.Double}($stringValue)$defaultValue"
196+
else -> throw CodegenException("unknown number shape: $shape")
197+
}
195198
}
196199

197200
private fun stringToDate(stringValue: String, tsFmt: TimestampFormatTrait.Format): String = when (tsFmt) {

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/middlewares/providers/HttpUrlPathProvider.kt‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,19 @@ class HttpUrlPathProvider(
9090
"$labelMemberName$enumRawValueSuffix$percentEncoded"
9191
}
9292
ShapeType.FLOAT, ShapeType.DOUBLE -> "$labelMemberName.encoded()"
93+
ShapeType.ENUM -> {
94+
val percentEncoded = if (!it.isGreedyLabel) ".urlPercentEncoding()" else ""
95+
"$labelMemberName.rawValue$percentEncoded"
96+
}
9397
else -> labelMemberName
9498
}
95-
val isBoxed = ctx.symbolProvider.toSymbol(targetShape).isBoxed()
99+
100+
// use member symbol to determine if we need to box the value
101+
// similar to how struct is generated
102+
val symbol = ctx.symbolProvider.toSymbol(binding.member)
96103

97104
// unwrap the label members if boxed
98-
if (isBoxed) {
105+
if (symbol.isBoxed()) {
99106
writer.openBlock("guard let $labelMemberName = $labelMemberName else {", "}") {
100107
writer.write("return nil")
101108
}

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/serde/formurl/MemberShapeEncodeFormURLGenerator.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ abstract class MemberShapeEncodeFormURLGenerator(
6565
writer.openBlock("for (index$level, $nestedMemberTargetName) in $memberName.enumerated() {", "}") {
6666
when (nestedMemberTarget) {
6767
is CollectionShape -> {
68-
val isBoxed = ctx.symbolProvider.toSymbol(nestedMemberTarget).isBoxed()
68+
val isBoxed = ctx.symbolProvider.toSymbol(memberTarget.member).isBoxed()
6969
if (isBoxed && !(nestedMemberTarget is SetShape)) {
7070
writer.openBlock("if let $nestedMemberTargetName = $nestedMemberTargetName {", "}") {
7171
renderNestedListEntryMember(nestedMemberTargetName, nestedMemberTarget, nestedMember, nestedMemberResolvedName, containerName, level)
@@ -119,7 +119,7 @@ abstract class MemberShapeEncodeFormURLGenerator(
119119
writer.openBlock("for (index$level, $nestedMemberTargetName) in $memberName.enumerated() {", "}") {
120120
when (nestedMemberTarget) {
121121
is CollectionShape -> {
122-
val isBoxed = ctx.symbolProvider.toSymbol(nestedMemberTarget).isBoxed()
122+
val isBoxed = ctx.symbolProvider.toSymbol(memberTarget.member).isBoxed()
123123
if (isBoxed && !(nestedMemberTarget is SetShape)) {
124124
writer.openBlock("if let $nestedMemberTargetName = $nestedMemberTargetName {", "}") {
125125
renderFlattenedListContainer(nestedMemberTargetName, nestedMemberTarget, nestedMember, memberName, member, containerName, level)
@@ -360,7 +360,7 @@ abstract class MemberShapeEncodeFormURLGenerator(
360360
}
361361

362362
fun renderScalarMember(member: MemberShape, memberTarget: Shape, containerName: String) {
363-
val symbol = ctx.symbolProvider.toSymbol(memberTarget)
363+
val symbol = ctx.symbolProvider.toSymbol(member)
364364
val memberName = ctx.symbolProvider.toMemberName(member)
365365
val resolvedMemberName = customizations.customNameTraitGenerator(member, member.memberName)
366366
val isBoxed = symbol.isBoxed()

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/serde/json/MemberShapeDecodeGenerator.kt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ abstract class MemberShapeDecodeGenerator(
5858
}
5959
}
6060

61-
fun writeDecodeForPrimitive(shape: Shape, member: MemberShape, containerName: String) {
62-
var symbol = ctx.symbolProvider.toSymbol(shape)
61+
fun writeDecodeForPrimitive(shape: Shape, member: MemberShape, containerName: String, ignoreDefaultValues: Boolean = false) {
62+
var symbol = ctx.symbolProvider.toSymbol(member)
6363
val memberName = ctx.symbolProvider.toMemberNames(member).second
6464
if (member.hasTrait(SwiftBoxTrait::class.java)) {
6565
symbol = symbol.recursiveSymbol()
@@ -69,7 +69,7 @@ abstract class MemberShapeDecodeGenerator(
6969
val decodedMemberName = "${memberName}Decoded"
7070

7171
// no need to assign nil to a member that is optional
72-
val defaultValueLiteral = if (defaultValue != null && defaultValue != "nil") " ?? $defaultValue" else ""
72+
val defaultValueLiteral = if (!ignoreDefaultValues && defaultValue != null && defaultValue != "nil") " ?? $defaultValue" else ""
7373

7474
writer.write("let \$L = try \$L.$decodeVerb(\$N.self, forKey: .\$L)$defaultValueLiteral", decodedMemberName, containerName, symbol, memberName)
7575
renderAssigningDecodedMember(member, decodedMemberName)
@@ -133,7 +133,7 @@ abstract class MemberShapeDecodeGenerator(
133133
level: Int = 0
134134
) {
135135
val symbolName = determineSymbolForShape(shape, true)
136-
val originalSymbol = ctx.symbolProvider.toSymbol(shape)
136+
val originalSymbol = ctx.symbolProvider.toSymbol(parentMember)
137137
val decodedMemberName = "${memberName.removeSurroundingBackticks()}Decoded$level"
138138
var insertMethod = when (shape) {
139139
is SetShape -> "insert"
@@ -249,7 +249,7 @@ abstract class MemberShapeDecodeGenerator(
249249
level: Int = 0
250250
) {
251251
val symbolName = determineSymbolForShape(shape, true)
252-
val originalSymbol = ctx.symbolProvider.toSymbol(shape)
252+
val originalSymbol = ctx.symbolProvider.toSymbol(topLevelMember)
253253
val decodedMemberName = "${memberName.removeSurroundingBackticks()}Decoded$level"
254254
val nestedTarget = ctx.model.expectShape(shape.value.target)
255255
if (level == 0) {

0 commit comments

Comments
 (0)