Fix @length-constrained collection shapes whose members are not con…

…strained (#2103) * Fix `@length`-constrained collection shapes whose members are not constrained The generated code these should have emitted was fixed in #2085 (it's bug number 2), but code generation is still crashing because the call to calculate the inner constraint violation symbol is performed _before_ checking that the collection's member can reach a constrained shape. The test that #2085 added in `constraints.smithy`: ```smithy @Length(max: 69) list LengthList { member: ConB } ``` was not exercising what it should have, since `ConB`, is its name hints at, is a constrained structure shape. * Add changelog entry Co-authored-by: Julian Antonielli <julianantonielli@gmail.com>
smithy-lang · Dec 15, 2022 · 0dd6c82 · 0dd6c82
1 parent 66fd6c9
commit 0dd6c82
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 7 deletions.
diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml
@@ -11,6 +11,12 @@
 # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
 # author = "rcoh"
 
+[[smithy-rs]]
+message = "In 0.52, `@length`-constrained collection shapes whose members are not constrained made the server code generator crash. This has been fixed."
+references = ["smithy-rs#2103"]
+meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server"}
+author = "david-perez"
+
 [[smithy-rs]]
 message = "The Rust client codegen plugin is now called `rust-client-codegen` instead of `rust-codegen`. Be sure to update your `smithy-build.json` files to refer to the correct plugin name."
 references = ["smithy-rs#2099"]

diff --git a/codegen-core/common-test-models/constraints.smithy b/codegen-core/common-test-models/constraints.smithy
@@ -837,21 +837,25 @@ list RecursiveList {
 }
 
 list ConBList {
-    member: LengthList
+    member: ConBListInner
+}
+
+list ConBListInner {
+    member: ConB
 }
 
 @length(max: 69)
 list LengthList {
-    member: ConB
+    member: String
 }
 
 // TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is
 //  just a `list` shape with `uniqueItems`, which hasn't been implemented yet.
 // set ConBSet {
-//     member: NestedSet
+//     member: ConBSetInner
 // }
 //
-// set NestedSet {
+// set ConBSetInner {
 //     member: String
 // }
 

diff --git a/...in/software/amazon/smithy/rust/codegen/server/smithy/ConstraintViolationSymbolProvider.kt b/...in/software/amazon/smithy/rust/codegen/server/smithy/ConstraintViolationSymbolProvider.kt
@@ -106,7 +106,9 @@ class ConstraintViolationSymbolProvider(
     }
 
     override fun toSymbol(shape: Shape): Symbol {
-        check(shape.canReachConstrainedShape(model, base))
+        check(shape.canReachConstrainedShape(model, base)) {
+            "`ConstraintViolationSymbolProvider` was called on shape that does not reach a constrained shape: $shape"
+        }
 
         return when (shape) {
             is MapShape, is CollectionShape, is UnionShape -> {

diff --git a/...e/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt b/...e/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt
@@ -89,8 +89,6 @@ class UnconstrainedCollectionGenerator(
     }
 
     private fun renderTryFromUnconstrainedForConstrained(writer: RustWriter) {
-        val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape)
-
         writer.rustBlock("impl std::convert::TryFrom<$name> for #{T}", constrainedSymbol) {
             rust("type Error = #T;", constraintViolationSymbol)
 
@@ -106,6 +104,7 @@ class UnconstrainedCollectionGenerator(
                     } else {
                         constrainedShapeSymbolProvider.toSymbol(innerShape)
                     }
+                    val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape)
 
                     rustTemplate(
                         """