-
Notifications
You must be signed in to change notification settings - Fork 249
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[smithy-rs] Render a Union member of type Unit to an enum variant wit…
…hout inner data (#1989) * Avoid explicitly emitting Unit type within Union This commit addresses #1546. Previously, the Unit type in a Union was rendered as an enum variant whose inner data was crate::model::Unit. The way such a variant appears in Rust code feels a bit odd as it usually does not carry inner data for `()`. We now render a Union member of type Unit to an enum variant without inner data. * Address test failures washed out in CI This commit updates places that need to be aware of the Unit type attached to a Union member. Those places will render the Union member in a way that the generated Rust code does not include inner data of type Unit. It should help pass `codegen-client-test:test` and `codegen-server-test:test`. Further refactoring is required because each place we updated performs an explicit if-else check for special casing, which we should avoid. * Update codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt Co-authored-by: John DiSanti <[email protected]> * Remove commented-out code * Add a helper for comparing against ShapeId for Unit This commit adds a helper for checking whether MemberShape targets the ShapeId of the Unit type. The benefit of the helper is that each call site does not have to construct a ShapeId for the Unit type every time comparison is made. * Move Unit type bifurcation logic to jsonObjectWriter This commit moves the if-else expression hard-coded in the StructureShape matching case within RustWriter.serializeMemberValue to jsonObjectWriter. The previous approach of inlining the if-else expression out of jsonObjectWriter to the StructureShape case and tailoring it to the call site violated the DRY principle. * Make QuerySerializerGenerator in sync with the change This commit brings the Unit type related change we've made so far to QuerySerializerGenerator. All tests in CI passed even without this commit, but noticing how similar QuerySerializerGenerator is to JsonSerializerGenerator, we should make the former in sync with the latter. * Update CHANGELOG.next.toml * Refactor ofTypeUnit -> isTargetUnit This commit addresses smithy-lang/smithy-rs#1989 (comment). The extension should be renamed to isTargetUnit and moved to Smithy.kt. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt Co-authored-by: John DiSanti <[email protected]> * Simplify if-else in jsonObjectWriter This commit addresses smithy-lang/smithy-rs#1989 (comment) * Avoid the union member's reference name being empty This commit addresses smithy-lang/smithy-rs#1989 (comment) Even if member's reference name "inner" is present, it will not be an issue when the member is the Unit type where the reference name "inner" cannot be extracted. The reason is jsonObjectWriter won't render the serialization function if the member is the Unit type. That said, the same change in QuerySerializerGenerator may not be the case so we'll run the tests in CI and see what breaks. * Ensure Union with Unit target can be serialized This commit updates serialization of a Union with a Unit target in different types of serializers. We first updated protocol tests by adding a new field of type Unit and some of them failed as expected. We worked our way back from those failed tests and fixed the said implementation for serialization accordingly. * Ensure Union with Unit target can be parsed This commit handles deserialization of a Union with a Unit target in XmlBindingTraitParserGenerator. Prior to the commit, we already handled the said deserialization correctly in JsonParserGenerator but missed it in XmlBindingTraitParserGenerator. We added a new field of type Unit to a Union in parser protocols tests, ran them, and worked our way back from the failed tests. * Ensure match arm for Unit works in custom Debug impl This commit handles a use case that came up as a result of combining two updates, implementing a custom Debug impl for a Union and avoid rendering the Unit type in a Union. In the custom Debug impl, we now make sure that if the target is of type Unit, we render the corresponding match arm without the inner data. We add a new unit test in UnionGeneratorTest. * Fix unused variables warnings in CI This commit adds the #[allow(unused_variables)] annotation to a block of code generated for a member being the Unit type. The annotation is put before we call the handleOptional function so that it covers the whole block that the function generates. * Fix E0658 on unused_variables This commit fixes an error where attributes on expressions are experimental. It does so by rearranging code in a way that achieves the same effect but without #[allow(unused_variables)] on an expression. Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: John DiSanti <[email protected]>
- Loading branch information
1 parent
be76de4
commit a7f25f3
Showing
277 changed files
with
10,522 additions
and
384 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.