From 2a9f251936424e689d44dd8957c24a4a630ec45b Mon Sep 17 00:00:00 2001 From: david-perez Date: Fri, 17 Feb 2023 18:24:25 +0100 Subject: [PATCH] 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 --- .../smithy/ValidateUnsupportedConstraints.kt | 51 +++++++++++++++++-- ...ateUnsupportedConstraintsAreNotUsedTest.kt | 42 ++++++++++++++- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt index 08c7e487b57..78d6078b05a 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt @@ -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 + $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) : 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().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) } diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt index 7e111f758cf..9a7ba2bf95f 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt @@ -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(