Detect if the uniqueItems trait is used (#2001)

And act like in the rest of the unsupported constraint traits cases. I missed this in the implementation of #1342.
smithy-lang · Nov 18, 2022 · 5eeb5d7 · 5eeb5d7
1 parent 0a610ec
commit 5eeb5d7
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 20 deletions.
diff --git a/codegen-core/common-test-models/constraints.smithy b/codegen-core/common-test-models/constraints.smithy
@@ -154,8 +154,10 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
     // @httpHeader("X-Length-MediaType")
     // lengthStringHeaderWithMediaType: MediaTypeLengthString,
 
-    @httpHeader("X-Length-Set")
-    lengthStringSetHeader: SetOfLengthString,
+    // 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.
+    // @httpHeader("X-Length-Set")
+    // lengthStringSetHeader: SetOfLengthString,
 
     @httpHeader("X-Length-List")
     lengthStringListHeader: ListOfLengthString,
@@ -176,8 +178,10 @@ structure ConstrainedHttpBoundShapesOperationInputOutput {
     @httpQuery("lengthStringList")
     lengthStringListQuery: ListOfLengthString,
 
-    @httpQuery("lengthStringSet")
-    lengthStringSetQuery: SetOfLengthString,
+    // 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.
+    // @httpQuery("lengthStringSet")
+    // lengthStringSetQuery: SetOfLengthString,
 
     @httpQuery("enumStringList")
     enumStringListQuery: ListOfEnumString,
@@ -277,7 +281,9 @@ structure ConA {
     conBList: ConBList,
     conBList2: ConBList2,
 
-    conBSet: ConBSet,
+    // 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.
+    // conBSet: ConBSet,
 
     conBMap: ConBMap,
 
@@ -287,7 +293,9 @@ structure ConA {
     enumString: EnumString,
 
     listOfLengthString: ListOfLengthString,
-    setOfLengthString: SetOfLengthString,
+    // 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.
+    // setOfLengthString: SetOfLengthString,
     mapOfLengthString: MapOfLengthString,
 
     nonStreamingBlob: NonStreamingBlob
@@ -315,7 +323,10 @@ map MapOfListOfEnumString {
 
 map MapOfSetOfLengthString {
     key: LengthString,
-    value: SetOfLengthString,
+    // 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.
+    // value: SetOfLengthString,
+    value: ListOfLengthString
 }
 
 @length(min: 2, max: 8)
@@ -346,7 +357,9 @@ union ConstrainedUnion {
 
     constrainedStructure: ConB,
     conBList: ConBList,
-    conBSet: ConBSet,
+    // 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.
+    // conBSet: ConBSet,
     conBMap: ConBMap,
 }
 
@@ -420,13 +433,15 @@ list NestedList {
     member: ConB
 }
 
-set ConBSet {
-    member: NestedSet
-}
-
-set NestedSet {
-    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
+// }
+//
+// set NestedSet {
+//     member: String
+// }
 
 @length(min: 1, max: 69)
 map ConBMap {

diff --git a/codegen-core/common-test-models/misc.smithy b/codegen-core/common-test-models/misc.smithy
@@ -255,6 +255,7 @@ list HeaderList {
     member: String
 }
 
-set HeaderSet {
+@uniqueItems
+list HeaderSet {
     member: String
 }
diff --git a/codegen-server-test/build.gradle.kts b/codegen-server-test/build.gradle.kts
@@ -54,7 +54,12 @@ val allCodegenTests = "../codegen-core/common-test-models".let { commonModels ->
             "constraints",
             imports = listOf("$commonModels/constraints.smithy"),
         ),
-        CodegenTest("aws.protocoltests.restjson#RestJson", "rest_json"),
+        CodegenTest(
+            "aws.protocoltests.restjson#RestJson",
+            "rest_json",
+            // TODO(https://github.com/awslabs/smithy-rs/issues/1401) `@uniqueItems` is used.
+            extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,
+        ),
         CodegenTest(
             "aws.protocoltests.restjson#RestJsonExtras",
             "rest_json_extras",
@@ -65,8 +70,18 @@ val allCodegenTests = "../codegen-core/common-test-models".let { commonModels ->
             extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,
         ),
         CodegenTest("aws.protocoltests.json10#JsonRpc10", "json_rpc10"),
-        CodegenTest("aws.protocoltests.json#JsonProtocol", "json_rpc11"),
-        CodegenTest("aws.protocoltests.misc#MiscService", "misc", imports = listOf("$commonModels/misc.smithy")),
+        CodegenTest(
+            "aws.protocoltests.json#JsonProtocol",
+            "json_rpc11",
+            extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,
+        ),
+        CodegenTest(
+            "aws.protocoltests.misc#MiscService",
+            "misc",
+            imports = listOf("$commonModels/misc.smithy"),
+            // TODO(https://github.com/awslabs/smithy-rs/issues/1401) `@uniqueItems` is used.
+            extraConfig = """, "codegen": { "ignoreUnsupportedConstraints": true } """,
+        ),
         CodegenTest(
             "com.amazonaws.ebs#Ebs", "ebs",
             imports = listOf("$commonModels/ebs.json"),

diff --git a/...otlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt b/...otlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt
@@ -94,6 +94,10 @@ private sealed class UnsupportedConstraintMessageKind {
                 level,
                 buildMessageShapeHasUnsupportedConstraintTrait(shape, rangeTrait, constraintTraitsUberIssue),
             )
+            is UnsupportedUniqueItemsTraitOnShape -> LogMessage(
+                level,
+                buildMessageShapeHasUnsupportedConstraintTrait(shape, uniqueItemsTrait, constraintTraitsUberIssue),
+            )
         }
     }
 }
@@ -104,6 +108,7 @@ private data class UnsupportedLengthTraitOnStreamingBlobShape(val shape: BlobSha
 private data class UnsupportedLengthTraitOnCollectionOrOnBlobShape(val shape: Shape, val lengthTrait: LengthTrait) : UnsupportedConstraintMessageKind()
 private data class UnsupportedPatternTraitOnStringShape(val shape: Shape, val patternTrait: PatternTrait) : UnsupportedConstraintMessageKind()
 private data class UnsupportedRangeTraitOnShape(val shape: Shape, val rangeTrait: RangeTrait) : UnsupportedConstraintMessageKind()
+private data class UnsupportedUniqueItemsTraitOnShape(val shape: Shape, val uniqueItemsTrait: UniqueItemsTrait) : UnsupportedConstraintMessageKind()
 
 data class LogMessage(val level: Level, val message: String)
 data class ValidationResult(val shouldAbort: Boolean, val messages: List<LogMessage>)
@@ -228,13 +233,23 @@ fun validateUnsupportedConstraints(model: Model, service: ServiceShape, codegenC
         .map { (shape, rangeTrait) -> UnsupportedRangeTraitOnShape(shape, rangeTrait as RangeTrait) }
         .toSet()
 
+    // 7. UniqueItems trait on any shape is used. It has not been implemented yet.
+    // TODO(https://github.com/awslabs/smithy-rs/issues/1401)
+    val unsupportedUniqueItemsTraitOnShapeSet = walker
+        .walkShapes(service)
+        .asSequence()
+        .filterMapShapesToTraits(setOf(UniqueItemsTrait::class.java))
+        .map { (shape, uniqueItemsTrait) -> UnsupportedUniqueItemsTraitOnShape(shape, uniqueItemsTrait as UniqueItemsTrait) }
+        .toSet()
+
     val messages =
         unsupportedConstraintOnMemberShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
             unsupportedLengthTraitOnStreamingBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
             unsupportedConstraintOnShapeReachableViaAnEventStreamSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
             unsupportedLengthTraitOnCollectionOrOnBlobShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
             unsupportedPatternTraitOnStringShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
-            unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) }
+            unsupportedRangeTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) } +
+            unsupportedUniqueItemsTraitOnShapeSet.map { it.intoLogMessage(codegenConfig.ignoreUnsupportedConstraints) }
 
     return ValidationResult(shouldAbort = messages.any { it.level == Level.SEVERE }, messages)
 }

diff --git a/.../amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt b/.../amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraintsAreNotUsedTest.kt
@@ -214,6 +214,27 @@ internal class ValidateUnsupportedConstraintsAreNotUsedTest {
         validationResult.messages[0].message shouldContain "The integer shape `test#RangeInteger` has the constraint trait `smithy.api#range` attached"
     }
 
+    @Test
+    fun `it should detect when the unique items trait is used`() {
+        val model =
+            """
+            $baseModel
+            
+            structure TestInputOutput {
+                uniqueItemsList: UniqueItemsList
+            }
+            
+            @uniqueItems
+            list UniqueItemsList {
+                member: String
+            }
+            """.asSmithyModel()
+        val validationResult = validateModel(model)
+
+        validationResult.messages shouldHaveSize 1
+        validationResult.messages[0].message shouldContain "The list shape `test#UniqueItemsList` has the constraint trait `smithy.api#uniqueItems` attached"
+    }
+
     @Test
     fun `it should abort when ignoreUnsupportedConstraints is false and unsupported constraints are used`() {
         val validationResult = validateModel(constraintTraitOnStreamingBlobShapeModel, ServerCodegenConfig())