diff --git a/apidiff/apidiff.go b/apidiff/apidiff.go index 3b6a9dfd9..92c24fee6 100644 --- a/apidiff/apidiff.go +++ b/apidiff/apidiff.go @@ -16,6 +16,8 @@ import ( "go/token" "go/types" "strings" + + "golang.org/x/tools/go/types/typeutil" ) // Changes reports on the differences between the APIs of the old and new packages. @@ -108,7 +110,7 @@ type differ struct { // Even though it is the named types (*types.Named) that correspond, we use // *types.TypeName as a map key because they are canonical. // The values can be either named types or basic types. - correspondMap map[*types.TypeName]types.Type + correspondMap typeutil.Map // Messages. incompatibles messageSet @@ -119,7 +121,6 @@ func newDiffer(old, new *types.Package) *differ { return &differ{ old: old, new: new, - correspondMap: map[*types.TypeName]types.Type{}, incompatibles: messageSet{}, compatibles: messageSet{}, } @@ -161,28 +162,36 @@ func (d *differ) checkPackage(oldRootPackagePath string) { // Whole-package satisfaction. // For every old exposed interface oIface and its corresponding new interface nIface... - for otn1, nt1 := range d.correspondMap { + d.correspondMap.Iterate(func(k1 types.Type, v1 any) { + ot1 := k1.(*types.Named) + otn1 := ot1.Obj() + nt1 := v1.(types.Type) oIface, ok := otn1.Type().Underlying().(*types.Interface) if !ok { - continue + return } nIface, ok := nt1.Underlying().(*types.Interface) if !ok { // If nt1 isn't an interface but otn1 is, then that's an incompatibility that // we've already noticed, so there's no need to do anything here. - continue + return } // For every old type that implements oIface, its corresponding new type must implement // nIface. - for otn2, nt2 := range d.correspondMap { + d.correspondMap.Iterate(func(k2 types.Type, v2 any) { + ot2 := k2.(*types.Named) + otn2 := ot2.Obj() + nt2 := v2.(types.Type) if otn1 == otn2 { - continue + return } if types.Implements(otn2.Type(), oIface) && !types.Implements(nt2, nIface) { + // TODO(jba): the type name is not sufficient information here; we need the type args + // if this is an instantiated generic type. d.incompatible(objectWithSide{otn2, false}, "", "no longer implements %s", objectString(otn1, oldRootPackagePath)) } - } - } + }) + }) } func (d *differ) checkObjects(old, new types.Object) { diff --git a/apidiff/correspondence.go b/apidiff/correspondence.go index 4ef96f83f..f92b2891c 100644 --- a/apidiff/correspondence.go +++ b/apidiff/correspondence.go @@ -124,7 +124,6 @@ func (d *differ) corr(old, new types.Type, p *ifacePair) bool { } if new, ok := new.(*types.Basic); ok { // Basic types are defined types, too, so we have to support them. - return d.establishCorrespondence(old, new) } @@ -167,8 +166,8 @@ func (d *differ) establishCorrespondence(old *types.Named, new types.Type) bool oldname := old.Obj() // If there already is a corresponding new type for old, check that they // are the same. - if c := d.correspondMap[oldname]; c != nil { - return typesEquivalent(c, new) + if c := d.correspondMap.At(old); c != nil { + return types.Identical(c.(types.Type), new) } // Attempt to establish a correspondence. // Assume the types don't correspond unless they have the same @@ -196,69 +195,58 @@ func (d *differ) establishCorrespondence(old *types.Named, new types.Type) bool if old.Obj().Pkg() != d.old || newn.Obj().Pkg() != d.new { return old.Obj().Id() == newn.Obj().Id() } - // Prior to generics, any two named types could correspond. - // Two named types cannot correspond if their type parameter lists don't match. - if !d.typeParamListsCorrespond(old.TypeParams(), newn.TypeParams()) { - return false + // Two generic named types correspond if their type parameter lists correspond. + // Since one or the other of those lists will be empty, it doesn't hurt + // to check both. + oldOrigin := old.Origin() + newOrigin := newn.Origin() + if oldOrigin != old { + // old is an instantiated type. + if newOrigin == newn { + // new is not; they cannot correspond. + return false + } + // Two instantiated types correspond if their origins correspond and + // their type argument lists correspond. + if !d.correspond(oldOrigin, newOrigin) { + return false + } + if !d.typeListsCorrespond(old.TypeArgs(), newn.TypeArgs()) { + return false + } + } else { + if !d.typeParamListsCorrespond(old.TypeParams(), newn.TypeParams()) { + return false + } } } // If there is no correspondence, create one. - d.correspondMap[oldname] = new + d.correspondMap.Set(old, new) // Check that the corresponding types are compatible. d.checkCompatibleDefined(oldname, old, new) return true } -// Two list of type parameters correspond if they are the same length, and -// the constraints of corresponding type parameters correspond. -func (d *differ) typeParamListsCorrespond(tps1, tps2 *types.TypeParamList) bool { - if tps1.Len() != tps2.Len() { +func (d *differ) typeListsCorrespond(tl1, tl2 *types.TypeList) bool { + if tl1.Len() != tl2.Len() { return false } - for i := 0; i < tps1.Len(); i++ { - if !d.correspond(tps1.At(i).Constraint(), tps2.At(i).Constraint()) { + for i := 0; i < tl1.Len(); i++ { + if !d.correspond(tl1.At(i), tl2.At(i)) { return false } } return true } -// typesEquivalent reports whether two types are identical, or if -// the types have identical type param lists except that one type has nil -// constraints. -// -// This allows us to match a Type from a method receiver or arg to the Type from -// the declaration. -func typesEquivalent(t1, t2 types.Type) bool { - if types.Identical(t1, t2) { - return true - } - // Handle two types with the same type params, one - // having constraints and one not. - oldn, ok := t1.(*types.Named) - if !ok { - return false - } - newn, ok := t2.(*types.Named) - if !ok { - return false - } - oldps := oldn.TypeParams() - newps := newn.TypeParams() - if oldps.Len() != newps.Len() { - return false - } - if oldps.Len() == 0 { - // Not generic types. +// Two list of type parameters correspond if they are the same length, and +// the constraints of corresponding type parameters correspond. +func (d *differ) typeParamListsCorrespond(tps1, tps2 *types.TypeParamList) bool { + if tps1.Len() != tps2.Len() { return false } - for i := 0; i < oldps.Len(); i++ { - oldp := oldps.At(i) - newp := newps.At(i) - if oldp.Constraint() == nil || newp.Constraint() == nil { - return true - } - if !types.Identical(oldp.Constraint(), newp.Constraint()) { + for i := 0; i < tps1.Len(); i++ { + if !d.correspond(tps1.At(i).Constraint(), tps2.At(i).Constraint()) { return false } } diff --git a/apidiff/testdata/generics.go b/apidiff/testdata/generics.go index 7dc096e26..12e1c780e 100644 --- a/apidiff/testdata/generics.go +++ b/apidiff/testdata/generics.go @@ -48,3 +48,37 @@ type custom interface { } type GT3[E custom] map[E]int + +// Instantiated types: +// Two instantiations of generic types +// with different type parameters are different. + +// both +type H[T any] []T + +// old +var V1 H[int] + +type T int + +var V2 H[T] + +// new +// i V1: changed from H[int] to H[bool] +var V1 H[bool] + +// i T: changed from int to bool +type T bool + +// OK: we reported on T, so we don't need to here. +var V2 H[T] + +// old +type t int + +// new +// i t: changed from int to byte +type t byte + +// both +var V3 H[t]