Repository: smithy-lang/smithy
Pull request #2051 (2023-11-24 11:59:50)

Rephrase auth trait documentation
This rephrases the documentation for auth traits to read more like documentation for the auth type rather than documentation for a function. This allows it to be more naturally used in documentation generation without change. This also adds inline links to external documentation rather than solely relying on the external docs trait. While that trait is useful still, rendered docs are more natural if there's inline links too.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.auth.smithy b/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.auth.smithy
index fcbb7e24c..7c67bb404 100644
--- a/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.auth.smithy
+++ b/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.auth.smithy
@@ -14,11 +14,11 @@ structure cognitoUserPools {
     providerArns: StringList
 }
 
-/// Signature Version 4 is the process to add authentication information to
-/// AWS requests sent by HTTP. For security, most requests to AWS must be
-/// signed with an access key, which consists of an access key ID and secret
-/// access key. These two keys are commonly referred to as your security
-/// credentials.
+/// [Signature Version 4](https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html)
+/// is the process to add authentication information to AWS requests sent by HTTP. For
+/// security, most requests to AWS must be signed with an access key, which consists
+/// of an access key ID and secret access key. These two keys are commonly referred to
+/// as your security credentials.
 @authDefinition(traits: [unsignedPayload])
 @externalDocumentation(
     Reference: "https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html"
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java
index 907904bbd..15ec4f750 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelInteropTransformer.java
@@ -66,7 +66,8 @@ final class ModelInteropTransformer {
             ShapeType.LONG,
             ShapeType.FLOAT,
             ShapeType.DOUBLE,
-            ShapeType.BOOLEAN);
+            ShapeType.BOOLEAN,
+            ShapeType.INT_ENUM); // intEnum is actually an integer in v1, but the ShapeType is different.
 
     private final Model model;
     private final List<ValidationEvent> events;
@@ -167,10 +168,9 @@ final class ModelInteropTransformer {
                && memberAndTargetAreNotAlreadyExplicitlyBoxed(member, target);
     }
 
