From 053afb299f04a636cf6d6c01195681deb2640c9c Mon Sep 17 00:00:00 2001 From: Bevan Arps Date: Mon, 2 Aug 2021 08:36:24 +1200 Subject: [PATCH] Code gardening for code generation (#1672) * Allocate locals only when we know we need them * Use helper method * Eliminate EnsureStatementBlock() and subsume into StatementBlock() * Allow SimpleIfElse() to handle multiple statements per branch * Use simplified SimpleIfElse() * Change SimpleAssignment() to always do assignment * Introduce SimpleDeclaration to always do variable := definition * fixup! Change SimpleAssignment() to always do assignment * fixup! Change SimpleAssignment() to always do assignment * Introduce SetVariable() * Modify SimpleDeclaration() to take string for variable name * Rename method to ShortDeclaration() * Renamed to `AssignmentStatement()` --- hack/generator/pkg/astbuilder/assignments.go | 43 ++++++++++++-- hack/generator/pkg/astbuilder/builder.go | 19 +++---- hack/generator/pkg/astbuilder/conditions.go | 6 +- hack/generator/pkg/astbuilder/lists.go | 4 +- hack/generator/pkg/astbuilder/maps.go | 4 +- .../convert_from_arm_function_builder.go | 3 +- .../astmodel/conversion_function_builder.go | 9 ++- .../kubernetes_admissions_defaults.go | 1 - .../kubernetes_admissions_validator.go | 8 +-- .../astmodel/kubernetes_resource_interface.go | 1 - .../pipeline/resource_registration_file.go | 34 ++--------- .../pkg/conversions/property_conversions.go | 57 ++++++++----------- .../writable_conversion_endpoint.go | 2 - .../functions/resource_conversion_function.go | 13 ++--- .../object_type_serialization_test_case.go | 43 ++++++-------- 15 files changed, 111 insertions(+), 136 deletions(-) diff --git a/hack/generator/pkg/astbuilder/assignments.go b/hack/generator/pkg/astbuilder/assignments.go index 01ec66af0e1..0a77482d51f 100644 --- a/hack/generator/pkg/astbuilder/assignments.go +++ b/hack/generator/pkg/astbuilder/assignments.go @@ -6,26 +6,61 @@ package astbuilder import ( + "fmt" "go/token" "github.com/dave/dst" ) // SimpleAssignment performs a simple assignment like: -// := // tok = token.DEFINE -// or = // tok = token.ASSIGN -func SimpleAssignment(lhs dst.Expr, tok token.Token, rhs dst.Expr) *dst.AssignStmt { +// +// = +// +// See also ShortDeclaration +func SimpleAssignment(lhs dst.Expr, rhs dst.Expr) *dst.AssignStmt { return &dst.AssignStmt{ Lhs: []dst.Expr{ dst.Clone(lhs).(dst.Expr), }, - Tok: tok, + Tok: token.ASSIGN, Rhs: []dst.Expr{ dst.Clone(rhs).(dst.Expr), }, } } +// ShortDeclaration performs a simple assignment like: +// +// := +// +// Method naming inspired by https://tour.golang.org/basics/10 +func ShortDeclaration(id string, rhs dst.Expr) *dst.AssignStmt { + return &dst.AssignStmt{ + Lhs: []dst.Expr{ + dst.NewIdent(id), + }, + Tok: token.DEFINE, + Rhs: []dst.Expr{ + dst.Clone(rhs).(dst.Expr), + }, + } +} + +// AssignmentStatement allows for either variable declaration or assignment by passing the required token +// Only token.DEFINE and token.ASSIGN are supported, other values will panic. +// Use SimpleAssignment or ShortDeclaration if possible; use this method only if you must. +func AssignmentStatement(lhs dst.Expr, tok token.Token, rhs dst.Expr) *dst.AssignStmt { + if tok != token.ASSIGN && tok != token.DEFINE { + panic(fmt.Sprintf("token %q not supported in VariableAssignment", tok)) + } + + return &dst.AssignStmt{ + Lhs: Expressions(lhs), + Tok: tok, + Rhs: Expressions(rhs), + } +} + // QualifiedAssignment performs a simple assignment like: // . := // tok = token.DEFINE // or . = // tok = token.ASSIGN diff --git a/hack/generator/pkg/astbuilder/builder.go b/hack/generator/pkg/astbuilder/builder.go index 4c90207859b..9534d81445d 100644 --- a/hack/generator/pkg/astbuilder/builder.go +++ b/hack/generator/pkg/astbuilder/builder.go @@ -386,20 +386,19 @@ func Nil() *dst.Ident { } // StatementBlock generates a block containing the supplied statements +// If we're given a single statement that's already a block, we won't double wrap it func StatementBlock(statements ...dst.Stmt) *dst.BlockStmt { - return &dst.BlockStmt{ - List: Statements(statements), - } -} + stmts := Statements(statements) -// EnsureStatementBlock wraps any statement into a block safely -// (without double wrapping an existing block) -func EnsureStatementBlock(statement dst.Stmt) *dst.BlockStmt { - if block, ok := statement.(*dst.BlockStmt); ok { - return block + if len(stmts) == 1 { + if block, ok := stmts[0].(*dst.BlockStmt); ok { + return block + } } - return StatementBlock(statement) + return &dst.BlockStmt{ + List: stmts, + } } // Statements creates a sequence of statements from the provided values, each of which may be a diff --git a/hack/generator/pkg/astbuilder/conditions.go b/hack/generator/pkg/astbuilder/conditions.go index e29541e8dd9..5729fd401e0 100644 --- a/hack/generator/pkg/astbuilder/conditions.go +++ b/hack/generator/pkg/astbuilder/conditions.go @@ -19,11 +19,11 @@ import ( // // } // -func SimpleIfElse(condition dst.Expr, trueBranch dst.Stmt, falseBranch dst.Stmt) *dst.IfStmt { +func SimpleIfElse(condition dst.Expr, trueBranch []dst.Stmt, falseBranch []dst.Stmt) *dst.IfStmt { result := &dst.IfStmt{ Cond: condition, - Body: EnsureStatementBlock(trueBranch), - Else: EnsureStatementBlock(falseBranch), + Body: StatementBlock(trueBranch...), + Else: StatementBlock(falseBranch...), } return result diff --git a/hack/generator/pkg/astbuilder/lists.go b/hack/generator/pkg/astbuilder/lists.go index 4efa46730f8..889fedb5383 100644 --- a/hack/generator/pkg/astbuilder/lists.go +++ b/hack/generator/pkg/astbuilder/lists.go @@ -6,8 +6,9 @@ package astbuilder import ( - "github.com/dave/dst" "go/token" + + "github.com/dave/dst" ) // MakeList returns the call expression for making a slice @@ -31,7 +32,6 @@ func MakeList(listType dst.Expr, len dst.Expr) *dst.CallExpr { func AppendList(lhs dst.Expr, rhs dst.Expr) dst.Stmt { return SimpleAssignment( dst.Clone(lhs).(dst.Expr), - token.ASSIGN, CallFunc("append", dst.Clone(lhs).(dst.Expr), dst.Clone(rhs).(dst.Expr))) } diff --git a/hack/generator/pkg/astbuilder/maps.go b/hack/generator/pkg/astbuilder/maps.go index 139b4b71ae0..5c5a0c7e150 100644 --- a/hack/generator/pkg/astbuilder/maps.go +++ b/hack/generator/pkg/astbuilder/maps.go @@ -6,8 +6,9 @@ package astbuilder import ( - "github.com/dave/dst" "go/token" + + "github.com/dave/dst" ) // MakeMap returns the call expression for making a map @@ -36,7 +37,6 @@ func InsertMap(mapExpr dst.Expr, key dst.Expr, rhs dst.Expr) *dst.AssignStmt { X: dst.Clone(mapExpr).(dst.Expr), Index: dst.Clone(key).(dst.Expr), }, - token.ASSIGN, dst.Clone(rhs).(dst.Expr)) } diff --git a/hack/generator/pkg/astmodel/armconversion/convert_from_arm_function_builder.go b/hack/generator/pkg/astmodel/armconversion/convert_from_arm_function_builder.go index 782b6c02d64..b86d6da5c8e 100644 --- a/hack/generator/pkg/astmodel/armconversion/convert_from_arm_function_builder.go +++ b/hack/generator/pkg/astmodel/armconversion/convert_from_arm_function_builder.go @@ -431,7 +431,7 @@ func (builder *convertFromARMBuilder) convertComplexTypeNameProperty(conversionB results = append(results, newVariable) results = append( results, - astbuilder.SimpleAssignment( + astbuilder.AssignmentStatement( dst.NewIdent("err"), tok, astbuilder.CallQualifiedFunc( @@ -442,7 +442,6 @@ func (builder *convertFromARMBuilder) convertComplexTypeNameProperty(conversionB results, astbuilder.SimpleAssignment( params.GetDestination(), - token.ASSIGN, dst.NewIdent(propertyLocalVar))) } else { results = append( diff --git a/hack/generator/pkg/astmodel/conversion_function_builder.go b/hack/generator/pkg/astmodel/conversion_function_builder.go index d080738d788..49ab97f3d22 100644 --- a/hack/generator/pkg/astmodel/conversion_function_builder.go +++ b/hack/generator/pkg/astmodel/conversion_function_builder.go @@ -210,7 +210,6 @@ func IdentityConvertComplexOptionalProperty(builder *ConversionFunctionBuilder, innerStatements, astbuilder.SimpleAssignment( params.GetDestination(), - token.ASSIGN, astbuilder.AddrOf(dst.NewIdent(tempVarIdent)))) result := &dst.IfStmt{ @@ -345,8 +344,8 @@ func IdentityConvertComplexMapProperty(builder *ConversionFunctionBuilder, param keyTypeAst := destinationType.KeyType().AsType(builder.CodeGenerationContext) valueTypeAst := destinationType.ValueType().AsType(builder.CodeGenerationContext) - makeMapStatement := astbuilder.SimpleAssignment( - dst.Clone(destination).(dst.Expr), + makeMapStatement := astbuilder.AssignmentStatement( + destination, makeMapToken, astbuilder.MakeMap(keyTypeAst, valueTypeAst)) rangeStatement := &dst.RangeStmt{ @@ -575,12 +574,12 @@ func IdentityDeepCopyJSON(builder *ConversionFunctionBuilder, params ConversionP // AssignmentHandlerDefine is an assignment handler for definitions, using := func AssignmentHandlerDefine(lhs dst.Expr, rhs dst.Expr) dst.Stmt { - return astbuilder.SimpleAssignment(lhs, token.DEFINE, rhs) + return astbuilder.AssignmentStatement(lhs, token.DEFINE, rhs) } // AssignmentHandlerAssign is an assignment handler for standard assignments to existing variables, using = func AssignmentHandlerAssign(lhs dst.Expr, rhs dst.Expr) dst.Stmt { - return astbuilder.SimpleAssignment(lhs, token.ASSIGN, rhs) + return astbuilder.SimpleAssignment(lhs, rhs) } // CreateLocal creates an unused local variable name. diff --git a/hack/generator/pkg/astmodel/kubernetes_admissions_defaults.go b/hack/generator/pkg/astmodel/kubernetes_admissions_defaults.go index 0dfcddd75c1..0c6f37c8aaf 100644 --- a/hack/generator/pkg/astmodel/kubernetes_admissions_defaults.go +++ b/hack/generator/pkg/astmodel/kubernetes_admissions_defaults.go @@ -60,7 +60,6 @@ func defaultAzureNameFunction(k *resourceFunction, codeGenerationContext *CodeGe Body: astbuilder.StatementBlock( astbuilder.SimpleAssignment( azureNameProp, - token.ASSIGN, nameProp)), }, }, diff --git a/hack/generator/pkg/astmodel/kubernetes_admissions_validator.go b/hack/generator/pkg/astmodel/kubernetes_admissions_validator.go index 5fcdf478380..1448367021f 100644 --- a/hack/generator/pkg/astmodel/kubernetes_admissions_validator.go +++ b/hack/generator/pkg/astmodel/kubernetes_admissions_validator.go @@ -266,7 +266,7 @@ func (v *ValidatorBuilder) validateBody(codeGenerationContext *CodeGenerationCon Tok: token.DEFINE, Body: &dst.BlockStmt{ List: []dst.Stmt{ - astbuilder.SimpleAssignment(dst.NewIdent("err"), token.DEFINE, astbuilder.CallFunc(validationIdent, args...)), + astbuilder.ShortDeclaration("err", astbuilder.CallFunc(validationIdent, args...)), astbuilder.CheckErrorAndSingleStatement(astbuilder.AppendList(dst.NewIdent(errsIdent), dst.NewIdent("err"))), }, }, @@ -279,9 +279,8 @@ func (v *ValidatorBuilder) validateBody(codeGenerationContext *CodeGenerationCon appendFuncCall.Ellipsis = true body := []dst.Stmt{ - astbuilder.SimpleAssignment( - dst.NewIdent(validationsIdent), - token.DEFINE, + astbuilder.ShortDeclaration( + validationsIdent, astbuilder.CallQualifiedFunc(receiverIdent, implFunctionName)), astbuilder.AssignToInterface(tempVarIdent, dst.NewIdent(receiverIdent)), &dst.IfStmt{ @@ -295,7 +294,6 @@ func (v *ValidatorBuilder) validateBody(codeGenerationContext *CodeGenerationCon // Not using astbuilder.AppendList here as we want to tack on a "..." at the end astbuilder.SimpleAssignment( dst.NewIdent(validationsIdent), - token.ASSIGN, appendFuncCall), }, }, diff --git a/hack/generator/pkg/astmodel/kubernetes_resource_interface.go b/hack/generator/pkg/astmodel/kubernetes_resource_interface.go index 540874cc144..6480eb10429 100644 --- a/hack/generator/pkg/astmodel/kubernetes_resource_interface.go +++ b/hack/generator/pkg/astmodel/kubernetes_resource_interface.go @@ -247,7 +247,6 @@ func setEnumAzureNameFunction(enumType TypeName) objectFunctionHandler { Body: []dst.Stmt{ astbuilder.SimpleAssignment( azureNameProp, - token.ASSIGN, &dst.CallExpr{ // cast from the string value to the enum Fun: enumTypeAST, diff --git a/hack/generator/pkg/codegen/pipeline/resource_registration_file.go b/hack/generator/pkg/codegen/pipeline/resource_registration_file.go index 7802cc5885c..68fd705d906 100644 --- a/hack/generator/pkg/codegen/pipeline/resource_registration_file.go +++ b/hack/generator/pkg/codegen/pipeline/resource_registration_file.go @@ -262,29 +262,11 @@ func (r *ResourceRegistrationFile) createCreateSchemeFunc(codeGenerationContext ignore := "_" addToScheme := "AddToScheme" - initSchemeVar := astbuilder.SimpleAssignment( - dst.NewIdent(scheme), - token.DEFINE, - &dst.CallExpr{ - Fun: &dst.SelectorExpr{ - X: dst.NewIdent(runtime), - Sel: dst.NewIdent("NewScheme"), - }, - Args: []dst.Expr{}, - }) + initSchemeVar := astbuilder.ShortDeclaration(scheme, astbuilder.CallQualifiedFunc(runtime, "NewScheme")) clientGoSchemeAssign := astbuilder.SimpleAssignment( dst.NewIdent(ignore), - token.ASSIGN, - &dst.CallExpr{ - Fun: &dst.SelectorExpr{ - X: dst.NewIdent(clientGoScheme), - Sel: dst.NewIdent(addToScheme), - }, - Args: []dst.Expr{ - dst.NewIdent(scheme), - }, - }) + astbuilder.CallQualifiedFunc(clientGoScheme, addToScheme, dst.NewIdent(scheme))) var importedPackageNames []string for pkg := range r.getImportedPackages() { @@ -304,16 +286,8 @@ func (r *ResourceRegistrationFile) createCreateSchemeFunc(codeGenerationContext for _, group := range importedPackageNames { groupSchemeAssign := astbuilder.SimpleAssignment( dst.NewIdent(ignore), - token.ASSIGN, - &dst.CallExpr{ - Fun: &dst.SelectorExpr{ - X: dst.NewIdent(group), - Sel: dst.NewIdent(addToScheme), - }, - Args: []dst.Expr{ - dst.NewIdent(scheme), - }, - }) + astbuilder.CallQualifiedFunc(group, addToScheme, dst.NewIdent(scheme))) + groupVersionAssignments = append(groupVersionAssignments, groupSchemeAssign) } diff --git a/hack/generator/pkg/conversions/property_conversions.go b/hack/generator/pkg/conversions/property_conversions.go index c4d5bf88afc..f2894f9af79 100644 --- a/hack/generator/pkg/conversions/property_conversions.go +++ b/hack/generator/pkg/conversions/property_conversions.go @@ -175,8 +175,6 @@ func assignToOptional( return nil } - local := destinationEndpoint.CreateLocal("", "Temp") - return func(reader dst.Expr, writer func(dst.Expr) []dst.Stmt, generationContext *astmodel.CodeGenerationContext) []dst.Stmt { // Create a writer that uses the address of the passed expression // If expr isn't a plain identifier (implying a local variable), we introduce one @@ -186,10 +184,11 @@ func assignToOptional( return writer(astbuilder.AddrOf(expr)) } - assignment := astbuilder.SimpleAssignment( - dst.NewIdent(local), - token.DEFINE, - expr) + // Only obtain our local variable name after we know we need it + // (this avoids reserving the name and not using it, which can interact with other conversions) + local := destinationEndpoint.CreateLocal("", "Temp") + + assignment := astbuilder.ShortDeclaration(local, expr) writing := writer(astbuilder.AddrOf(dst.NewIdent(local))) @@ -232,8 +231,6 @@ func assignFromOptional( return nil } - local := sourceEndpoint.CreateLocal("", "Read") - return func(reader dst.Expr, writer func(dst.Expr) []dst.Stmt, generationContext *astmodel.CodeGenerationContext) []dst.Stmt { var cacheOriginal dst.Stmt var actualReader dst.Expr @@ -249,10 +246,12 @@ func assignFromOptional( actualReader = reader default: // Something else, so we cache the original - cacheOriginal = astbuilder.SimpleAssignment( - dst.NewIdent(local), - token.DEFINE, - reader) + + // Only obtain our local variable name after we know we need it + // (this avoids reserving the name and not using it, which can interact with other conversions) + local := sourceEndpoint.CreateLocal("", "Read") + + cacheOriginal = astbuilder.ShortDeclaration(local, reader) actualReader = dst.NewIdent(local) } @@ -267,11 +266,10 @@ func assignFromOptional( writeZeroValue := writer( destinationEndpoint.Type().AsZero(conversionContext.Types(), generationContext)) - stmt := &dst.IfStmt{ - Cond: checkForNil, - Body: astbuilder.StatementBlock(writeActualValue...), - Else: astbuilder.StatementBlock(writeZeroValue...), - } + stmt := astbuilder.SimpleIfElse( + checkForNil, + writeActualValue, + writeZeroValue) return astbuilder.Statements(cacheOriginal, stmt) } @@ -554,9 +552,8 @@ func assignArrayFromArray( indexId := sourceEndpoint.CreateLocal("Index") tempId := sourceEndpoint.CreateLocal("List") - declaration := astbuilder.SimpleAssignment( - dst.NewIdent(tempId), - token.DEFINE, + declaration := astbuilder.ShortDeclaration( + tempId, astbuilder.MakeList(destinationArray.AsType(generationContext), astbuilder.CallFunc("len", reader))) writeToElement := func(expr dst.Expr) []dst.Stmt { @@ -566,12 +563,11 @@ func assignArrayFromArray( X: dst.NewIdent(tempId), Index: dst.NewIdent(indexId), }, - token.ASSIGN, expr), } } - avoidAliasing := astbuilder.SimpleAssignment(dst.NewIdent(itemId), token.DEFINE, dst.NewIdent(itemId)) + avoidAliasing := astbuilder.ShortDeclaration(itemId, dst.NewIdent(itemId)) avoidAliasing.Decs.Start.Append("// Shadow the loop variable to avoid aliasing") avoidAliasing.Decs.Before = dst.NewLine @@ -638,9 +634,8 @@ func assignMapFromMap( keyId := sourceEndpoint.CreateLocal("Key") tempId := sourceEndpoint.CreateLocal("Map") - declaration := astbuilder.SimpleAssignment( - dst.NewIdent(tempId), - token.DEFINE, + declaration := astbuilder.ShortDeclaration( + tempId, astbuilder.MakeMap(destinationMap.KeyType().AsType(generationContext), destinationMap.ValueType().AsType(generationContext))) assignToItem := func(expr dst.Expr) []dst.Stmt { @@ -650,12 +645,11 @@ func assignMapFromMap( X: dst.NewIdent(tempId), Index: dst.NewIdent(keyId), }, - token.ASSIGN, expr), } } - avoidAliasing := astbuilder.SimpleAssignment(dst.NewIdent(itemId), token.DEFINE, dst.NewIdent(itemId)) + avoidAliasing := astbuilder.ShortDeclaration(itemId, dst.NewIdent(itemId)) avoidAliasing.Decs.Start.Append("// Shadow the loop variable to avoid aliasing") avoidAliasing.Decs.Before = dst.NewLine @@ -719,10 +713,7 @@ func assignEnumFromEnum( local := destinationEndpoint.CreateLocal("", "As"+destinationName.Name(), "Value") return func(reader dst.Expr, writer func(dst.Expr) []dst.Stmt, ctx *astmodel.CodeGenerationContext) []dst.Stmt { result := []dst.Stmt{ - astbuilder.SimpleAssignment( - dst.NewIdent(local), - token.DEFINE, - astbuilder.CallFunc(destinationName.Name(), reader)), + astbuilder.ShortDeclaration(local, astbuilder.CallFunc(destinationName.Name(), reader)), } result = append(result, writer(dst.NewIdent(local))...) @@ -864,13 +855,13 @@ func assignObjectFromObject( var conversion dst.Stmt if destinationName.PackageReference.Equals(generationContext.CurrentPackage()) { // Destination is our current type - conversion = astbuilder.SimpleAssignment( + conversion = astbuilder.AssignmentStatement( errLocal, tok, astbuilder.CallExpr(localId, functionName, actualReader)) } else { // Destination is another type - conversion = astbuilder.SimpleAssignment( + conversion = astbuilder.AssignmentStatement( errLocal, tok, astbuilder.CallExpr(reader, functionName, astbuilder.AddrOf(localId))) diff --git a/hack/generator/pkg/conversions/writable_conversion_endpoint.go b/hack/generator/pkg/conversions/writable_conversion_endpoint.go index 9ad424a7a3c..45e75e8ba90 100644 --- a/hack/generator/pkg/conversions/writable_conversion_endpoint.go +++ b/hack/generator/pkg/conversions/writable_conversion_endpoint.go @@ -7,7 +7,6 @@ package conversions import ( "fmt" - "go/token" "github.com/dave/dst" @@ -39,7 +38,6 @@ func MakeWritableConversionEndpointForProperty( return []dst.Stmt{ astbuilder.SimpleAssignment( astbuilder.Selector(destination, propertyName), - token.ASSIGN, value), } }, diff --git a/hack/generator/pkg/functions/resource_conversion_function.go b/hack/generator/pkg/functions/resource_conversion_function.go index 3b10ddb5b41..6e899916c08 100644 --- a/hack/generator/pkg/functions/resource_conversion_function.go +++ b/hack/generator/pkg/functions/resource_conversion_function.go @@ -7,7 +7,6 @@ package functions import ( "fmt" - "go/token" "github.com/dave/dst" @@ -197,9 +196,8 @@ func (fn *ResourceConversionFunction) indirectConversionFromHub( localId, intermediateType.AsType(generationContext), "// intermediate variable for conversion") declareLocal.Decorations().Before = dst.NewLine - populateLocalFromHub := astbuilder.SimpleAssignment( - errIdent, - token.DEFINE, + populateLocalFromHub := astbuilder.ShortDeclaration( + "err", astbuilder.CallExpr(dst.NewIdent(localId), fn.Name(), dst.NewIdent("hub"))) populateLocalFromHub.Decs.Before = dst.EmptyLine @@ -209,7 +207,6 @@ func (fn *ResourceConversionFunction) indirectConversionFromHub( populateReceiverFromLocal := astbuilder.SimpleAssignment( errIdent, - token.ASSIGN, astbuilder.CallExpr(dst.NewIdent(receiverName), fn.propertyFunction.Name(), astbuilder.AddrOf(dst.NewIdent(localId)))) populateReceiverFromLocal.Decs.Before = dst.EmptyLine @@ -257,9 +254,8 @@ func (fn *ResourceConversionFunction) indirectConversionToHub( localId, intermediateType.AsType(generationContext), "// intermediate variable for conversion") declareLocal.Decorations().Before = dst.NewLine - populateLocalFromReceiver := astbuilder.SimpleAssignment( - errIdent, - token.DEFINE, + populateLocalFromReceiver := astbuilder.ShortDeclaration( + "err", astbuilder.CallExpr(dst.NewIdent(receiverName), fn.propertyFunction.Name(), astbuilder.AddrOf(dst.NewIdent(localId)))) checkForErrorsPopulatingLocal := astbuilder.CheckErrorAndWrap( @@ -268,7 +264,6 @@ func (fn *ResourceConversionFunction) indirectConversionToHub( populateHubFromLocal := astbuilder.SimpleAssignment( errIdent, - token.ASSIGN, astbuilder.CallExpr(dst.NewIdent(localId), fn.Name(), dst.NewIdent("hub"))) checkForErrorsPopulatingHub := astbuilder.CheckErrorAndWrap( diff --git a/hack/generator/pkg/testcases/object_type_serialization_test_case.go b/hack/generator/pkg/testcases/object_type_serialization_test_case.go index 30eb792eb07..18ae152c74a 100644 --- a/hack/generator/pkg/testcases/object_type_serialization_test_case.go +++ b/hack/generator/pkg/testcases/object_type_serialization_test_case.go @@ -178,9 +178,8 @@ func (o ObjectSerializationTestCase) createTestRunner(codegenContext *astmodel.C t := dst.NewIdent("t") // parameters := gopter.DefaultTestParameters() - defineParameters := astbuilder.SimpleAssignment( - dst.NewIdent(parametersLocal), - token.DEFINE, + defineParameters := astbuilder.ShortDeclaration( + parametersLocal, astbuilder.CallQualifiedFunc(gopterPackage, "DefaultTestParameters")) // parameters.MaxSize = 10 @@ -191,9 +190,8 @@ func (o ObjectSerializationTestCase) createTestRunner(codegenContext *astmodel.C astbuilder.IntLiteral(10)) // properties := gopter.NewProperties(parameters) - defineProperties := astbuilder.SimpleAssignment( - dst.NewIdent(propertiesLocal), - token.DEFINE, + defineProperties := astbuilder.ShortDeclaration( + propertiesLocal, astbuilder.CallQualifiedFunc(gopterPackage, "NewProperties", dst.NewIdent(parametersLocal))) // partial expression: description of the test @@ -265,7 +263,6 @@ func (o ObjectSerializationTestCase) createTestMethod(codegenContext *astmodel.C // err = json.Unmarshal(bin, &actual) deserialize := astbuilder.SimpleAssignment( dst.NewIdent("err"), - token.ASSIGN, astbuilder.CallQualifiedFunc(jsonPackage, "Unmarshal", dst.NewIdent(binId), &dst.UnaryExpr{ @@ -280,9 +277,8 @@ func (o ObjectSerializationTestCase) createTestMethod(codegenContext *astmodel.C // match := cmp.Equal(subject, actual, cmpopts.EquateEmpty()) equateEmpty := astbuilder.CallQualifiedFunc(cmpoptsPackage, "EquateEmpty") - compare := astbuilder.SimpleAssignment( - dst.NewIdent(matchId), - token.DEFINE, + compare := astbuilder.ShortDeclaration( + matchId, astbuilder.CallQualifiedFunc(cmpPackage, "Equal", dst.NewIdent(subjectId), dst.NewIdent(actualId), @@ -296,17 +292,14 @@ func (o ObjectSerializationTestCase) createTestMethod(codegenContext *astmodel.C }, Body: &dst.BlockStmt{ List: []dst.Stmt{ - astbuilder.SimpleAssignment( - dst.NewIdent(actualFmtId), - token.DEFINE, + astbuilder.ShortDeclaration( + actualFmtId, astbuilder.CallQualifiedFunc(prettyPackage, "Sprint", dst.NewIdent(actualId))), - astbuilder.SimpleAssignment( - dst.NewIdent(subjectFmtId), - token.DEFINE, + astbuilder.ShortDeclaration( + subjectFmtId, astbuilder.CallQualifiedFunc(prettyPackage, "Sprint", dst.NewIdent(subjectId))), - astbuilder.SimpleAssignment( - dst.NewIdent(resultId), - token.DEFINE, + astbuilder.ShortDeclaration( + resultId, astbuilder.CallQualifiedFunc(diffPackage, "Diff", dst.NewIdent(subjectFmtId), dst.NewIdent(actualFmtId))), astbuilder.Returns(dst.NewIdent(resultId)), }, @@ -390,9 +383,8 @@ func (o ObjectSerializationTestCase) createGeneratorMethod(ctx *astmodel.CodeGen // Create a simple version of the generator that does not reference generators for related types // This serves to terminate any dependency cycles that might occur during creation of a more fully fledged generator - makeIndependentMap := astbuilder.SimpleAssignment( - dst.NewIdent("independentGenerators"), - token.DEFINE, + makeIndependentMap := astbuilder.ShortDeclaration( + "independentGenerators", astbuilder.MakeMap( dst.NewIdent("string"), astbuilder.QualifiedTypeName(gopterPackage, "Gen"))) @@ -403,7 +395,6 @@ func (o ObjectSerializationTestCase) createGeneratorMethod(ctx *astmodel.CodeGen createIndependentGenerator := astbuilder.SimpleAssignment( dst.NewIdent(o.idOfSubjectGeneratorGlobal()), - token.ASSIGN, astbuilder.CallQualifiedFunc( genPackage, "Struct", @@ -418,9 +409,8 @@ func (o ObjectSerializationTestCase) createGeneratorMethod(ctx *astmodel.CodeGen // Have to call the factory method twice as the simple generator above has captured the map; // if we reuse or modify the map, chaos ensues. - makeAllMap := astbuilder.SimpleAssignment( - dst.NewIdent("allGenerators"), - token.DEFINE, + makeAllMap := astbuilder.ShortDeclaration( + "allGenerators", astbuilder.MakeMap( dst.NewIdent("string"), astbuilder.QualifiedTypeName(gopterPackage, "Gen"))) @@ -440,7 +430,6 @@ func (o ObjectSerializationTestCase) createGeneratorMethod(ctx *astmodel.CodeGen createFullGenerator := astbuilder.SimpleAssignment( dst.NewIdent(o.idOfSubjectGeneratorGlobal()), - token.ASSIGN, astbuilder.CallQualifiedFunc( genPackage, "Struct",