Fix directly constrained List shape with indirectly constrained aggregate type member shape

[drganjoo]

This PR fixes a bug in the code generation for a directly constrained List shape that has an indirectly constrained aggregate type as a member shape.

For example, in the following model:

@http(uri: "/sample", method: "POST")
operation SampleOp {
  input := {
      a: ItemList
  }
  errors: [ValidationException]
}
@length(min: 1 max: 100)
list ItemList {
  member: Item
}
map Item {
  key: ItemName
  value: ItemDescription
}
@length(min: 0 max: 65535)
string ItemName
string ItemDescription

A TryFrom implementation is generated that converts from ItemListUnconstrained to ItemList by converting each member of the inner list of ItemUnconstrained to ItemConstrained and then converting it to Vec<Vec<ItemName>>:

    impl std::convert::TryFrom<ItemListUnconstrained> for crate::model::ItemList {
        type Error = crate::model::item_list::ConstraintViolation;
        fn try_from(value: ItemListUnconstrained) -> Result<Self, Self::Error> {
            let res: Result<
                ::std::vec::Vec<
                    ::std::collections::HashMap<crate::model::ItemName, ::std::string::String>,
                >,
                (usize, crate::model::item::ConstraintViolation),
            > = value
                .0
                .into_iter()
                .enumerate()
                .map(|(idx, inner)| {
                    inner.try_into()
                        .map(|ic: crate::constrained::item_constrained::ItemConstrained| ic.into())
                        .map_err(|inner_violation| (idx, inner_violation))
                })
                .collect();
            let inner =
                res.map_err(|(idx, inner_violation)| Self::Error::Member(idx, inner_violation))?;
            Self::try_from(inner)
        }
    }

Partially fixes issue: #3885 for publicly constrained shapes only.

[github-actions]

A new generated diff is ready to view.

• AWS SDK (ignoring whitespace)
• Client Test (ignoring whitespace)
• Server Test (ignoring whitespace)
• Server Test Python (ignoring whitespace)
• No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

[github-actions]

A new generated diff is ready to view.

• AWS SDK (ignoring whitespace)
• Client Test (ignoring whitespace)
• Server Test (ignoring whitespace)
• Server Test Python (ignoring whitespace)
• No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

[github-actions]

A new generated diff is ready to view.

• AWS SDK (ignoring whitespace)
• Client Test (ignoring whitespace)
• Server Test (ignoring whitespace)
• Server Test Python (ignoring whitespace)
• No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

[github-actions]

A new generated diff is ready to view.

• No codegen difference in the AWS SDK
• No codegen difference in the Client Test
• Server Test (ignoring whitespace)
• Server Test Python (ignoring whitespace)
• No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

[david-perez]

Instead of putting the if statement within the writable, can we write things in two phases and add a statement that does another pass, converting the value? Similar to how we tackle this case in UnconstrainedMapGenerator, where we have an if (constrainedValueTypeIsNotFinalType) { and explain within it this corner case and do a pass calling .into().

[drganjoo]

I benchmarked performing this in separate phases, as done in UnconstrainedMapGenerator, and found it to be about 5ns slower on a vector of 4 string values. Therefore, I decided to keep this version instead of adding a second pass. I'll refactor it slightly and move the if statement out. If that doesn’t improve readability, I’ll opt for the double pass.

[david-perez]

I'd rather move all these to constraints.smithy, since, as noted here, simply compiling the crate is sufficient. Each of these is generating and compiling a brand new crate, which adds time. Instead, constraints.smithy constitues a "kitchen-sink"-style test for all constraints-related things.

[drganjoo]

Completely agreed. I should have added a TODO here to clarify why these tests are included. There’s an earlier commit in this PR that adds them to constraints.smithy, but our pubConstrainedTypes = false implementation is currently incomplete and has several edge cases, causing the PR to fail. I’ve created a new issue (#3895) to document all related edge cases. I initially tried to address these within this PR but encountered cases requiring more refactoring than is feasible here.

[github-actions]

A new generated diff is ready to view.

• No codegen difference in the AWS SDK
• No codegen difference in the Client Test
• Server Test (ignoring whitespace)
• Server Test Python (ignoring whitespace)
• No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

[github-actions]

A new generated diff is ready to view.

• No codegen difference in the AWS SDK
• No codegen difference in the Client Test
• Server Test (ignoring whitespace)
• Server Test Python (ignoring whitespace)
• No codegen difference in the Server Test Typescript

A new doc preview is ready to view.