From 707beb0c6308e0b247f3ab133a7a3bbfade2ff2e Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Thu, 24 Mar 2022 17:13:27 -0400 Subject: [PATCH] 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 Run-TryBot: Robert Findley Reviewed-by: Hyang-Ah Hana Kim gopls-CI: kokoro TryBot-Result: Gopher Robot --- internal/lsp/source/types_format.go | 7 +++++-- internal/lsp/testdata/func_rank/func_rank.go.in | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/lsp/source/types_format.go b/internal/lsp/source/types_format.go index 34d5f28b52..fcbf228eca 100644 --- a/internal/lsp/source/types_format.go +++ b/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 diff --git a/internal/lsp/testdata/func_rank/func_rank.go.in b/internal/lsp/testdata/func_rank/func_rank.go.in index 3706009583..905010b3d4 100644 --- a/internal/lsp/testdata/func_rank/func_rank.go.in +++ b/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)