Remove attributes annotation of Builder struct

The `struct` created for a Builder should not have attributes because these structs only derive from `Debug`, `PartialEq`, `Clone` and `Default`. None of these support attributes. This becomes a problem if a plugin tries to add attributes to the `metadata` field to configure the generated code for the `struct`. In this case, the attributes will also be added to the `Builder` struct; which is wrong.
smithy-lang · Jun 6, 2024 · 2b2a04b · 2b2a04b
1 parent 0cbeef3
commit 2b2a04b
Show file tree

Hide file tree

Showing 5 changed files with 149 additions and 3 deletions.
diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt
@@ -560,6 +560,7 @@ class Attribute(val inner: Writable, val isDeriveHelper: Boolean = false) {
         val AllowUnusedMut = Attribute(allow("unused_mut"))
         val AllowUnusedVariables = Attribute(allow("unused_variables"))
         val CfgTest = Attribute(cfg("test"))
+        val DenyDeprecated = Attribute(deny("deprecated"))
         val DenyMissingDocs = Attribute(deny("missing_docs"))
         val DocHidden = Attribute(doc("hidden"))
         val DocInline = Attribute(doc("inline"))

diff --git a/...ain/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt b/...ain/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt
@@ -189,6 +189,11 @@ class BuilderGenerator(
         metadata.derives.filter {
             it == RuntimeType.Debug || it == RuntimeType.PartialEq || it == RuntimeType.Clone
         } + RuntimeType.Default
+    // Filter out attributes
+    private val builderAttributes =
+        metadata.additionalAttributes.filter {
+            it == Attribute.NonExhaustive
+        }
     private val builderName = symbolProvider.symbolForBuilder(shape).name
 
     fun render(writer: RustWriter) {
@@ -306,8 +311,8 @@ class BuilderGenerator(
 
     private fun renderBuilder(writer: RustWriter) {
         writer.docs("A builder for #D.", structureSymbol)
-        metadata.additionalAttributes.render(writer)
         Attribute(derive(builderDerives)).render(writer)
+        this.builderAttributes.render(writer)
         writer.rustBlock("pub struct $builderName") {
             for (member in members) {
                 val memberName = symbolProvider.toMemberName(member)

diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt
@@ -538,11 +538,14 @@ fun RustCrate.integrationTest(
     writable: Writable,
 ) = this.withFile("tests/$name.rs", writable)
 
-fun TestWriterDelegator.unitTest(test: Writable): TestWriterDelegator {
+fun TestWriterDelegator.unitTest(
+    additionalAttributes: List<Attribute> = emptyList(),
+    test: Writable
+): TestWriterDelegator {
     lib {
         val name = safeName("test")
         withInlineModule(RustModule.inlineTests(name), TestModuleDocProvider) {
-            unitTest(name) {
+            unitTest(name, additionalAttributes = additionalAttributes) {
                 test(this)
             }
         }

diff --git a/...kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt b/...kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt
@@ -11,6 +11,7 @@ import software.amazon.smithy.model.Model
 import software.amazon.smithy.model.knowledge.NullableIndex
 import software.amazon.smithy.model.shapes.Shape
 import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
 import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.AllowDeprecated
 import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
 import software.amazon.smithy.rust.codegen.core.rustlang.implBlock
@@ -19,6 +20,8 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
 import software.amazon.smithy.rust.codegen.core.smithy.Default
 import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
 import software.amazon.smithy.rust.codegen.core.smithy.WrappingSymbolProvider
+import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata
+import software.amazon.smithy.rust.codegen.core.smithy.meta
 import software.amazon.smithy.rust.codegen.core.smithy.setDefault
 import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace
 import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
@@ -240,4 +243,58 @@ internal class BuilderGeneratorTest {
         }
         project.compileAndTest()
     }
+
+
+    @Test
+    fun `builder doesn't inherit attributes from struct`() {
+        /**
+         * This test checks if the generated Builder doesn't inherit the macro attributes added to the main struct.
+         *
+         * The strategy is to:
+         * 1) mark the `Inner` struct with `#[deprecated]`
+         * 2) deny use of deprecated in the test
+         * 3) allow use of deprecated by the Builder
+         * 4) Ensure that the builder can be instantiated
+         */
+        class SymbolProviderWithExtraAnnotation(val base: RustSymbolProvider): RustSymbolProvider by base {
+            override fun toSymbol(shape: Shape): Symbol {
+                val baseSymbol = base.toSymbol(shape)
+                val name = baseSymbol.name
+                if (name == "Inner") {
+                    var metadata = baseSymbol.expectRustMetadata()
+                    val attribute = Attribute.Deprecated
+                    metadata = metadata.copy(additionalAttributes = metadata.additionalAttributes + listOf(attribute))
+                    return baseSymbol.toBuilder().meta(metadata).build()
+                } else {
+                    return baseSymbol
+                }
+            }
+        }
+
+        val provider = SymbolProviderWithExtraAnnotation(testSymbolProvider(model))
+        val project = TestWorkspace.testProject(provider)
+        project.moduleFor(inner) {
+            rust("##![allow(deprecated)]")
+            generator(model, provider, this, inner).render()
+            implBlock(provider.toSymbol(inner)) {
+                BuilderGenerator.renderConvenienceMethod(this, provider, inner)
+            }
+            unitTest("test", additionalAttributes = listOf(Attribute.DenyDeprecated), block = {
+                rust(
+                    // Notice that the builder is instantiated directly, and not through the Inner::builder() method.
+                    // This is because Inner is marked with `deprecated`, so any usage of `Inner` inside the test will
+                    // fail the compilation.
+                    //
+                    // This piece of code would fail though if the Builder inherits the attributes from Inner.
+                    """
+                    let _ = test_inner::Builder::default();
+                    """,
+                )
+            })
+        }
+        project.withModule(provider.moduleForBuilder(inner)) {
+            BuilderGenerator(model, provider, inner, emptyList()).render(this)
+        }
+        project.compileAndTest()
+    }
 }
diff --git a/...oftware/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt b/...oftware/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt
@@ -6,10 +6,17 @@
 package software.amazon.smithy.rust.codegen.server.smithy.generators
 
 import org.junit.jupiter.api.Test
+import software.amazon.smithy.codegen.core.Symbol
+import software.amazon.smithy.model.shapes.Shape
 import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
 import software.amazon.smithy.rust.codegen.core.rustlang.implBlock
 import software.amazon.smithy.rust.codegen.core.rustlang.rust
+import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider
+import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata
+import software.amazon.smithy.rust.codegen.core.smithy.generators.BuilderGenerator
 import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureGenerator
+import software.amazon.smithy.rust.codegen.core.smithy.meta
 import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace
 import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
 import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest
@@ -79,4 +86,77 @@ class ServerBuilderGeneratorTest {
         }
         project.compileAndTest()
     }
+
+    @Test
+    fun `builder doesn't inherit attributes from struct`() {
+        /**
+         * This test checks if the generated Builder doesn't inherit the macro attributes added to the main struct.
+         *
+         * The strategy is to:
+         * 1) mark the `Inner` struct with `#[deprecated]`
+         * 2) deny use of deprecated in the test
+         * 3) allow use of deprecated by the Builder
+         * 4) Ensure that the builder can be instantiated
+         */
+        val model =
+            """
+            namespace test
+
+            structure Inner {}
+            """.asSmithyModel()
+
+        class SymbolProviderWithExtraAnnotation(val base: RustSymbolProvider): RustSymbolProvider by base {
+            override fun toSymbol(shape: Shape): Symbol {
+                val baseSymbol = base.toSymbol(shape)
+                val name = baseSymbol.name
+                if (name == "Inner") {
+                    var metadata = baseSymbol.expectRustMetadata()
+                    val attribute = Attribute.Deprecated
+                    metadata = metadata.copy(additionalAttributes = metadata.additionalAttributes + listOf(attribute))
+                    return baseSymbol.toBuilder().meta(metadata).build()
+                } else {
+                    return baseSymbol
+                }
+            }
+        }
+
+        val codegenContext = serverTestCodegenContext(model)
+        val provider = SymbolProviderWithExtraAnnotation(codegenContext.symbolProvider)
+        val project = TestWorkspace.testProject(provider)
+        project.withModule(ServerRustModule.Model) {
+            val shape = model.lookup<StructureShape>("test#Inner")
+            val writer = this
+
+            rust("##![allow(deprecated)]")
+            StructureGenerator(model, provider, writer, shape, emptyList(), codegenContext.structSettings()).render()
+            val builderGenerator =
+                ServerBuilderGenerator(
+                    codegenContext,
+                    shape,
+                    SmithyValidationExceptionConversionGenerator(codegenContext),
+                    ServerRestJsonProtocol(codegenContext),
+                )
+
+            builderGenerator.render(project, writer)
+
+            writer.implBlock(provider.toSymbol(shape)) {
+                builderGenerator.renderConvenienceMethod(this)
+            }
+
+            project.renderInlineMemoryModules()
+        }
+        project.unitTest(additionalAttributes = listOf(Attribute.DenyDeprecated), test = {
+            rust(
+                // Notice that the builder is instantiated directly, and not through the Inner::builder() method.
+                // This is because Inner is marked with `deprecated`, so any usage of `Inner` inside the test will
+                // fail the compilation.
+                //
+                // This piece of code would fail though if the Builder inherits the attributes from Inner.
+                """
+                let _ = crate::model::inner::Builder::default();
+                """,
+            )
+        })
+        project.compileAndTest()
+    }
 }