Unverified Commit 198c5009 authored by david-perez's avatar david-perez Committed by GitHub
Browse files

Disallow `@uniqueItems`-constrained list shapes that reach a map shape (#2375) 

* Disallow `@uniqueItems`-constrained list shapes that reach a map shape

The server SDK codegen generates Rust code that does not compile when a
`@uniqueItems`-constrained list shape reaches a map shape, essentially
because `std::collections::HashMap` does not implement
`std::hash::Hash`.

A ticket with the Smithy team was opened in awslabs/smithy#1567 to
disallow such models.

This commit makes it so that codegen aborts with an informative message
when such models are encountered, instead of going ahead and producing
code that does not compile.

This commit also slightly adjusts the error messages produced when
unsupported constraint traits are found.

* ./gradlew ktlintFormat
parent b9f8090c

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

+47 −4
Original line number Diff line number Diff line
@@ -10,7 +10,9 @@ import software.amazon.smithy.model.shapes.BlobShape 
import software.amazon.smithy.model.shapes.ByteShape

import software.amazon.smithy.model.shapes.EnumShape

import software.amazon.smithy.model.shapes.IntegerShape

import software.amazon.smithy.model.shapes.ListShape

import software.amazon.smithy.model.shapes.LongShape

import software.amazon.smithy.model.shapes.MapShape

import software.amazon.smithy.model.shapes.MemberShape

import software.amazon.smithy.model.shapes.OperationShape

import software.amazon.smithy.model.shapes.ServiceShape
@@ -36,13 +38,17 @@ private sealed class UnsupportedConstraintMessageKind { 
    private val constraintTraitsUberIssue = "https://github.com/awslabs/smithy-rs/issues/1401"



    fun intoLogMessage(ignoreUnsupportedConstraints: Boolean): LogMessage {

        fun buildMessage(intro: String, willSupport: Boolean, trackingIssue: String, canBeIgnored: Boolean = true): String {

        fun buildMessage(intro: String, willSupport: Boolean, trackingIssue: String? = null, canBeIgnored: Boolean = true): String {

            var msg = """

                    $intro

                    This is not supported in the smithy-rs server SDK."""

            if (willSupport) {

                msg += """

                    It will be supported in the future. See the tracking issue ($trackingIssue)."""

                    It will be supported in the future."""

            }

            if (trackingIssue != null) {

                msg += """

                    For more information, and to report if you're affected by this, please use the tracking issue: $trackingIssue."""

            }

            if (canBeIgnored) {

                msg += """
@@ -106,6 +112,19 @@ private sealed class UnsupportedConstraintMessageKind { 
                level,

                buildMessageShapeHasUnsupportedConstraintTrait(shape, uniqueItemsTrait, constraintTraitsUberIssue),

            )



            is UnsupportedMapShapeReachableFromUniqueItemsList -> LogMessage(

                Level.SEVERE,

                buildMessage(

                    """

                    The map shape `${mapShape.id}` is reachable from the list shape `${listShape.id}`, which has the 

                    `@uniqueItems` trait attached.

                    """.trimIndent().replace("\n", " "),

                    willSupport = false,

                    trackingIssue = "https://github.com/awslabs/smithy/issues/1567",

                    canBeIgnored = false,

                ),

            )

        }

    }

}
@@ -129,6 +148,12 @@ private data class UnsupportedRangeTraitOnShape(val shape: Shape, val rangeTrait 
private data class UnsupportedUniqueItemsTraitOnShape(val shape: Shape, val uniqueItemsTrait: UniqueItemsTrait) :

    UnsupportedConstraintMessageKind()



private data class UnsupportedMapShapeReachableFromUniqueItemsList(

    val listShape: ListShape,

    val uniqueItemsTrait: UniqueItemsTrait,

    val mapShape: MapShape,

) : UnsupportedConstraintMessageKind()



data class LogMessage(val level: Level, val message: String)

data class ValidationResult(val shouldAbort: Boolean, val messages: List<LogMessage>) :

    Throwable(message = messages.joinToString("\n") { it.message })
@@ -246,11 +271,29 @@ fun validateUnsupportedConstraints( 
        .map { (shape, rangeTrait) -> UnsupportedRangeTraitOnShape(shape, rangeTrait as RangeTrait) }

        .toSet()



    // 5. `@uniqueItems` cannot reach a map shape.

    // See https://github.com/awslabs/smithy/issues/1567.

    val mapShapeReachableFromUniqueItemsListShapeSet = walker

        .walkShapes(service)

        .asSequence()

        .filterMapShapesToTraits(setOf(UniqueItemsTrait::class.java))

        .flatMap { (listShape, uniqueItemsTrait) ->

            walker.walkShapes(listShape).filterIsInstance<MapShape>().map { mapShape ->

                UnsupportedMapShapeReachableFromUniqueItemsList(

                    listShape as ListShape,

                    uniqueItemsTrait as UniqueItemsTrait,

                    mapShape,

                )

            }

        }

        .toSet()



    val messages =

        unsupportedConstraintOnMemberShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedLengthTraitOnStreamingBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedConstraintShapeReachableViaAnEventStreamSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) }

            unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +

            mapShapeReachableFromUniqueItemsListShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) }



    return ValidationResult(shouldAbort = messages.any { it.level == Level.SEVERE }, messages)

}

codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt

+41 −1
Original line number Diff line number Diff line
@@ -27,7 +27,6 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        namespace test



        service TestService {

            version: "123",

            operations: [TestOperation]

        }
@@ -186,6 +185,47 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest { 
        }

    }



    private val mapShapeReachableFromUniqueItemsListShapeModel =

        """

        $baseModel

        

        structure TestInputOutput {

            uniqueItemsList: UniqueItemsList

        }

        

        @uniqueItems

        list UniqueItemsList {

            member: Map

        }

        

        map Map {

            key: String

            value: String

        }

        """.asSmithyModel()



    @Test

    fun `it should detect when a map shape is reachable from a uniqueItems list shape`() {

        val validationResult = validateModel(mapShapeReachableFromUniqueItemsListShapeModel)



        validationResult.messages shouldHaveSize 1

        validationResult.shouldAbort shouldBe true

        validationResult.messages[0].message shouldContain """

            The map shape `test#Map` is reachable from the list shape `test#UniqueItemsList`, which has the 

            `@uniqueItems` trait attached.

        """.trimIndent().replace("\n", " ")

    }



    @Test

    fun `it should abort when a map shape is reachable from a uniqueItems list shape, despite opting into ignoreUnsupportedConstraintTraits`() {

        val validationResult = validateModel(

            mapShapeReachableFromUniqueItemsListShapeModel,

            ServerCodegenConfig().copy(ignoreUnsupportedConstraints = true),

        )



        validationResult.shouldAbort shouldBe true

    }



    @Test

    fun `it should abort when constraint traits in event streams are used, despite opting into ignoreUnsupportedConstraintTraits`() {

        val validationResult = validateModel(