Merge pull request #887 from jasdel/denseListMap

codegen: Add support for list and maps generated as value members instead of pointer

Makefile

@@ -2,7 +2,7 @@
 LINTIGNORESINGLEFIGHT='internal/sync/singleflight/singleflight.go:.+error should be the last type'
 
 UNIT_TEST_TAGS=
-BUILD_TAGS=-tags "example,codegen,integration,ec2env"
+BUILD_TAGS=-tags "example,codegen,integration,ec2env,perftest"
 
 SMITHY_GO_SRC ?= $(shell pwd)/../smithy-go
 

codegen/protocol-test-codegen/build.gradle.kts

@@ -20,7 +20,7 @@ plugins {
 }
 
 dependencies {
-    implementation("software.amazon.smithy:smithy-aws-protocol-tests:[1.1.0,2.0.0[")
+    implementation("software.amazon.smithy:smithy-aws-protocol-tests:[1.3.0,2.0.0[")
     compile(project(":smithy-aws-go-codegen"))
 }
 

codegen/smithy-aws-go-codegen/build.gradle.kts

@@ -31,7 +31,7 @@ tasks.withType<Test> {
 }
 
 dependencies {
-    api("software.amazon.smithy:smithy-aws-traits:[1.1.0,2.0.0[")
+    api("software.amazon.smithy:smithy-aws-traits:[1.3.0,2.0.0[")
     api("software.amazon.smithy:smithy-go-codegen:0.1.0")
     testCompile("org.junit.jupiter:junit-jupiter-api:5.4.0")
     testRuntime("org.junit.jupiter:junit-jupiter-engine:5.4.0")

codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/AwsGoDependency.java

@@ -82,6 +82,6 @@ protected static GoDependency module(
     }
 
     private static final class Versions {
-        private static final String AWS_SDK = "v0.29.1-0.20201112231636-9ae467d8157d";
+        private static final String AWS_SDK = "v0.29.1-0.20201113222241-726e4a15683d";
     }
 }

codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/AwsProtocolUtils.java

@@ -113,6 +113,24 @@ static void generateHttpProtocolTests(GenerationContext context) {
                         .operation(ShapeId.from("aws.protocoltests.restjson#InlineDocumentAsPayload"))
                         .build(),
 
+                // Null lists/maps without sparse tag
+                HttpProtocolUnitTestGenerator.SkipTest.builder()
+                        .service(ShapeId.from("aws.protocoltests.restjson#RestJson"))
+                        .operation(ShapeId.from("aws.protocoltests.restjson#JsonLists"))
+                        .addTestName("RestJsonListsSerializeNull")
+                        .build(),
+                HttpProtocolUnitTestGenerator.SkipTest.builder()
+                        .service(ShapeId.from("aws.protocoltests.restjson#RestJson"))
+                        .operation(ShapeId.from("aws.protocoltests.restjson#JsonMaps"))
+                        .addTestName("RestJsonSerializesNullMapValues")
+                        .build(),
+                HttpProtocolUnitTestGenerator.SkipTest.builder()
+                        .service(ShapeId.from("aws.protocoltests.json#JsonProtocol"))
+                        .operation(ShapeId.from("aws.protocoltests.json#NullOperation"))
+                        .addTestName("AwsJson11MapsSerializeNullValues")
+                        .addTestName("AwsJson11ListsSerializeNull")
+                        .build(),
+
                 // JSON RPC Documents
                 HttpProtocolUnitTestGenerator.SkipTest.builder()
                         .service(ShapeId.from("aws.protocoltests.json#JsonProtocol"))
@@ -125,7 +143,7 @@ static void generateHttpProtocolTests(GenerationContext context) {
                 HttpProtocolUnitTestGenerator.SkipTest.builder()
                         .service(ShapeId.from("aws.protocoltests.restxml#RestXml"))
                         .operation(ShapeId.from("aws.protocoltests.restxml#HttpPrefixHeaders"))
-                        .testName("HttpPrefixHeadersAreNotPresent")
+                        .addTestName("HttpPrefixHeadersAreNotPresent")
                         .build()
         ));
 

codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/DocumentMemberSerVisitor.java

@@ -22,6 +22,7 @@
 import software.amazon.smithy.go.codegen.integration.ProtocolGenerator;
 import software.amazon.smithy.go.codegen.integration.ProtocolGenerator.GenerationContext;
 import software.amazon.smithy.go.codegen.integration.ProtocolUtils;
