From 0dd6c82ec4ee5a754396167782aa9a65fe76801c Mon Sep 17 00:00:00 2001
From: david-perez <d@vidp.dev>
Date: Thu, 15 Dec 2022 16:10:32 +0100
Subject: [PATCH] Fix `@length`-constrained collection shapes whose members are
 not constrained (#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>
---
 CHANGELOG.next.toml                                  |  6 ++++++
 codegen-core/common-test-models/constraints.smithy   | 12 ++++++++----
 .../smithy/ConstraintViolationSymbolProvider.kt      |  4 +++-
 .../generators/UnconstrainedCollectionGenerator.kt   |  3 +--
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml
index 6ca0411734..638664274a 100644
--- 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
index 798b17b855..138c9e632f 100644
--- 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/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintViolationSymbolProvider.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintViolationSymbolProvider.kt
index 185fae1e4a..28b71ce2c5 100644
--- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstraintViolationSymbolProvider.kt
+++ b/codegen-server/src/main/kotlin/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/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt
index d11300a136..ef7b77c405 100644
--- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt
+++ b/codegen-server/src/main/kotlin/software/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(
                         """