Fix bug where string builtIns were not supported

smithy-lang · Dec 30, 2022 · 2922165 · 2922165
1 parent 81028c6
commit 2922165
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 11 deletions.
diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml
@@ -87,3 +87,9 @@ message = "The constraint `@length` on non-streaming blob shapes is supported."
 references = ["smithy-rs#2131"]
 meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "server"}
 author = "82marbag"
+
+[[smithy-rs]]
+message = "Fix bug where string default values were not supported for endpoint parameters"
+references = ["smithy-rs#2150"]
+meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client"}
+author = "rcoh"
diff --git a/...n/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt b/...n/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/EndpointsDecorator.kt
@@ -28,6 +28,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.writable
 import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
 import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationCustomization
 import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationSection
+import software.amazon.smithy.rust.codegen.core.util.PANIC
 import software.amazon.smithy.rust.codegen.core.util.dq
 import software.amazon.smithy.rust.codegen.core.util.orNull
 
@@ -167,13 +168,24 @@ class EndpointsDecorator : ClientCodegenDecorator {
             }
         }
 
+        private fun Node.toWritable(): Writable {
+            val node = this
+            return writable {
+                when (node) {
+                    is StringNode -> rust("Some(${node.value.dq()}.to_string())")
+                    is BooleanNode -> rust("Some(${node.value})")
+                    else -> PANIC("unsupported default value: $node")
+                }
+            }
+        }
+
         private fun builderFields(params: Parameters, section: OperationSection.MutateInput) = writable {
             val memberParams = idx.getContextParams(operationShape)
             val builtInParams = params.toList().filter { it.isBuiltIn }
             // first load builtins and their defaults
             builtInParams.forEach { param ->
-                typesGenerator.builtInFor(param, section.config)?.also { defaultValue ->
-                    rust(".set_${param.name.rustName()}(#W)", defaultValue)
+                typesGenerator.builtInFor(param, section.config) ?: param.defaultValue.map { v ->
+                    v.toNode().toWritable()
                 }
             }
 
@@ -189,13 +201,7 @@ class EndpointsDecorator : ClientCodegenDecorator {
 
             idx.getStaticContextParams(operationShape).orNull()?.parameters?.forEach { (name, param) ->
                 val setterName = EndpointParamsGenerator.setterName(name)
-                val value = writable {
-                    when (val v = param.value) {
-                        is BooleanNode -> rust("Some(${v.value})")
-                        is StringNode -> rust("Some(${v.value.dq()}.to_string())")
-                        else -> TODO("Unexpected static value type: $v")
-                    }
-                }
+                val value = param.value.toWritable()
                 rust(".$setterName(#W)", value)
             }
 

diff --git a/...e/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsGenerator.kt b/...e/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsGenerator.kt
@@ -255,10 +255,11 @@ internal class EndpointParamsGenerator(private val parameters: Parameters) {
                 "ParamsError" to paramsError(),
             ) {
                 val params = writable {
+                    Attribute.Custom("allow(clippy::unnecessary_lazy_evaluations)").render(this)
                     rustBlockTemplate("#{Params}", "Params" to paramsStruct()) {
                         parameters.toList().forEach { parameter ->
                             rust("${parameter.memberName()}: self.${parameter.memberName()}")
-                            parameter.default.orNull()?.also { default -> rust(".or(Some(${value(default)}))") }
+                            parameter.default.orNull()?.also { default -> rust(".or_else(||Some(${value(default)}))") }
                             if (parameter.isRequired) {
                                 rustTemplate(
                                     ".ok_or_else(||#{Error}::missing(${parameter.memberName().dq()}))?",

diff --git a/...test/kotlin/software/amazon/smithy/rust/codegen/client/endpoint/EndpointsDecoratorTest.kt b/...test/kotlin/software/amazon/smithy/rust/codegen/client/endpoint/EndpointsDecoratorTest.kt
@@ -56,6 +56,8 @@ class EndpointsDecoratorTest {
             "parameters": {
                 "Bucket": { "required": false, "type": "String" },
                 "Region": { "required": false, "type": "String", "builtIn": "AWS::Region" },
+                "BuiltInWithDefault": { "required": true, "type": "String", "builtIn": "AWS::DefaultBuiltIn", "default": "some-default" },
+                "BoolBuiltInWithDefault": { "required": true, "type": "Boolean", "builtIn": "AWS::FooBar", "default": true },
                 "AStringParam": { "required": false, "type": "String" },
                 "ABoolParam": { "required": false, "type": "Boolean" }
             }
@@ -129,13 +131,15 @@ class EndpointsDecoratorTest {
                             use $moduleName::endpoint::{Params};
                             use aws_smithy_http::endpoint::Result;
                             let props = operation.properties();
+                            let endpoint_result = dbg!(props.get::<Result>().expect("endpoint result in the bag"));
                             let endpoint_params = props.get::<Params>().expect("endpoint params in the bag");
-                            let endpoint_result = props.get::<Result>().expect("endpoint result in the bag");
                             let endpoint = endpoint_result.as_ref().expect("endpoint resolved properly");
                             assert_eq!(
                                 endpoint_params,
                                 &Params::builder()
                                     .bucket("bucket-name".to_string())
+                                    .built_in_with_default("some-default")
+                                    .bool_built_in_with_default(true)
                                     .a_bool_param(false)
                                     .a_string_param("hello".to_string())
                                     .region("us-east-2".to_string())