Add #[doc(hidden)] to struct fields to encourage accessor use (#1573)

smithy-lang · Aug 3, 2022 · a2ccdf7 · a2ccdf7
1 parent ed2a866
commit a2ccdf7
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 5 deletions.
diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml
@@ -225,3 +225,15 @@ message = "Support @deprecated trait for aggregate shapes"
 references = ["smithy-rs#1570"]
 meta = { "breaking" = true, "tada" = true, "bug" = false }
 author = "weihanglo"
+
+[[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" = false, "tada" = false, "bug" = false }
+author = "jdisanti"
diff --git a/...lin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt b/...lin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonCodegenServerPlugin.kt
@@ -79,7 +79,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

diff --git a/.../main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/RustCodegenServerPlugin.kt b/.../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
@@ -333,6 +333,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
@@ -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/...rc/main/kotlin/software/amazon/smithy/rust/codegen/smithy/StreamingTraitSymbolProvider.kt b/...rc/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
@@ -7,13 +7,15 @@ 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
 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<Attribute>,
 ) : 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 {

diff --git a/.../src/test/kotlin/software/amazon/smithy/rust/codegen/generators/StructureGeneratorTest.kt b/.../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
@@ -358,4 +360,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<StructureShape>("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<StructureShape>("com.test#MyStruct")
+
+        val provider = testSymbolProvider(model)
+        RustWriter.forModule("test").let { writer ->
+            StructureGenerator(model, provider, writer, struct).render()
+            writer.toString().shouldNotContain("#[doc(hidden)]")
+        }
+    }
 }