From ec14b2965194f60e706c3b6fe884dfdae094ddcf Mon Sep 17 00:00:00 2001 From: Iskander Sharipov Date: Fri, 6 Dec 2019 20:59:56 +0300 Subject: [PATCH] internal/lsp/source: fix all types in resolveInvalid This CL teaches lsp to report `**T` instead of `**invalid type`, `func (badParam) badResult` instead of `func (invalid type) invalid type`, etc. To do that, we need to detect "invalid type" inside any part of a type. I've added typeIsValid() function for that. To simplify type formating code in resolveInvalid(), formatNode function is added that can also format *ast.StarExpr (of any depth). Since we already used AST printer in the same file, I added formatNode function that is now used in both places. While at it, replaced bytes.Buffer to strings.Builder there. Change-Id: I3bb84c58c417b175cceefb410e238c48425f7cee Reviewed-on: https://go-review.googlesource.com/c/tools/+/210357 Run-TryBot: Iskander Sharipov TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/source/completion.go | 5 +- internal/lsp/source/util.go | 69 +++++++++++++++--------- internal/lsp/testdata/bad/bad1.go | 13 +++-- internal/lsp/testdata/summary.txt.golden | 2 +- 4 files changed, 59 insertions(+), 30 deletions(-) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index d9d22a893e..788a528ebd 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -714,7 +714,7 @@ func (c *completer) lexical() error { } // If obj's type is invalid, find the AST node that defines the lexical block // containing the declaration of obj. Don't resolve types for packages. - if _, ok := obj.(*types.PkgName); !ok && obj.Type() == types.Typ[types.Invalid] { + if _, ok := obj.(*types.PkgName); !ok && !typeIsValid(obj.Type()) { // Match the scope to its ast.Node. If the scope is the package scope, // use the *ast.File as the starting node. var node ast.Node @@ -724,7 +724,8 @@ func (c *completer) lexical() error { node = c.path[i-1] } if node != nil { - if resolved := resolveInvalid(obj, node, c.pkg.GetTypesInfo()); resolved != nil { + fset := c.snapshot.View().Session().Cache().FileSet() + if resolved := resolveInvalid(fset, obj, node, c.pkg.GetTypesInfo()); resolved != nil { obj = resolved } } diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index ada8444419..f36f88651b 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -5,9 +5,7 @@ package source import ( - "bytes" "context" - "fmt" "go/ast" "go/printer" "go/token" @@ -320,24 +318,42 @@ func fieldSelections(T types.Type) (fields []*types.Var) { return fields } +// typeIsValid reports whether typ doesn't contain any Invalid types. +func typeIsValid(typ types.Type) bool { + switch typ := typ.Underlying().(type) { + case *types.Basic: + return typ.Kind() != types.Invalid + case *types.Array: + return typeIsValid(typ.Elem()) + case *types.Slice: + return typeIsValid(typ.Elem()) + case *types.Pointer: + return typeIsValid(typ.Elem()) + case *types.Map: + return typeIsValid(typ.Key()) && typeIsValid(typ.Elem()) + case *types.Chan: + return typeIsValid(typ.Elem()) + case *types.Signature: + return typeIsValid(typ.Params()) && typeIsValid(typ.Results()) + case *types.Tuple: + for i := 0; i < typ.Len(); i++ { + if !typeIsValid(typ.At(i).Type()) { + return false + } + } + return true + case *types.Struct, *types.Interface, *types.Named: + // Don't bother checking structs, interfaces, or named types for validity. + return true + default: + return false + } +} + // resolveInvalid traverses the node of the AST that defines the scope // containing the declaration of obj, and attempts to find a user-friendly // name for its invalid type. The resulting Object and its Type are fake. -func resolveInvalid(obj types.Object, node ast.Node, info *types.Info) types.Object { - // Construct a fake type for the object and return a fake object with this type. - formatResult := func(expr ast.Expr) types.Object { - var typename string - switch t := expr.(type) { - case *ast.SelectorExpr: - typename = fmt.Sprintf("%s.%s", t.X, t.Sel) - case *ast.Ident: - typename = t.String() - default: - return nil - } - typ := types.NewNamed(types.NewTypeName(token.NoPos, obj.Pkg(), typename, nil), types.Typ[types.Invalid], nil) - return types.NewVar(obj.Pos(), obj.Pkg(), obj.Name(), typ) - } +func resolveInvalid(fset *token.FileSet, obj types.Object, node ast.Node, info *types.Info) types.Object { var resultExpr ast.Expr ast.Inspect(node, func(node ast.Node) bool { switch n := node.(type) { @@ -355,12 +371,14 @@ func resolveInvalid(obj types.Object, node ast.Node, info *types.Info) types.Obj } } return false - // TODO(rstambler): Handle range statements. default: return true } }) - return formatResult(resultExpr) + // Construct a fake type for the object and return a fake object with this type. + typename := formatNode(fset, resultExpr) + typ := types.NewNamed(types.NewTypeName(token.NoPos, obj.Pkg(), typename, nil), types.Typ[types.Invalid], nil) + return types.NewVar(obj.Pos(), obj.Pkg(), obj.Name(), typ) } func isPointer(T types.Type) bool { @@ -489,12 +507,15 @@ func formatFieldType(s Snapshot, srcpkg Package, obj types.Object, qf types.Qual if !ok { return "", errors.Errorf("ident %s is not a field type", ident.Name) } - var typeNameBuf bytes.Buffer - fset := s.View().Session().Cache().FileSet() - if err := printer.Fprint(&typeNameBuf, fset, f.Type); err != nil { - return "", err + return formatNode(s.View().Session().Cache().FileSet(), f.Type), nil +} + +func formatNode(fset *token.FileSet, n ast.Node) string { + var buf strings.Builder + if err := printer.Fprint(&buf, fset, n); err != nil { + return "" } - return typeNameBuf.String(), nil + return buf.String() } func formatResults(tup *types.Tuple, qf types.Qualifier) ([]string, bool) { diff --git a/internal/lsp/testdata/bad/bad1.go b/internal/lsp/testdata/bad/bad1.go index aece2c803d..a04cb7b536 100644 --- a/internal/lsp/testdata/bad/bad1.go +++ b/internal/lsp/testdata/bad/bad1.go @@ -10,9 +10,9 @@ func random() int { //@item(random, "random", "func() int", "func") } func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func"),item(bad_y_param, "y", "int", "var") - x := 6 //@item(x, "x", "int", "var"),diag("x", "compiler", "x declared but not used") - var q blah //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used"),diag("blah", "compiler", "undeclared name: blah") - var t blob //@item(t, "t", "blob", "var"),diag("t", "compiler", "t declared but not used"),diag("blob", "compiler", "undeclared name: blob") + x := 6 //@item(x, "x", "int", "var"),diag("x", "compiler", "x declared but not used") + var q blah //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used"),diag("blah", "compiler", "undeclared name: blah") + var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared but not used"),diag("blob", "compiler", "undeclared name: blob") //@complete("", q, t, x, bad_y_param, global_a, bob, random, random2, random3, stuff) return y @@ -20,4 +20,11 @@ func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func") func random3(y ...int) { //@item(random3, "random3", "func(y ...int)", "func"),item(y_variadic_param, "y", "[]int", "var") //@complete("", y_variadic_param, global_a, bob, random, random2, random3, stuff) + + var ch chan (favType1) //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared but not used"),diag("favType1", "compiler", "undeclared name: favType1") + var m map[keyType]int //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared but not used"),diag("keyType", "compiler", "undeclared name: keyType") + var arr []favType2 //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared but not used"),diag("favType2", "compiler", "undeclared name: favType2") + var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared but not used"),diag("badResult", "compiler", "undeclared name: badResult") + var fn2 func(badParam) //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared but not used"),diag("badParam", "compiler", "undeclared name: badParam") + //@complete("", arr, ch, fn1, fn2, m, y_variadic_param, global_a, bob, random, random2, random3, stuff) } diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index 2a0b15d62c..eae6fc31b7 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -6,7 +6,7 @@ DeepCompletionsCount = 5 FuzzyCompletionsCount = 7 RankedCompletionsCount = 26 CaseSensitiveCompletionsCount = 4 -DiagnosticsCount = 24 +DiagnosticsCount = 34 FoldingRangesCount = 2 FormatCount = 6 ImportCount = 7