From 835b2b6db6c37b5b0faa90444b83564eb7d7c930 Mon Sep 17 00:00:00 2001
From: david-perez <d@vidp.dev>
Date: Wed, 23 Oct 2024 17:34:46 +0200
Subject: [PATCH] Don't add `serde` feature unnecessarily (#3891)

If the model is not using the `@smithy.rust#serde` trait, there's no
need for the code-generated crate to have a `serde` feature.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
---
 .../rust/codegen/serde/SerdeDecorator.kt      | 31 ++++++++-----------
 .../rust/codegen/serde/SerdeDecoratorTest.kt  | 30 ++++++++++++++++++
 2 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/codegen-serde/src/main/kotlin/software/amazon/smithy/rust/codegen/serde/SerdeDecorator.kt b/codegen-serde/src/main/kotlin/software/amazon/smithy/rust/codegen/serde/SerdeDecorator.kt
index e9aa1f34e..5e5cff78e 100644
--- a/codegen-serde/src/main/kotlin/software/amazon/smithy/rust/codegen/serde/SerdeDecorator.kt
+++ b/codegen-serde/src/main/kotlin/software/amazon/smithy/rust/codegen/serde/SerdeDecorator.kt
@@ -33,19 +33,7 @@ class ClientSerdeDecorator : ClientCodegenDecorator {
     override fun extras(
         codegenContext: ClientCodegenContext,
         rustCrate: RustCrate,
-    ) {
-        rustCrate.mergeFeature(SerdeFeature)
-        val generator = SerializeImplGenerator(codegenContext)
-        rustCrate.withModule(SerdeModule) {
-            serializationRoots(codegenContext).forEach {
-                generator.generateRootSerializerForShape(
-                    it,
-                )(this)
-            }
-            addDependency(SupportStructures.serializeRedacted().toSymbol())
-            addDependency(SupportStructures.serializeUnredacted().toSymbol())
-        }
-    }
+    ) = extrasCommon(codegenContext, rustCrate)
 }
 
 class ServerSerdeDecorator : ServerCodegenDecorator {
@@ -55,14 +43,21 @@ class ServerSerdeDecorator : ServerCodegenDecorator {
     override fun extras(
         codegenContext: ServerCodegenContext,
         rustCrate: RustCrate,
-    ) {
+    ) = extrasCommon(codegenContext, rustCrate)
+}
+
+// Just a common function to keep things DRY.
+private fun extrasCommon(
+    codegenContext: CodegenContext,
+    rustCrate: RustCrate,
+) {
+    val roots = serializationRoots(codegenContext)
+    if (roots.isNotEmpty()) {
         rustCrate.mergeFeature(SerdeFeature)
         val generator = SerializeImplGenerator(codegenContext)
         rustCrate.withModule(SerdeModule) {
-            serializationRoots(codegenContext).forEach {
-                generator.generateRootSerializerForShape(
-                    it,
-                )(this)
+            roots.forEach {
+                generator.generateRootSerializerForShape(it)(this)
             }
             addDependency(SupportStructures.serializeRedacted().toSymbol())
             addDependency(SupportStructures.serializeUnredacted().toSymbol())
diff --git a/codegen-serde/src/test/kotlin/software/amazon/smithy/rust/codegen/serde/SerdeDecoratorTest.kt b/codegen-serde/src/test/kotlin/software/amazon/smithy/rust/codegen/serde/SerdeDecoratorTest.kt
index 4a1dc3a52..47907d432 100644
--- a/codegen-serde/src/test/kotlin/software/amazon/smithy/rust/codegen/serde/SerdeDecoratorTest.kt
+++ b/codegen-serde/src/test/kotlin/software/amazon/smithy/rust/codegen/serde/SerdeDecoratorTest.kt
@@ -7,9 +7,13 @@ package software.amazon.smithy.rust.codegen.serde
 
 import org.junit.jupiter.api.Test
 import software.amazon.smithy.rust.codegen.client.testutil.clientIntegrationTest
+import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
+import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.cfg
+import software.amazon.smithy.rust.codegen.core.rustlang.Attribute.Companion.feature
 import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
 import software.amazon.smithy.rust.codegen.core.rustlang.CratesIo
 import software.amazon.smithy.rust.codegen.core.rustlang.RustType
+import software.amazon.smithy.rust.codegen.core.rustlang.rust
 import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
 import software.amazon.smithy.rust.codegen.core.testutil.IntegrationTestParams
 import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
@@ -231,6 +235,32 @@ class SerdeDecoratorTest {
         }
     }
 
+    @Test
+    fun `feature should not be added if trait is not used`() {
+        val model =
+            """
+            namespace com.example
+            use aws.protocols#awsJson1_0
+            
+            @awsJson1_0
+            service MyService {
+                operations: [MyOperation]
+            }
+            
+            operation MyOperation { }
+        """.asSmithyModel(smithyVersion = "2")
+
+        val params =
+            IntegrationTestParams(cargoCommand = "cargo test --all-features", service = "com.example#MyService")
+        serverIntegrationTest(model, params = params) { _, crate ->
+            crate.integrationTest("test_serde") {
+                unitTest("fails_if_serde_feature_exists", additionalAttributes = listOf(Attribute(cfg(feature("serde"))))) {
+                    rust("assert!(false);")
+                }
+            }
+        }
+    }
+
     @Test
     fun generateSerializersThatWorkServer() {
         serverIntegrationTest(simpleModel, params = params) { ctx, crate ->
-- 
GitLab