Address feedback

milesziemer · Nov 11, 2024 · fbd772b · fbd772b
1 parent 81b6261
commit fbd772b
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 39 deletions.
diff --git a/smithy-aws-protocol-tests/model/awsQuery/main.smithy b/smithy-aws-protocol-tests/model/awsQuery/main.smithy
@@ -1,15 +1,5 @@
 $version: "2.0"
 
-metadata suppressions = [
-    {
-        id: "XmlFlattenedTrait"
-        namespace: "aws.protocoltests.query"
-        reason: """
-            Some tests are for asserting the correct behavior in the case that
-            xmlFlattened is wrong and trips this validator."""
-    }
-]
-
 namespace aws.protocoltests.query
 
 use aws.api#service

diff --git a/smithy-aws-protocol-tests/model/awsQuery/xml-lists.smithy b/smithy-aws-protocol-tests/model/awsQuery/xml-lists.smithy
@@ -185,8 +185,10 @@ structure XmlListsOutput {
     @xmlName("renamed")
     renamedListMembers: RenamedListMembers,
 
+    @suppress(["XmlFlattenedTrait"])
     @xmlFlattened
     // The xmlname on the targeted list is ignored, and the member name is used.
+    // The validation that flags this is suppressed so we can test this behavior.
     flattenedList: RenamedListMembers,
 
     @xmlName("customName")

diff --git a/smithy-aws-protocol-tests/model/ec2Query/main.smithy b/smithy-aws-protocol-tests/model/ec2Query/main.smithy
@@ -26,16 +26,6 @@
 
 $version: "2.0"
 
-metadata suppressions = [
-    {
-        id: "XmlFlattenedTrait"
-        namespace: "aws.protocoltests.ec2"
-        reason: """
-            Some tests are for asserting the correct behavior in the case that
-            xmlFlattened is wrong and trips this validator."""
-    }
-]
-
 namespace aws.protocoltests.ec2
 
 use aws.api#service

diff --git a/smithy-aws-protocol-tests/model/ec2Query/xml-lists.smithy b/smithy-aws-protocol-tests/model/ec2Query/xml-lists.smithy
@@ -183,8 +183,10 @@ structure XmlListsOutput {
     @xmlName("renamed")
     renamedListMembers: RenamedListMembers,
 
+    @suppress(["XmlFlattenedTrait"])
     @xmlFlattened
     // The xmlname on the targeted list is ignored, and the member name is used.
+    // The validation that flags this is suppressed so we can test this behavior.
     flattenedList: RenamedListMembers,
 
     @xmlName("customName")

diff --git a/smithy-aws-protocol-tests/model/restXml/document-lists.smithy b/smithy-aws-protocol-tests/model/restXml/document-lists.smithy
@@ -352,8 +352,10 @@ structure XmlListsInputOutput {
     @xmlName("renamed")
     renamedListMembers: RenamedListMembers,
 
+    @suppress(["XmlFlattenedTrait"])
     @xmlFlattened
     // The xmlname on the targeted list is ignored, and the member name is used.
+    // The validation that flags this is suppressed so we can test this behavior.
     flattenedList: RenamedListMembers,
 
     @xmlName("customName")
@@ -376,7 +378,10 @@ structure XmlListsInputOutput {
     @xmlName("myStructureList")
     structureList: StructureList,
 
+    @suppress(["XmlFlattenedTrait"])
     @xmlFlattened
+    // The xmlname on the targeted list is ignored, and the member name is used.
+    // The validation that flags this is suppressed so we can test this behavior.
     flattenedStructureList: StructureList
 }
 

diff --git a/smithy-aws-protocol-tests/model/restXml/main.smithy b/smithy-aws-protocol-tests/model/restXml/main.smithy
@@ -1,15 +1,5 @@
 $version: "2.0"
 
-metadata suppressions = [
-    {
-        id: "XmlFlattenedTrait"
-        namespace: "aws.protocoltests.restxml"
-        reason: """
-            Some tests are for asserting the correct behavior in the case that
-            xmlFlattened is wrong and trips this validator."""
-    }
-]
-
 namespace aws.protocoltests.restxml
 
 use aws.api#service

diff --git a/...n/java/software/amazon/smithy/model/validation/validators/XmlFlattenedTraitValidator.java b/...n/java/software/amazon/smithy/model/validation/validators/XmlFlattenedTraitValidator.java
@@ -7,7 +7,6 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.shapes.ListShape;
 import software.amazon.smithy.model.shapes.MemberShape;
@@ -31,21 +30,25 @@ public List<ValidationEvent> validate(Model model) {
                 continue;
             }
 
-            model.getShape(member.getTarget())
-                    .flatMap(Shape::asListShape)
-                    .flatMap(listShape -> validateMemberTargetingList(member, listShape))
-                    .ifPresent(events::add);
+            Shape target = model.expectShape(member.getTarget());
+            if (target instanceof ListShape) {
+                ListShape targetList = (ListShape) target;
+                validateMemberTargetingList(member, targetList, events);
+            }
         }
         return events;
     }
 
-    private Optional<ValidationEvent> validateMemberTargetingList(MemberShape member, ListShape targetList) {
-        return targetList.getMember().getTrait(XmlNameTrait.class)
-                .filter(xmlName -> !member.getMemberName().equals(xmlName.getValue()))
-                .map(xmlName -> warning(member, String.format(
+    private void validateMemberTargetingList(MemberShape member, ListShape targetList, List<ValidationEvent> events) {
+        if (targetList.getMember().hasTrait(XmlNameTrait.class)) {
+            XmlNameTrait xmlName = targetList.getMember().expectTrait(XmlNameTrait.class);
+            if (!member.getMemberName().equals(xmlName.getValue())) {
+                events.add(warning(member, String.format(
                         "Member is `@xmlFlattened`, so `@xmlName` of target's member (`%s`) has no effect."
                         + " The flattened list elements will have the name of this member - `%s`. If this"
                         + " is unintended, you can add `@xmlName(\"%s\")` to this member.",
                         targetList.getMember().getId(), member.getMemberName(), xmlName.getValue())));
+            }
+        }
     }
 }