Merge pull request kubernetes#90296 from mandelsoft/master

go-to-protobuf: fix rewrite of embedded struct fields

Author: k8s-ci-robot

staging/src/k8s.io/code-generator/cmd/go-to-protobuf/gogo

@@ -37,6 +37,7 @@ go_test(
     srcs = [
         "cmd_test.go",
         "namer_test.go",
+        "parser_test.go",
     ],
     embed = [":go_default_library"],
 )

staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/BUILD

@@ -375,6 +375,21 @@ func RewriteTypesWithProtobufStructTags(name string, structTags map[string]map[s
 	})
 }
 
+func getFieldName(expr ast.Expr, structname string) (name string, err error) {
+	for {
+		switch t := expr.(type) {
+		case *ast.Ident:
+			return t.Name, nil
+		case *ast.SelectorExpr:
+			return t.Sel.Name, nil
+		case *ast.StarExpr:
+			expr = t.X
+		default:
+			return "", fmt.Errorf("unable to get name for tag from struct %q, field %#v", structname, t)
+		}
+	}
+}
+
 func updateStructTags(decl ast.Decl, structTags map[string]map[string]string, toCopy []string) []error {
 	var errs []error
 	t, ok := decl.(*ast.GenDecl)
@@ -403,14 +418,11 @@ func updateStructTags(decl ast.Decl, structTags map[string]map[string]string, to
 		for i := range st.Fields.List {
 			f := st.Fields.List[i]
 			var name string
+			var err error
 			if len(f.Names) == 0 {
-				switch t := f.Type.(type) {
-				case *ast.Ident:
-					name = t.Name
-				case *ast.SelectorExpr:
-					name = t.Sel.Name
-				default:
-					errs = append(errs, fmt.Errorf("unable to get name for tag from struct %q, field %#v", spec.Name.Name, t))
+				name, err = getFieldName(f.Type, spec.Name.Name)
+				if err != nil {
+					errs = append(errs, err)
 					continue
 				}
 			} else {

staging/src/k8s.io/code-generator/cmd/go-to-protobuf/protobuf/parser.go

@@ -0,0 +1,121 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package protobuf
+
+import (
+	"go/ast"
+	"testing"
+)
+
+/*
+	struct fields in go AST:
+
+     type Struct struct {
+		// fields with a direct field Name as <Ident>
+        A X  // regular fields
+        B *X // pointer fields
+        C    // embedded type field
+
+        // qualified embedded type fields use an <SelExpr> in the AST
+        v1.TypeMeta // X=v1, Sel=TypeMeta
+
+        // fields without a direct name, but
+        // a <StarExpr> in the go-AST
+        *D   // type field embedded as pointer
+        *v1.ListMeta   // qualified type field embedded as pointer
+                       // with <StarExpr> pointing to <SelExpr>
+     }
+*/
+
+func TestProtoParser(t *testing.T) {
+	ident := ast.NewIdent("FieldName")
+	tests := []struct {
+		expr ast.Expr
+		err  bool
+	}{
+		// valid struct field expressions
+		{
+			expr: ident,
+			err:  false,
+		},
+		{
+			expr: &ast.SelectorExpr{
+				Sel: ident,
+			},
+			err: false,
+		},
+		{
+			expr: &ast.StarExpr{
+				X: ident,
+			},
+			err: false,
+		},
+		{
+			expr: &ast.StarExpr{
+				X: &ast.StarExpr{
+					X: ident,
+				},
+			},
+			err: false,
+		},
+		{
+			expr: &ast.StarExpr{
+				X: &ast.SelectorExpr{
+					Sel: ident,
+				},
+			},
+			err: false,
+		},
+
+		// something else should provide an error
+		{
+			expr: &ast.KeyValueExpr{
+				Key:   ident,
+				Colon: 0,
+				Value: ident,
+			},
+			err: true,
+		},
+		{
+			expr: &ast.StarExpr{
+				X: &ast.KeyValueExpr{
+					Key:   ident,
+					Colon: 0,
+					Value: ident,
+				},
+			},
+			err: true,
+		},
+	}
+
+	for _, test := range tests {
+		actual, err := getFieldName(test.expr, "Struct")
+		if !test.err {
+			if err != nil {
+				t.Errorf("%s: unexpected error %s", test.expr, err)
+			} else {
+				if actual != ident.Name {
+					t.Errorf("%s: expected %s, got %s", test.expr, ident.Name, actual)
+				}
+			}
+		} else {
+			if err == nil {
+				t.Errorf("%s: expected error did not occur, got %s instead", test.expr, actual)
+			}
+		}
+	}
+}