From 2231ea6211effeef66d8a1545b386ccdce381c7a Mon Sep 17 00:00:00 2001 From: John Ryan Date: Tue, 6 Sep 2022 13:37:00 -0700 Subject: [PATCH] Merge "any" and "null" types when both are present Resolves #723 --- pkg/cmd/template/schema_author_test.go | 31 -------------- pkg/cmd/template/schema_consumer_test.go | 27 +++++++++++++ pkg/schema/annotations.go | 51 ++++++++---------------- pkg/schema/schema.go | 6 +-- 4 files changed, 45 insertions(+), 70 deletions(-) diff --git a/pkg/cmd/template/schema_author_test.go b/pkg/cmd/template/schema_author_test.go index 3d723b2e..16028ce9 100644 --- a/pkg/cmd/template/schema_author_test.go +++ b/pkg/cmd/template/schema_author_test.go @@ -331,37 +331,6 @@ schema.yml: assertFails(t, filesToProcess, expectedErr, opts) }) }) - t.Run("when schema/type and schema/nullable annotate a map", func(t *testing.T) { - schemaYAML := `#@data/values-schema ---- -#@schema/type any=True -#@schema/nullable -foo: 0 -` - - expectedErr := ` -Invalid schema -============== - -@schema/nullable, and @schema/type any=True are mutually exclusive -schema.yml: - | - 3 | #@schema/type any=True - 4 | #@schema/nullable - 5 | foo: 0 - | - - = found: both @schema/nullable, and @schema/type any=True annotations - = expected: one of schema/nullable, or schema/type any=True -` - - filesToProcess := files.NewSortedFiles([]*files.File{ - files.MustNewFileFromSource(files.NewBytesSource("schema.yml", []byte(schemaYAML))), - }) - - assertFails(t, filesToProcess, expectedErr, opts) - - }) t.Run("when schema/default annotation value", func(t *testing.T) { t.Run("is empty", func(t *testing.T) { diff --git a/pkg/cmd/template/schema_consumer_test.go b/pkg/cmd/template/schema_consumer_test.go index 25f28572..c8d7c368 100644 --- a/pkg/cmd/template/schema_consumer_test.go +++ b/pkg/cmd/template/schema_consumer_test.go @@ -1420,6 +1420,33 @@ rendered: #@ data.values assertSucceeds(t, filesToProcess, expected, opts) }) + + t.Run("when the type is also explicitly set", func(t *testing.T) { + schemaYAML := `#@data/values-schema +--- +#@schema/nullable +#@schema/type any=True +foo: 0 +#@schema/type any=True +#@schema/nullable +bar: "" +` + templateYAML := `#@ load("@ytt:data", "data") +--- +rendered: #@ data.values +` + expected := `rendered: + foo: null + bar: null +` + + filesToProcess := files.NewSortedFiles([]*files.File{ + files.MustNewFileFromSource(files.NewBytesSource("schema.yml", []byte(schemaYAML))), + files.MustNewFileFromSource(files.NewBytesSource("template.yml", []byte(templateYAML))), + }) + + assertSucceeds(t, filesToProcess, expected, opts) + }) t.Run("when any is set on maps and arrays with nested dvs and overlay/replace", func(t *testing.T) { schemaYAML := `#@data/values-schema --- diff --git a/pkg/schema/annotations.go b/pkg/schema/annotations.go index 1e08042b..bcaf6b27 100644 --- a/pkg/schema/annotations.go +++ b/pkg/schema/annotations.go @@ -658,52 +658,35 @@ func processValidationAnnotation(node yamlmeta.Node) (*ValidationAnnotation, err return nil, nil } -func getTypeFromAnnotations(anns []Annotation, pos *filepos.Position) (Type, error) { +func getTypeFromAnnotations(anns []Annotation) (Type, error) { annsCopy := append([]Annotation{}, anns...) - if len(annsCopy) == 0 { - return nil, nil - } - if len(annsCopy) == 1 { - typeFromAnn, err := annsCopy[0].NewTypeFromAnn() - if err != nil { - return nil, err - } - return typeFromAnn, nil - } - - var conflictingTypeAnns []Annotation + var typeFromAnn Type for _, ann := range annsCopy { switch typedAnn := ann.(type) { - case *NullableAnnotation: - conflictingTypeAnns = append(conflictingTypeAnns, ann) case *TypeAnnotation: if typedAnn.IsAny() { - conflictingTypeAnns = append(conflictingTypeAnns, ann) + var err error + typeFromAnn, err = typedAnn.NewTypeFromAnn() + if err != nil { + return nil, err + } + } + case *NullableAnnotation: + if typeFromAnn == nil { + var err error + typeFromAnn, err = typedAnn.NewTypeFromAnn() + if err != nil { + return nil, err + } + } else { + typeFromAnn.SetDefaultValue(nil) } default: continue } } - if len(conflictingTypeAnns) > 1 { - annPositions := make([]*filepos.Position, len(conflictingTypeAnns)) - for i, a := range conflictingTypeAnns { - annPositions[i] = a.GetPosition() - } - return nil, schemaAssertionError{ - annPositions: annPositions, - position: pos, - description: fmt.Sprintf("@%v, and @%v any=True are mutually exclusive", AnnotationNullable, AnnotationType), - expected: fmt.Sprintf("one of %v, or %v any=True", AnnotationNullable, AnnotationType), - found: fmt.Sprintf("both @%v, and @%v any=True annotations", AnnotationNullable, AnnotationType), - } - } - - typeFromAnn, err := conflictingTypeAnns[0].NewTypeFromAnn() - if err != nil { - return nil, err - } return typeFromAnn, nil } diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 660ada1d..b5c6a382 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -116,7 +116,7 @@ func getType(node yamlmeta.Node) (Type, error) { if err != nil { return nil, NewSchemaError("Invalid schema", err) } - typeOfValue, err = getTypeFromAnnotations(anns, node.GetPosition()) + typeOfValue, err = getTypeFromAnnotations(anns) if err != nil { return nil, NewSchemaError("Invalid schema", err) } @@ -156,10 +156,6 @@ func getValue(node yamlmeta.Node, t Type) (interface{}, error) { } } - if _, ok := t.(*AnyType); ok { - return node.GetValues()[0], nil - } - return t.GetDefaultValue(), nil }