try only boxing primitive types

smithy-lang · Jul 26, 2024 · 78e9745 · 78e9745
1 parent a0fd2c5
commit 78e9745
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 42 deletions.
diff --git a/...odegen/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkDotNetNameResolver.java b/...odegen/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkDotNetNameResolver.java
@@ -53,6 +53,14 @@ private boolean isGeneratedInSdk(final ShapeId shapeId) {
     return ModelUtils.isInServiceNamespace(shapeId, getServiceShape());
   }
 
+  @Override
+  protected String baseTypeForMember(final MemberShape memberShape) {
+    // The AWS SDK uses primitive types for required members.
+    return memberShapeIsOptional(memberShape)
+            ? super.baseTypeForMember(memberShape)
+            : baseTypeForShape(memberShape.getTarget());
+  }
+
   @Override
   protected String baseTypeForString(final StringShape stringShape) {
     if (

diff --git a/...gen/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkTypeConversionCodegen.java b/...gen/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkTypeConversionCodegen.java
@@ -82,7 +82,7 @@ public TokenTree generateExtractOptionalMember(MemberShape memberShape) {
     final String propertyName = nameResolver.classPropertyForStructureMember(
       memberShape
     );
-    return TokenTree.of(type, varName, "= value.%s;".formatted(propertyName));
+    return TokenTree.of(type, varName, "= (%s)value.%s;".formatted(type, propertyName));
   }
 
   @Override

diff --git a/...afny-codegen/src/main/java/software/amazon/polymorph/smithydotnet/DotNetNameResolver.java b/...afny-codegen/src/main/java/software/amazon/polymorph/smithydotnet/DotNetNameResolver.java
@@ -412,9 +412,8 @@ protected String baseTypeForUnion(final UnionShape unionShape) {
   }
 
   protected String baseTypeForMember(final MemberShape memberShape) {
-    final String baseType = baseTypeForShape(memberShape.getTarget());
-    final boolean isOptional = memberShapeIsOptional(memberShape);
-    return isOptional ? baseTypeForOptionalMember(memberShape) : baseType;
+    // We always use nullable types for safety.
+    return baseTypeForOptionalMember(memberShape);
   }
 
   protected String baseTypeForOptionalMember(final MemberShape memberShape) {

diff --git a/...y-codegen/src/main/java/software/amazon/polymorph/smithydotnet/TypeConversionCodegen.java b/...y-codegen/src/main/java/software/amazon/polymorph/smithydotnet/TypeConversionCodegen.java
@@ -788,20 +788,6 @@ public TypeConverter generateMemberConverter(final MemberShape memberShape) {
       TO_DAFNY
     );
 
-    if (!nameResolver.memberShapeIsOptional(memberShape)) {
-      final TokenTree fromDafnyBody = Token.of(
-        "return %s(value);".formatted(targetFromDafnyConverterName)
-      );
-      final TokenTree toDafnyBody = Token.of(
-        "return %s(value);".formatted(targetToDafnyConverterName)
-      );
-      return buildConverterFromMethodBodies(
-        memberShape,
-        fromDafnyBody,
-        toDafnyBody
-      );
-    }
-
     String cSharpTypeUnModified;
     if (
       StringUtils.equals(
@@ -819,6 +805,20 @@ public TypeConverter generateMemberConverter(final MemberShape memberShape) {
         cSharpTypeUnModified.substring(0, (cSharpTypeUnModified.length() - 1));
     }
 
+    if (!nameResolver.memberShapeIsOptional(memberShape)) {
+      final TokenTree fromDafnyBody = Token.of(
+              "return %s(value);".formatted(targetFromDafnyConverterName)
+      );
+      final TokenTree toDafnyBody = Token.of(
+              "return %s((%s)value);".formatted(targetToDafnyConverterName, cSharpTypeUnModified)
+      );
+      return buildConverterFromMethodBodies(
+              memberShape,
+              fromDafnyBody,
+              toDafnyBody
+      );
+    }
+
     final String cSharpType = cSharpTypeUnModified;
     final String cSharpOptionType;
     if (
@@ -891,6 +891,7 @@ public TypeConverter generateUnionConverter(final UnionShape unionShape) {
           .map(memberShape -> {
             final String propertyName =
               nameResolver.classPropertyForStructureMember(memberShape);
+            final String propertyType = nameResolver.classPropertyTypeForStructureMember(memberShape);
             final String memberFromDafnyConverterName = typeConverterForShape(
               memberShape.getId(),
               FROM_DAFNY
@@ -921,8 +922,9 @@ public TypeConverter generateUnionConverter(final UnionShape unionShape) {
               .append(
                 TokenTree
                   .of(
-                    "converted.%s = %s(concrete.dtor_%s);".formatted(
+                    "converted.%s = (%s)(%s(concrete.dtor_%s));".formatted(
                         propertyName,
+                        propertyType,
                         memberFromDafnyConverterName,
                         destructorValue
                       ),

diff --git a/...dafny-codegen/src/main/java/software/amazon/polymorph/smithyjava/nameresolver/Native.java b/...dafny-codegen/src/main/java/software/amazon/polymorph/smithyjava/nameresolver/Native.java
@@ -32,7 +32,6 @@
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.ShapeType;
-import software.amazon.smithy.model.traits.BoxTrait;
 import software.amazon.smithy.model.traits.EnumTrait;
 
 /**
@@ -75,13 +74,13 @@ public class Native extends NameResolver {
     NATIVE_TYPES_BY_SIMPLE_SHAPE_TYPE =
       Map.ofEntries(
         Map.entry(ShapeType.BLOB, ClassName.get(ByteBuffer.class)),
-        Map.entry(ShapeType.BOOLEAN, TypeName.BOOLEAN),
-        Map.entry(ShapeType.BYTE, TypeName.BYTE),
-        Map.entry(ShapeType.SHORT, TypeName.SHORT),
-        Map.entry(ShapeType.INTEGER, TypeName.INT),
-        Map.entry(ShapeType.LONG, TypeName.LONG),
-        Map.entry(ShapeType.FLOAT, TypeName.FLOAT),
-        Map.entry(ShapeType.DOUBLE, TypeName.DOUBLE),
+        Map.entry(ShapeType.BOOLEAN, TypeName.BOOLEAN.box()),
+        Map.entry(ShapeType.BYTE, TypeName.BYTE.box()),
+        Map.entry(ShapeType.SHORT, TypeName.SHORT.box()),
+        Map.entry(ShapeType.INTEGER, TypeName.INT.box()),
+        Map.entry(ShapeType.LONG, TypeName.LONG.box()),
+        Map.entry(ShapeType.FLOAT, TypeName.FLOAT.box()),
+        Map.entry(ShapeType.DOUBLE, TypeName.DOUBLE.box()),
         // TODO: AWS SDK V2 uses Instant, not Date
         Map.entry(ShapeType.TIMESTAMP, ClassName.get(Date.class)),
         Map.entry(ShapeType.BIG_DECIMAL, ClassName.get(BigDecimal.class)),
@@ -143,21 +142,8 @@ public TypeName typeForShape(final ShapeId shapeId) {
     }
 
     return switch (shape.getType()) {
-      case BOOLEAN, BYTE, SHORT, INTEGER, LONG, FLOAT, DOUBLE -> {
-        /* From the Smithy Docs:
-         * "Boolean, byte, short, integer, long, float, and double shapes
-         * are only considered boxed if they are marked with the box trait.
-         * All other shapes are always considered boxed."
-         * https://smithy.io/1.0/spec/core/type-refinement-traits.html#box-trait
-         * While Smithy Models SHOULD use Smithy Prelude shapes to avoid this confusion,
-         * they do not have to.
-         * Hence, the need to check if these shapes have the box trait
-         */
-        final TypeName typeName = NATIVE_TYPES_BY_SIMPLE_SHAPE_TYPE.get(
-          shape.getType()
-        );
-        yield shape.hasTrait(BoxTrait.class) ? typeName.box() : typeName;
-      }
+      // All primitives are boxed for safety, even if @required
+      case BOOLEAN, BYTE, SHORT, INTEGER, LONG, FLOAT, DOUBLE -> NATIVE_TYPES_BY_SIMPLE_SHAPE_TYPE.get(shape.getType());
       // For supported simple shapes, just map to native types
       case BLOB,
         TIMESTAMP,