Skip to content

Commit d4c11dd

Browse files
vcjanaysaito1001
andauthored
Try different protocols against union in question (#4333)
## Motivation and Context Addresses unused variable warnings identified in #4306. While #4306 fixed XmlBindingTraitSerializerGenerator, it noted that QuerySerializerGenerator contained the same bug when handling union variants with empty structs. This PR completes the fix for RestXml and AwsQuery protocols. The issue was discovered in S3Control's ObjectEncryptionFilter where builds with -D warnings were failing. ## Description Modified `QuerySerializerGenerator` to prefix unused variables with underscore for empty struct variants: - Added `isEmptyStruct()` to detect empty structures - Use `_inner` for empty structs, `inner` for non-empty structs - No changes to JsonSerializerGenerator or CborSerializerGenerator (as these are not affected) ## Testing Added tests for RestXml, AwsQuery, and RestJson (documents immunity) Tests enforce `-D warnings` via `clientIntegrationTest` Verified S3Control compiles cleanly with `RUSTFLAGS="-D warnings"` <!---## Checklist If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> <!---- [ ] [x] For changes to the smithy-rs codegen or runtime crates, I have created a changelog entry Markdown file in the .changelog directory, specifying "client," "server," or both in the applies_to key.--> ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: ysaito1001 <awsaito@amazon.com>
1 parent a87ffdf commit d4c11dd

File tree

5 files changed

+249
-55
lines changed

5 files changed

+249
-55
lines changed

codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/AwsQueryTest.kt

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,50 @@ class AwsQueryTest {
3434
}
3535
""".asSmithyModel()
3636

37+
private val inputUnionWithEmptyStructure =
38+
"""
39+
namespace test
40+
use aws.protocols#awsQuery
41+
42+
@awsQuery
43+
@xmlNamespace(uri: "https://example.com/")
44+
service TestService {
45+
version: "2019-12-16",
46+
operations: [TestOp]
47+
}
48+
49+
operation TestOp {
50+
input: TestInput
51+
}
52+
53+
structure TestInput {
54+
testUnion: TestUnion
55+
}
56+
57+
union TestUnion {
58+
// Empty struct - should generate _inner to avoid unused variable warning
59+
emptyStruct: EmptyStruct,
60+
61+
// Normal struct - should generate inner (without underscore)
62+
normalStruct: NormalStruct
63+
}
64+
65+
structure EmptyStruct {}
66+
67+
structure NormalStruct {
68+
value: String
69+
}
70+
""".asSmithyModel()
71+
3772
@Test
3873
fun `generate an aws query service that compiles`() {
3974
clientIntegrationTest(model) { _, _ -> }
4075
}
76+
77+
@Test
78+
fun `union with empty struct generates warning-free code`() {
79+
// This test will fail with unused variable warnings if the fix is not applied
80+
// clientIntegrationTest enforces -D warnings via codegenIntegrationTest
81+
clientIntegrationTest(inputUnionWithEmptyStructure) { _, _ -> }
82+
}
4183
}

codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/RestJsonTest.kt

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,54 @@ internal class RestJsonTest {
3939
}
4040
""".asSmithyModel()
4141

42+
private val inputUnionWithEmptyStructure =
43+
"""
44+
namespace test
45+
use aws.protocols#restJson1
46+
use aws.api#service
47+
48+
@service(sdkId: "Rest Json Empty Struct")
49+
@restJson1
50+
service RestJsonEmptyStruct {
51+
version: "2019-12-16",
52+
operations: [TestOp]
53+
}
54+
55+
@http(uri: "/test", method: "POST")
56+
operation TestOp {
57+
input: TestInput
58+
}
59+
60+
structure TestInput {
61+
testUnion: TestUnion
62+
}
63+
64+
union TestUnion {
65+
// Empty struct - RestJson ALWAYS uses inner variable, no warning
66+
emptyStruct: EmptyStruct,
67+
68+
// Normal struct - RestJson uses inner variable
69+
normalStruct: NormalStruct
70+
}
71+
72+
structure EmptyStruct {}
73+
74+
structure NormalStruct {
75+
value: String
76+
}
77+
""".asSmithyModel()
78+
4279
@Test
4380
fun `generate a rest json service that compiles`() {
4481
clientIntegrationTest(model) { _, _ -> }
4582
}
83+
84+
@Test
85+
fun `union with empty struct always uses inner variable`() {
86+
// This test documents that RestJson protocol is immune to unused variable issues.
87+
// Unlike RestXml/AwsQuery, RestJson serializers always reference the inner variable
88+
// even for empty structs, so no underscore prefix is needed.
89+
// This test passes without any code changes, proving RestJson immunity.
90+
clientIntegrationTest(inputUnionWithEmptyStructure) { _, _ -> }
91+
}
4692
}

