From 5c9fe3834d36034388a0835f4304366cfe7df150 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Fri, 25 Sep 2020 12:49:21 -0700 Subject: [PATCH 1/2] Add docs for nested and embedded structs Test: m soong_docs Test: go bpdoc tests Change-Id: I99f15405e1a7d4a819f6fb20fda22372afe253e1 --- bootstrap/bpdoc/bpdoc.go | 83 +++++++++++++++++++++++++++++----- bootstrap/bpdoc/bpdoc_test.go | 57 ++++++++++++++++++----- bootstrap/bpdoc/properties.go | 8 ++++ bootstrap/bpdoc/reader_test.go | 32 +++++++++++++ 4 files changed, 158 insertions(+), 22 deletions(-) diff --git a/bootstrap/bpdoc/bpdoc.go b/bootstrap/bpdoc/bpdoc.go index 4abf2e7..1a9d820 100644 --- a/bootstrap/bpdoc/bpdoc.go +++ b/bootstrap/bpdoc/bpdoc.go @@ -5,6 +5,7 @@ import ( "html/template" "reflect" "sort" + "strings" "github.com/google/blueprint/proptools" ) @@ -58,6 +59,7 @@ type Property struct { OtherTexts []template.HTML Properties []Property Default string + Anonymous bool } func AllPackages(pkgFiles map[string][]string, moduleTypeNameFactories map[string]reflect.Value, @@ -75,6 +77,7 @@ func AllPackages(pkgFiles map[string][]string, moduleTypeNameFactories map[strin return nil, err } // Some pruning work + removeAnonymousProperties(mtInfo) removeEmptyPropertyStructs(mtInfo) collapseDuplicatePropertyStructs(mtInfo) collapseNestedPropertyStructs(mtInfo) @@ -128,7 +131,9 @@ func assembleModuleTypeInfo(r *Reader, name string, factory reflect.Value, } ps.ExcludeByTag("blueprint", "mutated") - for nestedName, nestedValue := range nestedPropertyStructs(v) { + for _, nestedProperty := range nestedPropertyStructs(v) { + nestedName := nestedProperty.nestPoint + nestedValue := nestedProperty.value nestedType := nestedValue.Type() // Ignore property structs with unexported or unnamed types @@ -140,12 +145,27 @@ func assembleModuleTypeInfo(r *Reader, name string, factory reflect.Value, return nil, err } nested.ExcludeByTag("blueprint", "mutated") - nestPoint := ps.GetByName(nestedName) - if nestPoint == nil { - return nil, fmt.Errorf("nesting point %q not found", nestedName) + if nestedName == "" { + ps.Nest(nested) + } else { + nestPoint := ps.GetByName(nestedName) + if nestPoint == nil { + return nil, fmt.Errorf("nesting point %q not found", nestedName) + } + nestPoint.Nest(nested) } - nestPoint.Nest(nested) + if nestedProperty.anonymous { + if nestedName != "" { + nestedName += "." + } + nestedName += proptools.PropertyNameForField(nested.Name) + nestedProp := ps.GetByName(nestedName) + if nestedProp == nil { + return nil, fmt.Errorf("could not find nested property %q", nestedName) + } + nestedProp.SetAnonymous() + } } mt.PropertyStructs = append(mt.PropertyStructs, ps) } @@ -153,10 +173,31 @@ func assembleModuleTypeInfo(r *Reader, name string, factory reflect.Value, return mt, nil } -func nestedPropertyStructs(s reflect.Value) map[string]reflect.Value { - ret := make(map[string]reflect.Value) +type nestedProperty struct { + nestPoint string + value reflect.Value + anonymous bool +} + +func nestedPropertyStructs(s reflect.Value) []nestedProperty { + ret := make([]nestedProperty, 0) var walk func(structValue reflect.Value, prefix string) walk = func(structValue reflect.Value, prefix string) { + var nestStruct func(field reflect.StructField, value reflect.Value, fieldName string) + nestStruct = func(field reflect.StructField, value reflect.Value, fieldName string) { + nestPoint := prefix + if field.Anonymous { + nestPoint = strings.TrimSuffix(nestPoint, ".") + } else { + nestPoint = nestPoint + proptools.PropertyNameForField(fieldName) + } + ret = append(ret, nestedProperty{nestPoint: nestPoint, value: value, anonymous: field.Anonymous}) + if nestPoint != "" { + nestPoint += "." + } + walk(value, nestPoint) + } + typ := structValue.Type() for i := 0; i < structValue.NumField(); i++ { field := typ.Field(i) @@ -174,8 +215,9 @@ func nestedPropertyStructs(s reflect.Value) map[string]reflect.Value { case reflect.Bool, reflect.String, reflect.Slice, reflect.Int, reflect.Uint: // Nothing case reflect.Struct: - walk(fieldValue, prefix+proptools.PropertyNameForField(field.Name)+".") + nestStruct(field, fieldValue, field.Name) case reflect.Ptr, reflect.Interface: + if !fieldValue.IsNil() { // We leave the pointer intact and zero out the struct that's // pointed to. @@ -188,9 +230,7 @@ func nestedPropertyStructs(s reflect.Value) map[string]reflect.Value { elem = elem.Elem() } if elem.Kind() == reflect.Struct { - nestPoint := prefix + proptools.PropertyNameForField(field.Name) - ret[nestPoint] = elem - walk(elem, nestPoint+".") + nestStruct(field, elem, field.Name) } } default: @@ -214,6 +254,27 @@ func removeEmptyPropertyStructs(mt *ModuleType) { } } +// Remove any property structs that are anonymous +func removeAnonymousProperties(mt *ModuleType) { + var removeAnonymousProps func(props []Property) []Property + removeAnonymousProps = func(props []Property) []Property { + newProps := make([]Property, 0, len(props)) + for _, p := range props { + if p.Anonymous { + continue + } + if len(p.Properties) > 0 { + p.Properties = removeAnonymousProps(p.Properties) + } + newProps = append(newProps, p) + } + return newProps + } + for _, ps := range mt.PropertyStructs { + ps.Properties = removeAnonymousProps(ps.Properties) + } +} + // Squashes duplicates of the same property struct into single entries func collapseDuplicatePropertyStructs(mt *ModuleType) { var collapsed []*PropertyStruct diff --git a/bootstrap/bpdoc/bpdoc_test.go b/bootstrap/bpdoc/bpdoc_test.go index 3856933..70834ef 100644 --- a/bootstrap/bpdoc/bpdoc_test.go +++ b/bootstrap/bpdoc/bpdoc_test.go @@ -36,20 +36,23 @@ func TestNestedPropertyStructs(t *testing.T) { // mutated shouldn't be found because it's a mutated property. expected := []string{"child", "child.child"} if len(allStructs) != len(expected) { - t.Errorf("expected %d structs, got %d, all entries: %q", + t.Fatalf("expected %d structs, got %d, all entries: %v", len(expected), len(allStructs), allStructs) } - for _, e := range expected { - if _, ok := allStructs[e]; !ok { - t.Errorf("missing entry %q, all entries: %q", e, allStructs) - } + got := []string{} + for _, s := range allStructs { + got = append(got, s.nestPoint) + } + + if !reflect.DeepEqual(got, expected) { + t.Errorf("Expected nested properties:\n\t %q,\n but got\n\t %q", expected, got) } } func TestAllPackages(t *testing.T) { packages, err := AllPackages(pkgFiles, moduleTypeNameFactories, moduleTypeNamePropertyStructs) if err != nil { - t.Errorf("expected nil error for AllPackages(%v, %v, %v), got %s", pkgFiles, moduleTypeNameFactories, moduleTypeNamePropertyStructs, err) + t.Fatalf("expected nil error for AllPackages(%v, %v, %v), got %s", pkgFiles, moduleTypeNameFactories, moduleTypeNamePropertyStructs, err) } if numPackages := len(packages); numPackages != 1 { @@ -58,24 +61,56 @@ func TestAllPackages(t *testing.T) { pkg := packages[0] + expectedProps := map[string][]string{ + "bar": []string{ + "a", + "nested", + "nested.c", + "nested_struct", + "nested_struct.e", + "struct_has_embed", + "struct_has_embed.nested_in_embedded", + "struct_has_embed.nested_in_embedded.e", + "struct_has_embed.f", + "nested_in_other_embedded", + "nested_in_other_embedded.g", + "h", + }, + "foo": []string{ + "a", + }, + } + for _, m := range pkg.ModuleTypes { + foundProps := []string{} for _, p := range m.PropertyStructs { - for _, err := range noMutatedProperties(p.Properties) { + nestedProps, errs := findAllProperties("", p.Properties) + foundProps = append(foundProps, nestedProps...) + for _, err := range errs { t.Errorf("%s", err) } } + if wanted, ok := expectedProps[m.Name]; ok { + if !reflect.DeepEqual(foundProps, wanted) { + t.Errorf("For %s, expected\n\t %q,\nbut got\n\t %q", m.Name, wanted, foundProps) + } + } } } -func noMutatedProperties(properties []Property) []error { +func findAllProperties(prefix string, properties []Property) ([]string, []error) { + foundProps := []string{} errs := []error{} for _, p := range properties { + foundProps = append(foundProps, prefix+p.Name) if hasTag(p.Tag, "blueprint", "mutated") { - err := fmt.Errorf("Property %s has `blueprint:\"mutated\" tag but should have been excluded.", p) + err := fmt.Errorf("Property %s has `blueprint:\"mutated\" tag but should have been excluded.", p.Name) errs = append(errs, err) } - errs = append(errs, noMutatedProperties(p.Properties)...) + nestedProps, nestedErrs := findAllProperties(prefix+p.Name+".", p.Properties) + foundProps = append(foundProps, nestedProps...) + errs = append(errs, nestedErrs...) } - return errs + return foundProps, errs } diff --git a/bootstrap/bpdoc/properties.go b/bootstrap/bpdoc/properties.go index dcbb80e..a28384e 100644 --- a/bootstrap/bpdoc/properties.go +++ b/bootstrap/bpdoc/properties.go @@ -142,6 +142,10 @@ func (ps *PropertyStruct) GetByName(name string) *Property { return getByName(name, "", &ps.Properties) } +func (ps *PropertyStruct) Nest(nested *PropertyStruct) { + ps.Properties = append(ps.Properties, nested.Properties...) +} + func getByName(name string, prefix string, props *[]Property) *Property { for i := range *props { if prefix+(*props)[i].Name == name { @@ -157,6 +161,10 @@ func (p *Property) Nest(nested *PropertyStruct) { p.Properties = append(p.Properties, nested.Properties...) } +func (p *Property) SetAnonymous() { + p.Anonymous = true +} + func newPropertyStruct(t *doc.Type) (*PropertyStruct, error) { typeSpec := t.Decl.Specs[0].(*ast.TypeSpec) ps := PropertyStruct{ diff --git a/bootstrap/bpdoc/reader_test.go b/bootstrap/bpdoc/reader_test.go index 8493e14..69077e2 100644 --- a/bootstrap/bpdoc/reader_test.go +++ b/bootstrap/bpdoc/reader_test.go @@ -36,6 +36,32 @@ func barFactory() (blueprint.Module, []interface{}) { return nil, []interface{}{&complexProps{}} } +type structToNest struct { + E string +} + +type StructToEmbed struct { + Nested_in_embedded structToNest + + // F string + F string +} + +type otherStructToNest struct { + G string +} + +type OtherStructToEmbed struct { + Nested_in_other_embedded otherStructToNest + + // F string + H string +} + +type StructWithEmbedded struct { + StructToEmbed +} + // for bpdoc_test.go type complexProps struct { A string @@ -45,6 +71,12 @@ type complexProps struct { C string D_mutated string `blueprint:"mutated"` } + + Nested_struct structToNest + + Struct_has_embed StructWithEmbedded + + OtherStructToEmbed } // props docs. From 2068e08a2c5c2d7db345f2a46c85509aebcfdf88 Mon Sep 17 00:00:00 2001 From: Liz Kammer Date: Mon, 5 Oct 2020 14:47:47 -0700 Subject: [PATCH 2/2] Correct bug in generating anonymous nested props Some anonymous nested properties are missing from property structs, since setting the property to anonymous is to allow future filtering, there is no issue if we cannot find the struct. test: go bpdoc tests test: m soong_docs --- bootstrap/bpdoc/bpdoc.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bootstrap/bpdoc/bpdoc.go b/bootstrap/bpdoc/bpdoc.go index 1a9d820..8ed02c2 100644 --- a/bootstrap/bpdoc/bpdoc.go +++ b/bootstrap/bpdoc/bpdoc.go @@ -161,10 +161,11 @@ func assembleModuleTypeInfo(r *Reader, name string, factory reflect.Value, } nestedName += proptools.PropertyNameForField(nested.Name) nestedProp := ps.GetByName(nestedName) - if nestedProp == nil { - return nil, fmt.Errorf("could not find nested property %q", nestedName) + // Anonymous properties may have already been omitted, no need to ensure they are filtered later + if nestedProp != nil { + // Set property to anonymous to allow future filtering + nestedProp.SetAnonymous() } - nestedProp.SetAnonymous() } } mt.PropertyStructs = append(mt.PropertyStructs, ps)