v2/parser: prefer original comment when evaluating aliased Type

Fixes the comment parsing on a Type so that it does not
get overwritten by an aliased Type with a different comment.
Resolves kubernetes#292

Signed-off-by: Shashank Ram <[email protected]>

Author: shashankram

v2/parser/parse.go

@@ -488,7 +488,12 @@ func (p *Parser) addPkgToUniverse(pkg *packages.Package, u *types.Universe) erro
 		switch obj := s.Lookup(n).(type) {
 		case *gotypes.TypeName:
 			t := p.walkType(*u, nil, obj.Type())
-			p.addCommentsToType(obj, t)
+			// If the resolved type belongs to a different package, it implies
+			// that it is a type alias, in which case we should not update the
+			// comment lines on the original type with that of the alias
+			if t.Name.Package == pkgPath {
+				p.addCommentsToType(obj, t)
+			}
 		case *gotypes.Func:
 			// We only care about functions, not concrete/abstract methods.
 			if obj.Type() != nil && obj.Type().(*gotypes.Signature).Recv() == nil {

v2/parser/parse_test.go

@@ -823,12 +823,14 @@ func TestAddOnePkgToUniverse(t *testing.T) {
 func TestStructParse(t *testing.T) {
 	testCases := []struct {
 		description string
-		testFile    string
+		testFiles   []string
 		expected    func() *types.Type
 	}{
 		{
 			description: "basic comments",
-			testFile:    "k8s.io/gengo/v2/parser/testdata/basic",
+			testFiles: []string{
+				"k8s.io/gengo/v2/parser/testdata/basic",
+			},
 			expected: func() *types.Type {
 				return &types.Type{
 					Name: types.Name{
@@ -860,7 +862,9 @@ func TestStructParse(t *testing.T) {
 		},
 		{
 			description: "generic",
-			testFile:    "./testdata/generic",
+			testFiles: []string{
+				"./testdata/generic",
+			},
 			expected: func() *types.Type {
 				return &types.Type{
 					Name: types.Name{
@@ -897,7 +901,9 @@ func TestStructParse(t *testing.T) {
 		},
 		{
 			description: "generic on field",
-			testFile:    "./testdata/generic-field",
+			testFiles: []string{
+				"./testdata/generic-field",
+			},
 			expected: func() *types.Type {
 				fieldType := &types.Type{
 					Name: types.Name{
@@ -953,7 +959,9 @@ func TestStructParse(t *testing.T) {
 		},
 		{
 			description: "generic multiple",
-			testFile:    "./testdata/generic-multi",
+			testFiles: []string{
+				"./testdata/generic-multi",
+			},
 			expected: func() *types.Type {
 				return &types.Type{
 					Name: types.Name{
@@ -1026,7 +1034,9 @@ func TestStructParse(t *testing.T) {
 		},
 		{
 			description: "generic recursive",
-			testFile:    "./testdata/generic-recursive",
+			testFiles: []string{
+				"./testdata/generic-recursive",
+			},
 			expected: func() *types.Type {
 				recursiveT := &types.Type{
 					Name: types.Name{
@@ -1095,18 +1105,53 @@ func TestStructParse(t *testing.T) {
 				}
 			},
 		},
+		{
+			description: "comments on aliased type should not overwrite original type's comments",
+			testFiles: []string{
+				"k8s.io/gengo/v2/parser/testdata/type-alias/v1",
+				"k8s.io/gengo/v2/parser/testdata/type-alias/v2",
+			},
+			expected: func() *types.Type {
+				return &types.Type{
+					Name: types.Name{
+						Package: "k8s.io/gengo/v2/parser/testdata/type-alias/v1",
+						Name:    "Blah",
+					},
+					Kind:                      types.Struct,
+					CommentLines:              []string{"Blah is a test.", "A test, I tell you."},
+					SecondClosestCommentLines: []string{""},
+					Members: []types.Member{
+						{
+							Name:         "A",
+							Embedded:     false,
+							CommentLines: []string{"A is the first field."},
+							Tags:         `json:"a"`,
+							Type:         types.Int64,
+						},
+						{
+							Name:         "B",
+							Embedded:     false,
+							CommentLines: []string{"B is the second field.", "Multiline comments work."},
+							Tags:         `json:"b"`,
+							Type:         types.String,
+						},
+					},
+					TypeParams: map[string]*types.Type{},
+				}
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
 			parser := New()
 
-			pkgs, err := parser.loadPackages(tc.testFile)
+			_, err := parser.loadPackages(tc.testFiles...)
 			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 			}
-			u := types.Universe{}
-			if err := parser.addPkgToUniverse(pkgs[0], &u); err != nil {
+			u, err := parser.NewUniverse()
+			if err != nil {
 				t.Errorf("unexpected error: %v", err)
 			}
 
@@ -1153,3 +1198,37 @@ func TestGoNameToName(t *testing.T) {
 		})
 	}
 }
+
+func TestCommentsWithAliasedType(t *testing.T) {
+	for i := 0; i < 50; i++ {
+		parser := NewWithOptions(Options{BuildTags: []string{"ignore_autogenerated"}})
+		if _, err := parser.loadPackages("./testdata/type-alias/..."); err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		u, err := parser.NewUniverse()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		for name, pkg := range u {
+			if name != "k8s.io/gengo/v2/parser/testdata/type-alias/v1" {
+				continue
+			}
+			for _, typ := range pkg.Types {
+				if typ.Name.Name != "Blah" {
+					continue
+				}
+				if len(typ.CommentLines) != 2 {
+					t.Fatalf("expected 2 comments, got %d", len(typ.CommentLines))
+				}
+				if typ.CommentLines[0] != "Blah is a test." {
+					t.Errorf("expected comment %q, got %q", "Blah is a test.", typ.CommentLines[0])
+				}
+				if typ.CommentLines[1] != "A test, I tell you." {
+					t.Errorf("expected comment %q, got %q", "A test, I tell you.", typ.CommentLines[1])
+				}
+			}
+		}
+	}
+}

v2/parser/testdata/type-alias/v1/file.go

@@ -0,0 +1,12 @@
+package v1
+
+// Blah is a test.
+// A test, I tell you.
+type Blah struct {
+	// A is the first field.
+	A int64 `json:"a"`
+
+	// B is the second field.
+	// Multiline comments work.
+	B string `json:"b"`
+}

v2/parser/testdata/type-alias/v2/file.go

@@ -0,0 +1,6 @@
+package v2
+
+import v1 "k8s.io/gengo/v2/parser/testdata/type-alias/v1"
+
+// This is an alias for v1.Blah.
+type Blah = v1.Blah