go/types: fix typo causing loss of embedded interfaces

Simplified the code per prior suggestion to avoid that
kind of error in the first place.

Also: Fix subtle error in Interface.Complete where an
interface may have ended up incomplete if both the list
of methods and the list of embedded interfaces was nil.

Expanded existing test to cover all these cases.

Fixes golang/go#25577

Change-Id: If8723a8b0c4570f02b3dadfa390f96dd98ce11c8
Reviewed-on: https://go-review.googlesource.com/114504
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/go/types/type.go b/src/go/types/type.go
index cc87f1edb57d95a754dfd287fc18dea4389b7338..60e3efaec314e52172f02af41e2865a296e7dcc7 100644 (file)
--- a/src/go/types/type.go
+++ b/src/go/types/type.go
@@ -264,12 +264,9 @@ var markComplete = make([]*Func, 0)
 // to be embedded. This is necessary for interfaces that embed alias type names referring to
 // non-defined (literal) interface types.
 func NewInterface(methods []*Func, embeddeds []*Named) *Interface {
-       var tnames []Type
-       if len(embeddeds) > 0 {
-               tnames := make([]Type, len(embeddeds))
-               for i, t := range embeddeds {
-                       tnames[i] = t
-               }
+       tnames := make([]Type, len(embeddeds))
+       for i, t := range embeddeds {
+               tnames[i] = t
        }
        return NewInterface2(methods, tnames)
 }
@@ -356,27 +353,24 @@ func (t *Interface) Complete() *Interface {
        }
 
        var allMethods []*Func
-       if t.embeddeds == nil {
-               if t.methods == nil {
-                       allMethods = make([]*Func, 0, 1)
-               } else {
-                       allMethods = t.methods
-               }
-       } else {
-               allMethods = append(allMethods, t.methods...)
-               for _, et := range t.embeddeds {
-                       it := et.Underlying().(*Interface)
-                       it.Complete()
-                       for _, tm := range it.allMethods {
-                               // Make a copy of the method and adjust its receiver type.
-                               newm := *tm
-                               newmtyp := *tm.typ.(*Signature)
-                               newm.typ = &newmtyp
-                               newmtyp.recv = NewVar(newm.pos, newm.pkg, "", t)
-                               allMethods = append(allMethods, &newm)
-                       }
+       allMethods = append(allMethods, t.methods...)
+       for _, et := range t.embeddeds {
+               it := et.Underlying().(*Interface)
+               it.Complete()
+               for _, tm := range it.allMethods {
+                       // Make a copy of the method and adjust its receiver type.
+                       newm := *tm
+                       newmtyp := *tm.typ.(*Signature)
+                       newm.typ = &newmtyp
+                       newmtyp.recv = NewVar(newm.pos, newm.pkg, "", t)
+                       allMethods = append(allMethods, &newm)
                }
-               sort.Sort(byUniqueMethodName(allMethods))
+       }
+       sort.Sort(byUniqueMethodName(allMethods))
+
+       // t.methods and/or t.embeddeds may have been empty
+       if allMethods == nil {
+               allMethods = markComplete
        }
        t.allMethods = allMethods
 

diff --git a/src/go/types/typestring_test.go b/src/go/types/typestring_test.go
index 78f67d1f050028af821188e2d2e24e3a352c9022..6ed2d75dfe3f17d2b2a6dc2eed52bebe630db9b1 100644 (file)
--- a/src/go/types/typestring_test.go
+++ b/src/go/types/typestring_test.go
@@ -140,16 +140,41 @@ func TestTypeString(t *testing.T) {
 
 func TestIncompleteInterfaces(t *testing.T) {
        sig := NewSignature(nil, nil, nil, false)
+       m := NewFunc(token.NoPos, nil, "m", sig)
        for _, test := range []struct {
                typ  *Interface
                want string
        }{
                {new(Interface), "interface{/* incomplete */}"},
                {new(Interface).Complete(), "interface{}"},
+
+               {NewInterface(nil, nil), "interface{/* incomplete */}"},
+               {NewInterface(nil, nil).Complete(), "interface{}"},
+               {NewInterface([]*Func{}, nil), "interface{/* incomplete */}"},
+               {NewInterface([]*Func{}, nil).Complete(), "interface{}"},
+               {NewInterface(nil, []*Named{}), "interface{/* incomplete */}"},
+               {NewInterface(nil, []*Named{}).Complete(), "interface{}"},
+               {NewInterface([]*Func{m}, nil), "interface{m() /* incomplete */}"},
+               {NewInterface([]*Func{m}, nil).Complete(), "interface{m()}"},
+               {NewInterface(nil, []*Named{newDefined(new(Interface).Complete())}), "interface{T /* incomplete */}"},
+               {NewInterface(nil, []*Named{newDefined(new(Interface).Complete())}).Complete(), "interface{T}"},
+               {NewInterface(nil, []*Named{newDefined(NewInterface([]*Func{m}, nil))}), "interface{T /* incomplete */}"},
+               {NewInterface(nil, []*Named{newDefined(NewInterface([]*Func{m}, nil).Complete())}), "interface{T /* incomplete */}"},
+               {NewInterface(nil, []*Named{newDefined(NewInterface([]*Func{m}, nil).Complete())}).Complete(), "interface{T}"},
+
                {NewInterface2(nil, nil), "interface{/* incomplete */}"},
                {NewInterface2(nil, nil).Complete(), "interface{}"},
-               {NewInterface2([]*Func{NewFunc(token.NoPos, nil, "m", sig)}, nil), "interface{m() /* incomplete */}"},
-               {NewInterface2([]*Func{NewFunc(token.NoPos, nil, "m", sig)}, nil).Complete(), "interface{m()}"},
+               {NewInterface2([]*Func{}, nil), "interface{/* incomplete */}"},
+               {NewInterface2([]*Func{}, nil).Complete(), "interface{}"},
+               {NewInterface2(nil, []Type{}), "interface{/* incomplete */}"},
+               {NewInterface2(nil, []Type{}).Complete(), "interface{}"},
+               {NewInterface2([]*Func{m}, nil), "interface{m() /* incomplete */}"},
+               {NewInterface2([]*Func{m}, nil).Complete(), "interface{m()}"},
+               {NewInterface2(nil, []Type{new(Interface).Complete()}), "interface{interface{} /* incomplete */}"},
+               {NewInterface2(nil, []Type{new(Interface).Complete()}).Complete(), "interface{interface{}}"},
+               {NewInterface2(nil, []Type{NewInterface2([]*Func{m}, nil)}), "interface{interface{m() /* incomplete */} /* incomplete */}"},
+               {NewInterface2(nil, []Type{NewInterface2([]*Func{m}, nil).Complete()}), "interface{interface{m()} /* incomplete */}"},
+               {NewInterface2(nil, []Type{NewInterface2([]*Func{m}, nil).Complete()}).Complete(), "interface{interface{m()}}"},
        } {
                got := test.typ.String()
                if got != test.want {
@@ -158,6 +183,13 @@ func TestIncompleteInterfaces(t *testing.T) {
        }
 }
 
+// newDefined creates a new defined type named T with the given underlying type.
+// Helper function for use with TestIncompleteInterfaces only.
+func newDefined(underlying Type) *Named {
+       tname := NewTypeName(token.NoPos, nil, "T", nil)
+       return NewNamed(tname, underlying, nil)
+}
+
 func TestQualifiedTypeString(t *testing.T) {
        p, _ := pkgFor("p.go", "package p; type T int", nil)
        q, _ := pkgFor("q.go", "package q", nil)