codegen-client/src/test/kotlin/software/amazon/smithy/rust/codegen/client/smithy/protocols/RestXmlTest.kt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,51 @@ internal class RestXmlTest {
8383
8484
""".asSmithyModel()
8585

86+
private val inputUnionWithEmptyStructure =
87+
"""
88+
namespace test
89+
use aws.protocols#restXml
90+
use aws.api#service
91+
92+
@service(sdkId: "Rest XML Empty Struct")
93+
@restXml
94+
service RestXmlEmptyStruct {
95+
version: "2019-12-16",
96+
operations: [TestOp]
97+
}
98+
99+
@http(uri: "/test", method: "POST")
100+
operation TestOp {
101+
input: TestInput
102+
}
103+
104+
structure TestInput {
105+
testUnion: TestUnion
106+
}
107+
108+
union TestUnion {
109+
// Empty struct - should generate _inner to avoid unused variable warning
110+
emptyStruct: EmptyStruct,
111+
// Normal struct - should generate inner (without underscore)
112+
normalStruct: NormalStruct
113+
}
114+
115+
structure EmptyStruct {}
116+
117+
structure NormalStruct {
118+
value: String
119+
}
120+
""".asSmithyModel()
121+
86122
@Test
87123
fun `generate a rest xml service that compiles`() {
88124
clientIntegrationTest(model) { _, _ -> }
89125
}
126+
127+
@Test
128+
fun `union with empty struct generates warning-free code`() {
129+
// This test will fail with unused variable warnings if the fix is not applied
130+
// clientIntegrationTest enforces -D warnings via codegenIntegrationTest
131+
clientIntegrationTest(inputUnionWithEmptyStructure) { _, _ -> }
132+
}
90133
}

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/QuerySerializerGenerator.kt

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -345,29 +345,51 @@ abstract class QuerySerializerGenerator(private val codegenContext: CodegenConte
345345
}
346346
}
347347

348+
/**
349+
* Determines if a struct shape is empty (has no members).
350+
* Empty structs result in unused variables in union match arms since the inner value is never referenced.
351+
*/
352+
private fun isEmptyStruct(shape: Shape): Boolean =
353+
when (shape) {
354+
is StructureShape -> shape.members().isEmpty()
355+
else -> false
356+
}
357+
348358
private fun RustWriter.serializeUnion(context: Context<UnionShape>) {
349359
val unionSymbol = symbolProvider.toSymbol(context.shape)
360+
361+
// Check if any union member uses the writer (non-empty structs)
362+
val hasNonEmptyMember =
363+
context.shape.members().any { member ->
364+
!member.isTargetUnit() && !isEmptyStruct(model.expectShape(member.target))
365+
}
366+
val writerVarName = if (hasNonEmptyMember) "writer" else "_writer"
367+
350368
val unionSerializer =
351369
protocolFunctions.serializeFn(context.shape) { fnName ->
352370
Attribute.AllowUnusedMut.render(this)
353371
rustBlockTemplate(
354-
"pub fn $fnName(mut writer: #{QueryValueWriter}, input: &#{Input}) -> #{Result}<(), #{Error}>",
372+
"pub fn $fnName(mut $writerVarName: #{QueryValueWriter}, input: &#{Input}) -> #{Result}<(), #{Error}>",
355373
"Input" to unionSymbol,
356374
*codegenScope,
357375
) {
358376
rustBlock("match input") {
359377
for (member in context.shape.members()) {
378+
val targetShape = model.expectShape(member.target)
379+
// Use underscore prefix for empty structs to avoid unused variable warnings
380+
val innerVarName = if (isEmptyStruct(targetShape)) "_inner" else "inner"
381+
360382
val variantName =
361383
if (member.isTargetUnit()) {
362384
"${symbolProvider.toMemberName(member)}"
363385
} else {
364-
"${symbolProvider.toMemberName(member)}(inner)"
386+
"${symbolProvider.toMemberName(member)}($innerVarName)"
365387
}
366388
withBlock("#T::$variantName => {", "},", unionSymbol) {
367389
serializeMember(
368390
MemberContext.unionMember(
369-
context.copy(writerExpression = "writer"),
370-
"inner",
391+
context.copy(writerExpression = writerVarName),
392+
innerVarName,
371393
member,
372394
),
373395
)

0 commit comments

Comments
 (0)