Fine tune type-alias handling

Empirically, we visit the "real" type before any aliases, so if we find
that a type already has comments, this MUST be an alias.

Test errors look like:

```
W0902 12:41:47.282429 3445399 parse.go:451] Mismatched comments on type k8s.io/gengo/v2/parser/testdata/type-alias/v1.Blah.
  Using comments:
    ```
    Blah is a test.
    A test, I tell you.
    ```
  Ignoring comments from presumed type alias:
    ```
    This is an alias within the same package.
    It comes before the type it aliases.
    ```
W0902 12:41:47.282468 3445399 parse.go:451] Mismatched comments on type k8s.io/gengo/v2/parser/testdata/type-alias/v1.Blah.
  Using comments:
    ```
    Blah is a test.
    A test, I tell you.
    ```
  Ignoring comments from presumed type alias:
    ```
    This is an alias for v1.Blah.
    ```
```

Author: thockin

v2/parser/parse.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/constant"
+	"go/parser"
 	"go/token"
 	gotypes "go/types"
 	"maps"
@@ -37,6 +38,8 @@ import (
 	"k8s.io/klog/v2"
 )
 
+var typeAliasEnabled = goTypeAliasEnabled()
+
 // Parser lets you add all the go files in all the packages that you care
 // about, then constructs the type source data.
 type Parser struct {
@@ -416,61 +419,81 @@ func minimize(lines []string) []string {
 func (p *Parser) addCommentsToType(obj gotypes.Object, t *types.Type) {
 	if newLines, oldLines := p.docComment(obj.Pos()), t.CommentLines; len(newLines) > 0 {
 		switch {
-		case len(oldLines) == 0, reflect.DeepEqual(oldLines, newLines):
+		case reflect.DeepEqual(oldLines, newLines):
+			// nothing needed
+
+		case len(oldLines) == 0:
 			// no comments associated, or comments match exactly
 			t.CommentLines = newLines
 
-		case isTypeAlias(obj.Type()):
-			// ignore mismatched comments from obj because it's an alias
-			if !reflect.DeepEqual(minimize(oldLines), minimize(newLines)) {
-				klog.Warningf(
-					"Mismatched comments seen for type %v. Using comments:\n%s\nIgnoring comments from type alias:\n%s\n",
+		case typeAliasEnabled:
+			if isTypeAlias(obj.Type()) {
+				if !reflect.DeepEqual(minimize(oldLines), minimize(newLines)) {
+					// ignore mismatched comments from obj because it's an alias
+					klog.Warningf(
+						"Mismatched comments on type %v.\n  Using comments:\n%s\n  Ignoring comments from type alias:\n%s\n",
+						t.GoType,
+						formatCommentBlock(oldLines),
+						formatCommentBlock(newLines),
+					)
+				}
+			} else {
+				panic(fmt.Sprintf("type %v already has comments; somehow we processed it again, but it's not an alias\n  old:\n%s\n  new:\n%s",
 					t.GoType,
 					formatCommentBlock(oldLines),
 					formatCommentBlock(newLines),
-				)
+				))
 			}
 
-		case !isTypeAlias(obj.Type()):
-			// overwrite existing comments with ones from obj because obj is not an alias
-			t.CommentLines = newLines
+		case !typeAliasEnabled:
+			// ignore mismatched comments from obj because it's an PROBABLY alias
 			if !reflect.DeepEqual(minimize(oldLines), minimize(newLines)) {
 				klog.Warningf(
-					"Mismatched comments seen for type %v. Using comments:\n%s\nIgnoring comments from type alias:\n%s\n",
+					"Mismatched comments on type %v.\n  Using comments:\n%s\n  Ignoring comments from presumed type alias:\n%s\n",
 					t.GoType,
-					formatCommentBlock(newLines),
 					formatCommentBlock(oldLines),
+					formatCommentBlock(newLines),
 				)
 			}
 		}
 	}
 
 	if newLines, oldLines := p.priorDetachedComment(obj.Pos()), t.SecondClosestCommentLines; len(newLines) > 0 {
 		switch {
-		case len(oldLines) == 0, reflect.DeepEqual(oldLines, newLines):
+		case reflect.DeepEqual(oldLines, newLines):
+			// nothing needed
+
+		case len(oldLines) == 0:
 			// no comments associated, or comments match exactly
 			t.SecondClosestCommentLines = newLines
 
-		case isTypeAlias(obj.Type()):
-			// ignore mismatched comments from obj because it's an alias
-			if !reflect.DeepEqual(minimize(oldLines), minimize(newLines)) {
-				klog.Warningf(
-					"Mismatched secondClosestCommentLines seen for type %v. Using comments:\n%s\nIgnoring comments from type alias:\n%s\n",
+		case typeAliasEnabled:
+			if isTypeAlias(obj.Type()) {
+				if !reflect.DeepEqual(minimize(oldLines), minimize(newLines)) {
+					// ignore mismatched comments from obj because it's an alias
+					klog.Warningf(
+						"Mismatched secondClosestCommentLines on type %v.\n  Using comments:\n%s\n  Ignoring comments from type alias:\n%s\n",
+						t.GoType,
+						formatCommentBlock(oldLines),
+						formatCommentBlock(newLines),
+					)
+				}
+			} else {
+				panic(fmt.Sprintf("type %v already has comments; somehow we processed it again, but it's not an alias\n  old:\n%s\n  new:\n%s",
 					t.GoType,
 					formatCommentBlock(oldLines),
 					formatCommentBlock(newLines),
-				)
+				))
 			}
 
-		case !isTypeAlias(obj.Type()):
-			// overwrite existing comments with ones from obj because obj is not an alias
-			t.SecondClosestCommentLines = newLines
+		case !typeAliasEnabled:
+			// ignore mismatched comments from obj because it's an PROBABLY alias
 			if !reflect.DeepEqual(minimize(oldLines), minimize(newLines)) {
 				klog.Warningf(
-					"Mismatched secondClosestCommentLines seen for type %v. Using comments:\n%s\nIgnoring comments from type alias:\n%s\n",
+					"Mismatched secondClosestCommentLines on type %v.\n  Using comments:\n%s\n  Ignoring comments from presumed type alias:\n%s\n",
 					t.GoType,
-					formatCommentBlock(newLines),
 					formatCommentBlock(oldLines),
+					formatCommentBlock(newLines),
 				)
 			}
 		}
@@ -984,3 +1007,22 @@ func (p *Parser) addConstant(u types.Universe, useName *types.Name, in *gotypes.
 	out.ConstValue = &constval
 	return out
 }
+
+// Copied from https://github.com/golang/tools/blob/3e377036196f644e59e757af8a38ea6afa07677c/internal/aliases/aliases_go122.go#L64
+func goTypeAliasEnabled() bool {
+	// The only reliable way to compute the answer is to invoke go/types.
+	// We don't parse the GODEBUG environment variable, because
+	// (a) it's tricky to do so in a manner that is consistent
+	//     with the godebug package; in particular, a simple
+	//     substring check is not good enough. The value is a
+	//     rightmost-wins list of options. But more importantly:
+	// (b) it is impossible to detect changes to the effective
+	//     setting caused by os.Setenv("GODEBUG"), as happens in
+	//     many tests. Therefore any attempt to cache the result
+	//     is just incorrect.
+	fset := token.NewFileSet()
+	f, _ := parser.ParseFile(fset, "a.go", "package p; type A = int", parser.SkipObjectResolution)
+	pkg, _ := new(gotypes.Config).Check("p", fset, []*ast.File{f}, nil)
+	_, enabled := pkg.Scope().Lookup("A").Type().(*gotypes.Alias)
+	return enabled
+}

v2/parser/parse_test.go

@@ -19,10 +19,6 @@ package parser
 import (
 	"encoding/json"
 	"fmt"
-	"go/ast"
-	"go/parser"
-	"go/token"
-	gotypes "go/types"
 	"path/filepath"
 	"reflect"
 	"sort"
@@ -34,8 +30,6 @@ import (
 	"k8s.io/gengo/v2/types"
 )
 
-var typeAliasEnabled = goTypeAliasEnabled()
-
 func TestNew(t *testing.T) {
 	parser := New()
 	if parser.goPkgs == nil {
@@ -1121,10 +1115,6 @@ func TestStructParse(t *testing.T) {
 			},
 			expected: func() *types.Type {
 				expectedTypeComments := []string{"Blah is a test.", "A test, I tell you."}
-				if !typeAliasEnabled {
-					// Comments from the last processed package wins.
-					expectedTypeComments = []string{"This is an alias for v1.Blah."}
-				}
 
 				return &types.Type{
 					Name: types.Name{
@@ -1231,10 +1221,6 @@ func TestCommentsWithAliasedType(t *testing.T) {
 		}
 
 		expectedTypeComments := []string{"Blah is a test.", "A test, I tell you."}
-		if !typeAliasEnabled {
-			// Comments from the last processed package wins.
-			expectedTypeComments = []string{"This is an alias for v1.Blah."}
-		}
 		for _, typ := range pkg.Types {
 			if typ.Name.Name != "Blah" {
 				continue
@@ -1246,22 +1232,3 @@ func TestCommentsWithAliasedType(t *testing.T) {
 		}
 	}
 }
-
-// Copied from https://github.com/golang/tools/blob/3e377036196f644e59e757af8a38ea6afa07677c/internal/aliases/aliases_go122.go#L64
-func goTypeAliasEnabled() bool {
-	// The only reliable way to compute the answer is to invoke go/types.
-	// We don't parse the GODEBUG environment variable, because
-	// (a) it's tricky to do so in a manner that is consistent
-	//     with the godebug package; in particular, a simple
-	//     substring check is not good enough. The value is a
-	//     rightmost-wins list of options. But more importantly:
-	// (b) it is impossible to detect changes to the effective
-	//     setting caused by os.Setenv("GODEBUG"), as happens in
-	//     many tests. Therefore any attempt to cache the result
-	//     is just incorrect.
-	fset := token.NewFileSet()
-	f, _ := parser.ParseFile(fset, "a.go", "package p; type A = int", parser.SkipObjectResolution)
-	pkg, _ := new(gotypes.Config).Check("p", fset, []*ast.File{f}, nil)
-	_, enabled := pkg.Scope().Lookup("A").Type().(*gotypes.Alias)
-	return enabled
-}

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

@@ -1,5 +1,9 @@
 package v1
 
+// This is an alias within the same package.
+// It comes before the type it aliases.
+type LocalBlahAlias = Blah
+
 // Blah is a test.
 // A test, I tell you.
 type Blah struct {
@@ -11,18 +15,15 @@ type Blah struct {
 	B string `json:"b"`
 }
 
-// This is an alias within the same package.
-type LocalBlahAlias = Blah
-
-// This is for back compat.
-// It has the same number of lines as Blah's comment.
+// Blah is a test.
+// A test, I tell you.
 //
-// Deprecated: use Blah instead.
+// Deprecated: use Blah instead. This is another alias within the same package.
 type LocalBlahAliasDeprecated = Blah
 
-// This is for back compat.
-// It has the same number of lines as Blah's comment.
+// Blah is a test.
+// A test, I tell you.
 //
-// Deprecated: use Blah instead.
+// Deprecated: use Blah instead. This is a third alias within the same package.
 // It's a whole paragraph of deprecated notes
 type LocalBlahAliasDeprecatedLong = Blah