x/tools: add explicit Unalias operations

This change is the result of an audit of all type assertions
and type switches whose operand is a types.Type. (These were
enumerated with an analyzer tool.)

If the operand is already the result of a call to Underlying
or Unalias, there is nothing to do, but in other cases,
explicit Unalias operations were added in order to preserve
the existing behavior when go/types starts creating explicit
Alias types.

This change does not address any desired behavior changes
required for the ideal handling of aliases; they will wait
for a followup. In a number of places I have added comments
matching "TODO.*alias".

It may be prudent to split this change by top-level directory,
both for ease of review, and of later bisection if needed.

During the audit, there appeared to be a recurring need
for the following operators:
- (*types.Func).Signature (golang/go#65772);
- Deref(Type): it's easy to forget to strip off the
  Alias constructor;
- ReceiverName (CL 565075), for destructuring receiver
  types such as T and *T, in which up to two Aliases
  might be present.

Updates golang/go#65294

Change-Id: I5180b9bae1c9191807026b8e0dc6f15ed4953b9a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/565035
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

cmd/godex/print.go

@@ -12,6 +12,8 @@ import (
 	"go/types"
 	"io"
 	"math/big"
+
+	"golang.org/x/tools/internal/aliases"
 )
 
 // TODO(gri) use tabwriter for alignment?
@@ -56,7 +58,7 @@ func (p *printer) printf(format string, args ...interface{}) {
 // denoted by obj is not an interface and has methods. Otherwise it returns
 // the zero value.
 func methodsFor(obj *types.TypeName) (*types.Named, []*types.Selection) {
-	named, _ := obj.Type().(*types.Named)
+	named, _ := aliases.Unalias(obj.Type()).(*types.Named)
 	if named == nil {
 		// A type name's type can also be the
 		// exported basic type unsafe.Pointer.

cmd/godex/writetype.go

@@ -12,7 +12,11 @@
 
 package main
 
-import "go/types"
+import (
+	"go/types"
+
+	"golang.org/x/tools/internal/aliases"
+)
 
 func (p *printer) writeType(this *types.Package, typ types.Type) {
 	p.writeTypeInternal(this, typ, make([]types.Type, 8))
@@ -173,6 +177,10 @@ func (p *printer) writeTypeInternal(this *types.Package, typ types.Type, visited
 			p.print(")")
 		}
 
+	case *aliases.Alias:
+		// TODO(adonovan): display something aliasy.
+		p.writeTypeInternal(this, aliases.Unalias(t), visited)
+
 	case *types.Named:
 		s := "<Named w/o object>"
 		if obj := t.Obj(); obj != nil {

cmd/guru/describe.go

@@ -19,6 +19,7 @@ import (
 	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/go/loader"
 	"golang.org/x/tools/go/types/typeutil"
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/typesinternal"
 )
 
@@ -358,7 +359,7 @@ func appendNames(names []*types.Named, typ types.Type) []*types.Named {
 		Elem() types.Type
 	}
 
-	switch t := typ.(type) {
+	switch t := aliases.Unalias(typ).(type) {
 	case *types.Named:
 		names = append(names, t)
 	case *types.Map:
@@ -469,7 +470,7 @@ func describeType(qpos *queryPos, path []ast.Node) (*describeTypeResult, error)
 			description = "alias of "
 		} else if obj.Pos() == n.Pos() {
 			description = "definition of " // (Named type)
-		} else if _, ok := typ.(*types.Basic); ok {
+		} else if _, ok := aliases.Unalias(typ).(*types.Basic); ok {
 			description = "reference to built-in "
 		} else {
 			description = "reference to " // (Named type)
@@ -486,7 +487,7 @@ func describeType(qpos *queryPos, path []ast.Node) (*describeTypeResult, error)
 	description = description + "type " + qpos.typeString(typ)
 
 	// Show sizes for structs and named types (it's fairly obvious for others).
-	switch typ.(type) {
+	switch aliases.Unalias(typ).(type) {
 	case *types.Named, *types.Struct:
 		szs := types.StdSizes{WordSize: 8, MaxAlign: 8} // assume amd64
 		description = fmt.Sprintf("%s (size %d, align %d)", description,
@@ -576,7 +577,7 @@ func (r *describeTypeResult) PrintPlain(printf printfFunc) {
 	printf(r.node, "%s", r.description)
 
 	// Show the underlying type for a reference to a named type.
-	if nt, ok := r.typ.(*types.Named); ok && r.node.Pos() != nt.Obj().Pos() {
+	if nt, ok := aliases.Unalias(r.typ).(*types.Named); ok && r.node.Pos() != nt.Obj().Pos() {
 		// TODO(adonovan): improve display of complex struct/interface types.
 		printf(nt.Obj(), "defined as %s", r.qpos.typeString(nt.Underlying()))
 	}
@@ -585,7 +586,7 @@ func (r *describeTypeResult) PrintPlain(printf printfFunc) {
 	if len(r.methods) == 0 {
 		// Only report null result for type kinds
 		// capable of bearing methods.
-		switch r.typ.(type) {
+		switch aliases.Unalias(r.typ).(type) {
 		case *types.Interface, *types.Struct, *types.Named:
 			printf(r.node, "No methods.")
 		}
@@ -596,7 +597,7 @@ func (r *describeTypeResult) PrintPlain(printf printfFunc) {
 
 func (r *describeTypeResult) JSON(fset *token.FileSet) []byte {
 	var namePos, nameDef string
-	if nt, ok := r.typ.(*types.Named); ok {
+	if nt, ok := aliases.Unalias(r.typ).(*types.Named); ok {
 		namePos = fset.Position(nt.Obj().Pos()).String()
 		nameDef = nt.Underlying().String()
 	}
@@ -727,7 +728,7 @@ func formatMember(obj types.Object, maxname int) string {
 		}
 		var typestr string
 		// Abbreviate long aggregate type names.
-		switch typ := typ.(type) {
+		switch typ := aliases.Unalias(typ).(type) {
 		case *types.Interface:
 			if typ.NumMethods() > 1 {
 				typestr = "interface{...}"

cmd/guru/implements.go

@@ -16,6 +16,7 @@ import (
 	"golang.org/x/tools/cmd/guru/serial"
 	"golang.org/x/tools/go/loader"
 	"golang.org/x/tools/go/types/typeutil"
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/refactor/importgraph"
 )
 
@@ -157,7 +158,7 @@ func implements(q *Query) error {
 	}
 
 	var pos interface{} = qpos
-	if nt, ok := deref(T).(*types.Named); ok {
+	if nt, ok := aliases.Unalias(deref(T)).(*types.Named); ok {
 		pos = nt.Obj()
 	}
 
@@ -230,7 +231,7 @@ func (r *implementsResult) PrintPlain(printf printfFunc) {
 		for i, sub := range r.to {
 			if !isInterface(sub) {
 				if r.method == nil {
-					printf(deref(sub).(*types.Named).Obj(), "\t%s %s type %s",
+					printf(aliases.Unalias(deref(sub)).(*types.Named).Obj(), "\t%s %s type %s",
 						relation, typeKind(sub), r.qpos.typeString(sub))
 				} else {
 					meth(r.toMethod[i])
@@ -240,7 +241,7 @@ func (r *implementsResult) PrintPlain(printf printfFunc) {
 		for i, sub := range r.to {
 			if isInterface(sub) {
 				if r.method == nil {
-					printf(sub.(*types.Named).Obj(), "\t%s %s type %s",
+					printf(aliases.Unalias(sub).(*types.Named).Obj(), "\t%s %s type %s",
 						relation, typeKind(sub), r.qpos.typeString(sub))
 				} else {
 					meth(r.toMethod[i])
@@ -251,7 +252,7 @@ func (r *implementsResult) PrintPlain(printf printfFunc) {
 		relation = "implements"
 		for i, super := range r.from {
 			if r.method == nil {
-				printf(super.(*types.Named).Obj(), "\t%s %s",
+				printf(aliases.Unalias(super).(*types.Named).Obj(), "\t%s %s",
 					relation, r.qpos.typeString(super))
 			} else {
 				meth(r.fromMethod[i])
@@ -270,7 +271,7 @@ func (r *implementsResult) PrintPlain(printf printfFunc) {
 			}
 			for i, super := range r.from {
 				if r.method == nil {
-					printf(super.(*types.Named).Obj(), "\t%s %s",
+					printf(aliases.Unalias(super).(*types.Named).Obj(), "\t%s %s",
 						relation, r.qpos.typeString(super))
 				} else {
 					meth(r.fromMethod[i])
@@ -288,7 +289,7 @@ func (r *implementsResult) PrintPlain(printf printfFunc) {
 
 			for i, psuper := range r.fromPtr {
 				if r.method == nil {
-					printf(psuper.(*types.Named).Obj(), "\t%s %s",
+					printf(aliases.Unalias(psuper).(*types.Named).Obj(), "\t%s %s",
 						relation, r.qpos.typeString(psuper))
 				} else {
 					meth(r.fromPtrMethod[i])
@@ -332,7 +333,7 @@ func makeImplementsTypes(tt []types.Type, fset *token.FileSet) []serial.Implemen
 
 func makeImplementsType(T types.Type, fset *token.FileSet) serial.ImplementsType {
 	var pos token.Pos
-	if nt, ok := deref(T).(*types.Named); ok { // implementsResult.t may be non-named
+	if nt, ok := aliases.Unalias(deref(T)).(*types.Named); ok { // implementsResult.t may be non-named
 		pos = nt.Obj().Pos()
 	}
 	return serial.ImplementsType{

go/analysis/passes/composite/composite.go

@@ -15,6 +15,7 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/typeparams"
 )
 
@@ -71,7 +72,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			return
 		}
 		var structuralTypes []types.Type
-		switch typ := typ.(type) {
+		switch typ := aliases.Unalias(typ).(type) {
 		case *types.TypeParam:
 			terms, err := typeparams.StructuralTerms(typ)
 			if err != nil {
@@ -84,7 +85,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			structuralTypes = append(structuralTypes, typ)
 		}
 		for _, typ := range structuralTypes {
-			under := deref(typ.Underlying())
+			// TODO(adonovan): this operation is questionable.
+			under := aliases.Unalias(deref(typ.Underlying()))
 			strct, ok := under.(*types.Struct)
 			if !ok {
 				// skip non-struct composite literals
@@ -142,9 +144,11 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
+// Note: this is not the usual deref operator!
+// It strips off all Pointer constructors (and their Aliases).
 func deref(typ types.Type) types.Type {
 	for {
-		ptr, ok := typ.(*types.Pointer)
+		ptr, ok := aliases.Unalias(typ).(*types.Pointer)
 		if !ok {
 			break
 		}
@@ -153,18 +157,18 @@ func deref(typ types.Type) types.Type {
 	return typ
 }
 
+// isLocalType reports whether typ belongs to the same package as pass.
+// TODO(adonovan): local means "internal to a function"; rename to isSamePackageType.
 func isLocalType(pass *analysis.Pass, typ types.Type) bool {
-	switch x := typ.(type) {
+	switch x := aliases.Unalias(typ).(type) {
 	case *types.Struct:
 		// struct literals are local types
 		return true
 	case *types.Pointer:
 		return isLocalType(pass, x.Elem())
-	case *types.Named:
+	case interface{ Obj() *types.TypeName }: // *Named or *TypeParam (aliases were removed already)
 		// names in package foo are local to foo_test too
 		return strings.TrimSuffix(x.Obj().Pkg().Path(), "_test") == strings.TrimSuffix(pass.Pkg.Path(), "_test")
-	case *types.TypeParam:
-		return strings.TrimSuffix(x.Obj().Pkg().Path(), "_test") == strings.TrimSuffix(pass.Pkg.Path(), "_test")
 	}
 	return false
 }

go/analysis/passes/copylock/copylock.go

@@ -18,6 +18,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
 	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/typeparams"
 )
 
@@ -255,7 +256,7 @@ func lockPath(tpkg *types.Package, typ types.Type, seen map[types.Type]bool) typ
 	}
 	seen[typ] = true
 
-	if tpar, ok := typ.(*types.TypeParam); ok {
+	if tpar, ok := aliases.Unalias(typ).(*types.TypeParam); ok {
 		terms, err := typeparams.StructuralTerms(tpar)
 		if err != nil {
 			return nil // invalid type

go/analysis/passes/deepequalerrors/deepequalerrors.go

@@ -102,8 +102,7 @@ func containsError(typ types.Type) bool {
 					return true
 				}
 			}
-		case *types.Named,
-			*aliases.Alias:
+		case *types.Named, *aliases.Alias:
 			return check(t.Underlying())
 
 		// We list the remaining valid type kinds for completeness.

go/analysis/passes/httpresponse/httpresponse.go

@@ -14,6 +14,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
 	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/typesinternal"
 )
 
@@ -136,7 +137,7 @@ func isHTTPFuncOrMethodOnClient(info *types.Info, expr *ast.CallExpr) bool {
 	if analysisutil.IsNamedType(typ, "net/http", "Client") {
 		return true // method on http.Client.
 	}
-	ptr, ok := typ.(*types.Pointer)
+	ptr, ok := aliases.Unalias(typ).(*types.Pointer)
 	return ok && analysisutil.IsNamedType(ptr.Elem(), "net/http", "Client") // method on *http.Client.
 }
 

go/analysis/passes/ifaceassert/parameterized.go

@@ -7,6 +7,7 @@ package ifaceassert
 import (
 	"go/types"
 
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/typeparams"
 )
 
@@ -94,6 +95,10 @@ func (w *tpWalker) isParameterized(typ types.Type) (res bool) {
 	case *types.Chan:
 		return w.isParameterized(t.Elem())
 
+	case *aliases.Alias:
+		// TODO(adonovan): think about generic aliases.
+		return w.isParameterized(aliases.Unalias(t))
+
 	case *types.Named:
 		list := t.TypeArgs()
 		for i, n := 0, list.Len(); i < n; i++ {

go/analysis/passes/internal/analysisutil/util.go

@@ -14,6 +14,7 @@ import (
 	"go/types"
 	"os"
 
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/analysisinternal"
 )
 
@@ -115,7 +116,7 @@ func Imports(pkg *types.Package, path string) bool {
 // This function avoids allocating the concatenation of "pkg.Name",
 // which is important for the performance of syntax matching.
 func IsNamedType(t types.Type, pkgPath string, names ...string) bool {
-	n, ok := t.(*types.Named)
+	n, ok := aliases.Unalias(t).(*types.Named)
 	if !ok {
 		return false
 	}

go/analysis/passes/printf/printf.go

@@ -24,6 +24,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/typeutil"
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/typeparams"
 )
 
@@ -959,6 +960,8 @@ func isStringer(sig *types.Signature) bool {
 // It is almost always a mistake to print a function value.
 func isFunctionValue(pass *analysis.Pass, e ast.Expr) bool {
 	if typ := pass.TypesInfo.Types[e].Type; typ != nil {
+		// Don't call Underlying: a named func type with a String method is ok.
+		// TODO(adonovan): it would be more precise to check isStringer.
 		_, ok := typ.(*types.Signature)
 		return ok
 	}
@@ -1010,7 +1013,7 @@ func checkPrint(pass *analysis.Pass, call *ast.CallExpr, fn *types.Func) {
 		// Skip checking functions with unknown type.
 		return
 	}
-	if sig, ok := typ.(*types.Signature); ok {
+	if sig, ok := typ.Underlying().(*types.Signature); ok {
 		if !sig.Variadic() {
 			// Skip checking non-variadic functions.
 			return
@@ -1020,7 +1023,7 @@ func checkPrint(pass *analysis.Pass, call *ast.CallExpr, fn *types.Func) {
 
 		typ := params.At(firstArg).Type()
 		typ = typ.(*types.Slice).Elem()
-		it, ok := typ.(*types.Interface)
+		it, ok := aliases.Unalias(typ).(*types.Interface)
 		if !ok || !it.Empty() {
 			// Skip variadic functions accepting non-interface{} args.
 			return

go/analysis/passes/printf/types.go

@@ -10,6 +10,7 @@ import (
 	"go/types"
 
 	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/typeparams"
 )
 
@@ -72,7 +73,7 @@ func (m *argMatcher) match(typ types.Type, topLevel bool) bool {
 		return true
 	}
 
-	if typ, _ := typ.(*types.TypeParam); typ != nil {
+	if typ, _ := aliases.Unalias(typ).(*types.TypeParam); typ != nil {
 		// Avoid infinite recursion through type parameters.
 		if m.seen[typ] {
 			return true
@@ -275,7 +276,7 @@ func (m *argMatcher) match(typ types.Type, topLevel bool) bool {
 }
 
 func isConvertibleToString(typ types.Type) bool {
-	if bt, ok := typ.(*types.Basic); ok && bt.Kind() == types.UntypedNil {
+	if bt, ok := aliases.Unalias(typ).(*types.Basic); ok && bt.Kind() == types.UntypedNil {
 		// We explicitly don't want untyped nil, which is
 		// convertible to both of the interfaces below, as it
 		// would just panic anyway.