Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render a Union member of type Unit to an enum variant without inner data #1989

Merged
merged 25 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
bdf7858
Avoid explicitly emitting Unit type within Union
ysaito1001 Nov 15, 2022
2b148aa
Address test failures washed out in CI
ysaito1001 Nov 16, 2022
1f702df
Update codegen-core/src/test/kotlin/software/amazon/smithy/rust/codeg…
ysaito1001 Nov 16, 2022
11caf0c
Remove commented-out code
ysaito1001 Nov 16, 2022
a7002aa
Merge branch 'ysaito/remove-unit-from-generated-rust-enum' of https:/…
ysaito1001 Nov 16, 2022
fd60317
Add a helper for comparing against ShapeId for Unit
ysaito1001 Nov 17, 2022
2455eb6
Merge branch 'main' into ysaito/remove-unit-from-generated-rust-enum
ysaito1001 Nov 17, 2022
74041ca
Move Unit type bifurcation logic to jsonObjectWriter
ysaito1001 Nov 17, 2022
5802bd5
Make QuerySerializerGenerator in sync with the change
ysaito1001 Nov 17, 2022
d48b115
Update CHANGELOG.next.toml
ysaito1001 Nov 17, 2022
ccd2e63
Merge branch 'main' into ysaito/remove-unit-from-generated-rust-enum
ysaito1001 Nov 17, 2022
de1f5cf
Refactor ofTypeUnit -> isTargetUnit
ysaito1001 Nov 30, 2022
7c3a0a8
Merge branch 'main' into ysaito/remove-unit-from-generated-rust-enum
ysaito1001 Nov 30, 2022
5e95a55
Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codeg…
ysaito1001 Dec 3, 2022
4b9a355
Simplify if-else in jsonObjectWriter
ysaito1001 Dec 3, 2022
e4f6559
Avoid the union member's reference name being empty
ysaito1001 Dec 3, 2022
0e8b621
CHANGELOG.next.toml
ysaito1001 Dec 3, 2022
45ef80d
Ensure Union with Unit target can be serialized
ysaito1001 Dec 6, 2022
884155e
Ensure Union with Unit target can be parsed
ysaito1001 Dec 6, 2022
9962214
Merge branch 'main' into ysaito/remove-unit-from-generated-rust-enum
ysaito1001 Dec 6, 2022
2269361
Ensure match arm for Unit works in custom Debug impl
ysaito1001 Dec 6, 2022
5445282
Merge branch 'main' into ysaito/remove-unit-from-generated-rust-enum
ysaito1001 Dec 6, 2022
fd8e8fc
Fix unused variables warnings in CI
ysaito1001 Dec 7, 2022
2d1f01d
Fix E0658 on unused_variables
ysaito1001 Dec 7, 2022
d0c8b0b
Merge branch 'main' into ysaito/remove-unit-from-generated-rust-enum
ysaito1001 Dec 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package software.amazon.smithy.rust.codegen.core.smithy.generators

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.codegen.core.SymbolProvider
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.MemberShape
Expand Down Expand Up @@ -49,16 +50,18 @@ class UnionGenerator(
private val sortedMembers: List<MemberShape> = shape.allMembers.values.sortedBy { symbolProvider.toMemberName(it) }

fun render() {
renderUnion()
}

private fun renderUnion() {
writer.documentShape(shape, model)
writer.deprecatedShape(shape)

val unionSymbol = symbolProvider.toSymbol(shape)
val containerMeta = unionSymbol.expectRustMetadata()
containerMeta.render(writer)

renderUnion(unionSymbol)
renderImplBlock(unionSymbol)
}

private fun renderUnion(unionSymbol: Symbol) {
writer.rustBlock("enum ${unionSymbol.name}") {
sortedMembers.forEach { member ->
val memberSymbol = symbolProvider.toSymbol(member)
Expand All @@ -67,7 +70,7 @@ class UnionGenerator(
documentShape(member, model, note = note)
deprecatedShape(member)
memberSymbol.expectRustMetadata().renderAttributes(this)
write("${symbolProvider.toMemberName(member)}(#T),", symbolProvider.toSymbol(member))
writer.renderVariant(symbolProvider, member, memberSymbol)
}
if (renderUnknownVariant) {
docs("""The `Unknown` variant represents cases where new union variant was received. Consider upgrading the SDK to the latest available version.""")
Expand All @@ -82,6 +85,9 @@ class UnionGenerator(
rust("Unknown,")
}
}
}

private fun renderImplBlock(unionSymbol: Symbol) {
writer.rustBlock("impl ${unionSymbol.name}") {
sortedMembers.forEach { member ->
val memberSymbol = symbolProvider.toSymbol(member)
Expand All @@ -91,11 +97,7 @@ class UnionGenerator(
if (sortedMembers.size == 1) {
Attribute.Custom("allow(irrefutable_let_patterns)").render(this)
}
rust("/// Tries to convert the enum instance into [`$variantName`](#T::$variantName), extracting the inner #D.", unionSymbol, memberSymbol)
rust("/// Returns `Err(&Self)` if it can't be converted.")
rustBlock("pub fn as_$funcNamePart(&self) -> std::result::Result<&#T, &Self>", memberSymbol) {
rust("if let ${unionSymbol.name}::$variantName(val) = &self { Ok(val) } else { Err(self) }")
}
writer.renderAsVariant(member, variantName, funcNamePart, unionSymbol, memberSymbol)
rust("/// Returns true if this is a [`$variantName`](#T::$variantName).", unionSymbol)
rustBlock("pub fn is_$funcNamePart(&self) -> bool") {
rust("self.as_$funcNamePart().is_ok()")
Expand All @@ -114,7 +116,44 @@ class UnionGenerator(
const val UnknownVariantName = "Unknown"
}
}

fun unknownVariantError(union: String) =
"Cannot serialize `$union::${UnionGenerator.UnknownVariantName}` for the request. " +
"The `Unknown` variant is intended for responses only. " +
"It occurs when an outdated client is used after a new enum variant was added on the server side."

private fun RustWriter.renderVariant(symbolProvider: SymbolProvider, member: MemberShape, memberSymbol: Symbol) {
if (member.target.name == "Unit") {
ysaito1001 marked this conversation as resolved.
Show resolved Hide resolved
write("${symbolProvider.toMemberName(member)},")
} else {
write("${symbolProvider.toMemberName(member)}(#T),", memberSymbol)
}
}

private fun RustWriter.renderAsVariant(
member: MemberShape,
variantName: String,
funcNamePart: String,
unionSymbol: Symbol,
memberSymbol: Symbol,
) {
if (member.target.name == "Unit") {
rust(
"/// Tries to convert the enum instance into [`$variantName`], extracting the inner `()`.",
)
rust("/// Returns `Err(&Self)` if it can't be converted.")
rustBlock("pub fn as_$funcNamePart(&self) -> std::result::Result<(), &Self>") {
ysaito1001 marked this conversation as resolved.
Show resolved Hide resolved
rust("if let ${unionSymbol.name}::$variantName = &self { Ok(()) } else { Err(self) }")
}
} else {
rust(
"/// Tries to convert the enum instance into [`$variantName`](#T::$variantName), extracting the inner #D.",
unionSymbol,
memberSymbol,
)
rust("/// Returns `Err(&Self)` if it can't be converted.")
rustBlock("pub fn as_$funcNamePart(&self) -> std::result::Result<&#T, &Self>", memberSymbol) {
rust("if let ${unionSymbol.name}::$variantName(val) = &self { Ok(val) } else { Err(self) }")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,17 @@ class UnionGeneratorTest {
writer.compileAndTest()
}

@Test
fun `unit types should not appear in generated enum`() {
val writer = generateUnion("union MyUnion { a: Unit, b: String }", unknownVariant = true)
writer.compileAndTest(
"""
let a = MyUnion::A;
assert!(a.as_a() == Ok(()));
ysaito1001 marked this conversation as resolved.
Show resolved Hide resolved
""",
)
}

private fun generateUnion(modelSmithy: String, unionName: String = "MyUnion", unknownVariant: Boolean = true): RustWriter {
val model = "namespace test\n$modelSmithy".asSmithyModel()
val provider: SymbolProvider = testSymbolProvider(model)
Expand Down