diff --git a/pkg/generators/rules/names_match.go b/pkg/generators/rules/names_match.go index af30edc5e..d7655f0d9 100644 --- a/pkg/generators/rules/names_match.go +++ b/pkg/generators/rules/names_match.go @@ -32,14 +32,6 @@ var ( "-", ) - // Blacklist of JSON names that should skip match evaluation - jsonNameBlacklist = sets.NewString( - // Empty name is used for inline struct field (e.g. metav1.TypeMeta) - "", - // Special case for object and list meta - "metadata", - ) - // List of substrings that aren't allowed in Go name and JSON name disallowedNameSubstrings = sets.NewString( // Underscore is not allowed in either name @@ -73,12 +65,11 @@ is also considered matched. HTTPJSONSpec httpjsonSpec true -NOTE: JSON names in jsonNameBlacklist should skip evaluation +NOTE: an empty JSON name is valid only for inlined structs or pointer to structs. +It cannot be empty for anything else because capitalization must be set explicitly. - true - podSpec true - podSpec - true - podSpec metadata true +NOTE: metav1.ListMeta and metav1.ObjectMeta by convention must have "metadata" as name. +Other fields may have that JSON name if the field name matches. */ type NamesMatch struct{} @@ -109,7 +100,7 @@ func (n *NamesMatch) Validate(t *types.Type) ([]string, error) { continue } jsonName := strings.Split(jsonTag, ",")[0] - if !namesMatch(goName, jsonName) { + if !nameIsOkay(m, jsonName) { fields = append(fields, goName) } } @@ -117,6 +108,22 @@ func (n *NamesMatch) Validate(t *types.Type) ([]string, error) { return fields, nil } +func nameIsOkay(member types.Member, jsonName string) bool { + if jsonName == "" { + return member.Type.Kind == types.Struct || + member.Type.Kind == types.Pointer && member.Type.Elem.Kind == types.Struct + } + + typeName := member.Type.String() + switch typeName { + case "k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta", + "k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta": + return jsonName == "metadata" + } + + return namesMatch(member.Name, jsonName) +} + // namesMatch evaluates if goName and jsonName match the API rule // TODO: Use an off-the-shelf CamelCase solution instead of implementing this logic. The following existing // @@ -129,9 +136,6 @@ func (n *NamesMatch) Validate(t *types.Type) ([]string, error) { // about why they don't satisfy our need. What we need can be a function that detects an acronym at the // beginning of a string. func namesMatch(goName, jsonName string) bool { - if jsonNameBlacklist.Has(jsonName) { - return true - } if !isAllowedName(goName) || !isAllowedName(jsonName) { return false } diff --git a/pkg/generators/rules/names_match_test.go b/pkg/generators/rules/names_match_test.go index e45e6d1bb..1e0954704 100644 --- a/pkg/generators/rules/names_match_test.go +++ b/pkg/generators/rules/names_match_test.go @@ -24,6 +24,39 @@ import ( ) func TestNamesMatch(t *testing.T) { + someStruct := &types.Type{ + Name: types.Name{Name: "SomeStruct"}, + Kind: types.Struct, + } + someStructPtr := &types.Type{ + Name: types.Name{Name: "SomeStructPtr"}, + Kind: types.Pointer, + Elem: someStruct, + } + intPtr := &types.Type{ + Name: types.Name{Name: "IntPtr"}, + Kind: types.Pointer, + Elem: types.Int, + } + listMeta := &types.Type{ + Name: types.Name{Package: "k8s.io/apimachinery/pkg/apis/meta/v1", Name: "ListMeta"}, + Kind: types.Struct, + } + listMetaPtr := &types.Type{ + Name: types.Name{Package: "k8s.io/apimachinery/pkg/apis/meta/v1", Name: "ListMetaPtr"}, + Kind: types.Pointer, + Elem: listMeta, + } + objectMeta := &types.Type{ + Name: types.Name{Package: "k8s.io/apimachinery/pkg/apis/meta/v1", Name: "ObjectMeta"}, + Kind: types.Struct, + } + objectMetaPtr := &types.Type{ + Name: types.Name{Package: "k8s.io/apimachinery/pkg/apis/meta/v1", Name: "ObjectMetaPtr"}, + Kind: types.Pointer, + Elem: objectMeta, + } + tcs := []struct { // name of test case name string @@ -231,36 +264,7 @@ func TestNamesMatch(t *testing.T) { }, expected: []string{"PodSpec"}, }, - // NOTE: JSON names in jsonNameBlacklist should skip evaluation - // {"", "", true}, - { - name: "unspecified", - t: &types.Type{ - Kind: types.Struct, - Members: []types.Member{ - { - Name: "", - Tags: `json:""`, - }, - }, - }, - expected: []string{}, - }, - // {"podSpec", "", true}, - { - name: "blacklist_empty", - t: &types.Type{ - Kind: types.Struct, - Members: []types.Member{ - { - Name: "podSpec", - Tags: `json:""`, - }, - }, - }, - expected: []string{}, - }, - // {"podSpec", "metadata", true}, + // {"podSpec", "metadata", false}, { name: "blacklist_metadata", t: &types.Type{ @@ -272,7 +276,7 @@ func TestNamesMatch(t *testing.T) { }, }, }, - expected: []string{}, + expected: []string{"podSpec"}, }, { name: "non_struct", @@ -338,6 +342,92 @@ func TestNamesMatch(t *testing.T) { }, expected: []string{"Pod_Spec"}, }, + { + name: "empty_JSON_name", + t: &types.Type{ + Kind: types.Struct, + Members: []types.Member{ + { + Name: "Int", + Tags: `json:""`, // Not okay! + Type: types.Int, + }, + { + Name: "Struct", + Tags: `json:""`, // Okay, inlined. + Type: someStruct, + }, + { + Name: "IntPtr", + Tags: `json:""`, // Not okay! + Type: intPtr, + }, + { + Name: "StructPtr", + Tags: `json:""`, // Okay, inlined. + Type: someStructPtr, + }, + }, + }, + expected: []string{ + "Int", + "IntPtr", + }, + }, + { + name: "metadata_no_pointers", + t: &types.Type{ + Kind: types.Struct, + Members: []types.Member{ + { + Name: "ListMeta", + Tags: `json:"listMeta"`, // Not okay, should be "metadata"! + Type: listMeta, + }, + { + Name: "ObjectMeta", + Tags: `json:"objectMeta"`, // Not okay, should be metadata"! + Type: objectMeta, + }, + { + Name: "SomeStruct", + Tags: `json:"metadata"`, // Not okay, name mismatch! + Type: someStruct, + }, + }, + }, + expected: []string{ + "ListMeta", + "ObjectMeta", + "SomeStruct", + }, + }, + { + name: "metadata_pointers", + t: &types.Type{ + Kind: types.Struct, + Members: []types.Member{ + { + Name: "ListMeta", + Tags: `json:"listMeta"`, // Okay, convention only applies to struct. + Type: listMetaPtr, + }, + { + Name: "ObjectMeta", + Tags: `json:"objectMeta"`, // Okay, convention only applies to struct. + Type: objectMetaPtr, + }, + { + Name: "SomeStruct", + Tags: `json:"metadata"`, // Not okay, name mismatch! + Type: someStructPtr, + }, + }, + }, + expected: []string{ + "SomeStruct", + }, + }, } n := &NamesMatch{}