Add validator for xmlFlattened + xmlName (#2439)

Adds a validator for the xmlFlattened trait that checks if the member's
target is a list that has a member with xmlName, and that xmlName
doesn't match the name of the xmlFlattened member. xmlFlattened causes
the xmlName of the list member to be ignored. This is called out in our
docs:
https://smithy.io/2.0/spec/protocol-traits.html#flattened-list-serialization,
but we were missing a validator for it. It is a warning because you might
mean to do this, i.e. if the list is used in multiple places.

I also added some suppressions to our protocol tests that are testing
the behavior for models that would trip up this validator.

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

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

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
 }
 

smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/XmlFlattenedTraitValidator.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.model.validation.validators;
+
+import java.util.ArrayList;
+import java.util.List;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.shapes.ListShape;
+import software.amazon.smithy.model.shapes.MemberShape;
+import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.traits.XmlFlattenedTrait;
+import software.amazon.smithy.model.traits.XmlNameTrait;
+import software.amazon.smithy.model.validation.AbstractValidator;
+import software.amazon.smithy.model.validation.ValidationEvent;
+
+/**
+ * Validates that xmlFlattened members aren't unintentionally ignoring the
+ * xmlName of their targets.
+ */
+public final class XmlFlattenedTraitValidator extends AbstractValidator {
+    @Override
+    public List<ValidationEvent> validate(Model model) {
+        List<ValidationEvent> events = new ArrayList<>();
+        for (MemberShape member : model.getMemberShapesWithTrait(XmlFlattenedTrait.class)) {
+            // Don't emit the event if they're being explicit about the xmlName on this member
+            if (member.hasTrait(XmlNameTrait.class)) {
+                continue;
+            }
+
+            Shape target = model.expectShape(member.getTarget());
+            if (target instanceof ListShape) {
+                ListShape targetList = (ListShape) target;
+                validateMemberTargetingList(member, targetList, events);
+            }
+        }
+        return events;
+    }
+
+    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())));
+            }
+        }
+    }
+}

smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator

@@ -59,3 +59,4 @@ software.amazon.smithy.model.validation.validators.UnitTypeValidator
 software.amazon.smithy.model.validation.validators.UnstableTraitValidator
 software.amazon.smithy.model.validation.validators.XmlNamespaceTraitValidator
 software.amazon.smithy.model.validation.validators.TraitValidatorsValidator
+software.amazon.smithy.model.validation.validators.XmlFlattenedTraitValidator

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/xml-flattened-and-name.errors

@@ -0,0 +1 @@
+[WARNING] smithy.example#Struct$flattenedNoXmlName: Member is `@xmlFlattened`, so `@xmlName` of target's member (`smithy.example#ListWithXmlName$member`) has no effect. The flattened list elements will have the name of this member - `flattenedNoXmlName`. If this is unintended, you can add `@xmlName("customMember")` to this member. | XmlFlattenedTrait

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/xml-flattened-and-name.smithy

@@ -0,0 +1,43 @@
+$version: "2"
+
+namespace smithy.example
+
+structure Struct {
+    // No event because not flattened
+    notFlattened: ListWithXmlName
+
+    // Event because flattened and non-matching member name
+    @xmlFlattened
+    flattenedNoXmlName: ListWithXmlName
+
+    // No event because the member name matches the xml name
+    @xmlFlattened
+    customMember: ListWithXmlName
+
+    // No event because you're being explicit about the name to use
+    @xmlFlattened
+    @xmlName("customMember")
+    flattenedMatchingXmlName: ListWithXmlName
+
+    // No event because you're being explicit about the name to use
+    @xmlFlattened
+    @xmlName("Bar")
+    flattenedNonMatchingXmlName: ListWithXmlName
+
+    // Validator doesn't apply to maps
+    @xmlFlattened
+    flattenedMap: MapWithXmlName
+}
+
+list ListWithXmlName {
+    @xmlName("customMember")
+    member: String
+}
+
+map MapWithXmlName {
+    @xmlName("customKey")
+    key: String
+
+    @xmlName("customValue")
+    value: String
+}