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

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

Merged
merged 10 commits into from
Aug 3, 2022
14 changes: 13 additions & 1 deletion CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,16 @@ author = "crisidev"
message = "Change detailed logs in CredentialsProviderChain from info to debug"
references = ["smithy-rs#1578"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "lkts"
author = "lkts"

[[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"
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<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)]")
}
}
}