Skip to content

Commit

Permalink
Fix @length-constrained collection shapes whose members are not con…
Browse files Browse the repository at this point in the history
…strained

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.
  • Loading branch information
david-perez committed Dec 14, 2022
1 parent 2cc7c24 commit 093dd12
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
12 changes: 8 additions & 4 deletions codegen-core/common-test-models/constraints.smithy
Original file line number Diff line number Diff line change
Expand Up @@ -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
// }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -106,6 +104,7 @@ class UnconstrainedCollectionGenerator(
} else {
constrainedShapeSymbolProvider.toSymbol(innerShape)
}
val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape)

rustTemplate(
"""
Expand Down

0 comments on commit 093dd12

Please sign in to comment.