+import software.amazon.smithy.go.codegen.knowledge.GoPointableIndex;
 import software.amazon.smithy.model.shapes.BigDecimalShape;
 import software.amazon.smithy.model.shapes.BigIntegerShape;
 import software.amazon.smithy.model.shapes.BlobShape;
@@ -58,6 +59,7 @@ public class DocumentMemberSerVisitor implements ShapeVisitor<Void> {
     private final String dataDest;
     private final Format timestampFormat;
     private final MemberShape member;
+    private final GoPointableIndex pointableIndex;
 
     public DocumentMemberSerVisitor(
             GenerationContext context,
@@ -71,67 +73,68 @@ public DocumentMemberSerVisitor(
         this.dataSource = dataSource;
         this.dataDest = dataDest;
         this.timestampFormat = timestampFormat;
+        this.pointableIndex = GoPointableIndex.of(context.getModel());
     }
 
     @Override
     public Void blobShape(BlobShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         context.getWriter().write("$L.Base64EncodeBytes($L)", dataDest, source);
         return null;
     }
 
     @Override
     public Void booleanShape(BooleanShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         context.getWriter().write("$L.Boolean($L)", dataDest, source);
         return null;
     }
 
     @Override
     public Void byteShape(ByteShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         context.getWriter().write("$L.Byte($L)", dataDest, source);
         return null;
     }
 
     @Override
     public Void shortShape(ShortShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         context.getWriter().write("$L.Short($L)", dataDest, source);
         return null;
     }
 
     @Override
     public Void integerShape(IntegerShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         context.getWriter().write("$L.Integer($L)", dataDest, source);
         return null;
     }
 
     @Override
     public Void longShape(LongShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         context.getWriter().write("$L.Long($L)", dataDest, source);
         return null;
     }
 
     @Override
     public Void floatShape(FloatShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         context.getWriter().write("$L.Float($L)", dataDest, source);
         return null;
     }
 
     @Override
     public Void doubleShape(DoubleShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         context.getWriter().write("$L.Double($L)", dataDest, source);
         return null;
     }
 
     @Override
     public Void timestampShape(TimestampShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         GoWriter writer = context.getWriter();
         writer.addUseImports(SmithyGoDependency.SMITHY_TIME);
 
@@ -153,23 +156,14 @@ public Void timestampShape(TimestampShape shape) {
 
     @Override
     public Void stringShape(StringShape shape) {
-        String source = conditionallyDereference(shape, dataSource);
+        String source = CodegenUtils.getAsValueIfDereferencable(pointableIndex, member, dataSource);
         if (shape.hasTrait(EnumTrait.class)) {
             source = String.format("string(%s)", source);
         }
         context.getWriter().write("$L.String($L)", dataDest, source);
         return null;
     }
 
-    private String conditionallyDereference(Shape shape, String dataSource) {
-        boolean shouldDereference = CodegenUtils.isShapePassByReference(shape);
-        if (context.getModel().expectShape(member.getContainer()).isUnionShape()) {
-            Shape target = context.getModel().expectShape(member.getTarget());
-            shouldDereference &= ProtocolUtils.usesScalarWhenUnionValue(target);
-        }
-        return shouldDereference ? "*" + dataSource : dataSource;
-    }
-
     @Override
     public Void bigIntegerShape(BigIntegerShape shape) {
         // Fail instead of losing precision through Number.
@@ -184,7 +178,7 @@ public Void bigDecimalShape(BigDecimalShape shape) {
         return null;
     }
 
-    private String unsupportedShape(Shape shape) {
+    private void unsupportedShape(Shape shape) {
         throw new CodegenException(String.format("Cannot serialize shape type %s on protocol, shape: %s.",
                 shape.getType(), shape.getId()));
     }
@@ -248,8 +242,10 @@ public Void mapShape(MapShape shape) {
     private void writeDelegateFunction(Shape shape) {
         String serFunctionName = ProtocolGenerator.getDocumentSerializerFunctionName(shape, context.getProtocolName());
         GoWriter writer = context.getWriter();
-        writer.openBlock("if err := $L($L, $L); err != nil {", "}", serFunctionName, dataSource, dataDest, () -> {
-            writer.write("return err");
+
+        ProtocolUtils.writeSerDelegateFunction(context, writer, member, dataSource, (srcVar) -> {
+            writer.openBlock("if err := $L($L, $L); err != nil {", "}", serFunctionName, srcVar, dataDest,
+                    () -> writer.write("return err"));
         });
     }
 }

codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/JsonMemberDeserVisitor.java

@@ -22,6 +22,8 @@
 import software.amazon.smithy.go.codegen.SmithyGoDependency;
 import software.amazon.smithy.go.codegen.integration.ProtocolGenerator;
 import software.amazon.smithy.go.codegen.integration.ProtocolGenerator.GenerationContext;
+import software.amazon.smithy.go.codegen.integration.ProtocolUtils;
+import software.amazon.smithy.go.codegen.knowledge.GoPointableIndex;
 import software.amazon.smithy.model.shapes.BigDecimalShape;
 import software.amazon.smithy.model.shapes.BigIntegerShape;
 import software.amazon.smithy.model.shapes.BlobShape;
@@ -55,13 +57,17 @@
  */
 public class JsonMemberDeserVisitor implements ShapeVisitor<Void> {
     private final GenerationContext context;
+    private final MemberShape member;
     private final String dataDest;
     private final Format timestampFormat;
+    private final GoPointableIndex pointableIndex;
 
-    public JsonMemberDeserVisitor(GenerationContext context, String dataDest, Format timestampFormat) {
+    public JsonMemberDeserVisitor(GenerationContext context, MemberShape member, String dataDest, Format timestampFormat) {
         this.context = context;
+        this.member = member;
         this.dataDest = dataDest;
         this.timestampFormat = timestampFormat;
+        this.pointableIndex = GoPointableIndex.of(context.getModel());
     }
 
     @Override
@@ -70,13 +76,20 @@ public Void blobShape(BlobShape shape) {
         writer.addUseImports(SmithyGoDependency.FMT);
         writer.addUseImports(SmithyGoDependency.BASE64);
         final String typeError = "return fmt.Errorf(\"expected $L to be []byte, got %T instead\", value)";
+
         writer.openBlock("if value != nil {", "}", () -> {
             writer.write("jtv, ok := value.(string)");
-            writer.openBlock("if !ok {", "}", () -> writer.write(typeError, shape.getId().getName()));
+            writer.openBlock("if !ok {", "}", () -> {
+                writer.write(typeError, shape.getId().getName());
+            });
+
             writer.write("dv, err := base64.StdEncoding.DecodeString(jtv)");
-            writer.openBlock("if err != nil {", "}", () -> writer.write("return fmt.Errorf(\"failed to base64 decode "
-                    + "$L, %w\", err)", shape.getId().getName()));
-            writer.write("$L = dv", dataDest);
+            writer.openBlock("if err != nil {", "}", () -> {
+                writer.write("return fmt.Errorf(\"failed to base64 decode $L, %w\", err)", shape.getId().getName());
+            });
+
+            writer.write("$L = $L", dataDest, CodegenUtils.getAsPointerIfPointable(context.getModel(),
+                    context.getWriter(), pointableIndex, member, "dv"));
         });
         return null;
     }
@@ -91,7 +104,8 @@ public Void booleanShape(BooleanShape shape) {
                 writer.write("return fmt.Errorf(\"expected $L to be of type *bool, got %T instead\", value)",
                         shape.getId().getName());
             });
-            writer.write("$L = &jtv", dataDest);
+            writer.write("$L = $L", dataDest, CodegenUtils.getAsPointerIfPointable(context.getModel(),
+                    context.getWriter(), pointableIndex, member, "jtv"));
         });
         return null;
     }
@@ -100,27 +114,31 @@ public Void booleanShape(BooleanShape shape) {
     public Void byteShape(ByteShape shape) {
         GoWriter writer = context.getWriter();
         // Smithy's byte shape represents a signed 8-bit int, which doesn't line up with Go's unsigned byte
-        handleInteger(shape, CodegenUtils.generatePointerValueIfPointable(writer, shape, "int8(i64)"));
+        handleInteger(shape, CodegenUtils.getAsPointerIfPointable(context.getModel(), writer,
+                pointableIndex, member, "int8(i64)"));
         return null;
     }
 
     @Override
     public Void shortShape(ShortShape shape) {
         GoWriter writer = context.getWriter();
-        handleInteger(shape, CodegenUtils.generatePointerValueIfPointable(writer, shape, "int16(i64)"));
+        handleInteger(shape, CodegenUtils.getAsPointerIfPointable(context.getModel(), writer,
+                pointableIndex, member, "int16(i64)"));
         return null;
     }
 
     @Override
     public Void integerShape(IntegerShape shape) {
         GoWriter writer = context.getWriter();
-        handleInteger(shape, CodegenUtils.generatePointerValueIfPointable(writer, shape, "int32(i64)"));
+        handleInteger(shape, CodegenUtils.getAsPointerIfPointable(context.getModel(), writer,
+                pointableIndex, member, "int32(i64)"));
         return null;
     }
 
     @Override
     public Void longShape(LongShape shape) {
-        handleInteger(shape, "&i64");
+        handleInteger(shape, CodegenUtils.getAsPointerIfPointable(context.getModel(), context.getWriter(),
+                pointableIndex, member, "i64"));
         return null;
     }
 
@@ -165,14 +183,15 @@ private void handleNumber(Shape shape, Runnable r) {
 
     @Override
     public Void floatShape(FloatShape shape) {
-        GoWriter writer = context.getWriter();
-        handleFloat(shape, CodegenUtils.generatePointerValueIfPointable(writer, shape, "float32(f64)"));
+        handleFloat(shape, CodegenUtils.getAsPointerIfPointable(context.getModel(), context.getWriter(),
+                pointableIndex, member, "float32(f64)"));
         return null;
     }
 
     @Override
     public Void doubleShape(DoubleShape shape) {
-        handleFloat(shape, "&f64");
+        handleFloat(shape, CodegenUtils.getAsPointerIfPointable(context.getModel(), context.getWriter(),
+                pointableIndex, member, "f64"));
         return null;
     }
 
@@ -201,7 +220,8 @@ public Void stringShape(StringShape shape) {
         if (shape.hasTrait(EnumTrait.class)) {
             handleString(shape, () -> writer.write("$L = $P(jtv)", dataDest, symbol));
         } else {
-            handleString(shape, () -> writer.write("$L = &jtv", dataDest));
+            handleString(shape, () -> writer.write("$L = $L", dataDest, CodegenUtils.getAsPointerIfPointable(
+                    context.getModel(), context.getWriter(), pointableIndex, member, "jtv")));
         }
 
         return null;
@@ -239,19 +259,22 @@ public Void timestampShape(TimestampShape shape) {
                 handleString(shape, () -> {
                     writer.write("t, err := smithytime.ParseDateTime(jtv)");
                     writer.write("if err != nil { return err }");
-                    writer.write("$L = &t", dataDest);
+                    writer.write("$L = $L", dataDest, CodegenUtils.getAsPointerIfPointable(context.getModel(),
+                            context.getWriter(), pointableIndex, member, "t"));
                 });
                 break;
             case HTTP_DATE:
                 handleString(shape, () -> {
                     writer.write("t, err := smithytime.ParseHTTPDate(jtv)");
                     writer.write("if err != nil { return err }");
-                    writer.write("$L = &t", dataDest);
+                    writer.write("$L = $L", dataDest, CodegenUtils.getAsPointerIfPointable(context.getModel(),
+                            context.getWriter(), pointableIndex, member, "t"));
                 });
                 break;
             case EPOCH_SECONDS:
                 writer.addUseImports(SmithyGoDependency.SMITHY_PTR);
-                handleFloat(shape, "ptr.Time(smithytime.ParseEpochSeconds(f64))");
+                handleFloat(shape, CodegenUtils.getAsPointerIfPointable(context.getModel(), context.getWriter(),
+                        pointableIndex, member, "smithytime.ParseEpochSeconds(f64)"));
                 break;
             default:
                 throw new CodegenException(String.format("Unknown timestamp format %s", timestampFormat));
@@ -340,8 +363,11 @@ public Void mapShape(MapShape shape) {
     private void writeDelegateFunction(Shape shape) {
         String functionName = ProtocolGenerator.getDocumentDeserializerFunctionName(shape, context.getProtocolName());
         GoWriter writer = context.getWriter();
-        writer.openBlock("if err := $L(&$L, value); err != nil {", "}", functionName, dataDest, () -> {
-            writer.write("return err");
+
+        ProtocolUtils.writeDeserDelegateFunction(context, writer, member, dataDest, (destVar) -> {
+            writer.openBlock("if err := $L(&$L, value); err != nil {", "}", functionName, destVar, () -> {
+                writer.write("return err");
+            });
         });
     }
 }