From 1109cd96b7693d67e193d48f0d75c7f0c05d2f3c Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 31 Aug 2021 00:07:30 +0000 Subject: [PATCH] bpdocs for struct types created using reflection struct types defined using reflection have empty PkgPaths(), due to which they were not added to the generated bpdocs. This CL allows us to generate bpdocs for such struct types. For each field in the struct, the algorithm checks the following and creates a PropertyStruct object accordingly 1. If field is a primitive type (Base condition) 2. If field is another exported struct (Base condition) 3. If field is another struct created using reflection (Recurse) Test: m soong_docs Test: java_sdk_library_import before https://ci.android.com/builds/submitted/7710820/linux/latest/view/java.html#java_sdk_library_import and after https://spandandas.users.x20web.corp.google.com/docs/java.html#java_sdk_library_import Bug: 172797653 Change-Id: I0349e405fd290d427fa0b38a109b4212aace50c6 --- bootstrap/bpdoc/bpdoc.go | 7 +- bootstrap/bpdoc/properties.go | 23 +++++- bootstrap/bpdoc/properties_test.go | 126 +++++++++++++++++++++++++++++ bootstrap/bpdoc/reader.go | 39 ++++++++- 4 files changed, 186 insertions(+), 9 deletions(-) diff --git a/bootstrap/bpdoc/bpdoc.go b/bootstrap/bpdoc/bpdoc.go index 8ed02c2..b2e1330 100644 --- a/bootstrap/bpdoc/bpdoc.go +++ b/bootstrap/bpdoc/bpdoc.go @@ -121,16 +121,12 @@ func assembleModuleTypeInfo(r *Reader, name string, factory reflect.Value, v := reflect.ValueOf(s).Elem() t := v.Type() - // Ignore property structs with unexported or unnamed types - if t.PkgPath() == "" { - continue - } ps, err := r.PropertyStruct(t.PkgPath(), t.Name(), v) + if err != nil { return nil, err } ps.ExcludeByTag("blueprint", "mutated") - for _, nestedProperty := range nestedPropertyStructs(v) { nestedName := nestedProperty.nestPoint nestedValue := nestedProperty.value @@ -357,7 +353,6 @@ propertyLoop: s := &n[i] if s.SameSubProperties(child) { s.OtherNames = append(s.OtherNames, child.Name) - s.OtherTexts = append(s.OtherTexts, child.Text) continue propertyLoop } } diff --git a/bootstrap/bpdoc/properties.go b/bootstrap/bpdoc/properties.go index 2ca8e65..31b93b1 100644 --- a/bootstrap/bpdoc/properties.go +++ b/bootstrap/bpdoc/properties.go @@ -143,7 +143,26 @@ func (ps *PropertyStruct) GetByName(name string) *Property { } func (ps *PropertyStruct) Nest(nested *PropertyStruct) { - ps.Properties = append(ps.Properties, nested.Properties...) + ps.Properties = nestUnique(ps.Properties, nested.Properties) +} + +// Adds a target element to src if it does not exist in src +func nestUnique(src []Property, target []Property) []Property { + var ret []Property + ret = append(ret, src...) + for _, elem := range target { + isUnique := true + for _, retElement := range ret { + if elem.Equal(retElement) { + isUnique = false + break + } + } + if isUnique { + ret = append(ret, elem) + } + } + return ret } func getByName(name string, prefix string, props *[]Property) *Property { @@ -158,7 +177,7 @@ func getByName(name string, prefix string, props *[]Property) *Property { } func (p *Property) Nest(nested *PropertyStruct) { - p.Properties = append(p.Properties, nested.Properties...) + p.Properties = nestUnique(p.Properties, nested.Properties) } func (p *Property) SetAnonymous() { diff --git a/bootstrap/bpdoc/properties_test.go b/bootstrap/bpdoc/properties_test.go index 085bcdf..b0b3ae4 100644 --- a/bootstrap/bpdoc/properties_test.go +++ b/bootstrap/bpdoc/properties_test.go @@ -16,6 +16,7 @@ package bpdoc import ( "reflect" + "strings" "testing" ) @@ -51,6 +52,131 @@ func TestIncludeByTag(t *testing.T) { } } +func TestPropertiesOfReflectionStructs(t *testing.T) { + testCases := []struct { + fields map[string]interface{} + expectedProperties map[string]Property + description string + }{ + { + fields: map[string]interface{}{ + "A": "A is a string", + "B": 0, //B is an int + }, + expectedProperties: map[string]Property{ + "a": *createProperty("a", "string", ""), + "b": *createProperty("b", "int", ""), + }, + description: "struct is composed of primitive types", + }, + { + fields: map[string]interface{}{ + "A": "A is a string", + "B": 0, //B is an int + "C": props{}, + }, + expectedProperties: map[string]Property{ + "a": *createProperty("a", "string", ""), + "b": *createProperty("b", "int", ""), + "c": *createProperty("c", "props", "props docs."), + }, + description: "struct is composed of primitive types and other structs", + }, + } + + r := NewReader(pkgFiles) + for _, testCase := range testCases { + structType := reflectionStructType(testCase.fields) + ps, err := r.PropertyStruct(structType.PkgPath(), structType.String(), reflect.New(structType).Elem()) + if err != nil { + t.Fatal(err) + } + for _, actualProperty := range ps.Properties { + propName := actualProperty.Name + assertProperties(t, testCase.expectedProperties[propName], actualProperty) + } + } +} + +func TestNestUnique(t *testing.T) { + testCases := []struct { + src []Property + target []Property + expected []Property + description string + }{ + { + src: []Property{}, + target: []Property{}, + expected: []Property{}, + description: "Nest Unique fails for empty slice", + }, + { + src: []Property{*createProperty("a", "string", ""), *createProperty("b", "string", "")}, + target: []Property{}, + expected: []Property{*createProperty("a", "string", ""), *createProperty("b", "string", "")}, + description: "Nest Unique fails when all elements are unique", + }, + { + src: []Property{*createProperty("a", "string", ""), *createProperty("b", "string", "")}, + target: []Property{*createProperty("c", "string", "")}, + expected: []Property{*createProperty("a", "string", ""), *createProperty("b", "string", ""), *createProperty("c", "string", "")}, + description: "Nest Unique fails when all elements are unique", + }, + { + src: []Property{*createProperty("a", "string", ""), *createProperty("b", "string", "")}, + target: []Property{*createProperty("a", "string", "")}, + expected: []Property{*createProperty("a", "string", ""), *createProperty("b", "string", "")}, + description: "Nest Unique fails when nested elements are duplicate", + }, + } + + errMsgTemplate := "%s. Expected: %q, Actual: %q" + for _, testCase := range testCases { + actual := nestUnique(testCase.src, testCase.target) + if len(actual) != len(testCase.expected) { + t.Errorf(errMsgTemplate, testCase.description, testCase.expected, actual) + } + for i := 0; i < len(actual); i++ { + if !actual[i].Equal(testCase.expected[i]) { + t.Errorf(errMsgTemplate, testCase.description, testCase.expected[i], actual[i]) + } + } + } +} + +// Creates a struct using reflection and return its type +func reflectionStructType(fields map[string]interface{}) reflect.Type { + var structFields []reflect.StructField + for fieldname, obj := range fields { + structField := reflect.StructField{ + Name: fieldname, + Type: reflect.TypeOf(obj), + } + structFields = append(structFields, structField) + } + return reflect.StructOf(structFields) +} + +// Creates a Property object with a subset of its props populated +func createProperty(propName string, propType string, propDocs string) *Property { + return &Property{Name: propName, Type: propType, Text: formatText(propDocs)} +} + +// Asserts that two Property objects are "similar" +// Name, Type and Text properties are checked for similarity +func assertProperties(t *testing.T, expected Property, actual Property) { + assertStrings(t, expected.Name, actual.Name) + assertStrings(t, expected.Type, actual.Type) + assertStrings(t, strings.TrimSpace(string(expected.Text)), strings.TrimSpace(string(actual.Text))) +} + +func assertStrings(t *testing.T, expected string, actual string) { + if expected != actual { + t.Errorf("expected: %s, actual: %s", expected, actual) + } +} + func actualProperties(t *testing.T, props []Property) []string { t.Helper() diff --git a/bootstrap/bpdoc/reader.go b/bootstrap/bpdoc/reader.go index a39ee3c..7aa655b 100644 --- a/bootstrap/bpdoc/reader.go +++ b/bootstrap/bpdoc/reader.go @@ -83,7 +83,7 @@ func (r *Reader) ModuleType(name string, factory reflect.Value) (*ModuleType, er // Return the PropertyStruct associated with a property struct type. The type should be in the // format . -func (r *Reader) PropertyStruct(pkgPath, name string, defaults reflect.Value) (*PropertyStruct, error) { +func (r *Reader) propertyStruct(pkgPath, name string, defaults reflect.Value) (*PropertyStruct, error) { ps := r.getPropertyStruct(pkgPath, name) if ps == nil { @@ -113,6 +113,43 @@ func (r *Reader) PropertyStruct(pkgPath, name string, defaults reflect.Value) (* return ps, nil } +// Return the PropertyStruct associated with a struct type using recursion +// This method is useful since golang structs created using reflection have an empty PkgPath() +func (r *Reader) PropertyStruct(pkgPath, name string, defaults reflect.Value) (*PropertyStruct, error) { + var props []Property + + // Base case: primitive type + if defaults.Kind() != reflect.Struct { + props = append(props, Property{Name: name, + Type: defaults.Type().String()}) + return &PropertyStruct{Properties: props}, nil + } + + // Base case: use r.propertyStruct if struct has a non empty pkgpath + if pkgPath != "" { + return r.propertyStruct(pkgPath, name, defaults) + } + + numFields := defaults.NumField() + for i := 0; i < numFields; i++ { + field := defaults.Type().Field(i) + // Recurse + ps, err := r.PropertyStruct(field.Type.PkgPath(), field.Type.Name(), reflect.New(field.Type).Elem()) + + if err != nil { + return nil, err + } + prop := Property{ + Name: strings.ToLower(field.Name), + Text: formatText(ps.Text), + Type: field.Type.Name(), + Properties: ps.Properties, + } + props = append(props, prop) + } + return &PropertyStruct{Properties: props}, nil +} + func (r *Reader) getModuleTypeDoc(pkgPath, factoryFuncName string) (string, error) { goPkg, err := r.goPkg(pkgPath) if err != nil {