From 8caad7cbacc90a222fc2504296af1d1775f31254 Mon Sep 17 00:00:00 2001 From: Amund Tenstad Date: Mon, 4 Jul 2022 13:17:56 +0200 Subject: [PATCH] Support for inlining fields of embedded types (#25) --- processor/processor.go | 45 +++++++------- test/api/v1/guestbook_types.go | 22 +++++++ test/api/v1/zz_generated.deepcopy.go | 88 ++++++++++++++++++++++++++++ test/config.yaml | 2 +- test/expected.asciidoc | 23 ++++++++ test/expected.md | 22 +++++++ types/types.go | 71 +++++++++++++++++++++- 7 files changed, 245 insertions(+), 28 deletions(-) diff --git a/processor/processor.go b/processor/processor.go index 27f5343..17792b4 100644 --- a/processor/processor.go +++ b/processor/processor.go @@ -46,7 +46,7 @@ type groupVersionInfo struct { *loader.Package doc string kinds map[string]struct{} - types map[string]*types.Type + types types.TypeMap } func Process(config *config.Config) ([]types.GroupVersionDetails, error) { @@ -61,6 +61,8 @@ func Process(config *config.Config) ([]types.GroupVersionDetails, error) { return nil, fmt.Errorf("failed to find API types in directory %s:%w", config.SourcePath, err) } + p.types.InlineTypes() + // collect references between types for typeName, refs := range p.references { typeDef, ok := p.types[typeName] @@ -83,11 +85,16 @@ func Process(config *config.Config) ([]types.GroupVersionDetails, error) { details.Kinds = append(details.Kinds, k) } - details.Types = make(map[string]*types.Type) - for t, _ := range gvi.types { - key := fmt.Sprintf("%s.%s", gvi.Package.PkgPath, t) + details.Types = make(types.TypeMap) + for name, t := range gvi.types { + key := types.Key(t) + + if p.shouldIgnoreType(key) { + zap.S().Debugw("Skipping excluded type", "type", name) + continue + } if typeDef, ok := p.types[key]; ok && typeDef != nil { - details.Types[t] = typeDef + details.Types[name] = typeDef } else { zap.S().Fatalw("Type not loaded", "type", key) } @@ -121,7 +128,7 @@ func newProcessor(compiledConfig *compiledConfig, maxDepth int) *processor { Checker: &loader.TypeChecker{}, }, groupVersions: make(map[schema.GroupVersion]*groupVersionInfo), - types: make(map[string]*types.Type), + types: make(types.TypeMap), references: make(map[string]map[string]struct{}), } @@ -134,7 +141,7 @@ type processor struct { maxDepth int parser *crd.Parser groupVersions map[schema.GroupVersion]*groupVersionInfo - types map[string]*types.Type + types types.TypeMap references map[string]map[string]struct{} } @@ -167,7 +174,7 @@ func (p *processor) findAPITypes(directory string) error { } if gvInfo.types == nil { - gvInfo.types = make(map[string]*types.Type) + gvInfo.types = make(types.TypeMap) } // locate the kinds @@ -265,11 +272,6 @@ func (p *processor) processType(pkg *loader.Package, info *markers.TypeInfo, dep Doc: info.Doc, } - if p.shouldIgnoreType(types.Key(typeDef)) { - zap.S().Debugw("Skipping excluded type", "type", typeDef.String()) - return nil - } - // if the field list is non-empty, this is a struct if len(info.Fields) > 0 { typeDef.Kind = types.StructKind @@ -301,11 +303,6 @@ func (p *processor) processStructFields(parentType *types.Type, pkg *loader.Pack parentTypeKey := types.Key(parentType) for _, f := range info.Fields { - if p.shouldIgnoreField(parentTypeKey, f.Name) { - zap.S().Debugw("Skipping excluded field", "type", parentType.String(), "field", f.Name) - continue - } - t := pkg.TypesInfo.TypeOf(f.RawField.Type) if t == nil { zap.S().Debugw("Failed to determine type of field", "field", f.Name) @@ -331,11 +328,15 @@ func (p *processor) processStructFields(parentType *types.Type, pkg *loader.Pack continue } - if fieldDef.Embedded && fieldDef.Name == "" { - fieldDef.Name = fieldDef.Type.Name + if fieldDef.Embedded { + fieldDef.Inlined = fieldDef.Name == "" + if fieldDef.Name == "" { + fieldDef.Name = fieldDef.Type.Name + } } if p.shouldIgnoreField(parentTypeKey, fieldDef.Name) { + zap.S().Debugw("Skipping excluded field", "type", parentType.String(), "field", fieldDef.Name) continue } @@ -355,10 +356,6 @@ func (p *processor) loadType(pkg *loader.Package, t gotypes.Type, depth int) *ty } typeDef := mkType(pkg, t) - if p.shouldIgnoreType(types.Key(typeDef)) { - zap.S().Debugw("Skipping excluded type", "type", t.String()) - return nil - } zap.S().Debugw("Load", "package", typeDef.Package, "name", typeDef.Name) diff --git a/test/api/v1/guestbook_types.go b/test/api/v1/guestbook_types.go index e92a647..47cb5ca 100644 --- a/test/api/v1/guestbook_types.go +++ b/test/api/v1/guestbook_types.go @@ -21,6 +21,28 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// +kubebuilder:object:root=true + +type Embedded struct { + A string `json:"a,omitempty"` + Embedded1 `json:",inline"` +} +type Embedded1 struct { + Embedded2 `json:",inline"` + E string `json:"e,omitempty"` +} +type Embedded2 struct { + B string `json:"b,omitempty"` + Embedded3 `json:",inline"` +} +type Embedded3 struct { + Embedded4 `json:",inline"` + D string `json:"d,omitempty"` +} +type Embedded4 struct { + C string `json:"c,omitempty"` +} + // GuestbookSpec defines the desired state of Guestbook. type GuestbookSpec struct { // Page indicates the page number diff --git a/test/api/v1/zz_generated.deepcopy.go b/test/api/v1/zz_generated.deepcopy.go index bde5968..2ef8b0a 100644 --- a/test/api/v1/zz_generated.deepcopy.go +++ b/test/api/v1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated // Licensed to Elasticsearch B.V. under one or more contributor @@ -25,6 +26,93 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Embedded) DeepCopyInto(out *Embedded) { + *out = *in + out.Embedded1 = in.Embedded1 +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Embedded. +func (in *Embedded) DeepCopy() *Embedded { + if in == nil { + return nil + } + out := new(Embedded) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *Embedded) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Embedded1) DeepCopyInto(out *Embedded1) { + *out = *in + out.Embedded2 = in.Embedded2 +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Embedded1. +func (in *Embedded1) DeepCopy() *Embedded1 { + if in == nil { + return nil + } + out := new(Embedded1) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Embedded2) DeepCopyInto(out *Embedded2) { + *out = *in + out.Embedded3 = in.Embedded3 +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Embedded2. +func (in *Embedded2) DeepCopy() *Embedded2 { + if in == nil { + return nil + } + out := new(Embedded2) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Embedded3) DeepCopyInto(out *Embedded3) { + *out = *in + out.Embedded4 = in.Embedded4 +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Embedded3. +func (in *Embedded3) DeepCopy() *Embedded3 { + if in == nil { + return nil + } + out := new(Embedded3) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Embedded4) DeepCopyInto(out *Embedded4) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Embedded4. +func (in *Embedded4) DeepCopy() *Embedded4 { + if in == nil { + return nil + } + out := new(Embedded4) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Guestbook) DeepCopyInto(out *Guestbook) { *out = *in diff --git a/test/config.yaml b/test/config.yaml index 24f01da..11060fe 100644 --- a/test/config.yaml +++ b/test/config.yaml @@ -2,7 +2,7 @@ processor: ignoreGroupVersions: - "GVK" ignoreTypes: - - "Status$" + - "Embedded[0-9]$" ignoreFields: - "status$" - "TypeMeta$" diff --git a/test/expected.asciidoc b/test/expected.asciidoc index ee2c53b..2076a27 100644 --- a/test/expected.asciidoc +++ b/test/expected.asciidoc @@ -14,11 +14,32 @@ Package v1 contains API Schema definitions for the webapp v1 API group .Resource Types +- xref:{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-embedded[$$Embedded$$] - xref:{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-guestbook[$$Guestbook$$] - xref:{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-guestbooklist[$$GuestbookList$$] +[id="{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-embedded"] +==== Embedded + + + + + +[cols="25a,75a", options="header"] +|=== +| Field | Description +| *`apiVersion`* __string__ | `webapp.test.k8s.elastic.co/v1` +| *`kind`* __string__ | `Embedded` +| *`a`* __string__ | +| *`b`* __string__ | +| *`c`* __string__ | +| *`d`* __string__ | +| *`e`* __string__ | +|=== + + [id="{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-guestbook"] ==== Guestbook @@ -110,6 +131,8 @@ GuestbookSpec defines the desired state of Guestbook. |=== + + [id="{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-rating"] ==== Rating (string) diff --git a/test/expected.md b/test/expected.md index 4d420ea..1471acb 100644 --- a/test/expected.md +++ b/test/expected.md @@ -9,11 +9,31 @@ Package v1 contains API Schema definitions for the webapp v1 API group ### Resource Types +- [Embedded](#embedded) - [Guestbook](#guestbook) - [GuestbookList](#guestbooklist) +#### Embedded + + + + + + + +| Field | Description | +| --- | --- | +| `apiVersion` _string_ | `webapp.test.k8s.elastic.co/v1` +| `kind` _string_ | `Embedded` +| `a` _string_ | | +| `b` _string_ | | +| `c` _string_ | | +| `d` _string_ | | +| `e` _string_ | | + + #### Guestbook @@ -92,6 +112,8 @@ _Appears in:_ | `headers` _[GuestbookHeader](#guestbookheader) array_ | Headers contains a list of header items to include in the page | + + #### Rating _Underlying type:_ `string` diff --git a/types/types.go b/types/types.go index 9f44f90..f1e4300 100644 --- a/types/types.go +++ b/types/types.go @@ -22,6 +22,7 @@ import ( "sort" "strings" + "go.uber.org/zap" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -107,7 +108,7 @@ type Type struct { UnderlyingType *Type `json:"underlyingType"` // for aliases, slices and pointers KeyType *Type `json:"keyType"` // for maps ValueType *Type `json:"valueType"` // for maps - Fields []*Field `json:"fields"` // for structs + Fields Fields `json:"fields"` // for structs References []*Type `json:"-"` // other types that refer to this type } @@ -142,7 +143,7 @@ func (t *Type) IsBasic() bool { } } -func (t *Type) Members() []*Field { +func (t *Type) Members() Fields { if t == nil { return nil } @@ -210,14 +211,78 @@ func (t *Type) SortedReferences() []*Type { return t.References } +func (t *Type) ContainsInlinedTypes() bool { + for _, f := range t.Members() { + if f.Inlined { + return true + } + } + return false +} + +// TypeMap is a map of Type elements +type TypeMap map[string]*Type + +func (types TypeMap) InlineTypes() { + // If C is inlined in B, and B is inlined in A; the fields of C are copied + // into B before the fields of B is copied into A. The ideal order of + // iterating and inlining fields is NOT known. Worst-case, only one type's + // fields are inlined in its parent type in each iteration. + maxDepth := 100 + var numTypesToBeInlined int + for iteration := 0; iteration < maxDepth; iteration++ { + numTypesToBeInlined = 0 + for _, t := range types { + // By iterating backwards, it is safe to delete field at current index + // and copy the fields of the inlined type. + for i := len(t.Fields) - 1; i >= 0; i-- { + if !t.Fields[i].Inlined { + continue + } + numTypesToBeInlined += 1 + + embeddedType, ok := types[Key(t.Fields[i].Type)] + if !ok { + zap.S().Warnw("Unable to find embedded type", "type", t, + "embeddedType", t.Fields[i].Type) + continue + } + + // Only inline type's fields if the inlined type itself has no + // types yet to be inlined. + if !embeddedType.ContainsInlinedTypes() { + zap.S().Debugw("Inlining embedded type", "type", t, + "embeddedType", t.Fields[i].Type) + t.Fields.inlineType(i, embeddedType) + } + } + } + if numTypesToBeInlined == 0 { + return + } + } + zap.S().Warnw("Failed to inline all inlined types", "remaining", numTypesToBeInlined) +} + // Field describes a field in a struct. type Field struct { Name string Embedded bool + Inlined bool Doc string Type *Type } +type Fields []*Field + +// inlineType replaces field at index i with the fields of inlined type. +func (fields *Fields) inlineType(i int, inlined *Type) { + new := make([]*Field, 0, len(*fields)+len(inlined.Fields)-1) + new = append(new, (*fields)[:i]...) + new = append(new, inlined.Fields...) + *fields = append(new, (*fields)[i+1:]...) +} + // Key generates the unique name for the give type. func Key(t *Type) string { if t.Package == "" { @@ -232,7 +297,7 @@ type GroupVersionDetails struct { schema.GroupVersion Doc string Kinds []string - Types map[string]*Type + Types TypeMap } func (gvd GroupVersionDetails) GroupVersionString() string {