From 3d3b9bbc2c35b6a39941a681d1d9e9fa31734010 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Mon, 25 Jul 2022 18:15:18 -0700 Subject: [PATCH 1/4] Add `#[doc(hidden)]` to struct fields to encourage accessor use --- .../server/smithy/RustCodegenServerPlugin.kt | 2 +- .../smithy/rust/codegen/rustlang/RustTypes.kt | 1 + .../rust/codegen/smithy/RustCodegenPlugin.kt | 2 +- .../smithy/StreamingTraitSymbolProvider.kt | 5 +++- .../codegen/smithy/SymbolMetadataProvider.kt | 26 ++++++++++++++++++- 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt index 43c1b342dc..793cb11939 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt @@ -70,7 +70,7 @@ class RustCodegenServerPlugin : SmithyBuildPlugin { // Generate [ByteStream] instead of `Blob` for streaming binary shapes (e.g. S3 GetObject) .let { StreamingShapeSymbolProvider(it, model) } // Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes - .let { BaseSymbolMetadataProvider(it, additionalAttributes = listOf()) } + .let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf()) } // Streaming shapes need different derives (e.g. they cannot derive Eq) .let { StreamingShapeMetadataProvider(it, model) } // Rename shapes that clash with Rust reserved words & and other SDK specific features e.g. `send()` cannot diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustTypes.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustTypes.kt index 84673f2239..e1af9ec67c 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustTypes.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustTypes.kt @@ -334,6 +334,7 @@ sealed class Attribute { */ val NonExhaustive = Custom("non_exhaustive") val AllowUnusedMut = Custom("allow(unused_mut)") + val DocHidden = Custom("doc(hidden)") val DocInline = Custom("doc(inline)") } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustCodegenPlugin.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustCodegenPlugin.kt index 9e02e6c67a..9f43da2a3b 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustCodegenPlugin.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/RustCodegenPlugin.kt @@ -66,7 +66,7 @@ class RustCodegenPlugin : SmithyBuildPlugin { // Generate `ByteStream` instead of `Blob` for streaming binary shapes (e.g. S3 GetObject) .let { StreamingShapeSymbolProvider(it, model) } // Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes - .let { BaseSymbolMetadataProvider(it, additionalAttributes = listOf(NonExhaustive)) } + .let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf(NonExhaustive)) } // Streaming shapes need different derives (e.g. they cannot derive Eq) .let { StreamingShapeMetadataProvider(it, model) } // Rename shapes that clash with Rust reserved words & and other SDK specific features e.g. `send()` cannot diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/StreamingTraitSymbolProvider.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/StreamingTraitSymbolProvider.kt index b76eb3aa85..e3bde3719e 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/StreamingTraitSymbolProvider.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/StreamingTraitSymbolProvider.kt @@ -56,7 +56,10 @@ class StreamingShapeSymbolProvider(private val base: RustSymbolProvider, private * * Note that since streaming members can only be used on the root shape, this can only impact input and output shapes. */ -class StreamingShapeMetadataProvider(private val base: RustSymbolProvider, private val model: Model) : SymbolMetadataProvider(base) { +class StreamingShapeMetadataProvider( + private val base: RustSymbolProvider, + private val model: Model, +) : SymbolMetadataProvider(base) { override fun memberMeta(memberShape: MemberShape): RustMetadata { return base.toSymbol(memberShape).expectRustMetadata() } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/SymbolMetadataProvider.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/SymbolMetadataProvider.kt index 6521efaca1..f2b3381d2e 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/SymbolMetadataProvider.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/SymbolMetadataProvider.kt @@ -7,6 +7,7 @@ package software.amazon.smithy.rust.codegen.smithy import software.amazon.smithy.codegen.core.CodegenException import software.amazon.smithy.codegen.core.Symbol +import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.MemberShape import software.amazon.smithy.model.shapes.Shape import software.amazon.smithy.model.shapes.StringShape @@ -14,6 +15,7 @@ import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.shapes.UnionShape import software.amazon.smithy.model.traits.EnumDefinition import software.amazon.smithy.model.traits.EnumTrait +import software.amazon.smithy.model.traits.StreamingTrait import software.amazon.smithy.rust.codegen.rustlang.Attribute import software.amazon.smithy.rust.codegen.rustlang.RustMetadata import software.amazon.smithy.rust.codegen.rustlang.Visibility @@ -73,6 +75,7 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr */ class BaseSymbolMetadataProvider( base: RustSymbolProvider, + private val model: Model, additionalAttributes: List, ) : SymbolMetadataProvider(base) { private val containerDefault = RustMetadata( @@ -82,7 +85,28 @@ class BaseSymbolMetadataProvider( ) override fun memberMeta(memberShape: MemberShape): RustMetadata { - return RustMetadata(visibility = Visibility.PUBLIC) + val container = model.expectShape(memberShape.container) + return when { + container.isStructureShape -> { + // TODO(https://github.com/awslabs/smithy-rs/issues/943): Once streaming accessors are usable, + // then also make streaming members `#[doc(hidden)]` + if (memberShape.getMemberTrait(model, StreamingTrait::class.java).isPresent) { + RustMetadata(visibility = Visibility.PUBLIC) + } else { + RustMetadata( + // At some point, visibility will be made PRIVATE, so make these `#[doc(hidden)]` for now + visibility = Visibility.PUBLIC, + additionalAttributes = listOf(Attribute.DocHidden) + ) + } + } + container.isUnionShape || + container.isListShape || + container.isSetShape || + container.isMapShape + -> RustMetadata(visibility = Visibility.PUBLIC) + else -> TODO("Unrecognized container type: $container") + } } override fun structureMeta(structureShape: StructureShape): RustMetadata { From 8d3f933c777132a420ce1ba23c38a55304c19169 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Mon, 25 Jul 2022 18:19:57 -0700 Subject: [PATCH 2/4] Update changelog --- CHANGELOG.next.toml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 5df921107a..af6b88dea4 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -142,3 +142,15 @@ An example from the SDK is in [transcribe streaming](https://github.com/awslabs/ references = ["smithy-rs#1157"] meta = { "breaking" = true, "tada" = false, "bug" = false } author = "82marbag" + +[[smithy-rs]] +message = "Non-streaming struct members are now marked `#[doc(hidden)]` since they will be removed in the future" +references = ["smithy-rs#1573", "smithy-rs#1569"] +meta = { "breaking" = false, "tada" = false, "bug" = false } +author = "jdisanti" + +[[aws-sdk-rust]] +message = "Non-streaming struct members are now marked `#[doc(hidden)]` since they will be removed in the future" +references = ["smithy-rs#1573", "smithy-rs#1569"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "jdisanti" From 5adb3f6124cf48d3449fb4e7d70b18f80ff49137 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 26 Jul 2022 09:39:17 -0700 Subject: [PATCH 3/4] Add unit tests --- .../generators/StructureGeneratorTest.kt | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/generators/StructureGeneratorTest.kt b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/generators/StructureGeneratorTest.kt index a893e9bba4..e9f917657d 100644 --- a/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/generators/StructureGeneratorTest.kt +++ b/codegen/src/test/kotlin/software/amazon/smithy/rust/codegen/generators/StructureGeneratorTest.kt @@ -6,6 +6,8 @@ package software.amazon.smithy.rust.codegen.generators import io.kotest.matchers.string.shouldContainInOrder +import io.kotest.matchers.string.shouldNotContain +import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.rust.codegen.rustlang.Attribute @@ -293,4 +295,41 @@ class StructureGeneratorTest { } project.compileAndTest() } + + @Test + fun `non-streaming fields are doc-hidden`() { + val model = """ + namespace com.test + structure MyStruct { + foo: String, + bar: PrimitiveInteger, + baz: Integer, + ts: Timestamp, + byteValue: Byte, + } + """.asSmithyModel() + val struct = model.lookup("com.test#MyStruct") + + val provider = testSymbolProvider(model) + RustWriter.forModule("test").let { writer -> + StructureGenerator(model, provider, writer, struct).render() + assertEquals(6, writer.toString().split("#[doc(hidden)]").size, "there should be 5 doc-hiddens") + } + } + + @Test + fun `streaming fields are NOT doc-hidden`() { + val model = """ + namespace com.test + @streaming blob SomeStreamingThing + structure MyStruct { foo: SomeStreamingThing } + """.asSmithyModel() + val struct = model.lookup("com.test#MyStruct") + + val provider = testSymbolProvider(model) + RustWriter.forModule("test").let { writer -> + StructureGenerator(model, provider, writer, struct).render() + writer.toString().shouldNotContain("#[doc(hidden)]") + } + } } From 6945b1e84ddc3c7d28ba0ca8bb8b5678743e3274 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 26 Jul 2022 14:12:56 -0700 Subject: [PATCH 4/4] Fix Python server build --- .../codegen/server/python/smithy/PythonCodegenServerPlugin.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt index 5c89e2094b..1236ac2f04 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt @@ -75,7 +75,7 @@ class PythonCodegenServerPlugin : SmithyBuildPlugin { // `aws_smithy_http_server_python::types`. .let { PythonServerSymbolProvider(it, model) } // Add Rust attributes (like `#[derive(PartialEq)]`) to generated shapes - .let { BaseSymbolMetadataProvider(it, additionalAttributes = listOf()) } + .let { BaseSymbolMetadataProvider(it, model, additionalAttributes = listOf()) } // Streaming shapes need different derives (e.g. they cannot derive Eq) .let { StreamingShapeMetadataProvider(it, model) } // Rename shapes that clash with Rust reserved words & and other SDK specific features e.g. `send()` cannot