-    // Only apply box to members where the trait can be applied. Note that intEnum is treated
-    // like a normal integer in v1 implementations, so it is allowed to be synthetically boxed.
+    // Only apply box to members where the trait can be applied.
     private boolean canBoxTargetThisKindOfShape(Shape target) {
-        return HAD_DEFAULT_VALUE_IN_1_0.contains(target.getType()) || target.isIntEnumShape();
+        return HAD_DEFAULT_VALUE_IN_1_0.contains(target.getType());
     }
 
     private boolean memberAndTargetAreNotAlreadyExplicitlyBoxed(MemberShape member, Shape target) {
diff --git a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
index 007f60a90..947082d00 100644
--- a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
+++ b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude.smithy
@@ -246,8 +246,7 @@ structure authDefinition {
     traits: TraitShapeIdList
 }
 
-/// Enables HTTP Basic Authentication as defined in RFC 2617 on a service
-/// or operation.
+/// HTTP Basic Authentication as defined in [RFC 2617](https://tools.ietf.org/html/rfc2617.html).
 @trait(
     selector: "service"
     breakingChanges: [
@@ -258,8 +257,7 @@ structure authDefinition {
 @externalDocumentation("RFC 2617": "https://tools.ietf.org/html/rfc2617.html")
 structure httpBasicAuth {}
 
-/// Enables HTTP Digest Authentication as defined in RFC 2617 on a service
-/// or operation.
+/// HTTP Digest Authentication as defined in [RFC 2617](https://tools.ietf.org/html/rfc2617.html).
 @trait(
     selector: "service"
     breakingChanges: [
@@ -270,8 +268,7 @@ structure httpBasicAuth {}
 @externalDocumentation("RFC 2617": "https://tools.ietf.org/html/rfc2617.html")
 structure httpDigestAuth {}
 
-/// Enables HTTP Bearer Authentication as defined in RFC 6750 on a service
-/// or operation.
+/// HTTP Bearer Authentication as defined in [RFC 6750](https://tools.ietf.org/html/rfc6750.html).
 @trait(
     selector: "service"
     breakingChanges: [


Test patch:
diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelInteropTransformerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelInteropTransformerTest.java
index d8c4bbc3c..c55f8a902 100644
--- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelInteropTransformerTest.java
+++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelInteropTransformerTest.java
@@ -360,11 +360,28 @@ public class ModelInteropTransformerTest {
                                                 + "@default(0)\n"
                                                 + "integer PrimitiveInteger\n"
                                                 + "\n"
+                                                + "intEnum BoxedIntEnum {\n"
+                                                + "    ONE = 1\n"
+                                                + "}\n"
+                                                + "\n"
+                                                + "@default(1)\n"
+                                                + "intEnum BoxedIntEnumWithDefault {\n"
+                                                + "    ONE = 1\n"
+                                                + "}\n"
+                                                + "\n"
+                                                + "@default(0)\n"
+                                                + "intEnum PrimitiveIntEnum {\n"
+                                                + "    ZERO = 0\n"
+                                                + "}\n"
+                                                + "\n"
                                                 + "structure Foo {\n"
                                                 + "    DefaultString: DefaultString = \"\"\n"
                                                 + "    BoxedInteger: BoxedInteger\n"
                                                 + "    PrimitiveInteger: PrimitiveInteger = 0\n"
                                                 + "    BoxedIntegerWithDefault: BoxedIntegerWithDefault = 1\n"
+                                                + "    BoxedIntEnum: BoxedIntEnum\n"
+                                                + "    BoxedIntEnumWithDefault: BoxedIntEnumWithDefault = 1\n"
+                                                + "    PrimitiveIntEnum: PrimitiveIntEnum = 0\n"
                                                 +"}\n")
                 .assemble()
                 .unwrap();
@@ -385,11 +402,20 @@ public class ModelInteropTransformerTest {
         ShapeId fooBoxedInteger = ShapeId.from("smithy.example#Foo$BoxedInteger");
 
         ShapeId boxedIntegerWithDefault = ShapeId.from("smithy.example#BoxedIntegerWithDefault");
-        ShapeId fooBoxedIntegerWithDefault = ShapeId.from("smithy.example#BoxedIntegerWithDefault");
+        ShapeId fooBoxedIntegerWithDefault = ShapeId.from("smithy.example#Foo$BoxedIntegerWithDefault");
 
         ShapeId primitiveInteger = ShapeId.from("smithy.example#PrimitiveInteger");
         ShapeId fooPrimitiveInteger = ShapeId.from("smithy.example#Foo$PrimitiveInteger");
 
+        ShapeId boxedIntEnum = ShapeId.from("smithy.example#BoxedIntEnum");
+        ShapeId fooBoxedIntEnum = ShapeId.from("smithy.example#Foo$BoxedIntEnum");
+
+        ShapeId boxedIntEnumWithDefault = ShapeId.from("smithy.example#BoxedIntEnumWithDefault");
+        ShapeId fooBoxedIntEnumWithDefault = ShapeId.from("smithy.example#Foo$BoxedIntEnumWithDefault");
+
+        ShapeId primitiveIntEnum = ShapeId.from("smithy.example#PrimitiveIntEnum");
+        ShapeId fooPrimitiveIntEnum = ShapeId.from("smithy.example#Foo$PrimitiveIntEnum");
+
         // Do not box strings for v1 compatibility.
         assertThat(model.expectShape(defaultString).hasTrait(BoxTrait.class), is(false));
         assertThat(model.expectShape(fooDefaultString).hasTrait(BoxTrait.class), is(false));
@@ -404,7 +430,7 @@ public class ModelInteropTransformerTest {
 
         // Add box to BoxedIntegerWithDefault because it has a default that isn't the v1 zero value.
         assertThat(model.expectShape(boxedIntegerWithDefault).hasTrait(BoxTrait.class), is(true));
-        assertThat(model.expectShape(fooBoxedIntegerWithDefault).hasTrait(BoxTrait.class), is(true)); // no need to box the member too
+        assertThat(model.expectShape(fooBoxedIntegerWithDefault).hasTrait(BoxTrait.class), is(false)); // no need to box the member too
         assertThat(model.expectShape(boxedIntegerWithDefault).hasTrait(DefaultTrait.class), is(true));
         assertThat(model.expectShape(fooBoxedIntegerWithDefault).hasTrait(DefaultTrait.class), is(true));
 
@@ -413,6 +439,24 @@ public class ModelInteropTransformerTest {
         assertThat(model.expectShape(fooPrimitiveInteger).hasTrait(BoxTrait.class), is(false));
         assertThat(model.expectShape(primitiveInteger).hasTrait(DefaultTrait.class), is(true));
         assertThat(model.expectShape(fooPrimitiveInteger).hasTrait(DefaultTrait.class), is(true));
+
+        // Add box to BoxedIntEnum because it has no default trait.
+        assertThat(model.expectShape(boxedIntEnum).hasTrait(BoxTrait.class), is(true));
+        assertThat(model.expectShape(fooBoxedIntEnum).hasTrait(BoxTrait.class), is(false)); // no need to box the member too
+        assertThat(model.expectShape(boxedIntEnum).hasTrait(DefaultTrait.class), is(false));
+        assertThat(model.expectShape(fooBoxedIntEnum).hasTrait(DefaultTrait.class), is(false));
+
+        // Add box to BoxedIntEnumWithDefault because it has a default that isn't the v1 zero value.
+        assertThat(model.expectShape(boxedIntEnumWithDefault).hasTrait(BoxTrait.class), is(true));
+        assertThat(model.expectShape(fooBoxedIntEnumWithDefault).hasTrait(BoxTrait.class), is(false)); // no need to box the member too
+        assertThat(model.expectShape(boxedIntEnumWithDefault).hasTrait(DefaultTrait.class), is(true));
+        assertThat(model.expectShape(fooBoxedIntEnumWithDefault).hasTrait(DefaultTrait.class), is(true));
+
+        // No box trait on PrimitiveIntEnum because it has a zero value default.
+        assertThat(model.expectShape(primitiveIntEnum).hasTrait(BoxTrait.class), is(false));
+        assertThat(model.expectShape(fooPrimitiveIntEnum).hasTrait(BoxTrait.class), is(false));
+        assertThat(model.expectShape(primitiveIntEnum).hasTrait(DefaultTrait.class), is(true));
+        assertThat(model.expectShape(fooPrimitiveIntEnum).hasTrait(DefaultTrait.class), is(true));
     }
 
     @Test
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/upgrade-box/2-to-1/intEnumSetToZeroValueToNonNullable.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/upgrade-box/2-to-1/intEnumSetToZeroValueToNonNullable.smithy
index 4a0d7660f..8b5d72b76 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/upgrade-box/2-to-1/intEnumSetToZeroValueToNonNullable.smithy
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/upgrade-box/2-to-1/intEnumSetToZeroValueToNonNullable.smithy
@@ -1,11 +1,11 @@
-// {"v1-box": false, "v1-client-zero-value": false, "v2": false}
+// {"v1-box": true, "v1-client-zero-value": false, "v2": false}
+// V1 style box checks will think this member is nullable because it
+// targets a shape with the box trait.
 $version: "2.0"
 
 namespace smithy.example
 
 structure Foo {
-    // V1 models treat intEnum as normal integers, so they just see a default zero value, hence this is
-    // non-nullable in v1 and v2.
     intEnumSetToZeroValueToNonNullable: MyIntEnum = 0
 }
 
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/ast-serialization/cases/v2/enums.1.0.json b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/ast-serialization/cases/v2/enums.1.0.json
index c02810a98..f294cb927 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/ast-serialization/cases/v2/enums.1.0.json
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/ast-serialization/cases/v2/enums.1.0.json
@@ -2,7 +2,10 @@
     "smithy": "1.0",
     "shapes": {
         "smithy.example#IntEnum": {
-            "type": "integer"
+            "type": "integer",
+            "traits": {
+                "smithy.api#box": {}
+            }
         },
         "smithy.example#StringEnum": {
             "type": "string",