Add synthetic box to root intEnums w/o a default (smithy-lang#2053)

Primitive root shapes without the default trait have a synthetic
box trait appplied when the shape is built in the loader (see
https://github.com/smithy-lang/smithy/blob/d457aabb80feb4088caa3ac27d337b84e3ebc43d/smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderShapeMap.java#L366)
if the shape would have a default of 0 in Smithy 1.0.
Previously, this didn't include intEnum, so if you convert to
1.0, the intEnum which previously had no default in 2.0 will have
a default of 0.

This commit updates ModelInteropTransformer to consider intEnum as
a shape that has a default value in 1.0, so the loader applies the
synthetic box trait to root intEnums.

Tests were updated to make sure box is added to root intEnums, and
to fix some incorrect assertions.

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 @@ private boolean v2ShapeNeedsBoxTrait(MemberShape member, Shape target) {
                && 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) {

smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelInteropTransformerTest.java

@@ -360,11 +360,28 @@ public void boxTraitOnlyAddedToRootWhenNotSetToZeroValueDefault() {
                                                 + "@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 @@ private void boxTraitRootAssertionsV2(Model model, int roundTrip) {
         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 @@ private void boxTraitRootAssertionsV2(Model model, int roundTrip) {
 
         // 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 @@ private void boxTraitRootAssertionsV2(Model model, int roundTrip) {
         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

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
 }
 

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