internal/lsp/source: descend into fields and field lists in qualifyExpr

We do a lot of gymnastics to format var types, working around the lack
of alias tracking in go/types. As part of this, we clone and qualify
expressions. In this case, we were not qualifying identifiers that were
contained within fields or field lists.

Fix this by updating our expression traversal to include *ast.Field and
*ast.FieldList.

Fixes golang/go#50539

Change-Id: I6531c6a51aa402bd784778b8bedaa3dccee75af0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/395678
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

internal/lsp/source/types_format.go

@@ -252,7 +252,7 @@ func NewSignature(ctx context.Context, s Snapshot, pkg Package, sig *types.Signa
 
 // FormatVarType formats a *types.Var, accounting for type aliases.
 // To do this, it looks in the AST of the file in which the object is declared.
-// On any errors, it always fallbacks back to types.TypeString.
+// On any errors, it always falls back to types.TypeString.
 func FormatVarType(ctx context.Context, snapshot Snapshot, srcpkg Package, obj *types.Var, qf types.Qualifier) string {
 	pkg, err := FindPackageFromPos(ctx, snapshot, obj.Pos())
 	if err != nil {
@@ -304,10 +304,13 @@ func qualifyExpr(expr ast.Expr, srcpkg, pkg Package, clonedInfo map[token.Pos]*t
 		switch n := n.(type) {
 		case *ast.ArrayType, *ast.ChanType, *ast.Ellipsis,
 			*ast.FuncType, *ast.MapType, *ast.ParenExpr,
-			*ast.StarExpr, *ast.StructType:
+			*ast.StarExpr, *ast.StructType, *ast.FieldList, *ast.Field:
 			// These are the only types that are cloned by cloneExpr below,
 			// so these are the only types that we can traverse and potentially
 			// modify. This is not an ideal approach, but it works for now.
+
+			// TODO(rFindley): can we eliminate this filtering entirely? This caused
+			// bugs in the past (golang/go#50539)
 			return true
 		case *ast.SelectorExpr:
 			// We may need to change any selectors in which the X is a package

internal/lsp/testdata/func_rank/func_rank.go.in

@@ -63,7 +63,7 @@ func _() {
 }
 
 func _() {
-	HandleFunc //@item(httpHandleFunc, "HandleFunc", "func(pattern string, handler func(ResponseWriter, *Request))", "func")
+	HandleFunc //@item(httpHandleFunc, "HandleFunc", "func(pattern string, handler func(http.ResponseWriter, *http.Request))", "func")
 	HandlerFunc //@item(httpHandlerFunc, "HandlerFunc", "func(http.ResponseWriter, *http.Request)", "type")
 
 	http.HandleFunc //@rank(" //", httpHandleFunc, httpHandlerFunc)