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>

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
 					}
 				}

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) {

internal/lsp/testdata/bad/bad1.go

@@ -12,7 +12,7 @@ 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")
+	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)
 }

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