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 <quasilyte@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go
index d9d22a8..788a528 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -714,7 +714,7 @@
}
// 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 @@
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 ada8444..f36f886 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 @@
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 @@
}
}
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 @@
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 aece2c8..a04cb7b 100644
--- a/internal/lsp/testdata/bad/bad1.go
+++ b/internal/lsp/testdata/bad/bad1.go
@@ -10,9 +10,9 @@
}
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 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 2a0b15d..eae6fc3 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -6,7 +6,7 @@
FuzzyCompletionsCount = 7
RankedCompletionsCount = 26
CaseSensitiveCompletionsCount = 4
-DiagnosticsCount = 24
+DiagnosticsCount = 34
FoldingRangesCount = 2
FormatCount = 6
ImportCount = 7
