Fix 3 bugs in @length-constrained collection and map shapes

codegen-core/common-test-models/constraints.smithy

@@ -467,13 +467,14 @@ structure ConA {
     fixedValueByte: FixedValueByte,
 
     conBList: ConBList,
-    conBList2: ConBList2,
+    lengthList: LengthList,
 
     // 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.
     // conBSet: ConBSet,
 
     conBMap: ConBMap,
+    lengthMap: LengthMap,
 
     mapOfMapOfListOfListOfConB: MapOfMapOfListOfListOfConB,
 
@@ -837,14 +838,11 @@ list RecursiveList {
 }
 
 list ConBList {
-    member: NestedList
+    member: LengthList
 }
 
-list ConBList2 {
-    member: ConB
-}
-
-list NestedList {
+@length(max: 69)
+list LengthList {
     member: ConB
 }
 
@@ -874,6 +872,12 @@ map ConBMap {
     value: LengthString
 }
 
+@length(min: 1, max: 69)
+map LengthMap {
+    key: String,
+    value: String
+}
+
 @error("client")
 structure ErrorWithLengthStringMessage {
     // TODO Doesn't work yet because constrained string types don't implement

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedCollectionGenerator.kt

@@ -7,6 +7,8 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators
 
 import software.amazon.smithy.codegen.core.Symbol
 import software.amazon.smithy.model.shapes.CollectionShape
+import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.model.shapes.UnionShape
 import software.amazon.smithy.model.traits.LengthTrait
 import software.amazon.smithy.model.traits.Trait
 import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
@@ -23,6 +25,7 @@ import software.amazon.smithy.rust.codegen.core.util.PANIC
 import software.amazon.smithy.rust.codegen.core.util.orNull
 import software.amazon.smithy.rust.codegen.server.smithy.PubCrateConstraintViolationSymbolProvider
 import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext
+import software.amazon.smithy.rust.codegen.server.smithy.canReachConstrainedShape
 import software.amazon.smithy.rust.codegen.server.smithy.supportedCollectionConstraintTraits
 import software.amazon.smithy.rust.codegen.server.smithy.validationErrorMessage
 
@@ -132,7 +135,12 @@ class ConstrainedCollectionGenerator(
             "ValidationFunctions" to constraintsInfo.map { it.validationFunctionDefinition(constraintViolation, inner) }.join("\n"),
         )
 
-        if (!publicConstrainedTypes && isValueConstrained(shape, model, symbolProvider)) {
+        val innerShape = model.expectShape(shape.member.target)
+        if (!publicConstrainedTypes &&
+            innerShape.canReachConstrainedShape(model, symbolProvider) &&
+            innerShape !is StructureShape &&
+            innerShape !is UnionShape
+        ) {
             writer.rustTemplate(
                 """
                 impl #{From}<$name> for #{FullyUnconstrainedSymbol} {

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedMapGenerator.kt

@@ -7,6 +7,8 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators
 
 import software.amazon.smithy.codegen.core.Symbol
 import software.amazon.smithy.model.shapes.MapShape
+import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.model.shapes.UnionShape
 import software.amazon.smithy.model.traits.LengthTrait
 import software.amazon.smithy.rust.codegen.core.rustlang.Attribute
 import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata
@@ -115,7 +117,12 @@ class ConstrainedMapGenerator(
             *codegenScope,
         )
 
-        if (!publicConstrainedTypes && isValueConstrained(shape, model, symbolProvider)) {
+        val valueShape = model.expectShape(shape.value.target)
+        if (!publicConstrainedTypes &&
+            isValueConstrained(valueShape, model, symbolProvider) &&
+            valueShape !is StructureShape &&
+            valueShape !is UnionShape
+        ) {
             writer.rustTemplate(
                 """
                 impl #{From}<$name> for #{FullyUnconstrainedSymbol} {

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedCollectionGenerator.kt

@@ -6,8 +6,12 @@
 package software.amazon.smithy.rust.codegen.server.smithy.generators
 
 import software.amazon.smithy.model.shapes.CollectionShape
+import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.model.shapes.UnionShape
 import software.amazon.smithy.rust.codegen.core.rustlang.RustType
 import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
+import software.amazon.smithy.rust.codegen.core.rustlang.rust
+import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock
 import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
 import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
 import software.amazon.smithy.rust.codegen.core.smithy.makeMaybeConstrained
@@ -38,6 +42,8 @@ class UnconstrainedCollectionGenerator(
     private val symbolProvider = codegenContext.symbolProvider
     private val unconstrainedShapeSymbolProvider = codegenContext.unconstrainedShapeSymbolProvider
     private val pubCrateConstrainedShapeSymbolProvider = codegenContext.pubCrateConstrainedShapeSymbolProvider
+    private val symbol = unconstrainedShapeSymbolProvider.toSymbol(shape)
+    private val name = symbol.name
     private val publicConstrainedTypes = codegenContext.settings.codegenConfig.publicConstrainedTypes
     private val constraintViolationSymbolProvider =
         with(codegenContext.constraintViolationSymbolProvider) {
@@ -47,22 +53,20 @@ class UnconstrainedCollectionGenerator(
                 PubCrateConstraintViolationSymbolProvider(this)
             }
         }
+    private val constraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(shape)
     private val constrainedShapeSymbolProvider = codegenContext.constrainedShapeSymbolProvider
     private val constrainedSymbol = if (shape.isDirectlyConstrained(symbolProvider)) {
         constrainedShapeSymbolProvider.toSymbol(shape)
     } else {
         pubCrateConstrainedShapeSymbolProvider.toSymbol(shape)
     }
+    private val innerShape = model.expectShape(shape.member.target)
 
     fun render() {
         check(shape.canReachConstrainedShape(model, symbolProvider))
 
-        val symbol = unconstrainedShapeSymbolProvider.toSymbol(shape)
-        val name = symbol.name
         val innerShape = model.expectShape(shape.member.target)
         val innerUnconstrainedSymbol = unconstrainedShapeSymbolProvider.toSymbol(innerShape)
-        val constraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(shape)
-        val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape)
 
         unconstrainedModuleWriter.withInlineModule(symbol.module()) {
             rustTemplate(
@@ -75,29 +79,58 @@ class UnconstrainedCollectionGenerator(
                         Self::Unconstrained(value)
                     }
                 }
+                """,
+                "InnerUnconstrainedSymbol" to innerUnconstrainedSymbol,
+                "MaybeConstrained" to constrainedSymbol.makeMaybeConstrained(),
+            )
+
+            renderTryFromUnconstrainedForConstrained(this)
+        }
+    }
+
+    private fun renderTryFromUnconstrainedForConstrained(writer: RustWriter) {
+        val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape)
 
-                impl #{TryFrom}<$name> for #{ConstrainedSymbol} {
-                    type Error = #{ConstraintViolationSymbol};
+        writer.rustBlock("impl std::convert::TryFrom<$name> for #{T}", constrainedSymbol) {
+            rust("type Error = #T;", constraintViolationSymbol)
 
-                    fn try_from(value: $name) -> Result<Self, Self::Error> {
-                        let res: Result<_, (usize, #{InnerConstraintViolationSymbol})> = value
+            rustBlock("fn try_from(value: $name) -> Result<Self, Self::Error>") {
+                if (innerShape.canReachConstrainedShape(model, symbolProvider)) {
+                    val resolvesToNonPublicConstrainedValueType =
+                        innerShape.canReachConstrainedShape(model, symbolProvider) &&
+                            !innerShape.isDirectlyConstrained(symbolProvider) &&
+                            innerShape !is StructureShape &&
+                            innerShape !is UnionShape
+                    val innerConstrainedSymbol = if (resolvesToNonPublicConstrainedValueType) {
+                        pubCrateConstrainedShapeSymbolProvider.toSymbol(innerShape)
+                    } else {
+                        constrainedShapeSymbolProvider.toSymbol(innerShape)
+                    }
+
+                    rustTemplate(
+                        """
+                        let res: Result<std::vec::Vec<#{InnerConstrainedSymbol}>, (usize, #{InnerConstraintViolationSymbol})> = value
                             .0
                             .into_iter()
                             .enumerate()
                             .map(|(idx, inner)| inner.try_into().map_err(|inner_violation| (idx, inner_violation)))
                             .collect();
-                        res.map(Self)
-                           .map_err(|(idx, inner_violation)| #{ConstraintViolationSymbol}::Member(idx, inner_violation))
-                    }
+                        let inner = res.map_err(|(idx, inner_violation)| Self::Error::Member(idx, inner_violation))?;
+                        """,
+                        "InnerConstrainedSymbol" to innerConstrainedSymbol,
+                        "InnerConstraintViolationSymbol" to innerConstraintViolationSymbol,
+                        "TryFrom" to RuntimeType.TryFrom,
+                    )
+                } else {
+                    rust("let inner = value.0;")
                 }
-                """,
-                "InnerUnconstrainedSymbol" to innerUnconstrainedSymbol,
-                "InnerConstraintViolationSymbol" to innerConstraintViolationSymbol,
-                "ConstrainedSymbol" to constrainedSymbol,
-                "ConstraintViolationSymbol" to constraintViolationSymbol,
-                "MaybeConstrained" to constrainedSymbol.makeMaybeConstrained(),
-                "TryFrom" to RuntimeType.TryFrom,
-            )
+
+                if (shape.isDirectlyConstrained(symbolProvider)) {
+                    rust("Self::try_from(inner)")
+                } else {
+                    rust("Ok(Self(inner))")
+                }
+            }
         }
     }
 }

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/UnconstrainedMapGenerator.kt

@@ -7,6 +7,8 @@ package software.amazon.smithy.rust.codegen.server.smithy.generators
 
 import software.amazon.smithy.model.shapes.MapShape
 import software.amazon.smithy.model.shapes.StringShape
+import software.amazon.smithy.model.shapes.StructureShape
+import software.amazon.smithy.model.shapes.UnionShape
 import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
 import software.amazon.smithy.rust.codegen.core.rustlang.join
 import software.amazon.smithy.rust.codegen.core.rustlang.rust
@@ -95,11 +97,12 @@ class UnconstrainedMapGenerator(
 
             rustBlock("fn try_from(value: $name) -> Result<Self, Self::Error>") {
                 if (isKeyConstrained(keyShape, symbolProvider) || isValueConstrained(valueShape, model, symbolProvider)) {
-                    val resolveToNonPublicConstrainedValueType =
+                    val resolvesToNonPublicConstrainedValueType =
                         isValueConstrained(valueShape, model, symbolProvider) &&
                             !valueShape.isDirectlyConstrained(symbolProvider) &&
-                            !valueShape.isStructureShape
-                    val constrainedValueSymbol = if (resolveToNonPublicConstrainedValueType) {
+                            valueShape !is StructureShape &&
+                            valueShape !is UnionShape
+                    val constrainedValueSymbol = if (resolvesToNonPublicConstrainedValueType) {
                         pubCrateConstrainedShapeSymbolProvider.toSymbol(valueShape)
                     } else {
                         constrainedShapeSymbolProvider.toSymbol(valueShape)
@@ -154,7 +157,7 @@ class UnconstrainedMapGenerator(
                     )
 
                     val constrainedValueTypeIsNotFinalType =
-                        resolveToNonPublicConstrainedValueType && shape.isDirectlyConstrained(symbolProvider)
+                        resolvesToNonPublicConstrainedValueType && shape.isDirectlyConstrained(symbolProvider)
                     if (constrainedValueTypeIsNotFinalType) {
                         // The map is constrained. Its value shape reaches a constrained shape, but the value shape itself
                         // is not directly constrained. The value shape must be an aggregate shape. But it is not a

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/protocol/ServerProtocolTestGenerator.kt

@@ -946,10 +946,6 @@ class ServerProtocolTestGenerator(
             // See https://github.com/awslabs/smithy-rs/issues/1401.
             FailingTest(RestJsonValidation, "RestJsonMalformedLengthBlob_case0", TestType.MalformedRequest),
             FailingTest(RestJsonValidation, "RestJsonMalformedLengthBlob_case1", TestType.MalformedRequest),
-            FailingTest(RestJsonValidation, "RestJsonMalformedLengthList_case0", TestType.MalformedRequest),
-            FailingTest(RestJsonValidation, "RestJsonMalformedLengthList_case1", TestType.MalformedRequest),
-            FailingTest(RestJsonValidation, "RestJsonMalformedLengthMapValue_case0", TestType.MalformedRequest),
-            FailingTest(RestJsonValidation, "RestJsonMalformedLengthMapValue_case1", TestType.MalformedRequest),
             FailingTest(RestJsonValidation, "RestJsonMalformedRangeFloat_case0", TestType.MalformedRequest),
             FailingTest(RestJsonValidation, "RestJsonMalformedRangeFloat_case1", TestType.MalformedRequest),
             FailingTest(RestJsonValidation, "RestJsonMalformedRangeMaxFloat", TestType.MalformedRequest),