Fix various codegen bugs (#1017)

* Fix various codegen bugs

Ensure unique var names when splitting arrays.
Fix fake type name generation for slices when slice-elements-byval is
enabled.
Fix time serde codegen when slice-elements-byval is enabled.
Include method parameter types when generating fake polymorphic helpers.
Fix fake codegen for types that coalesce to string.

* add tests

* bump version for impending release

.scripts/regeneration.js

@@ -128,7 +128,7 @@ generateFromReadme("armdataboxedge", databoxedge, 'package-2021-02-01', 'test/da
 const acr = './swagger/specification/containerregistry/data-plane/Azure.ContainerRegistry/stable/2021-07-01/containerregistry.json';
 generate("azacr", acr, 'test/acr/azacr', '--module="azacr" --module-version=0.1.0 --openapi-type="data-plane" --rawjson-as-bytes --generate-fakes --azcore-version=1.8.0-beta.2');
 
-generate("azalias", 'packages/autorest.go/test/swagger/alias.json', 'test/maps/azalias', '--security=AzureKey --module="azalias" --module-version=0.1.0 --openapi-type="data-plane" --generate-fakes --inject-spans --azcore-version=1.8.0-beta.2');
+generate("azalias", 'packages/autorest.go/test/swagger/alias.json', 'test/maps/azalias', '--security=AzureKey --module="azalias" --module-version=0.1.0 --openapi-type="data-plane" --generate-fakes --inject-spans --azcore-version=1.8.0-beta.2 --slice-elements-byval');
 
 function should_generate(name) {
     if (filter !== undefined) {

packages/autorest.go/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@autorest/go",
-  "version": "4.0.0-preview.55",
+  "version": "4.0.0-preview.56",
   "description": "AutoRest Go Generator",
   "main": "dist/exports.js",
   "typings": "dist/exports.d.ts",

packages/autorest.go/src/generator/fake/servers.ts

@@ -555,11 +555,12 @@ function createParamGroupParams(clientPkg: string, op: Operation, imports: Impor
       if ((<ArraySchema>param.schema).elementType.type !== SchemaType.String) {
         const asArray = <ArraySchema>param.schema;
         imports.add('strings');
-        content += `\telements := strings.Split(${paramValue}, "${getArraySeparator(param)}")\n`;
+        const elementsParam = `${paramValue}Elements`;
+        content += `\t${elementsParam} := strings.Split(${paramValue}, "${getArraySeparator(param)}")\n`;
         const localVar = createLocalVariableName(param, 'Param');
         const toType = `${clientPkg}.${asArray.elementType.language.go!.name}`;
-        content += `\t${localVar} := make([]${toType}, len(elements))\n`;
-        content += '\tfor i := 0; i < len(elements); i++ {\n';
+        content += `\t${localVar} := make([]${toType}, len(${elementsParam}))\n`;
+        content += `\tfor i := 0; i < len(${elementsParam}); i++ {\n`;
         let fromVar: string;
         switch (asArray.elementType.type) {
           case SchemaType.Choice:
@@ -571,18 +572,18 @@ function createParamGroupParams(clientPkg: string, op: Operation, imports: Impor
                 imports.add('strconv');
                 fromVar = 'parsedInt';
                 content += `\t\tvar ${fromVar} int64\n`;
-                content += `\t\t${fromVar}, err = strconv.ParseInt(elements[i], 10, 32)\n`;
+                content += `\t\t${fromVar}, err = strconv.ParseInt(${elementsParam}[i], 10, 32)\n`;
                 content += '\t\tif err != nil {\n\t\t\treturn nil, err\n\t\t}\n';
                 break;
               case SchemaType.Number:
                 imports.add('strconv');
                 fromVar = 'parsedNum';
                 content += `\t\tvar ${fromVar} float64\n`;
-                content += `\t\t${fromVar}, err = strconv.ParseFloat(elements[i], 32)\n`;
+                content += `\t\t${fromVar}, err = strconv.ParseFloat(${elementsParam}[i], 32)\n`;
                 content += '\t\tif err != nil {\n\t\t\treturn nil, err\n\t\t}\n';
                 break;
               case SchemaType.String:
-                fromVar = 'elements[i]';
+                fromVar = `${elementsParam}[i]`;
                 break;
               default:
                 throw new Error(`unhandled array element choice type ${asChoice.choiceType.type}`);
@@ -612,8 +613,8 @@ function createParamGroupParams(clientPkg: string, op: Operation, imports: Impor
       imports.add('encoding/base64');
       content += `\t${createLocalVariableName(param, 'Param')}, err := base64.StdEncoding.DecodeString(${paramValue})\n`;
       content += '\tif err != nil {\n\t\treturn nil, err\n\t}\n';
-    } else if (param.language.go!.paramGroup && (param.schema.type === SchemaType.Choice || param.schema.type === SchemaType.Constant || param.schema.type === SchemaType.Duration || param.schema.type === SchemaType.SealedChoice || param.schema.type === SchemaType.String)) {
-      // for params that don't require parsing, the value is inlined into the invocation of the fake.
+    } else if (param.language.go!.paramGroup && (param.schema.type === SchemaType.ArmId || param.schema.type === SchemaType.Choice || param.schema.type === SchemaType.Constant || param.schema.type === SchemaType.Credential || param.schema.type === SchemaType.Duration || param.schema.type === SchemaType.SealedChoice || param.schema.type === SchemaType.String || param.schema.type === SchemaType.Uuid || param.schema.type === SchemaType.Uri)) {
+      // for params that don't require parsing (all param types that coalesce to string), the value is inlined into the invocation of the fake.
       // but if it's grouped, then we need to create a local first which will later be copied into the param group.
       content += `\t${createLocalVariableName(param, 'Param')} := `;
       let paramValue = getParamValueWithCast(clientPkg, param, imports);

packages/autorest.go/src/generator/helpers.ts

@@ -68,7 +68,8 @@ export function formatTypeName(schema: Schema, pkgName?: string): string {
   // for array/dictionary, we need to splice the package name into the correct location
   const elementType = unwrapSchemaType(schema);
   if (elementType.type === SchemaType.Choice || elementType.type === SchemaType.SealedChoice || elementType.type === SchemaType.Object) {
-    return typeName.replace(`*${elementType.language.go!.name}`, `*${pkgName}.${elementType.language.go!.name}`);
+    // don't use a * prefix on the type name as --slice-elements-byval will remove it for slices
+    return typeName.replace(`${elementType.language.go!.name}`, `${pkgName}.${elementType.language.go!.name}`);
   }
 
   return typeName;

packages/autorest.go/src/generator/models.ts

@@ -265,9 +265,13 @@ function generateJSONMarshallerBody(obj: ObjectSchema, modelDef: ModelDef, impor
       marshaller += `\tpopulateByteArray(objectMap, "${prop.serializedName}", ${receiver}.${prop.language.go!.name}, runtime.Base64${base64Format}Format)\n`;
     } else if (isArraySchema(prop.schema) && prop.schema.elementType.language.go!.internalTimeType) {
       const source = `${receiver}.${prop.language.go!.name}`;
-      marshaller += `\taux := make([]*${prop.schema.elementType.language.go!.internalTimeType}, len(${source}), len(${source}))\n`;
+      let elementPtr = '';
+      if (<boolean>prop.schema.language.go!.elementIsPtr) {
+        elementPtr = '*';
+      }
+      marshaller += `\taux := make([]${elementPtr}${prop.schema.elementType.language.go!.internalTimeType}, len(${source}), len(${source}))\n`;
       marshaller += `\tfor i := 0; i < len(${source}); i++ {\n`;
-      marshaller += `\t\taux[i] = (*${prop.schema.elementType.language.go!.internalTimeType})(${source}[i])\n`;
+      marshaller += `\t\taux[i] = (${elementPtr}${prop.schema.elementType.language.go!.internalTimeType})(${source}[i])\n`;
       marshaller += '\t}\n';
       marshaller += `\tpopulate(objectMap, "${prop.serializedName}", aux)\n`;
     } else if (prop.schema.type === SchemaType.Constant) {
@@ -365,10 +369,14 @@ function generateJSONUnmarshallerBody(modelDef: ModelDef, imports: ImportManager
       unmarshalBody += `\t\t\t\terr = unpopulate${capitalize(prop.schema.language.go!.internalTimeType)}(val, "${prop.language.go!.name}", &${receiver}.${prop.language.go!.name})\n`;
     } else if (isArraySchema(prop.schema) && prop.schema.elementType.language.go!.internalTimeType) {
       imports.add('time');
-      unmarshalBody += `\t\t\tvar aux []*${prop.schema.elementType.language.go!.internalTimeType}\n`;
+      let elementPtr = '';
+      if (<boolean>prop.schema.language.go!.elementIsPtr) {
+        elementPtr = '*';
+      }
+      unmarshalBody += `\t\t\tvar aux []${elementPtr}${prop.schema.elementType.language.go!.internalTimeType}\n`;
       unmarshalBody += `\t\t\terr = unpopulate(val, "${prop.language.go!.name}", &aux)\n`;
       unmarshalBody += '\t\t\tfor _, au := range aux {\n';
-      unmarshalBody += `\t\t\t\t${receiver}.${prop.language.go!.name} = append(${receiver}.${prop.language.go!.name}, (*time.Time)(au))\n`;
+      unmarshalBody += `\t\t\t\t${receiver}.${prop.language.go!.name} = append(${receiver}.${prop.language.go!.name}, (${elementPtr}time.Time)(au))\n`;
       unmarshalBody += '\t\t\t}\n';
     } else if (prop.schema.type === SchemaType.ByteArray) {
       let base64Format = 'Std';

packages/autorest.go/src/generator/polymorphics.ts

@@ -4,14 +4,14 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { Session } from '@autorest/extension-base';
-import { CodeModel, ObjectSchema, Property } from '@autorest/codemodel';
+import { CodeModel, ObjectSchema, Property, Schema } from '@autorest/codemodel';
 import { values } from '@azure-tools/linq';
 import { isArraySchema, isDictionarySchema, recursiveUnwrapArrayDictionary } from '../common/helpers';
-import { contentPreamble, getParentImport, sortAscending } from './helpers';
+import { contentPreamble, getMethodParameters, getParentImport, sortAscending } from './helpers';
 import { ImportManager } from './imports';
 
 // Creates the content in polymorphic_helpers.go
-export async function generatePolymorphicHelpers(session: Session<CodeModel>, packageName?: string): Promise<string> {
+export async function generatePolymorphicHelpers(session: Session<CodeModel>, fakeServerPkg?: string): Promise<string> {
   if (!session.model.language.go!.discriminators) {
     // no polymorphic types
     return '';
@@ -21,10 +21,10 @@ export async function generatePolymorphicHelpers(session: Session<CodeModel>, pa
     // all polymorphic types omitted
     return '';
   }
-  let text = await contentPreamble(session, packageName);
+  let text = await contentPreamble(session, fakeServerPkg);
   const imports = new ImportManager();
   imports.add('encoding/json');
-  if (packageName) {
+  if (fakeServerPkg) {
     // content is being generated into a separate package, add the necessary import
     imports.add(await getParentImport(session));
   }
@@ -42,17 +42,17 @@ export async function generatePolymorphicHelpers(session: Session<CodeModel>, pa
   const scalars = new Set<string>();
   const arrays = new Set<string>();
   const maps = new Set<string>();
-  const trackDisciminator = function(prop: Property) {
-    if (prop.schema.language.go!.discriminatorInterface) {
-      scalars.add(prop.schema.language.go!.discriminatorInterface);
-    } else if (isArraySchema(prop.schema)) {
-      const discriminatorInterface = recursiveUnwrapArrayDictionary(prop.schema).language.go!.discriminatorInterface;
+  const trackDisciminator = function(schema: Schema) {
+    if (schema.language.go!.discriminatorInterface) {
+      scalars.add(schema.language.go!.discriminatorInterface);
+    } else if (isArraySchema(schema)) {
+      const discriminatorInterface = recursiveUnwrapArrayDictionary(schema).language.go!.discriminatorInterface;
       if (discriminatorInterface) {
         scalars.add(discriminatorInterface);
         arrays.add(discriminatorInterface);
       }
-    } else if (isDictionarySchema(prop.schema)) {
-      const discriminatorInterface = recursiveUnwrapArrayDictionary(prop.schema).language.go!.discriminatorInterface;
+    } else if (isDictionarySchema(schema)) {
+      const discriminatorInterface = recursiveUnwrapArrayDictionary(schema).language.go!.discriminatorInterface;
       if (discriminatorInterface) {
         scalars.add(discriminatorInterface);
         maps.add(discriminatorInterface);
@@ -62,14 +62,25 @@ export async function generatePolymorphicHelpers(session: Session<CodeModel>, pa
   // calculate which discriminator helpers we actually need to generate
   for (const obj of values(session.model.schemas.objects)) {
     for (const prop of values(obj.properties)) {
-      trackDisciminator(prop);
+      trackDisciminator(prop.schema);
     }
   }
   for (const respEnv of values(<Array<ObjectSchema>>session.model.language.go!.responseEnvelopes)) {
     if (respEnv.language.go!.resultProp) {
       const resultProp = <Property>respEnv.language.go!.resultProp;
       if (resultProp.isDiscriminator) {
-        trackDisciminator(resultProp);
+        trackDisciminator(resultProp.schema);
+      }
+    }
+  }
+  if (fakeServerPkg) {
+    // when generating for the fakes server, we must also look at operation parameters
+    for (const group of values(session.model.operationGroups)) {
+      for (const op of values(group.operations)) {
+        const params = getMethodParameters(op);
+        for (const param of values(params)) {
+          trackDisciminator(param.schema);
+        }
       }
     }
   }
@@ -83,7 +94,7 @@ export async function generatePolymorphicHelpers(session: Session<CodeModel>, pa
   discriminators.sort((a: ObjectSchema, b: ObjectSchema) => { return sortAscending(a.language.go!.discriminatorInterface, b.language.go!.discriminatorInterface); });
 
   let prefix = '';
-  if (packageName) {
+  if (fakeServerPkg) {
     // content is being generated into a separate package, set the type name prefix
     prefix = `${session.model.language.go!.packageName}.`;
   }

packages/autorest.go/test/maps/azalias/polymorphic_helpers_test.go

@@ -96,7 +96,7 @@ func TestGeoObjectNamedCollectionRoundTrip(t *testing.T) {
 
 func TestInterfaceRoundTrip(t *testing.T) {
 	props1 := ScheduleCreateOrUpdateProperties{
-		Aliases:     []*string{to.Ptr("foo")},
+		Aliases:     []string{"foo"},
 		Description: to.Ptr("funky"),
 		Interval:    false,
 		StartTime:   to.Ptr(time.Now().UTC()),
@@ -126,8 +126,8 @@ func TestInterfaceRoundTrip(t *testing.T) {
 	if *props1.StartTime != *props2.StartTime {
 		t.Fatalf("expected %v, got %v", *props1.StartTime, *props2.StartTime)
 	}
-	if *props1.Aliases[0] != *props2.Aliases[0] {
-		t.Fatalf("expected %v, got %v", *props1.Aliases[0], *props2.Aliases[0])
+	if props1.Aliases[0] != props2.Aliases[0] {
+		t.Fatalf("expected %v, got %v", props1.Aliases[0], props2.Aliases[0])
 	}
 }