+2
−1
Loading
Fix recursive constraint violations with paths over list and map shapes (#2371)
* Fix recursive constraint violations with paths over list and map shapes
There is a widespread assumption throughout the generation of constraint
violations that does not hold true all the time, namely, that a
recursive constraint violation graph has the same requirements with
regards to boxing as the regular shape graph.
Some types corresponding to recursive shapes are boxed to introduce
indirection and thus not generate an infinitely recursive type. The
algorithm however does not superfluously introduce boxes when the cycle
goes through a list shape or a map shape. Why list shapes and map
shapes? List shapes and map shapes get rendered in Rust as `Vec<T>` and
`HashMap<K, V>`, respectively, they're the only Smithy shapes that
"organically" introduce indirection (via a pointer to the heap) in the
recursive path. For other recursive paths, we thus have to introduce the
indirection artificially ourselves using `Box`. This is done in the
`RecursiveShapeBoxer` model transform.
However, the constraint violation graph needs to box types in recursive
paths more often. Since we don't collect constraint violations
(yet, see #2040), the constraint violation graph never holds
`Vec<T>`s or `HashMap<K, V>`s, only simple types. Indeed, the following simple
recursive model:
```smithy
union Recursive {
list: List
}
@length(min: 69)
list List {
member: Recursive
}
```
has a cycle that goes through a list shape, so no shapes in it need
boxing in the regular shape graph. However, the constraint violation
graph is infinitely recursive if we don't introduce boxing somewhere:
```rust
pub mod model {
pub mod list {
pub enum ConstraintViolation {
Length(usize),
Member(
usize,
crate::model::recursive::ConstraintViolation,
),
}
}
pub mod recursive {
pub enum ConstraintViolation {
List(crate::model::list::ConstraintViolation),
}
}
}
```
This commit fixes things by making the `RecursiveShapeBoxer` model
transform configurable so that the "cycles through lists and maps
introduce indirection" assumption can be lifted. This allows a server
model transform, `RecursiveConstraintViolationBoxer`, to tag member
shapes along recursive paths with a new trait,
`ConstraintViolationRustBoxTrait`, that the constraint violation type
generation then utilizes to ensure that no infinitely recursive
constraint violation types get generated.
For example, for the above model, the generated Rust code would now look
like:
```rust
pub mod model {
pub mod list {
pub enum ConstraintViolation {
Length(usize),
Member(
usize,
std::boxed::Box(crate::model::recursive::ConstraintViolation),
),
}
}
pub mod recursive {
pub enum ConstraintViolation {
List(crate::model::list::ConstraintViolation),
}
}
}
```
Likewise, places where constraint violations are handled (like where
unconstrained types are converted to constrained types) have been
updated to account for the scenario where they now are or need to be
boxed.
Parametrized tests have been added to exhaustively test combinations of
models exercising recursive paths going through (sparse and non-sparse)
list and map shapes, as well as union and structure shapes
(`RecursiveConstraintViolationsTest`). These tests even assert that the
specific member shapes along the cycles are tagged as expected
(`RecursiveConstraintViolationBoxerTest`).
* Address comments