From 7d395586a31a6d387b9e975122e83195c429870f Mon Sep 17 00:00:00 2001 From: George Pollard Date: Sun, 22 Nov 2020 22:44:14 +0000 Subject: [PATCH] Tweaks to field rendering --- hack/generator/pkg/astmodel/object_type.go | 6 ++++++ hack/generator/pkg/astmodel/property_definition.go | 7 ++++++- .../testdata/AllOf/AllOf_generates_wrapper_type.golden | 1 + .../Arm_test_dependent_resource_and_ownership.golden | 9 +++++++++ .../Arm_test_simple_resource_array_properties.golden | 5 +++++ .../Arm_test_simple_resource_complex_properties.golden | 5 +++++ .../Arm_test_simple_resource_json_fields.golden | 7 +++++++ .../Arm_test_simple_resource_map_properties.golden | 5 +++++ .../Arm_test_simple_resource_renders_spec.golden | 3 +++ .../EmbeddedTypes/Embedded_type_simple_resource.golden | 7 +++++++ .../testdata/EnumNames/Multi_valued_enum_name.golden | 3 +++ .../testdata/EnumNames/Single_valued_enum_name.golden | 3 +++ .../OneOf_generates_wrapper_for_inner_properties.golden | 1 - ...eOf_generates_wrapper_type_with_marshal_helper.golden | 1 - ...ype_with_named_properties_from_anonymous_types.golden | 1 - .../OneOf/OneOf_handles_properties_on_owning_node.golden | 1 - .../codegen/testdata/OneOf/OneOf_preserves_names.golden | 1 - .../testdata/Validations/Validate_property_type.golden | 3 +++ 18 files changed, 63 insertions(+), 6 deletions(-) diff --git a/hack/generator/pkg/astmodel/object_type.go b/hack/generator/pkg/astmodel/object_type.go index ee0cbdf49..304d26b81 100644 --- a/hack/generator/pkg/astmodel/object_type.go +++ b/hack/generator/pkg/astmodel/object_type.go @@ -168,6 +168,12 @@ func (objectType *ObjectType) AsType(codeGenerationContext *CodeGenerationContex fields = append(fields, f.AsField(codeGenerationContext)) } + if len(fields) > 0 { + // if first field has Before:EmptyLine decoration, switch it to NewLine + // this makes the output look nicer 🙂 + fields[0].Decs.Before = ast.NewLine + } + return &ast.StructType{ Fields: &ast.FieldList{ List: fields, diff --git a/hack/generator/pkg/astmodel/property_definition.go b/hack/generator/pkg/astmodel/property_definition.go index 217636ba5..1dcf9fe6d 100644 --- a/hack/generator/pkg/astmodel/property_definition.go +++ b/hack/generator/pkg/astmodel/property_definition.go @@ -255,12 +255,17 @@ func (property *PropertyDefinition) AsField(codeGenerationContext *CodeGeneratio AddValidationComments(&doc, validated.Validations().ToKubeBuilderValidations()) } + before := ast.NewLine + if len(doc) > 0 { + before = ast.EmptyLine + } + // We don't use StringLiteral() for the tag as it adds extra quotes result := &ast.Field{ Decs: ast.FieldDecorations{ NodeDecs: ast.NodeDecs{ - Before: ast.NewLine, Start: doc, + Before: before, }, }, Names: names, diff --git a/hack/generator/pkg/codegen/testdata/AllOf/AllOf_generates_wrapper_type.golden b/hack/generator/pkg/codegen/testdata/AllOf/AllOf_generates_wrapper_type.golden index 0dac84316..60e91b870 100644 --- a/hack/generator/pkg/codegen/testdata/AllOf/AllOf_generates_wrapper_type.golden +++ b/hack/generator/pkg/codegen/testdata/AllOf/AllOf_generates_wrapper_type.golden @@ -8,6 +8,7 @@ type Test struct { // +kubebuilder:validation:Required Enabled bool `json:"enabled"` Name *string `json:"name,omitempty"` + // +kubebuilder:validation:Required Size int `json:"size"` } diff --git a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_dependent_resource_and_ownership.golden b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_dependent_resource_and_ownership.golden index 2240279a0..ab6d9569c 100644 --- a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_dependent_resource_and_ownership.golden +++ b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_dependent_resource_and_ownership.golden @@ -54,8 +54,10 @@ type AList struct { type A_SpecArm struct { // +kubebuilder:validation:Required ApiVersion ASpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required Name string `json:"name"` + // +kubebuilder:validation:Required Type ASpecType `json:"type"` } @@ -121,8 +123,10 @@ type BList struct { type B_SpecArm struct { // +kubebuilder:validation:Required ApiVersion BSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required Name string `json:"name"` + // +kubebuilder:validation:Required Type BSpecType `json:"type"` } @@ -188,8 +192,10 @@ type CList struct { type C_SpecArm struct { // +kubebuilder:validation:Required ApiVersion CSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required Name string `json:"name"` + // +kubebuilder:validation:Required Type CSpecType `json:"type"` } @@ -228,6 +234,7 @@ type A_Spec struct { //AzureName: The name of the resource in Azure. This is often the same as the name //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } @@ -283,6 +290,7 @@ type B_Spec struct { //AzureName: The name of the resource in Azure. This is often the same as the name //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"test.infra.azure.com" json:"owner" kind:"A"` } @@ -338,6 +346,7 @@ type C_Spec struct { //AzureName: The name of the resource in Azure. This is often the same as the name //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"test.infra.azure.com" json:"owner" kind:"B"` } diff --git a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_array_properties.golden b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_array_properties.golden index cc5899022..02cb80a0c 100644 --- a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_array_properties.golden +++ b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_array_properties.golden @@ -54,13 +54,16 @@ type FakeResourceList struct { type FakeResource_SpecArm struct { // +kubebuilder:validation:Required ApiVersion FakeResourceSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required ArrayFoo []FooArm `json:"arrayFoo"` ArrayOfArrays [][]FooArm `json:"arrayOfArrays,omitempty"` ArrayOfEnums []Color `json:"arrayOfEnums,omitempty"` ArrayOfMaps []map[string]FooArm `json:"arrayOfMaps,omitempty"` + // +kubebuilder:validation:Required Name string `json:"name"` + // +kubebuilder:validation:Required Type FakeResourceSpecType `json:"type"` } @@ -105,6 +108,7 @@ const FakeResourceSpecTypeMicrosoftAzureFakeResource = FakeResourceSpecType("Mic type FakeResource_Spec struct { // +kubebuilder:validation:Required ApiVersion FakeResourceSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required ArrayFoo []Foo `json:"arrayFoo"` ArrayOfArrays [][]Foo `json:"arrayOfArrays,omitempty"` @@ -114,6 +118,7 @@ type FakeResource_Spec struct { //AzureName: The name of the resource in Azure. This is often the same as the name //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } diff --git a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_complex_properties.golden b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_complex_properties.golden index 547fc1598..6b572a845 100644 --- a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_complex_properties.golden +++ b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_complex_properties.golden @@ -55,11 +55,14 @@ type FakeResource_SpecArm struct { // +kubebuilder:validation:Required ApiVersion FakeResourceSpecApiVersion `json:"apiVersion"` Color *FakeResourceSpecColor `json:"color,omitempty"` + // +kubebuilder:validation:Required Foo FooArm `json:"foo"` + // +kubebuilder:validation:Required Name string `json:"name"` OptionalFoo *FooArm `json:"optionalFoo,omitempty"` + // +kubebuilder:validation:Required Type FakeResourceSpecType `json:"type"` } @@ -108,9 +111,11 @@ type FakeResource_Spec struct { //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` Color *FakeResourceSpecColor `json:"color,omitempty"` + // +kubebuilder:validation:Required Foo Foo `json:"foo"` OptionalFoo *Foo `json:"optionalFoo,omitempty"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } diff --git a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_json_fields.golden b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_json_fields.golden index 68d81a267..306fcd42f 100644 --- a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_json_fields.golden +++ b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_json_fields.golden @@ -55,13 +55,17 @@ type FakeResourceList struct { type FakeResource_SpecArm struct { // +kubebuilder:validation:Required ApiVersion FakeResourceSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required JsonObject map[string]v1.JSON `json:"jsonObject"` + // +kubebuilder:validation:Required MandatoryJson v1.JSON `json:"mandatoryJson"` + // +kubebuilder:validation:Required Name string `json:"name"` OptionalJson *v1.JSON `json:"optionalJson,omitempty"` + // +kubebuilder:validation:Required Type FakeResourceSpecType `json:"type"` } @@ -100,11 +104,14 @@ type FakeResource_Spec struct { //AzureName: The name of the resource in Azure. This is often the same as the name //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` + // +kubebuilder:validation:Required JsonObject map[string]v1.JSON `json:"jsonObject"` + // +kubebuilder:validation:Required MandatoryJson v1.JSON `json:"mandatoryJson"` OptionalJson *v1.JSON `json:"optionalJson,omitempty"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } diff --git a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_map_properties.golden b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_map_properties.golden index ee52648db..d112ffc86 100644 --- a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_map_properties.golden +++ b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_map_properties.golden @@ -54,14 +54,17 @@ type FakeResourceList struct { type FakeResource_SpecArm struct { // +kubebuilder:validation:Required ApiVersion FakeResourceSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required MapFoo map[string]FooArm `json:"mapFoo"` MapOfArrays map[string][]FooArm `json:"mapOfArrays,omitempty"` MapOfEnums map[string]Color `json:"mapOfEnums,omitempty"` MapOfMaps map[string]map[string]FooArm `json:"mapOfMaps,omitempty"` MapOfStrings map[string]string `json:"mapOfStrings,omitempty"` + // +kubebuilder:validation:Required Name string `json:"name"` + // +kubebuilder:validation:Required Type FakeResourceSpecType `json:"type"` } @@ -110,12 +113,14 @@ type FakeResource_Spec struct { //AzureName: The name of the resource in Azure. This is often the same as the name //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` + // +kubebuilder:validation:Required MapFoo map[string]Foo `json:"mapFoo"` MapOfArrays map[string][]Foo `json:"mapOfArrays,omitempty"` MapOfEnums map[string]Color `json:"mapOfEnums,omitempty"` MapOfMaps map[string]map[string]Foo `json:"mapOfMaps,omitempty"` MapOfStrings map[string]string `json:"mapOfStrings,omitempty"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } diff --git a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_renders_spec.golden b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_renders_spec.golden index 4a712f3ab..d0f062baa 100644 --- a/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_renders_spec.golden +++ b/hack/generator/pkg/codegen/testdata/ArmResource/Arm_test_simple_resource_renders_spec.golden @@ -54,8 +54,10 @@ type FakeResourceList struct { type FakeResource_SpecArm struct { // +kubebuilder:validation:Required ApiVersion FakeResourceSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required Name string `json:"name"` + // +kubebuilder:validation:Required Type FakeResourceSpecType `json:"type"` } @@ -94,6 +96,7 @@ type FakeResource_Spec struct { //AzureName: The name of the resource in Azure. This is often the same as the name //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } diff --git a/hack/generator/pkg/codegen/testdata/EmbeddedTypes/Embedded_type_simple_resource.golden b/hack/generator/pkg/codegen/testdata/EmbeddedTypes/Embedded_type_simple_resource.golden index ab46efd00..b78354f45 100644 --- a/hack/generator/pkg/codegen/testdata/EmbeddedTypes/Embedded_type_simple_resource.golden +++ b/hack/generator/pkg/codegen/testdata/EmbeddedTypes/Embedded_type_simple_resource.golden @@ -53,14 +53,18 @@ type FakeResourceList struct { type FakeResource_SpecArm struct { EmbeddedTestType `json:",inline"` + // +kubebuilder:validation:Required ApiVersion FakeResourceSpecApiVersion `json:"apiVersion"` Color *FakeResourceSpecColor `json:"color,omitempty"` + // +kubebuilder:validation:Required Foo FooArm `json:"foo"` + // +kubebuilder:validation:Required Name string `json:"name"` OptionalFoo *FooArm `json:"optionalFoo,omitempty"` + // +kubebuilder:validation:Required Type FakeResourceSpecType `json:"type"` } @@ -103,6 +107,7 @@ const FakeResourceSpecTypeMicrosoftAzureFakeResource = FakeResourceSpecType("Mic type FakeResource_Spec struct { EmbeddedTestType `json:",inline"` + // +kubebuilder:validation:Required ApiVersion FakeResourceSpecApiVersion `json:"apiVersion"` @@ -110,9 +115,11 @@ type FakeResource_Spec struct { //of the resource in Kubernetes but it doesn't have to be. AzureName string `json:"azureName"` Color *FakeResourceSpecColor `json:"color,omitempty"` + // +kubebuilder:validation:Required Foo Foo `json:"foo"` OptionalFoo *Foo `json:"optionalFoo,omitempty"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } diff --git a/hack/generator/pkg/codegen/testdata/EnumNames/Multi_valued_enum_name.golden b/hack/generator/pkg/codegen/testdata/EnumNames/Multi_valued_enum_name.golden index 0aca97695..7a832c324 100644 --- a/hack/generator/pkg/codegen/testdata/EnumNames/Multi_valued_enum_name.golden +++ b/hack/generator/pkg/codegen/testdata/EnumNames/Multi_valued_enum_name.golden @@ -54,8 +54,10 @@ type AResourceList struct { type AResource_SpecArm struct { // +kubebuilder:validation:Required ApiVersion AResourceSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required Name string `json:"name"` + // +kubebuilder:validation:Required Type AResourceSpecType `json:"type"` } @@ -94,6 +96,7 @@ type AResource_Spec struct { //AzureName: The name of the resource in Azure. This is often the same as the name //of the resource in Kubernetes but it doesn't have to be. AzureName AResourceSpecName `json:"azureName"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } diff --git a/hack/generator/pkg/codegen/testdata/EnumNames/Single_valued_enum_name.golden b/hack/generator/pkg/codegen/testdata/EnumNames/Single_valued_enum_name.golden index cc2374ad7..41036871f 100644 --- a/hack/generator/pkg/codegen/testdata/EnumNames/Single_valued_enum_name.golden +++ b/hack/generator/pkg/codegen/testdata/EnumNames/Single_valued_enum_name.golden @@ -47,8 +47,10 @@ const AResourceSpecNameOnlyonevalue = AResourceSpecName("onlyonevalue") type AResource_SpecArm struct { // +kubebuilder:validation:Required ApiVersion AResourceSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required Name string `json:"name"` + // +kubebuilder:validation:Required Type AResourceSpecType `json:"type"` } @@ -83,6 +85,7 @@ const AResourceSpecTypeMicrosoftAzureAResource = AResourceSpecType("Microsoft.Az type AResource_Spec struct { // +kubebuilder:validation:Required ApiVersion AResourceSpecApiVersion `json:"apiVersion"` + // +kubebuilder:validation:Required Owner genruntime.KnownResourceReference `group:"microsoft.resources.infra.azure.com" json:"owner" kind:"ResourceGroup"` } diff --git a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_for_inner_properties.golden b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_for_inner_properties.golden index e0bbcac2c..be23e7ec2 100644 --- a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_for_inner_properties.golden +++ b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_for_inner_properties.golden @@ -13,7 +13,6 @@ type Test struct { } type Test_Properties struct { - //Bar: Mutually exclusive with all other properties Bar *Bar `json:"bar,omitempty"` diff --git a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_type_with_marshal_helper.golden b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_type_with_marshal_helper.golden index a102ab03f..96af4beb3 100644 --- a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_type_with_marshal_helper.golden +++ b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_type_with_marshal_helper.golden @@ -7,7 +7,6 @@ import "encoding/json" //Generated from: https://test.test/schemas/2020-01-01/test.json type Test struct { - //Bar: Mutually exclusive with all other properties Bar *Bar `json:"bar,omitempty"` diff --git a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_type_with_named_properties_from_anonymous_types.golden b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_type_with_named_properties_from_anonymous_types.golden index 1f82b2fe9..fb351e944 100644 --- a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_type_with_named_properties_from_anonymous_types.golden +++ b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_generates_wrapper_type_with_named_properties_from_anonymous_types.golden @@ -7,7 +7,6 @@ import "encoding/json" //Generated from: https://test.test/schemas/2020-01-01/test.json type Test struct { - //Bool1: Mutually exclusive with all other properties Bool1 *bool `json:"bool1,omitempty"` diff --git a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_handles_properties_on_owning_node.golden b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_handles_properties_on_owning_node.golden index 8e2e4a365..e2640c560 100644 --- a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_handles_properties_on_owning_node.golden +++ b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_handles_properties_on_owning_node.golden @@ -7,7 +7,6 @@ import "encoding/json" //Generated from: https://test.test/schemas/2020-01-01/test.json type Test struct { - //Either: Mutually exclusive with all other properties Either *Test_Either `json:"either,omitempty"` diff --git a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_preserves_names.golden b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_preserves_names.golden index add8cf39b..5c9f7a661 100644 --- a/hack/generator/pkg/codegen/testdata/OneOf/OneOf_preserves_names.golden +++ b/hack/generator/pkg/codegen/testdata/OneOf/OneOf_preserves_names.golden @@ -7,7 +7,6 @@ import "encoding/json" //Generated from: https://test.test/schemas/2020-01-01/test.json type Test struct { - //Base: Mutually exclusive with all other properties Base *Test_Base `json:"base,omitempty"` diff --git a/hack/generator/pkg/codegen/testdata/Validations/Validate_property_type.golden b/hack/generator/pkg/codegen/testdata/Validations/Validate_property_type.golden index ab3bae4f0..e7f487853 100644 --- a/hack/generator/pkg/codegen/testdata/Validations/Validate_property_type.golden +++ b/hack/generator/pkg/codegen/testdata/Validations/Validate_property_type.golden @@ -10,17 +10,20 @@ type Test struct { // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:UniqueItems=true Apple []float64 `json:"apple"` + // +kubebuilder:validation:Required // +kubebuilder:validation:MaxLength=20 // +kubebuilder:validation:MinLength=10 // +kubebuilder:validation:Pattern="^[a-z]+$" Duck string `json:"duck"` + // +kubebuilder:validation:Required // +kubebuilder:validation:Maximum=2000 // +kubebuilder:validation:Minimum=1000 // +kubebuilder:validation:ExclusiveMinimum=true // +kubebuilder:validation:MultipleOf=3 Moon int `json:"moon"` + // +kubebuilder:validation:Required // +kubebuilder:validation:Maximum=200 // +kubebuilder:validation:ExclusiveMaximum=true