Make required trait part of httpLabel selector

We previously enforced that a structure member is required using separate validation, but it should really be encoded in the selector. This updates the selector, expected validation messages, and the documentation to do so.
smithy-lang · Jul 3, 2020 · 1851a85 · 1851a85
1 parent 9158a65
commit 1851a85
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 8 deletions.
diff --git a/docs/source/1.0/spec/core/http-traits.rst b/docs/source/1.0/spec/core/http-traits.rst
@@ -574,9 +574,9 @@ Summary
 Trait selector
     .. code-block:: none
 
-        structure > :test(member > :test(string, number, boolean, timestamp))
+        structure > member[trait|required] :test(> :test(string, number, boolean, timestamp))
 
-    *Structure members that target any simple type other than blobs*
+    *Required structure members that target a string, number, boolean, or timestamp*
 Value type
     Annotation trait.
 Conflicts with

diff --git a/...main/java/software/amazon/smithy/model/validation/validators/HttpLabelTraitValidator.java b/...main/java/software/amazon/smithy/model/validation/validators/HttpLabelTraitValidator.java
@@ -104,9 +104,6 @@ private List<ValidationEvent> validateBindings(
                     MemberShape member = pair.getLeft();
                     HttpLabelTrait trait = pair.getRight();
                     labels.remove(member.getMemberName());
-                    if (member.isOptional()) {
-                        events.add(error(member, trait, "Members with the `httpLabel` trait must be required."));
-                    }
 
                     // Emit an error if the member is not a valid label.
                     if (!http.getUri().getLabel(member.getMemberName()).isPresent()) {

diff --git a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy
@@ -473,7 +473,7 @@ structure http {
 }
 
 /// Binds an operation input structure member to an HTTP label.
-@trait(selector: "structure > :test(member > :test(string, number, boolean, timestamp))",
+@trait(selector: "structure > member[trait|required] :test(> :test(string, number, boolean, timestamp))",
         conflicts: [httpHeader, httpQuery, httpPrefixHeaders, httpPayload])
 @tags(["diff.error.const"])
 structure httpLabel {}

diff --git a/...software/amazon/smithy/model/errorfiles/validators/http-request-response-validator.errors b/...software/amazon/smithy/model/errorfiles/validators/http-request-response-validator.errors
@@ -20,12 +20,12 @@
 [ERROR] ns.foo#L: Operation URI, `/k`, conflicts with other operation URIs in the same service: [`ns.foo#K` (/k)] | HttpUriConflict
 [DANGER] ns.foo#MInput$a: httpHeader cannot be set to `Authorization` | HttpHeaderTrait
 [ERROR] ns.foo#NInput$a: This `a` structure member is marked with the `httpLabel` trait, but no corresponding `http` URI label could be found when used as the input of the `ns.foo#N` operation. | HttpLabelTrait
-[ERROR] ns.foo#OInput$a: Members with the `httpLabel` trait must be required. | HttpLabelTrait
 [ERROR] ns.foo#PInput$a: The `a` structure member corresponds to a greedy label when used as the input of the `ns.foo#P` operation. This member targets (integer: `ns.foo#Integer`), but greedy labels must target string shapes. | HttpLabelTrait
 [ERROR] ns.foo#RInput$b: `httpHeader` binding of `x-foo` conflicts with the `httpPrefixHeaders` binding of `ns.foo#RInput$a` to ``. `httpHeader` bindings must not case-insensitively start with any `httpPrefixHeaders` bindings. | HttpPrefixHeadersTrait
 [NOTE] ns.foo#BadError: The structure shape is not connected to from any service shape. | UnreferencedShape
 [NOTE] ns.foo#BadErrorMultipleBindings: The structure shape is not connected to from any service shape. | UnreferencedShape
 [ERROR] ns.foo#GInput$b: Trait `httpHeader` cannot be applied to `ns.foo#GInput$b`. This trait may only be applied to shapes that match the following selector: structure > :test(member > :test(boolean, number, string, timestamp, collection > member > :test(boolean, number, string, timestamp))) | TraitTarget
 [ERROR] ns.foo#GOutput$b: Trait `httpHeader` cannot be applied to `ns.foo#GOutput$b`. This trait may only be applied to shapes that match the following selector: structure > :test(member > :test(boolean, number, string, timestamp, collection > member > :test(boolean, number, string, timestamp))) | TraitTarget
 [ERROR] ns.foo#HInput$a: Trait `httpHeader` cannot be applied to `ns.foo#HInput$a`. This trait may only be applied to shapes that match the following selector: structure > :test(member > :test(boolean, number, string, timestamp, collection > member > :test(boolean, number, string, timestamp))) | TraitTarget
-[ERROR] ns.foo#EInput$label2: Trait `httpLabel` cannot be applied to `ns.foo#EInput$label2`. This trait may only be applied to shapes that match the following selector: structure > :test(member > :test(string, number, boolean, timestamp)) | TraitTarget
+[ERROR] ns.foo#EInput$label2: Trait `httpLabel` cannot be applied to `ns.foo#EInput$label2`. This trait may only be applied to shapes that match the following selector: structure > member[trait|required] :test(> :test(string, number, boolean, timestamp)) | TraitTarget
+[ERROR] ns.foo#OInput$a: Trait `httpLabel` cannot be applied to `ns.foo#OInput$a`. This trait may only be applied to shapes that match the following selector: structure > member[trait|required] :test(> :test(string, number, boolean, timestamp)) | TraitTarget