Fix ignored binding checks for mixins

smithy-lang · Sep 6, 2023 · 4815bf7 · 4815bf7
1 parent 0af2ca4
commit 4815bf7
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 19 deletions.
diff --git a/.../software/amazon/smithy/model/validation/validators/HttpBindingTraitIgnoredValidator.java b/.../software/amazon/smithy/model/validation/validators/HttpBindingTraitIgnoredValidator.java
@@ -22,6 +22,7 @@
 import software.amazon.smithy.model.neighbor.Relationship;
 import software.amazon.smithy.model.neighbor.RelationshipType;
 import software.amazon.smithy.model.shapes.MemberShape;
+import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.StructureShape;
 import software.amazon.smithy.model.traits.HttpHeaderTrait;
@@ -31,6 +32,7 @@
 import software.amazon.smithy.model.traits.HttpQueryParamsTrait;
 import software.amazon.smithy.model.traits.HttpQueryTrait;
 import software.amazon.smithy.model.traits.HttpResponseCodeTrait;
+import software.amazon.smithy.model.traits.MixinTrait;
 import software.amazon.smithy.model.traits.Trait;
 import software.amazon.smithy.model.validation.AbstractValidator;
 import software.amazon.smithy.model.validation.ValidationEvent;
@@ -70,6 +72,12 @@ public List<ValidationEvent> validate(Model model) {
         List<ValidationEvent> events = new ArrayList<>();
         NeighborProvider reverse = NeighborProviderIndex.of(model).getReverseProvider();
         for (MemberShape memberShape : model.getMemberShapes()) {
+            Shape container = model.expectShape(memberShape.getContainer());
+            // Skip non-structures (invalid) and mixins (handled at mixed site).
+            if (!container.isStructureShape() || container.hasTrait(MixinTrait.class)) {
+                continue;
+            }
+
             // Gather all traits that are HTTP member binding.
             // Keep the trait instance around so that it can be used it later for source location.
             Map<ShapeId, Trait> traits = new HashMap<>();
@@ -82,11 +90,9 @@ public List<ValidationEvent> validate(Model model) {
             // The traits set is now the HTTP binding traits that are ignored outside
             // the top level of an operation's components.
             if (!traits.isEmpty()) {
-                StructureShape containerShape = model.expectShape(memberShape.getContainer(), StructureShape.class);
-
                 // All relationships and trait possibilities are checked at once to de-duplicate
                 // several parts of the iteration logic.
-                events.addAll(checkRelationships(containerShape, memberShape, traits, reverse));
+                events.addAll(checkRelationships(container.asStructureShape().get(), memberShape, traits, reverse));
             }
         }
         return events;
@@ -151,19 +157,16 @@ private List<ValidationEvent> checkRelationships(
         // immediate grabbing of next traits is all that's necessary.
         if (!ignoredRelationships.isEmpty()) {
             if (!ignoredOutsideInputTraits.isEmpty()) {
-                ShapeId traitId = ignoredOutsideInputTraits.iterator().next();
-                events.add(emit("Input", memberShape, traits, traitId,
-                        checkedRelationshipCount, ignoredRelationships));
+                Trait trait = traits.get(ignoredOutsideInputTraits.iterator().next());
+                events.add(emit("Input", memberShape, trait, checkedRelationshipCount, ignoredRelationships));
 
             } else if (!ignoredOutsideOutputTraits.isEmpty()) {
-                ShapeId traitId = ignoredOutsideOutputTraits.iterator().next();
-                events.add(emit("Output", memberShape, traits, traitId,
-                        checkedRelationshipCount, ignoredRelationships));
+                Trait trait = traits.get(ignoredOutsideOutputTraits.iterator().next());
+                events.add(emit("Output", memberShape, trait, checkedRelationshipCount, ignoredRelationships));
             } else {
                 // The traits list is always non-empty here, so just grab the first.
-                ShapeId traitId = traits.keySet().iterator().next();
-                events.add(emit("TopLevel", memberShape, traits, traitId,
-                        checkedRelationshipCount, ignoredRelationships));
+                Trait trait = traits.get(traits.keySet().iterator().next());
+                events.add(emit("TopLevel", memberShape, trait, checkedRelationshipCount, ignoredRelationships));
             }
         }
 
@@ -180,18 +183,18 @@ private List<ShapeId> mergeShapeIdLists(List<ShapeId> shapeIds1, List<ShapeId> s
     private ValidationEvent emit(
             String type,
             MemberShape memberShape,
-            Map<ShapeId, Trait> traits,
-            ShapeId traitId,
+            Trait trait,
             int checkedRelationshipCount,
             Map<RelationshipType, List<ShapeId>> ignoredRelationships
     ) {
-        String message = "The `%s` trait applied to this member is ";
+        String mixedIn = memberShape.getMixins().isEmpty() ? "" : " mixed in";
+        String message = "The `%s` trait applied to this%s member is ";
         if (checkedRelationshipCount == ignoredRelationships.size()) {
             message += "ignored in all contexts.";
         } else {
-            message += format("ignored in some contexts: %s", formatIgnoredRelationships(ignoredRelationships));
+            message += "ignored in some contexts: " + formatIgnoredRelationships(ignoredRelationships);
         }
-        return warning(memberShape, traits.get(traitId), format(message, Trait.getIdiomaticTraitName(traitId)), type);
+        return warning(memberShape, trait, format(message, Trait.getIdiomaticTraitName(trait), mixedIn), type);
     }
 
     private String formatIgnoredRelationships(Map<RelationshipType, List<ShapeId>> ignoredRelationships) {

diff --git a/...rces/software/amazon/smithy/model/errorfiles/validators/http-binding-trait-ignored.errors b/...rces/software/amazon/smithy/model/errorfiles/validators/http-binding-trait-ignored.errors
@@ -15,3 +15,6 @@
 [WARNING] smithy.example#NestedBindings$stringMap2: The `httpPrefixHeaders` trait applied to this member is ignored in some contexts: {member target: [smithy.example#IgnoredBindingOperationInput$nestedBindings, smithy.example#IgnoredBindingOperationOutput$nestedBindings]} | HttpBindingTraitIgnored.TopLevel
 [WARNING] smithy.example#NestedBindings$code: The `httpResponseCode` trait applied to this member is ignored in some contexts: {member target: [smithy.example#IgnoredBindingOperationInput$nestedBindings, smithy.example#IgnoredBindingOperationOutput$nestedBindings], input: [smithy.example#NestedBindingOperation]} | HttpBindingTraitIgnored.Output
 [WARNING] smithy.example#NestedBindings$payload: The `httpPayload` trait applied to this member is ignored in some contexts: {member target: [smithy.example#IgnoredBindingOperationInput$nestedBindings, smithy.example#IgnoredBindingOperationOutput$nestedBindings]} | HttpBindingTraitIgnored.TopLevel
+[WARNING] smithy.example#MixedBindingOperationOutput$query: The `httpQuery` trait applied to this mixed in member is ignored in all contexts. | HttpBindingTraitIgnored.Input
+[WARNING] smithy.example#MixedBindingOperationInput$code: The `httpResponseCode` trait applied to this mixed in member is ignored in all contexts. | HttpBindingTraitIgnored.Output
+[WARNING] smithy.example#MixedBindingOperationOutput$code: The `httpResponseCode` trait applied to this mixed in member is ignored in some contexts: {member target: [smithy.example#MixedBindingOperationInput$content]} | HttpBindingTraitIgnored.Output
diff --git a/...rces/software/amazon/smithy/model/errorfiles/validators/http-binding-trait-ignored.smithy b/...rces/software/amazon/smithy/model/errorfiles/validators/http-binding-trait-ignored.smithy
@@ -3,11 +3,12 @@ $version: "2"
 namespace smithy.example
 
 service InvalidExample {
-    version: "2020-12-29",
+    version: "2020-12-29"
     operations: [
         IgnoredBindingOperation
         NestedBindingOperation
-    ],
+        MixedBindingOperation
+    ]
 }
 
 @http(method: "POST", uri: "/ignored-binding")
@@ -77,3 +78,27 @@ map StringMap {
     key: String
     value: String
 }
+
+@http(method: "POST", uri: "/mixed-binding")
+operation MixedBindingOperation {
+    input: MixedBindingOperationInput
+    output: MixedBindingOperationOutput
+}
+
+structure MixedBindingOperationInput with [MixedQuery, MixedResponseCode] {
+    content: MixedBindingOperationOutput
+}
+
+structure MixedBindingOperationOutput with [MixedQuery, MixedResponseCode] {}
+
+@mixin
+structure MixedQuery {
+    @httpQuery("query")
+    query: String
+}
+
+@mixin
+structure MixedResponseCode {
+    @httpResponseCode
+    code: Integer
+}