Unverified Commit 0dd6c82e authored by david-perez's avatar david-perez Committed by GitHub
Browse files

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: default avatarJulian Antonielli <julianantonielli@gmail.com>
parent 66fd6c9f

CHANGELOG.next.toml

+6 −0
Original line number Original line Diff line number Diff line
@@ -11,6 +11,12 @@ 
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}

# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}

# author = "rcoh"

# 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]]

[[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."

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"]

references = ["smithy-rs#2099"]

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

+8 −4
Original line number Original line Diff line number Diff line
@@ -837,21 +837,25 @@ list RecursiveList { 
}

}





list ConBList {

list ConBList {

    member: LengthList

    member: ConBListInner

}



list ConBListInner {

    member: ConB

}

}





@length(max: 69)

@length(max: 69)

list LengthList {

list LengthList {

    member: ConB

    member: String

}

}





// TODO(https://github.com/awslabs/smithy-rs/issues/1401): a `set` shape is

// 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.

//  just a `list` shape with `uniqueItems`, which hasn't been implemented yet.

// set ConBSet {

// set ConBSet {

//     member: NestedSet

//     member: ConBSetInner

// }

// }

//

//

// set NestedSet {

// set ConBSetInner {

//     member: String

//     member: String

// }

// }

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

+3 −1
Original line number Original line Diff line number Diff line
@@ -106,7 +106,9 @@ class ConstraintViolationSymbolProvider( 
    }

    }





    override fun toSymbol(shape: Shape): Symbol {

    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) {

        return when (shape) {

            is MapShape, is CollectionShape, is UnionShape -> {

            is MapShape, is CollectionShape, is UnionShape -> {

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

+1 −2
Original line number Original line Diff line number Diff line
@@ -89,8 +89,6 @@ class UnconstrainedCollectionGenerator( 
    }

    }





    private fun renderTryFromUnconstrainedForConstrained(writer: RustWriter) {

    private fun renderTryFromUnconstrainedForConstrained(writer: RustWriter) {

        val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape)



        writer.rustBlock("impl std::convert::TryFrom<$name> for #{T}", constrainedSymbol) {

        writer.rustBlock("impl std::convert::TryFrom<$name> for #{T}", constrainedSymbol) {

            rust("type Error = #T;", constraintViolationSymbol)

            rust("type Error = #T;", constraintViolationSymbol)
@@ -106,6 +104,7 @@ class UnconstrainedCollectionGenerator( 
                    } else {

                    } else {

                        constrainedShapeSymbolProvider.toSymbol(innerShape)

                        constrainedShapeSymbolProvider.toSymbol(innerShape)

                    }

                    }

                    val innerConstraintViolationSymbol = constraintViolationSymbolProvider.toSymbol(innerShape)





                    rustTemplate(

                    rustTemplate(

                        """

                        """