Skip to content

Commit

Permalink
apidiff: fix comparison of a type in an other package with a basic type
Browse files Browse the repository at this point in the history
A named type in a package other than the one being compared can
never correspond to a basic type.

Fixes golang/go#61385.

Change-Id: I1c440dfc52e3c9e2b58a7f013d9809974e94efeb
Reviewed-on: https://go-review.googlesource.com/c/exp/+/513675
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
jba committed Aug 11, 2023
1 parent 352e893 commit 3b0b5b6
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 13 deletions.
20 changes: 12 additions & 8 deletions apidiff/correspondence.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,7 @@ func (d *differ) corr(old, new types.Type, p *ifacePair) bool {
}

case *types.Named:
if new, ok := new.(*types.Named); ok {
return d.establishCorrespondence(old, new)
}
if new, ok := new.(*types.Basic); ok {
// Basic types are defined types, too, so we have to support them.
return d.establishCorrespondence(old, new)
}
return d.establishCorrespondence(old, new)

case *types.TypeParam:
if new, ok := new.(*types.TypeParam); ok {
Expand Down Expand Up @@ -191,7 +185,9 @@ func (d *differ) establishCorrespondence(old *types.Named, new types.Type) bool
// of the same path, doesn't correspond to something other than the new type.
// That is a bit hard, because there is no easy way to find a new package
// matching an old one.
if newn, ok := new.(*types.Named); ok {
switch new := new.(type) {
case *types.Named:
newn := new
oobj := old.Obj()
nobj := newn.Obj()
if oobj.Pkg() != d.old || nobj.Pkg() != d.new {
Expand Down Expand Up @@ -233,6 +229,14 @@ func (d *differ) establishCorrespondence(old *types.Named, new types.Type) bool
return false
}
}
case *types.Basic:
if old.Obj().Pkg() != d.old {
// A named type from a package other than old never corresponds to a basic type.
return false
}
default:
// Only named and basic types can correspond.
return false
}
// If there is no correspondence, create one.
d.correspondMap.Set(old, new)
Expand Down
14 changes: 14 additions & 0 deletions apidiff/testdata/aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,17 @@ type A = t1
// new
// i t1: changed from int to bool
type A = t2

// old
type B = int

// new
// i B: changed from int to B
type B int

// old
type C int

// new
// OK: merging types
type C = int
30 changes: 25 additions & 5 deletions apidiff/testdata/other_packages.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package p

// This test demonstrates correct handling of symbols
// These tests demonstrate the correct handling of symbols
// in packages other than two being compared.
// See the lines in establishCorrespondence after
// if newn, ok := new.(*types.Named); ok
// See the lines in establishCorrespondence beginning
//
// if newn, ok := new.(*types.Named); ok
package p

// both

Expand All @@ -14,6 +14,10 @@ import (
"text/tabwriter"
)

// Here we have two named types in different packages.
// They have the same package-relative name, but we compare
// the package-qualified names.

// old
var V io.Writer
var _ tabwriter.Writer
Expand All @@ -22,3 +26,19 @@ var _ tabwriter.Writer
// i V: changed from io.Writer to text/tabwriter.Writer
var V tabwriter.Writer
var _ io.Writer

// Here one type is a basic type.
// Example from https://go.dev/issue/61385.
// apidiff would previously report
// F2: changed from func(io.ReadCloser) to func(io.ReadCloser)
// io.ReadCloser: changed from interface{io.Reader; io.Closer} to int

// old
func F1(io.ReadCloser) {}

// new
// i F1: changed from func(io.ReadCloser) to func(int)
func F1(int) {}

// both
func F2(io.ReadCloser) {}

0 comments on commit 3b0b5b6

Please sign in to comment.