Skip to content

Commit 76c959c

Browse files
vcjanavcjana
committed
Fix union serialization unused variable warnings
Fixes unused variable warnings in union serialization for JSON, Query, and CBOR protocols. Changes: - JsonSerializerGenerator: Fix '_inner' -> 'inner' for non-unit structs - QuerySerializerGenerator: Use '_inner' for unit structs, 'inner' for others - CborSerializerGenerator: Fix '_inner' -> 'inner' for non-unit structs - Add regression tests for JSON and Query serializers - Tests generate actual protocol_serde modules to validate fix Addresses issue #4308 by following the same pattern as the XML serializer fix. Unit structs use '_inner' since the variable is never accessed in serialization.
1 parent b497b26 commit 76c959c

File tree

5 files changed

+2520
-77
lines changed

5 files changed

+2520
-77
lines changed

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,11 +357,18 @@ abstract class QuerySerializerGenerator(private val codegenContext: CodegenConte
357357
) {
358358
rustBlock("match input") {
359359
for (member in context.shape.members()) {
360+
val memberShape = model.expectShape(member.target)
361+
val memberName = symbolProvider.toMemberName(member)
360362
val variantName =
361363
if (member.isTargetUnit()) {
362-
"${symbolProvider.toMemberName(member)}"
364+
"$memberName"
365+
} else if (memberShape.isStructureShape &&
366+
memberShape.asStructureShape().get().allMembers.isEmpty()
367+
) {
368+
// Unit structs don't serialize inner, so it is never accessed
369+
"$memberName(_inner)"
363370
} else {
364-
"${symbolProvider.toMemberName(member)}(inner)"
371+
"$memberName(inner)"
365372
}
366373
withBlock("#T::$variantName => {", "},", unionSymbol) {
367374
serializeMember(

‎codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/AwsQuerySerializerGeneratorTest.kt‎

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,12 @@
55

66
package software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize
77

8-
import org.junit.jupiter.api.Test
98
import org.junit.jupiter.params.ParameterizedTest
109
import org.junit.jupiter.params.provider.CsvSource
1110
import software.amazon.smithy.model.knowledge.NullableIndex
1211
import software.amazon.smithy.model.shapes.OperationShape
1312
import software.amazon.smithy.model.shapes.StringShape
1413
import software.amazon.smithy.model.shapes.StructureShape
15-
import software.amazon.smithy.model.shapes.UnionShape
1614
import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget
1715
import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator.Companion.hasFallibleBuilder
1816
import software.amazon.smithy.rust.codegen.core.smithy.generators.EnumGenerator
@@ -25,7 +23,6 @@ import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
2523
import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest
2624
import software.amazon.smithy.rust.codegen.core.testutil.renderWithModelBuilder
2725
import software.amazon.smithy.rust.codegen.core.testutil.testCodegenContext
28-
import software.amazon.smithy.rust.codegen.core.testutil.testSymbolProvider
2926
import software.amazon.smithy.rust.codegen.core.testutil.unitTest
3027
import software.amazon.smithy.rust.codegen.core.util.inputShape
3128
import software.amazon.smithy.rust.codegen.core.util.lookup
@@ -328,33 +325,4 @@ class AwsQuerySerializerGeneratorTest {
328325
}
329326
project.compileAndTest()
330327
}
331-
332-
@Test
333-
fun `union with unit struct demonstrates query serialization bug`() {
334-
val model =
335-
"""
336-
namespace test
337-
338-
union TestUnion {
339-
unitMember: Unit,
340-
dataMember: String
341-
}
342-
343-
structure Unit {}
344-
""".asSmithyModel()
345-
346-
val project = TestWorkspace.testProject(testSymbolProvider(model))
347-
val codegenContext = testCodegenContext(model)
348-
349-
// Render the Unit structure
350-
model.lookup<StructureShape>("test#Unit").also { unit ->
351-
unit.renderWithModelBuilder(model, codegenContext.symbolProvider, project)
352-
}
353-
354-
// Render the Union
355-
project.moduleFor(model.lookup<UnionShape>("test#TestUnion")) {
356-
UnionGenerator(model, codegenContext.symbolProvider, this, model.lookup("test#TestUnion")).render()
357-
}
358-
project.compileAndTest()
359-
}
360328
}

‎codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGeneratorTest.kt‎

Lines changed: 53 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,28 @@ import software.amazon.smithy.rust.codegen.core.util.inputShape
3333
import software.amazon.smithy.rust.codegen.core.util.lookup
3434

3535
class JsonSerializerGeneratorTest {
36+
private val unionWithUnitStructModel =
37+
"""
38+
namespace test
39+
use aws.protocols#restJson1
40+
41+
union TestUnion {
42+
unitMember: Unit,
43+
dataMember: String
44+
}
45+
46+
structure Unit {}
47+
48+
@http(uri: "/test", method: "POST")
49+
operation TestOp {
50+
input: TestInput
51+
}
52+
53+
structure TestInput {
54+
union: TestUnion
55+
}
56+
""".asSmithyModel()
57+
3658
private val baseModel =
3759
"""
3860
namespace test
@@ -346,60 +368,48 @@ class JsonSerializerGeneratorTest {
346368
}
347369

348370
@Test
349-
fun `union with unit struct demonstrates serialization bug`() {
350-
val model =
351-
"""
352-
namespace test
353-
354-
union TestUnion {
355-
unitMember: Unit,
356-
dataMember: String
357-
}
371+
fun `union with unit struct doesn't cause unused variable warning`() {
372+
// Regression test for https://github.com/smithy-lang/smithy-rs/issues/4308
373+
val model = RecursiveShapeBoxer().transform(OperationNormalizer.transform(unionWithUnitStructModel))
358374

359-
structure Unit {}
360-
""".asSmithyModel()
361-
362-
val project = TestWorkspace.testProject(testSymbolProvider(model))
363375
val codegenContext = testCodegenContext(model)
376+
val symbolProvider = codegenContext.symbolProvider
377+
val project = TestWorkspace.testProject(symbolProvider)
364378

365-
// Render the Unit structure
366-
model.lookup<StructureShape>("test#Unit").also { unit ->
367-
unit.renderWithModelBuilder(model, codegenContext.symbolProvider, project)
368-
}
379+
// Generate the JSON serializer that will create the union serialization code
380+
val jsonSerializer =
381+
JsonSerializerGenerator(
382+
codegenContext,
383+
HttpTraitHttpBindingResolver(model, ProtocolContentTypes.consistent("application/json")),
384+
::restJsonFieldName,
385+
)
386+
val operationGenerator = jsonSerializer.operationInputSerializer(model.lookup("test#TestOp"))
387+
388+
// Render all necessary structures and unions
389+
model.lookup<StructureShape>("test#Unit").renderWithModelBuilder(model, symbolProvider, project)
390+
model.lookup<OperationShape>("test#TestOp").inputShape(model).renderWithModelBuilder(model, symbolProvider, project)
369391

370-
// Render the Union
371392
project.moduleFor(model.lookup<UnionShape>("test#TestUnion")) {
372-
UnionGenerator(model, codegenContext.symbolProvider, this, model.lookup("test#TestUnion")).render()
393+
UnionGenerator(model, symbolProvider, this, model.lookup("test#TestUnion")).render()
373394
}
374-
project.compileAndTest()
375-
}
376-
377-
@Test
378-
fun `union with unit struct demonstrates cbor serialization bug`() {
379-
val model =
380-
"""
381-
namespace test
382-
383-
union TestUnion {
384-
unitMember: Unit,
385-
dataMember: String
386-
}
387395

388-
structure Unit {}
389-
""".asSmithyModel()
396+
// Generate the actual protocol_serde module with union serialization
397+
project.lib {
398+
unitTest(
399+
"json_union_serialization",
400+
"""
401+
use test_model::{TestInput, TestUnion, Unit};
390402
391-
val project = TestWorkspace.testProject(testSymbolProvider(model))
392-
val codegenContext = testCodegenContext(model)
403+
// Generate the serialization function that contains union match arms with (inner)
404+
${format(operationGenerator!!)};
393405
394-
// Render the Unit structure
395-
model.lookup<StructureShape>("test#Unit").also { unit ->
396-
unit.renderWithModelBuilder(model, codegenContext.symbolProvider, project)
406+
// Test that the code compiles - this validates our fix works
407+
let _test_passed = true;
408+
""",
409+
)
397410
}
398411

399-
// Render the Union
400-
project.moduleFor(model.lookup<UnionShape>("test#TestUnion")) {
401-
UnionGenerator(model, codegenContext.symbolProvider, this, model.lookup("test#TestUnion")).render()
402-
}
412+
// The test passes if the generated code compiles without unused variable warnings
403413
project.compileAndTest()
404414
}
405415
}

‎codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/QuerySerializerGeneratorTest.kt‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package software.amazon.smithy.rust.codegen.core.smithy.protocols.serialize
7+
8+
import org.junit.jupiter.api.Test
9+
import software.amazon.smithy.model.shapes.OperationShape
10+
import software.amazon.smithy.model.shapes.StructureShape
11+
import software.amazon.smithy.model.shapes.UnionShape
12+
import software.amazon.smithy.rust.codegen.core.smithy.generators.UnionGenerator
13+
import software.amazon.smithy.rust.codegen.core.smithy.transformers.OperationNormalizer
14+
import software.amazon.smithy.rust.codegen.core.smithy.transformers.RecursiveShapeBoxer
15+
import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace
16+
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
17+
import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest
18+
import software.amazon.smithy.rust.codegen.core.testutil.renderWithModelBuilder
19+
import software.amazon.smithy.rust.codegen.core.testutil.testCodegenContext
20+
import software.amazon.smithy.rust.codegen.core.testutil.unitTest
21+
import software.amazon.smithy.rust.codegen.core.util.inputShape
22+
import software.amazon.smithy.rust.codegen.core.util.lookup
23+
24+
class QuerySerializerGeneratorTest {
25+
@Test
26+
fun `union with unit struct doesn't cause unused variable warning`() {
27+
// Regression test for https://github.com/smithy-lang/smithy-rs/issues/4308
28+
val model =
29+
RecursiveShapeBoxer().transform(
30+
OperationNormalizer.transform(
31+
"""
32+
namespace test
33+
34+
union TestUnion {
35+
unitMember: Unit,
36+
dataMember: String
37+
}
38+
39+
structure Unit {}
40+
41+
@http(uri: "/test", method: "POST")
42+
operation TestOp {
43+
input: TestInput
44+
}
45+
46+
structure TestInput {
47+
union: TestUnion
48+
}
49+
""".asSmithyModel(),
50+
),
51+
)
52+
53+
val codegenContext = testCodegenContext(model)
54+
val symbolProvider = codegenContext.symbolProvider
55+
val parserSerializer = AwsQuerySerializerGenerator(codegenContext)
56+
val operationGenerator = parserSerializer.operationInputSerializer(model.lookup("test#TestOp"))
57+
58+
val project = TestWorkspace.testProject(symbolProvider)
59+
60+
// Render all necessary structures and unions
61+
model.lookup<StructureShape>("test#Unit").renderWithModelBuilder(model, symbolProvider, project)
62+
model.lookup<OperationShape>("test#TestOp").inputShape(model).renderWithModelBuilder(model, symbolProvider, project)
63+
64+
project.moduleFor(model.lookup<UnionShape>("test#TestUnion")) {
65+
UnionGenerator(model, symbolProvider, this, model.lookup("test#TestUnion")).render()
66+
}
67+
68+
// Generate the serialization module that will contain the union serialization code
69+
project.lib {
70+
unitTest(
71+
"query_union_serialization",
72+
"""
73+
use test_model::{TestInput, TestUnion, Unit};
74+
75+
// This will generate the serialization code that should not have unused variable warnings
76+
${format(operationGenerator!!)};
77+
78+
// The test passes if the code compiles without unused variable warnings
79+
""",
80+
)
81+
}
82+
83+
project.compileAndTest()
84+
}
85+
}

0 commit comments

Comments
 (0)