diff --git a/internal/apidiff/README.md b/internal/apidiff/README.md new file mode 100644 index 0000000000..04a1f4e702 --- /dev/null +++ b/internal/apidiff/README.md @@ -0,0 +1,624 @@ +# Checking Go Package API Compatibility + +The `apidiff` tool in this directory determines whether two versions of the same +package are compatible. The goal is to help the developer make an informed +choice of semantic version after they have changed the code of their module. + +`apidiff` reports two kinds of changes: incompatible ones, which require +incrementing the major part of the semantic version, and compatible ones, which +require a minor version increment. If no API changes are reported but there are +code changes that could affect client code, then the patch version should +be incremented. + +Because `apidiff` ignores package import paths, it may be used to display API +differences between any two packages, not just different versions of the same +package. + +The current version of `apidiff` compares only packages, not modules. + + +## Compatibility Desiderata + +Any tool that checks compatibility can offer only an approximation. No tool can +detect behavioral changes; and even if it could, whether a behavioral change is +a breaking change or not depends on many factors, such as whether it closes a +security hole or fixes a bug. Even a change that causes some code to fail to +compile may not be considered a breaking change by the developers or their +users. It may only affect code marked as experimental or unstable, for +example, or the break may only manifest in unlikely cases. + +For a tool to be useful, its notion of compatibility must be relaxed enough to +allow reasonable changes, like adding a field to a struct, but strict enough to +catch significant breaking changes. A tool that is too lax will miss important +incompatibilities, and users will stop trusting it; one that is too strict may +generate so much noise that users will ignore it. + +To a first approximation, this tool reports a change as incompatible if it could +cause client code to stop compiling. But `apidiff` ignores five ways in which +code may fail to compile after a change. Three of them are mentioned in the +[Go 1 Compatibility Guarantee](https://golang.org/doc/go1compat). + +### Unkeyed Struct Literals + +Code that uses an unkeyed struct literal would fail to compile if a field was +added to the struct, making any such addition an incompatible change. An example: + +``` +// old +type Point struct { X, Y int } + +// new +type Point struct { X, Y, Z int } + +// client +p := pkg.Point{1, 2} // fails in new because there are more fields than expressions +``` +Here and below, we provide three snippets: the code in the old version of the +package, the code in the new version, and the code written in a client of the package, +which refers to it by the name `pkg`. The client code compiles against the old +code but not the new. + +### Embedding and Shadowing + +Adding an exported field to a struct can break code that embeds that struct, +because the newly added field may conflict with an identically named field +at the same struct depth. A selector referring to the latter would become +ambiguous and thus erroneous. + + +``` +// old +type Point struct { X, Y int } + +// new +type Point struct { X, Y, Z int } + +// client +type z struct { Z int } + +var v struct { + pkg.Point + z +} + +_ = v.Z // fails in new +``` +In the new version, the last line fails to compile because there are two embedded `Z` +fields at the same depth, one from `z` and one from `pkg.Point`. + + +### Using an Identical Type Externally + +If it is possible for client code to write a type expression representing the +underlying type of a defined type in a package, then external code can use it in +assignments involving the package type, making any change to that type incompatible. +``` +// old +type Point struct { X, Y int } + +// new +type Point struct { X, Y, Z int } + +// client +var p struct { X, Y int } = pkg.Point{} // fails in new because of Point's extra field +``` +Here, the external code could have used the provided name `Point`, but chose not +to. I'll have more to say about this and related examples later. + +### unsafe.Sizeof and Friends + +Since `unsafe.Sizeof`, `unsafe.Offsetof` and `unsafe.Alignof` are constant +expressions, they can be used in an array type literal: + +``` +// old +type S struct{ X int } + +// new +type S struct{ X, y int } + +// client +var a [unsafe.Sizeof(pkg.S{})]int = [8]int{} // fails in new because S's size is not 8 +``` +Use of these operations could make many changes to a type potentially incompatible. + + +### Type Switches + +A package change that merges two different types (with same underlying type) +into a single new type may break type switches in clients that refer to both +original types: + +``` +// old +type T1 int +type T2 int + +// new +type T1 int +type T2 = T1 + +// client +switch x.(type) { +case T1: +case T2: +} // fails with new because two cases have the same type +``` +This sort of incompatibility is sufficiently esoteric to ignore; the tool allows +merging types. + +## First Attempt at a Definition + +Our first attempt at defining compatibility captures the idea that all the +exported names in the old package must have compatible equivalents in the new +package. + +A new package is compatible with an old one if and only if: +- For every exported package-level name in the old package, the same name is + declared in the new at package level, and +- the names denote the same kind of object (e.g. both are variables), and +- the types of the objects are compatible. + +We will work out the details (and make some corrections) below, but it is clear +already that we will need to determine what makes two types compatible. And +whatever the definition of type compatibility, it's certainly true that if two +types are the same, they are compatible. So we will need to decide what makes an +old and new type the same. We will call this sameness relation _correspondence_. + +## Type Correspondence + +Go already has a definition of when two types are the same: +[type identity](https://golang.org/ref/spec#Type_identity). +But identity isn't adequate for our purpose: it says that two defined +types are identical if they arise from the same definition, but it's unclear +what "same" means when talking about two different packages (or two versions of +a single package). + +The obvious change to the definition of identity is to require that old and new +[defined types](https://golang.org/ref/spec#Type_definitions) +have the same name instead. But that doesn't work either, for two +reasons. First, type aliases can equate two defined types with different names: + +``` +// old +type E int + +// new +type t int +type E = t +``` +Second, an unexported type can be renamed: + +``` +// old +type u1 int +var V u1 + +// new +type u2 int +var V u2 +``` +Here, even though `u1` and `u2` are unexported, their exported fields and +methods are visible to clients, so they are part of the API. But since the name +`u1` is not visible to clients, it can be changed compatibly. We say that `u1` +and `u2` are _exposed_: a type is exposed if a client package can declare variables of that type. + +We will say that an old defined type _corresponds_ to a new one if they have the +same name, or one can be renamed to the other without otherwise changing the +API. In the first example above, old `E` and new `t` correspond. In the second, +old `u1` and new `u2` correspond. + +Two or more old defined types can correspond to a single new type: we consider +"merging" two types into one to be a compatible change. As mentioned above, +code that uses both names in a type switch will fail, but we deliberately ignore +this case. However, a single old type can correspond to only one new type. + +So far, we've explained what correspondence means for defined types. To extend +the definition to all types, we parallel the language's definition of type +identity. So, for instance, an old and a new slice type correspond if their +element types correspond. + +## Definition of Compatibility + +We can now present the definition of compatibility used by `apidiff`. + +### Package Compatibility + +> A new package is compatible with an old one if: +>1. Each exported name in the old package's scope also appears in the new +>package's scope, and the object (constant, variable, function or type) denoted +>by that name in the old package is compatible with the object denoted by the +>name in the new package, and +>2. For every exposed type that implements an exposed interface in the old package, +> its corresponding type should implement the corresponding interface in the new package. +> +>Otherwise the packages are incompatible. + +As an aside, the tool also finds exported names in the new package that are not +exported in the old, and marks them as compatible changes. + +Clause 2 is discussed further in "Whole-Package Compatibility." + +### Object Compatibility + +This section provides compatibility rules for constants, variables, functions +and types. + +#### Constants + +>A new exported constant is compatible with an old one of the same name if and only if +>1. Their types correspond, and +>2. Their values are identical. + +It is tempting to allow changing a typed constant to an untyped one. That may +seem harmless, but it can break code like this: + +``` +// old +const C int64 = 1 + +// new +const C = 1 + +// client +var x = C // old type is int64, new is int +var y int64 = x // fails with new: different types in assignment +``` + +A change to the value of a constant can break compatiblity if the value is used +in an array type: + +``` +// old +const C = 1 + +// new +const C = 2 + +// client +var a [C]int = [1]int{} // fails with new because [2]int and [1]int are different types +``` +Changes to constant values are rare, and determining whether they are compatible +or not is better left to the user, so the tool reports them. + +#### Variables + +>A new exported variable is compatible with an old one of the same name if and +>only if their types correspond. + +Correspondence doesn't look past names, so this rule does not prevent adding a +field to `MyStruct` if the package declares `var V MyStruct`. It does, however, mean that + +``` +var V struct { X int } +``` +is incompatible with +``` +var V struct { X, Y int } +``` +I discuss this at length below in the section "Compatibility, Types and Names." + +#### Functions + +>A new exported function or variable is compatible with an old function of the +>same name if and only if their types (signatures) correspond. + +This rule captures the fact that, although many signature changes are compatible +for all call sites, none are compatible for assignment: + +``` +var v func(int) = pkg.F +``` +Here, `F` must be of type `func(int)` and not, for instance, `func(...int)` or `func(interface{})`. + +Note that the rule permits changing a function to a variable. This is a common +practice, usually done for test stubbing, and cannot break any code at compile +time. + +#### Exported Types + +> A new exported type is compatible with an old one if and only if their +> names are the same and their types correspond. + +This rule seems far too strict. But, ignoring aliases for the moment, it demands only +that the old and new _defined_ types correspond. Consider: +``` +// old +type T struct { X int } + +// new +type T struct { X, Y int } +``` +The addition of `Y` is a compatible change, because this rule does not require +that the struct literals have to correspond, only that the defined types +denoted by `T` must correspond. (Remember that correspondence stops at type +names.) + +If one type is an alias that refers to the corresponding defined type, the +situation is the same: + +``` +// old +type T struct { X int } + +// new +type u struct { X, Y int } +type T = u +``` +Here, the only requirement is that old `T` corresponds to new `u`, not that the +struct types correspond. (We can't tell from this snippet that the old `T` and +the new `u` do correspond; that depends on whether `u` replaces `T` throughout +the API.) + +However, the following change is incompatible, because the names do not +denote corresponding types: + +``` +// old +type T = struct { X int } + +// new +type T = struct { X, Y int } +``` +### Type Literal Compatibility + +Only five kinds of types can differ compatibly: defined types, structs, +interfaces, channels and numeric types. We only consider the compatibility of +the last four when they are the underlying type of a defined type. See +"Compatibility, Types and Names" for a rationale. + +We justify the compatibility rules by enumerating all the ways a type +can be used, and by showing that the allowed changes cannot break any code that +uses values of the type in those ways. + +Values of all types can be used in assignments (including argument passing and +function return), but we do not require that old and new types are assignment +compatible. That is because we assume that the old and new packages are never +used together: any given binary will link in either the old package or the new. +So in describing how a type can be used in the sections below, we omit +assignment. + +Any type can also be used in a type assertion or conversion. The changes we allow +below may affect the run-time behavior of these operations, but they cannot affect +whether they compile. The only such breaking change would be to change +the type `T` in an assertion `x.T` so that it no longer implements the interface +type of `x`; but the rules for interfaces below disallow that. + +> A new type is compatible with an old one if and only if they correspond, or +> one of the cases below applies. + +#### Defined Types + +Other than assignment, the only ways to use a defined type are to access its +methods, or to make use of the properties of its underlying type. Rule 2 below +covers the latter, and rules 3 and 4 cover the former. + +> A new defined type is compatible with an old one if and only if all of the +> following hold: +>1. They correspond. +>2. Their underlying types are compatible. +>3. The new exported value method set is a superset of the old. +>4. The new exported pointer method set is a superset of the old. + +An exported method set is a method set with all unexported methods removed. +When comparing methods of a method set, we require identical names and +corresponding signatures. + +Removing an exported method is clearly a breaking change. But removing an +unexported one (or changing its signature) can be breaking as well, if it +results in the type no longer implementing an interface. See "Whole-Package +Compatibility," below. + +#### Channels + +> A new channel type is compatible with an old one if +> 1. The element types correspond, and +> 2. Either the directions are the same, or the new type has no direction. + +Other than assignment, the only ways to use values of a channel type are to send +and receive on them, to close them, and to use them as map keys. Changes to a +channel type cannot cause code that closes a channel or uses it as a map key to +fail to compile, so we need not consider those operations. + +Rule 1 ensures that any operations on the values sent or received will compile. +Rule 2 captures the fact that any program that compiles with a directed channel +must use either only sends, or only receives, so allowing the other operation +by removing the channel direction cannot break any code. + + +#### Interfaces + +> A new interface is compatible with an old one if and only if: +> 1. The old interface does not have an unexported method, and it corresponds +> to the new interfaces (i.e. they have the same method set), or +> 2. The old interface has an unexported method and the new exported method set is a +> superset of the old. + +Other than assignment, the only ways to use an interface are to implement it, +embed it, or call one of its methods. (Interface values can also be used as map +keys, but that cannot cause a compile-time error.) + +Certainly, removing an exported method from an interface could break a client +call, so neither rule allows it. + +Rule 1 also disallows adding a method to an interface without an existing unexported +method. Such an interface can be implemented in client code. If adding a method +were allowed, a type that implements the old interface could fail to implement +the new one: + +``` +type I interface { M1() } // old +type I interface { M1(); M2() } // new + +// client +type t struct{} +func (t) M1() {} +var i pkg.I = t{} // fails with new, because t lacks M2 +``` + +Rule 2 is based on the observation that if an interface has an unexported +method, the only way a client can implement it is to embed it. +Adding a method is compatible in this case, because the embedding struct will +continue to implement the interface. Adding a method also cannot break any call +sites, since no program that compiles could have any such call sites. + +#### Structs + +> A new struct is compatible with an old one if all of the following hold: +> 1. The new set of top-level exported fields is a superset of the old. +> 2. The new set of _selectable_ exported fields is a superset of the old. +> 3. If the old struct is comparable, so is the new one. + +The set of selectable exported fields is the set of exported fields `F` +such that `x.F` is a valid selector expression for a value `x` of the struct +type. `F` may be at the top level of the struct, or it may be a field of an +embedded struct. + +Two fields are the same if they have the same name and corresponding types. + +Other than assignment, there are only four ways to use a struct: write a struct +literal, select a field, use a value of the struct as a map key, or compare two +values for equality. The first clause ensures that struct literals compile; the +second, that selections compile; and the third, that equality expressions and +map index expressions compile. + +#### Numeric Types + +> A new numeric type is compatible with an old one if and only if they are +> both unsigned integers, both signed integers, both floats or both complex +> types, and the new one is at least as large as the old on both 32-bit and +> 64-bit architectures. + +Other than in assignments, numeric types appear in arithmetic and comparison +expressions. Since all arithmetic operations but shifts (see below) require that +operand types be identical, and by assumption the old and new types underly +defined types (see "Compatibility, Types and Names," below), there is no way for +client code to write an arithmetic expression that compiles with operands of the +old type but not the new. + +Numeric types can also appear in type switches and type assertions. Again, since +the old and new types underly defined types, type switches and type assertions +that compiled using the old defined type will continue to compile with the new +defined type. + +Going from an unsigned to a signed integer type is an incompatible change for +the sole reason that only an unsigned type can appear as the right operand of a +shift. If this rule is relaxed, then changes from an unsigned type to a larger +signed type would be compatible. See [this +issue](https://github.com/golang/go/issues/19113). + +Only integer types can be used in bitwise and shift operations, and for indexing +slices and arrays. That is why switching from an integer to a floating-point +type--even one that can represent all values of the integer type--is an +incompatible change. + + +Conversions from floating-point to complex types or vice versa are not permitted +(the predeclared functions real, imag, and complex must be used instead). To +prevent valid floating-point or complex conversions from becoming invalid, +changing a floating-point type to a complex type or vice versa is considered an +incompatible change. + +Although conversions between any two integer types are valid, assigning a +constant value to a variable of integer type that is too small to represent the +constant is not permitted. That is why the only compatible changes are to +a new type whose values are a superset of the old. The requirement that the new +set of values must include the old on both 32-bit and 64-bit machines allows +conversions from `int32` to `int` and from `int` to `int64`, but not the other +direction; and similarly for `uint`. + +Changing a type to or from `uintptr` is considered an incompatible change. Since +its size is not specified, there is no way to know whether the new type's values +are a superset of the old type's. + +## Whole-Package Compatibility + +Some changes that are compatible for a single type are not compatible when the +package is considered as a whole. For example, if you remove an unexported +method on a defined type, it may no longer implement an interface of the +package. This can break client code: + +``` +// old +type T int +func (T) m() {} +type I interface { m() } + +// new +type T int // no method m anymore + +// client +var i pkg.I = pkg.T{} // fails with new because T lacks m +``` + +Similarly, adding a method to an interface can cause defined types +in the package to stop implementing it. + +The second clause in the definition for package compatibility handles these +cases. To repeat: +> 2. For every exposed type that implements an exposed interface in the old package, +> its corresponding type should implement the corresponding interface in the new package. +Recall that a type is exposed if it is part of the package's API, even if it is +unexported. + +Other incompatibilities that involve more than one type in the package can arise +whenever two types with identical underlying types exist in the old or new +package. Here, a change "splits" an identical underlying type into two, breaking +conversions: + +``` +// old +type B struct { X int } +type C struct { X int } + +// new +type B struct { X int } +type C struct { X, Y int } + +// client +var b B +_ = C(b) // fails with new: cannot convert B to C +``` +Finally, changes that are compatible for the package in which they occur can +break downstream packages. That can happen even if they involve unexported +methods, thanks to embedding. + +The definitions given here don't account for these sorts of problems. + + +## Compatibility, Types and Names + +The above definitions state that the only types that can differ compatibly are +defined types and the types that underly them. Changes to other type literals +are considered incompatible. For instance, it is considered an incompatible +change to add a field to the struct in this variable declaration: + +``` +var V struct { X int } +``` +or this alias definition: +``` +type T = struct { X int } +``` + +We make this choice to keep the definition of compatibility (relatively) simple. +A more precise definition could, for instance, distinguish between + +``` +func F(struct { X int }) +``` +where any changes to the struct are incompatible, and + +``` +func F(struct { X, u int }) +``` +where adding a field is compatible (since clients cannot write the signature, +and thus cannot assign `F` to a variable of the signature type). The definition +should then also allow other function signature changes that only require +call-site compatibility, like + +``` +func F(struct { X, u int }, ...int) +``` +The result would be a much more complex definition with little benefit, since +the examples in this section rarely arise in practice. diff --git a/internal/apidiff/apidiff.go b/internal/apidiff/apidiff.go new file mode 100644 index 0000000000..dc0f0e7a78 --- /dev/null +++ b/internal/apidiff/apidiff.go @@ -0,0 +1,216 @@ +// TODO: test swap corresponding types (e.g. u1 <-> u2 and u2 <-> u1) +// TODO: test exported alias refers to something in another package -- does correspondence work then? +// TODO: CODE COVERAGE +// TODO: note that we may miss correspondences because we bail early when we compare a signature (e.g. when lengths differ; we could do up to the shorter) +// TODO: if you add an unexported method to an exposed interface, you have to check that +// every exposed type that previously implemented the interface still does. Otherwise +// an external assignment of the exposed type to the interface type could fail. +// TODO: check constant values: large values aren't representable by some types. +// TODO: Document all the incompatibilities we don't check for. + +package apidiff + +import ( + "fmt" + "go/constant" + "go/token" + "go/types" +) + +// Changes reports on the differences between the APIs of the old and new packages. +// It classifies each difference as either compatible or incompatible (breaking.) For +// a detailed discussion of what constitutes an incompatible change, see the package +// documentation. +func Changes(old, new *types.Package) Report { + d := newDiffer(old, new) + d.checkPackage() + return Report{ + Incompatible: d.incompatibles.collect(), + Compatible: d.compatibles.collect(), + } +} + +type differ struct { + old, new *types.Package + // Correspondences between named types. + // 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 + + // Messages. + incompatibles messageSet + compatibles messageSet +} + +func newDiffer(old, new *types.Package) *differ { + return &differ{ + old: old, + new: new, + correspondMap: map[*types.TypeName]types.Type{}, + incompatibles: messageSet{}, + compatibles: messageSet{}, + } +} + +func (d *differ) incompatible(obj types.Object, part, format string, args ...interface{}) { + addMessage(d.incompatibles, obj, part, format, args) +} + +func (d *differ) compatible(obj types.Object, part, format string, args ...interface{}) { + addMessage(d.compatibles, obj, part, format, args) +} + +func addMessage(ms messageSet, obj types.Object, part, format string, args []interface{}) { + ms.add(obj, part, fmt.Sprintf(format, args...)) +} + +func (d *differ) checkPackage() { + // Old changes. + for _, name := range d.old.Scope().Names() { + oldobj := d.old.Scope().Lookup(name) + if !oldobj.Exported() { + continue + } + newobj := d.new.Scope().Lookup(name) + if newobj == nil { + d.incompatible(oldobj, "", "removed") + continue + } + d.checkObjects(oldobj, newobj) + } + // New additions. + for _, name := range d.new.Scope().Names() { + newobj := d.new.Scope().Lookup(name) + if newobj.Exported() && d.old.Scope().Lookup(name) == nil { + d.compatible(newobj, "", "added") + } + } + + // Whole-package satisfaction. + // For every old exposed interface oIface and its corresponding new interface nIface... + for otn1, nt1 := range d.correspondMap { + oIface, ok := otn1.Type().Underlying().(*types.Interface) + if !ok { + continue + } + 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 + } + // For every old type that implements oIface, its corresponding new type must implement + // nIface. + for otn2, nt2 := range d.correspondMap { + if otn1 == otn2 { + continue + } + if types.Implements(otn2.Type(), oIface) && !types.Implements(nt2, nIface) { + d.incompatible(otn2, "", "no longer implements %s", objectString(otn1)) + } + } + } +} + +func (d *differ) checkObjects(old, new types.Object) { + switch old := old.(type) { + case *types.Const: + if new, ok := new.(*types.Const); ok { + d.constChanges(old, new) + return + } + case *types.Var: + if new, ok := new.(*types.Var); ok { + d.checkCorrespondence(old, "", old.Type(), new.Type()) + return + } + case *types.Func: + switch new := new.(type) { + case *types.Func: + d.checkCorrespondence(old, "", old.Type(), new.Type()) + return + case *types.Var: + d.compatible(old, "", "changed from func to var") + d.checkCorrespondence(old, "", old.Type(), new.Type()) + return + + } + case *types.TypeName: + if new, ok := new.(*types.TypeName); ok { + d.checkCorrespondence(old, "", old.Type(), new.Type()) + return + } + default: + panic("unexpected obj type") + } + // Here if kind of type changed. + d.incompatible(old, "", "changed from %s to %s", + objectKindString(old), objectKindString(new)) +} + +// Compare two constants. +func (d *differ) constChanges(old, new *types.Const) { + ot := old.Type() + nt := new.Type() + // Check for change of type. + if !d.correspond(ot, nt) { + d.typeChanged(old, "", ot, nt) + return + } + // Check for change of value. + // We know the types are the same, so constant.Compare shouldn't panic. + if !constant.Compare(old.Val(), token.EQL, new.Val()) { + d.incompatible(old, "", "value changed from %s to %s", old.Val(), new.Val()) + } +} + +func objectKindString(obj types.Object) string { + switch obj.(type) { + case *types.Const: + return "const" + case *types.Var: + return "var" + case *types.Func: + return "func" + case *types.TypeName: + return "type" + default: + return "???" + } +} + +func (d *differ) checkCorrespondence(obj types.Object, part string, old, new types.Type) { + if !d.correspond(old, new) { + d.typeChanged(obj, part, old, new) + } +} + +func (d *differ) typeChanged(obj types.Object, part string, old, new types.Type) { + old = removeNamesFromSignature(old) + new = removeNamesFromSignature(new) + olds := types.TypeString(old, types.RelativeTo(d.old)) + news := types.TypeString(new, types.RelativeTo(d.new)) + d.incompatible(obj, part, "changed from %s to %s", olds, news) +} + +// go/types always includes the argument and result names when formatting a signature. +// Since these can change without affecting compatibility, we don't want users to +// be distracted by them, so we remove them. +func removeNamesFromSignature(t types.Type) types.Type { + sig, ok := t.(*types.Signature) + if !ok { + return t + } + + dename := func(p *types.Tuple) *types.Tuple { + var vars []*types.Var + for i := 0; i < p.Len(); i++ { + v := p.At(i) + vars = append(vars, types.NewVar(v.Pos(), v.Pkg(), "", v.Type())) + } + return types.NewTuple(vars...) + } + + return types.NewSignature(sig.Recv(), dename(sig.Params()), dename(sig.Results()), sig.Variadic()) +} diff --git a/internal/apidiff/apidiff_test.go b/internal/apidiff/apidiff_test.go new file mode 100644 index 0000000000..4dcb4d87da --- /dev/null +++ b/internal/apidiff/apidiff_test.go @@ -0,0 +1,166 @@ +package apidiff + +import ( + "bufio" + "fmt" + "go/types" + "io/ioutil" + "os" + "path/filepath" + "reflect" + "sort" + "strings" + "testing" + + "golang.org/x/tools/go/packages" +) + +func TestChanges(t *testing.T) { + dir, err := ioutil.TempDir("", "apidiff_test") + if err != nil { + t.Fatal(err) + } + dir = filepath.Join(dir, "go") + wanti, wantc := splitIntoPackages(t, dir) + defer os.RemoveAll(dir) + sort.Strings(wanti) + sort.Strings(wantc) + + oldpkg, err := load("apidiff/old", dir) + if err != nil { + t.Fatal(err) + } + newpkg, err := load("apidiff/new", dir) + if err != nil { + t.Fatal(err) + } + + report := Changes(oldpkg.Types, newpkg.Types) + + if !reflect.DeepEqual(report.Incompatible, wanti) { + t.Errorf("incompatibles: got %v\nwant %v\n", report.Incompatible, wanti) + } + if !reflect.DeepEqual(report.Compatible, wantc) { + t.Errorf("compatibles: got %v\nwant %v\n", report.Compatible, wantc) + } +} + +func splitIntoPackages(t *testing.T, dir string) (incompatibles, compatibles []string) { + // Read the input file line by line. + // Write a line into the old or new package, + // dependent on comments. + // Also collect expected messages. + f, err := os.Open("testdata/tests.go") + if err != nil { + t.Fatal(err) + } + defer f.Close() + + if err := os.MkdirAll(filepath.Join(dir, "src", "apidiff"), 0700); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(filepath.Join(dir, "src", "apidiff", "go.mod"), []byte("module apidiff\n"), 0666); err != nil { + t.Fatal(err) + } + + oldd := filepath.Join(dir, "src/apidiff/old") + newd := filepath.Join(dir, "src/apidiff/new") + if err := os.MkdirAll(oldd, 0700); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(newd, 0700); err != nil && !os.IsExist(err) { + t.Fatal(err) + } + + oldf, err := os.Create(filepath.Join(oldd, "old.go")) + if err != nil { + t.Fatal(err) + } + newf, err := os.Create(filepath.Join(newd, "new.go")) + if err != nil { + t.Fatal(err) + } + + wl := func(f *os.File, line string) { + if _, err := fmt.Fprintln(f, line); err != nil { + t.Fatal(err) + } + } + writeBoth := func(line string) { wl(oldf, line); wl(newf, line) } + writeln := writeBoth + s := bufio.NewScanner(f) + for s.Scan() { + line := s.Text() + tl := strings.TrimSpace(line) + switch { + case tl == "// old": + writeln = func(line string) { wl(oldf, line) } + case tl == "// new": + writeln = func(line string) { wl(newf, line) } + case tl == "// both": + writeln = writeBoth + case strings.HasPrefix(tl, "// i "): + incompatibles = append(incompatibles, strings.TrimSpace(tl[4:])) + case strings.HasPrefix(tl, "// c "): + compatibles = append(compatibles, strings.TrimSpace(tl[4:])) + default: + writeln(line) + } + } + if s.Err() != nil { + t.Fatal(s.Err()) + } + return +} + +func load(importPath, goPath string) (*packages.Package, error) { + cfg := &packages.Config{ + Mode: packages.LoadTypes, + } + if goPath != "" { + cfg.Env = append(os.Environ(), "GOPATH="+goPath) + cfg.Dir = filepath.Join(goPath, "src", filepath.FromSlash(importPath)) + } + pkgs, err := packages.Load(cfg, importPath) + if err != nil { + return nil, err + } + if len(pkgs[0].Errors) > 0 { + return nil, pkgs[0].Errors[0] + } + return pkgs[0], nil +} + +func TestExportedFields(t *testing.T) { + pkg, err := load("golang.org/x/exp/apidiff/testdata/exported_fields", "") + if err != nil { + t.Fatal(err) + } + typeof := func(name string) types.Type { + return pkg.Types.Scope().Lookup(name).Type() + } + + s := typeof("S") + su := s.(*types.Named).Underlying().(*types.Struct) + + ef := exportedSelectableFields(su) + wants := []struct { + name string + typ types.Type + }{ + {"A1", typeof("A1")}, + {"D", types.Typ[types.Bool]}, + {"E", types.Typ[types.Int]}, + {"F", typeof("F")}, + {"S", types.NewPointer(s)}, + } + + if got, want := len(ef), len(wants); got != want { + t.Errorf("got %d fields, want %d\n%+v", got, want, ef) + } + for _, w := range wants { + if got := ef[w.name]; got != nil && !types.Identical(got.Type(), w.typ) { + t.Errorf("%s: got %v, want %v", w.name, got.Type(), w.typ) + } + } +} diff --git a/internal/apidiff/compatibility.go b/internal/apidiff/compatibility.go new file mode 100644 index 0000000000..f78da8f3c9 --- /dev/null +++ b/internal/apidiff/compatibility.go @@ -0,0 +1,361 @@ +package apidiff + +import ( + "fmt" + "go/types" + "reflect" +) + +func (d *differ) checkCompatible(otn *types.TypeName, old, new types.Type) { + switch old := old.(type) { + case *types.Interface: + if new, ok := new.(*types.Interface); ok { + d.checkCompatibleInterface(otn, old, new) + return + } + + case *types.Struct: + if new, ok := new.(*types.Struct); ok { + d.checkCompatibleStruct(otn, old, new) + return + } + + case *types.Chan: + if new, ok := new.(*types.Chan); ok { + d.checkCompatibleChan(otn, old, new) + return + } + + case *types.Basic: + if new, ok := new.(*types.Basic); ok { + d.checkCompatibleBasic(otn, old, new) + return + } + + case *types.Named: + panic("unreachable") + + default: + d.checkCorrespondence(otn, "", old, new) + return + + } + // Here if old and new are different kinds of types. + d.typeChanged(otn, "", old, new) +} + +func (d *differ) checkCompatibleChan(otn *types.TypeName, old, new *types.Chan) { + d.checkCorrespondence(otn, ", element type", old.Elem(), new.Elem()) + if old.Dir() != new.Dir() { + if new.Dir() == types.SendRecv { + d.compatible(otn, "", "removed direction") + } else { + d.incompatible(otn, "", "changed direction") + } + } +} + +func (d *differ) checkCompatibleBasic(otn *types.TypeName, old, new *types.Basic) { + // Certain changes to numeric types are compatible. Approximately, the info must + // be the same, and the new values must be a superset of the old. + if old.Kind() == new.Kind() { + // old and new are identical + return + } + if compatibleBasics[[2]types.BasicKind{old.Kind(), new.Kind()}] { + d.compatible(otn, "", "changed from %s to %s", old, new) + } else { + d.typeChanged(otn, "", old, new) + } +} + +// All pairs (old, new) of compatible basic types. +var compatibleBasics = map[[2]types.BasicKind]bool{ + {types.Uint8, types.Uint16}: true, + {types.Uint8, types.Uint32}: true, + {types.Uint8, types.Uint}: true, + {types.Uint8, types.Uint64}: true, + {types.Uint16, types.Uint32}: true, + {types.Uint16, types.Uint}: true, + {types.Uint16, types.Uint64}: true, + {types.Uint32, types.Uint}: true, + {types.Uint32, types.Uint64}: true, + {types.Uint, types.Uint64}: true, + {types.Int8, types.Int16}: true, + {types.Int8, types.Int32}: true, + {types.Int8, types.Int}: true, + {types.Int8, types.Int64}: true, + {types.Int16, types.Int32}: true, + {types.Int16, types.Int}: true, + {types.Int16, types.Int64}: true, + {types.Int32, types.Int}: true, + {types.Int32, types.Int64}: true, + {types.Int, types.Int64}: true, + {types.Float32, types.Float64}: true, + {types.Complex64, types.Complex128}: true, +} + +// Interface compatibility: +// If the old interface has an unexported method, the new interface is compatible +// if its exported method set is a superset of the old. (Users could not implement, +// only embed.) +// +// If the old interface did not have an unexported method, the new interface is +// compatible if its exported method set is the same as the old, and it has no +// unexported methods. (Adding an unexported method makes the interface +// unimplementable outside the package.) +// +// TODO: must also check that if any methods were added or removed, every exposed +// type in the package that implemented the interface in old still implements it in +// new. Otherwise external assignments could fail. +func (d *differ) checkCompatibleInterface(otn *types.TypeName, old, new *types.Interface) { + // Method sets are checked in checkCompatibleDefined. + + // Does the old interface have an unexported method? + if unexportedMethod(old) != nil { + d.checkMethodSet(otn, old, new, additionsCompatible) + } else { + // Perform an equivalence check, but with more information. + d.checkMethodSet(otn, old, new, additionsIncompatible) + if u := unexportedMethod(new); u != nil { + d.incompatible(otn, u.Name(), "added unexported method") + } + } +} + +// Return an unexported method from the method set of t, or nil if there are none. +func unexportedMethod(t *types.Interface) *types.Func { + for i := 0; i < t.NumMethods(); i++ { + if m := t.Method(i); !m.Exported() { + return m + } + } + return nil +} + +// We need to check three things for structs: +// 1. The set of exported fields must be compatible. This ensures that keyed struct +// literals continue to compile. (There is no compatibility guarantee for unkeyed +// struct literals.) +// 2. The set of exported *selectable* fields must be compatible. This includes the exported +// fields of all embedded structs. This ensures that selections continue to compile. +// 3. If the old struct is comparable, so must the new one be. This ensures that equality +// expressions and uses of struct values as map keys continue to compile. +// +// An unexported embedded struct can't appear in a struct literal outside the +// package, so it doesn't have to be present, or have the same name, in the new +// struct. +// +// Field tags are ignored: they have no compile-time implications. +func (d *differ) checkCompatibleStruct(obj types.Object, old, new *types.Struct) { + d.checkCompatibleObjectSets(obj, exportedFields(old), exportedFields(new)) + d.checkCompatibleObjectSets(obj, exportedSelectableFields(old), exportedSelectableFields(new)) + // Removing comparability from a struct is an incompatible change. + if types.Comparable(old) && !types.Comparable(new) { + d.incompatible(obj, "", "old is comparable, new is not") + } +} + +// exportedFields collects all the immediate fields of the struct that are exported. +// This is also the set of exported keys for keyed struct literals. +func exportedFields(s *types.Struct) map[string]types.Object { + m := map[string]types.Object{} + for i := 0; i < s.NumFields(); i++ { + f := s.Field(i) + if f.Exported() { + m[f.Name()] = f + } + } + return m +} + +// exportedSelectableFields collects all the exported fields of the struct, including +// exported fields of embedded structs. +// +// We traverse the struct breadth-first, because of the rule that a lower-depth field +// shadows one at a higher depth. +func exportedSelectableFields(s *types.Struct) map[string]types.Object { + var ( + m = map[string]types.Object{} + next []*types.Struct // embedded structs at the next depth + seen []*types.Struct // to handle recursive embedding + ) + for cur := []*types.Struct{s}; len(cur) > 0; cur, next = next, nil { + seen = append(seen, cur...) + // We only want to consider unambiguous fields. Ambiguous fields (where there + // is more than one field of the same name at the same level) are legal, but + // cannot be selected. + for name, f := range unambiguousFields(cur) { + // Record an exported field we haven't seen before. If we have seen it, + // it occurred a lower depth, so it shadows this field. + if f.Exported() && m[name] == nil { + m[name] = f + } + // Remember embedded structs for processing at the next depth, + // but only if we haven't seen the struct at this depth or above. + if !f.Anonymous() { + continue + } + t := f.Type().Underlying() + if p, ok := t.(*types.Pointer); ok { + t = p.Elem().Underlying() + } + if t, ok := t.(*types.Struct); ok && !contains(seen, t) { + next = append(next, t) + } + } + } + return m +} + +func contains(ts []*types.Struct, t *types.Struct) bool { + for _, s := range ts { + if types.Identical(s, t) { + return true + } + } + return false +} + +// Given a set of structs at the same depth, the unambiguous fields are the ones whose +// names appear exactly once. +func unambiguousFields(structs []*types.Struct) map[string]*types.Var { + fields := map[string]*types.Var{} + seen := map[string]bool{} + for _, s := range structs { + for i := 0; i < s.NumFields(); i++ { + f := s.Field(i) + name := f.Name() + if seen[name] { + delete(fields, name) + } else { + seen[name] = true + fields[name] = f + } + } + } + return fields +} + +// Anything removed or change from the old set is an incompatible change. +// Anything added to the new set is a compatible change. +func (d *differ) checkCompatibleObjectSets(obj types.Object, old, new map[string]types.Object) { + for name, oldo := range old { + newo := new[name] + if newo == nil { + d.incompatible(obj, name, "removed") + } else { + d.checkCorrespondence(obj, name, oldo.Type(), newo.Type()) + } + } + for name := range new { + if old[name] == nil { + d.compatible(obj, name, "added") + } + } +} + +func (d *differ) checkCompatibleDefined(otn *types.TypeName, old *types.Named, new types.Type) { + // We've already checked that old and new correspond. + d.checkCompatible(otn, old.Underlying(), new.Underlying()) + // If there are different kinds of types (e.g. struct and interface), don't bother checking + // the method sets. + if reflect.TypeOf(old.Underlying()) != reflect.TypeOf(new.Underlying()) { + return + } + // Interface method sets are checked in checkCompatibleInterface. + if _, ok := old.Underlying().(*types.Interface); ok { + return + } + + // A new method set is compatible with an old if the new exported methods are a superset of the old. + d.checkMethodSet(otn, old, new, additionsCompatible) + d.checkMethodSet(otn, types.NewPointer(old), types.NewPointer(new), additionsCompatible) +} + +const ( + additionsCompatible = true + additionsIncompatible = false +) + +func (d *differ) checkMethodSet(otn *types.TypeName, oldt, newt types.Type, addcompat bool) { + // TODO: find a way to use checkCompatibleObjectSets for this. + oldMethodSet := exportedMethods(oldt) + newMethodSet := exportedMethods(newt) + msname := otn.Name() + if _, ok := oldt.(*types.Pointer); ok { + msname = "*" + msname + } + for name, oldMethod := range oldMethodSet { + newMethod := newMethodSet[name] + if newMethod == nil { + var part string + // Due to embedding, it's possible that the method's receiver type is not + // the same as the defined type whose method set we're looking at. So for + // a type T with removed method M that is embedded in some other type U, + // we will generate two "removed" messages for T.M, one for its own type + // T and one for the embedded type U. We want both messages to appear, + // but the messageSet dedup logic will allow only one message for a given + // object. So use the part string to distinguish them. + if receiverNamedType(oldMethod).Obj() != otn { + part = fmt.Sprintf(", method set of %s", msname) + } + d.incompatible(oldMethod, part, "removed") + } else { + obj := oldMethod + // If a value method is changed to a pointer method and has a signature + // change, then we can get two messages for the same method definition: one + // for the value method set that says it's removed, and another for the + // pointer method set that says it changed. To keep both messages (since + // messageSet dedups), use newMethod for the second. (Slight hack.) + if !hasPointerReceiver(oldMethod) && hasPointerReceiver(newMethod) { + obj = newMethod + } + d.checkCorrespondence(obj, "", oldMethod.Type(), newMethod.Type()) + } + } + + // Check for added methods. + for name, newMethod := range newMethodSet { + if oldMethodSet[name] == nil { + if addcompat { + d.compatible(newMethod, "", "added") + } else { + d.incompatible(newMethod, "", "added") + } + } + } +} + +// exportedMethods collects all the exported methods of type's method set. +func exportedMethods(t types.Type) map[string]types.Object { + m := map[string]types.Object{} + ms := types.NewMethodSet(t) + for i := 0; i < ms.Len(); i++ { + obj := ms.At(i).Obj() + if obj.Exported() { + m[obj.Name()] = obj + } + } + return m +} + +func receiverType(method types.Object) types.Type { + return method.Type().(*types.Signature).Recv().Type() +} + +func receiverNamedType(method types.Object) *types.Named { + switch t := receiverType(method).(type) { + case *types.Pointer: + return t.Elem().(*types.Named) + case *types.Named: + return t + default: + panic("unreachable") + } +} + +func hasPointerReceiver(method types.Object) bool { + _, ok := receiverType(method).(*types.Pointer) + return ok +} diff --git a/internal/apidiff/correspondence.go b/internal/apidiff/correspondence.go new file mode 100644 index 0000000000..bd14c094b5 --- /dev/null +++ b/internal/apidiff/correspondence.go @@ -0,0 +1,219 @@ +package apidiff + +import ( + "go/types" + "sort" +) + +// Two types are correspond if they are identical except for defined types, +// which must correspond. +// +// Two defined types correspond if they can be interchanged in the old and new APIs, +// possibly after a renaming. +// +// This is not a pure function. If we come across named types while traversing, +// we establish correspondence. +func (d *differ) correspond(old, new types.Type) bool { + return d.corr(old, new, nil) +} + +// corr determines whether old and new correspond. The argument p is a list of +// known interface identities, to avoid infinite recursion. +// +// corr calls itself recursively as much as possible, to establish more +// correspondences and so check more of the API. E.g. if the new function has more +// parameters than the old, compare all the old ones before returning false. +// +// Compare this to the implementation of go/types.Identical. +func (d *differ) corr(old, new types.Type, p *ifacePair) bool { + // Structure copied from types.Identical. + switch old := old.(type) { + case *types.Basic: + return types.Identical(old, new) + + case *types.Array: + if new, ok := new.(*types.Array); ok { + return d.corr(old.Elem(), new.Elem(), p) && old.Len() == new.Len() + } + + case *types.Slice: + if new, ok := new.(*types.Slice); ok { + return d.corr(old.Elem(), new.Elem(), p) + } + + case *types.Map: + if new, ok := new.(*types.Map); ok { + return d.corr(old.Key(), new.Key(), p) && d.corr(old.Elem(), new.Elem(), p) + } + + case *types.Chan: + if new, ok := new.(*types.Chan); ok { + return d.corr(old.Elem(), new.Elem(), p) && old.Dir() == new.Dir() + } + + case *types.Pointer: + if new, ok := new.(*types.Pointer); ok { + return d.corr(old.Elem(), new.Elem(), p) + } + + case *types.Signature: + if new, ok := new.(*types.Signature); ok { + pe := d.corr(old.Params(), new.Params(), p) + re := d.corr(old.Results(), new.Results(), p) + return old.Variadic() == new.Variadic() && pe && re + } + + case *types.Tuple: + if new, ok := new.(*types.Tuple); ok { + for i := 0; i < old.Len(); i++ { + if i >= new.Len() || !d.corr(old.At(i).Type(), new.At(i).Type(), p) { + return false + } + } + return old.Len() == new.Len() + } + + case *types.Struct: + if new, ok := new.(*types.Struct); ok { + for i := 0; i < old.NumFields(); i++ { + if i >= new.NumFields() { + return false + } + of := old.Field(i) + nf := new.Field(i) + if of.Anonymous() != nf.Anonymous() || + old.Tag(i) != new.Tag(i) || + !d.corr(of.Type(), nf.Type(), p) || + !d.corrFieldNames(of, nf) { + return false + } + } + return old.NumFields() == new.NumFields() + } + + case *types.Interface: + if new, ok := new.(*types.Interface); ok { + // Deal with circularity. See the comment in types.Identical. + q := &ifacePair{old, new, p} + for p != nil { + if p.identical(q) { + return true // same pair was compared before + } + p = p.prev + } + oldms := d.sortedMethods(old) + newms := d.sortedMethods(new) + for i, om := range oldms { + if i >= len(newms) { + return false + } + nm := newms[i] + if d.methodID(om) != d.methodID(nm) || !d.corr(om.Type(), nm.Type(), q) { + return false + } + } + return old.NumMethods() == new.NumMethods() + } + + 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) + } + + default: + panic("unknown type kind") + } + return false +} + +// Compare old and new field names. We are determining correspondence across packages, +// so just compare names, not packages. For an unexported, embedded field of named +// type (non-named embedded fields are possible with aliases), we check that the type +// names correspond. We check the types for correspondence before this is called, so +// we've established correspondence. +func (d *differ) corrFieldNames(of, nf *types.Var) bool { + if of.Anonymous() && nf.Anonymous() && !of.Exported() && !nf.Exported() { + if on, ok := of.Type().(*types.Named); ok { + nn := nf.Type().(*types.Named) + return d.establishCorrespondence(on, nn) + } + } + return of.Name() == nf.Name() +} + +// Establish that old corresponds with new if it does not already +// correspond to something else. +func (d *differ) establishCorrespondence(old *types.Named, new types.Type) bool { + oldname := old.Obj() + oldc := d.correspondMap[oldname] + if oldc == nil { + // For now, assume the types don't correspond unless they are from the old + // and new packages, respectively. + // + // This is too conservative. For instance, + // [old] type A = q.B; [new] type A q.C + // could be OK if in package q, B is an alias for C. + // Or, using p as the name of the current old/new packages: + // [old] type A = q.B; [new] type A int + // could be OK if in q, + // [old] type B int; [new] type B = p.A + // In this case, p.A and q.B name the same type in both old and new worlds. + // Note that this case doesn't imply circular package imports: it's possible + // that in the old world, p imports q, but in the new, q imports p. + // + // However, if we didn't do something here, then we'd incorrectly allow cases + // like the first one above in which q.B is not an alias for q.C + // + // What we should do is check that the old type, in the new world's package + // 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 { + if old.Obj().Pkg() != d.old || newn.Obj().Pkg() != d.new { + return old.Obj().Id() == newn.Obj().Id() + } + } + // If there is no correspondence, create one. + d.correspondMap[oldname] = new + // Check that the corresponding types are compatible. + d.checkCompatibleDefined(oldname, old, new) + return true + } + return types.Identical(oldc, new) +} + +func (d *differ) sortedMethods(iface *types.Interface) []*types.Func { + ms := make([]*types.Func, iface.NumMethods()) + for i := 0; i < iface.NumMethods(); i++ { + ms[i] = iface.Method(i) + } + sort.Slice(ms, func(i, j int) bool { return d.methodID(ms[i]) < d.methodID(ms[j]) }) + return ms +} + +func (d *differ) methodID(m *types.Func) string { + // If the method belongs to one of the two packages being compared, use + // just its name even if it's unexported. That lets us treat unexported names + // from the old and new packages as equal. + if m.Pkg() == d.old || m.Pkg() == d.new { + return m.Name() + } + return m.Id() +} + +// Copied from the go/types package: + +// An ifacePair is a node in a stack of interface type pairs compared for identity. +type ifacePair struct { + x, y *types.Interface + prev *ifacePair +} + +func (p *ifacePair) identical(q *ifacePair) bool { + return p.x == q.x && p.y == q.y || p.x == q.y && p.y == q.x +} diff --git a/internal/apidiff/messageset.go b/internal/apidiff/messageset.go new file mode 100644 index 0000000000..135479053d --- /dev/null +++ b/internal/apidiff/messageset.go @@ -0,0 +1,79 @@ +// TODO: show that two-non-empty dotjoin can happen, by using an anon struct as a field type +// TODO: don't report removed/changed methods for both value and pointer method sets? + +package apidiff + +import ( + "fmt" + "go/types" + "sort" + "strings" +) + +// There can be at most one message for each object or part thereof. +// Parts include interface methods and struct fields. +// +// The part thing is necessary. Method (Func) objects have sufficient info, but field +// Vars do not: they just have a field name and a type, without the enclosing struct. +type messageSet map[types.Object]map[string]string + +// Add a message for obj and part, overwriting a previous message +// (shouldn't happen). +// obj is required but part can be empty. +func (m messageSet) add(obj types.Object, part, msg string) { + s := m[obj] + if s == nil { + s = map[string]string{} + m[obj] = s + } + if f, ok := s[part]; ok && f != msg { + fmt.Printf("! second, different message for obj %s, part %q\n", obj, part) + fmt.Printf(" first: %s\n", f) + fmt.Printf(" second: %s\n", msg) + } + s[part] = msg +} + +func (m messageSet) collect() []string { + var s []string + for obj, parts := range m { + // Format each object name relative to its own package. + objstring := objectString(obj) + for part, msg := range parts { + var p string + + if strings.HasPrefix(part, ",") { + p = objstring + part + } else { + p = dotjoin(objstring, part) + } + s = append(s, p+": "+msg) + } + } + sort.Strings(s) + return s +} + +func objectString(obj types.Object) string { + if f, ok := obj.(*types.Func); ok { + sig := f.Type().(*types.Signature) + if recv := sig.Recv(); recv != nil { + tn := types.TypeString(recv.Type(), types.RelativeTo(obj.Pkg())) + if tn[0] == '*' { + tn = "(" + tn + ")" + } + return fmt.Sprintf("%s.%s", tn, obj.Name()) + } + } + return obj.Name() +} + +func dotjoin(s1, s2 string) string { + if s1 == "" { + return s2 + } + if s2 == "" { + return s1 + } + return s1 + "." + s2 +} diff --git a/internal/apidiff/report.go b/internal/apidiff/report.go new file mode 100644 index 0000000000..fd346b158d --- /dev/null +++ b/internal/apidiff/report.go @@ -0,0 +1,55 @@ +package apidiff + +import ( + "bytes" + "fmt" + "io" +) + +// Report describes the changes detected by Changes. +type Report struct { + Incompatible, Compatible []string +} + +func (r Report) String() string { + var buf bytes.Buffer + if err := r.Text(&buf); err != nil { + return fmt.Sprintf("!!%v", err) + } + return buf.String() +} + +func (r Report) Text(w io.Writer) error { + if err := r.TextIncompatible(w, true); err != nil { + return err + } + return r.TextCompatible(w) +} + +func (r Report) TextIncompatible(w io.Writer, withHeader bool) error { + if withHeader { + return r.writeMessages(w, "Incompatible changes:", r.Incompatible) + } + return r.writeMessages(w, "", r.Incompatible) +} + +func (r Report) TextCompatible(w io.Writer) error { + return r.writeMessages(w, "Compatible changes:", r.Compatible) +} + +func (r Report) writeMessages(w io.Writer, header string, msgs []string) error { + if len(msgs) == 0 { + return nil + } + if header != "" { + if _, err := fmt.Fprintf(w, "%s\n", header); err != nil { + return err + } + } + for _, m := range msgs { + if _, err := fmt.Fprintf(w, "- %s\n", m); err != nil { + return err + } + } + return nil +} diff --git a/internal/apidiff/testdata/exported_fields/ef.go b/internal/apidiff/testdata/exported_fields/ef.go new file mode 100644 index 0000000000..19da716c46 --- /dev/null +++ b/internal/apidiff/testdata/exported_fields/ef.go @@ -0,0 +1,37 @@ +package exported_fields + +// Used for testing exportedFields. +// Its exported fields are: +// A1 [1]int +// D bool +// E int +// F F +// S *S +type ( + S struct { + int + *embed2 + embed + E int // shadows embed.E + alias + A1 + *S + } + + A1 [1]int + + embed struct { + E string + } + + embed2 struct { + embed3 + F // shadows embed3.F + } + embed3 struct { + F bool + } + alias = struct{ D bool } + + F int +) diff --git a/internal/apidiff/testdata/tests.go b/internal/apidiff/testdata/tests.go new file mode 100644 index 0000000000..014e813027 --- /dev/null +++ b/internal/apidiff/testdata/tests.go @@ -0,0 +1,924 @@ +// This file is split into two packages, old and new. +// It is syntactically valid Go so that gofmt can process it. +// +// If a comment begins with: Then: +// old write subsequent lines to the "old" package +// new write subsequent lines to the "new" package +// both write subsequent lines to both packages +// c expect a compatible error with the following text +// i expect an incompatible error with the following text +package ignore + +// both +import "io" + +//////////////// Basics + +//// Same type in both: OK. +// both +type A int + +//// Changing the type is an incompatible change. +// old +type B int + +// new +// i B: changed from int to string +type B string + +//// Adding a new type, whether alias or not, is a compatible change. +// new +// c AA: added +type AA = A + +// c B1: added +type B1 bool + +//// Change of type for an unexported name doesn't matter... +// old +type t int + +// new +type t string // OK: t isn't part of the API + +//// ...unless it is exposed. +// both +var V2 u + +// old +type u string + +// new +// i u: changed from string to int +type u int + +//// An exposed, unexported type can be renamed. +// both +type u2 int + +// old +type u1 int + +var V5 u1 + +// new +var V5 u2 // OK: V5 has changed type, but old u1 corresopnds to new u2 + +//// Splitting a single type into two is an incompatible change. +// both +type u3 int + +// old +type ( + Split1 = u1 + Split2 = u1 +) + +// new +type ( + Split1 = u2 // OK, since old u1 corresponds to new u2 + + // This tries to make u1 correspond to u3 + // i Split2: changed from u1 to u3 + Split2 = u3 +) + +//// Merging two types into one is OK. +// old +type ( + GoodMerge1 = u2 + GoodMerge2 = u3 +) + +// new +type ( + GoodMerge1 = u3 + GoodMerge2 = u3 +) + +//// Merging isn't OK here because a method is lost. +// both +type u4 int + +func (u4) M() {} + +// old +type ( + BadMerge1 = u3 + BadMerge2 = u4 +) + +// new +type ( + BadMerge1 = u3 + // i u4.M: removed + // What's really happening here is that old u4 corresponds to new u3, + // and new u3's method set is not a superset of old u4's. + BadMerge2 = u3 +) + +// old +type Rem int + +// new +// i Rem: removed + +//////////////// Constants + +//// type changes +// old +const ( + C1 = 1 + C2 int = 2 + C3 = 3 + C4 u1 = 4 +) + +var V8 int + +// new +const ( + // i C1: changed from untyped int to untyped string + C1 = "1" + // i C2: changed from int to untyped int + C2 = -1 + // i C3: changed from untyped int to int + C3 int = 3 + // i V8: changed from var to const + V8 int = 1 + C4 u2 = 4 // OK: u1 corresponds to u2 +) + +// value change +// old +const ( + Cr1 = 1 + Cr2 = "2" + Cr3 = 3.5 + Cr4 = complex(0, 4.1) +) + +// new +const ( + // i Cr1: value changed from 1 to -1 + Cr1 = -1 + // i Cr2: value changed from "2" to "3" + Cr2 = "3" + // i Cr3: value changed from 3.5 to 3.8 + Cr3 = 3.8 + // i Cr4: value changed from (0 + 4.1i) to (4.1 + 0i) + Cr4 = complex(4.1, 0) +) + +//////////////// Variables + +//// simple type changes +// old +var ( + V1 string + V3 A + V7 <-chan int +) + +// new +var ( + // i V1: changed from string to []string + V1 []string + V3 A // OK: same + // i V7: changed from <-chan int to chan int + V7 chan int +) + +//// interface type changes +// old +var ( + V9 interface{ M() } + V10 interface{ M() } + V11 interface{ M() } +) + +// new +var ( + // i V9: changed from interface{M()} to interface{} + V9 interface{} + // i V10: changed from interface{M()} to interface{M(); M2()} + V10 interface { + M2() + M() + } + // i V11: changed from interface{M()} to interface{M(int)} + V11 interface{ M(int) } +) + +//// struct type changes +// old +var ( + VS1 struct{ A, B int } + VS2 struct{ A, B int } + VS3 struct{ A, B int } + VS4 struct { + A int + u1 + } +) + +// new +var ( + // i VS1: changed from struct{A int; B int} to struct{B int; A int} + VS1 struct{ B, A int } + // i VS2: changed from struct{A int; B int} to struct{A int} + VS2 struct{ A int } + // i VS3: changed from struct{A int; B int} to struct{A int; B int; C int} + VS3 struct{ A, B, C int } + VS4 struct { + A int + u2 + } +) + +//////////////// Types + +// old +const C5 = 3 + +type ( + A1 [1]int + A2 [2]int + A3 [C5]int +) + +// new +// i C5: value changed from 3 to 4 +const C5 = 4 + +type ( + A1 [1]int + // i A2: changed from [2]int to [2]bool + A2 [2]bool + // i A3: changed from [3]int to [4]int + A3 [C5]int +) + +// old +type ( + Sl []int + P1 *int + P2 *u1 +) + +// new +type ( + // i Sl: changed from []int to []string + Sl []string + // i P1: changed from *int to **bool + P1 **bool + P2 *u2 // OK: u1 corresponds to u2 +) + +// old +type Bc1 int32 +type Bc2 uint +type Bc3 float32 +type Bc4 complex64 + +// new +// c Bc1: changed from int32 to int +type Bc1 int + +// c Bc2: changed from uint to uint64 +type Bc2 uint64 + +// c Bc3: changed from float32 to float64 +type Bc3 float64 + +// c Bc4: changed from complex64 to complex128 +type Bc4 complex128 + +// old +type Bi1 int32 +type Bi2 uint +type Bi3 float64 +type Bi4 complex128 + +// new +// i Bi1: changed from int32 to int16 +type Bi1 int16 + +// i Bi2: changed from uint to uint32 +type Bi2 uint32 + +// i Bi3: changed from float64 to float32 +type Bi3 float32 + +// i Bi4: changed from complex128 to complex64 +type Bi4 complex64 + +// old +type ( + M1 map[string]int + M2 map[string]int + M3 map[string]int +) + +// new +type ( + M1 map[string]int + // i M2: changed from map[string]int to map[int]int + M2 map[int]int + // i M3: changed from map[string]int to map[string]string + M3 map[string]string +) + +// old +type ( + Ch1 chan int + Ch2 <-chan int + Ch3 chan int + Ch4 <-chan int +) + +// new +type ( + // i Ch1, element type: changed from int to bool + Ch1 chan bool + // i Ch2: changed direction + Ch2 chan<- int + // i Ch3: changed direction + Ch3 <-chan int + // c Ch4: removed direction + Ch4 chan int +) + +// old +type I1 interface { + M1() + M2() +} + +// new +type I1 interface { + // M1() + // i I1.M1: removed + M2(int) + // i I1.M2: changed from func() to func(int) + M3() + // i I1.M3: added + m() + // i I1.m: added unexported method +} + +// old +type I2 interface { + M1() + m() +} + +// new +type I2 interface { + M1() + // m() Removing an unexported method is OK. + m2() // OK, because old already had an unexported method + // c I2.M2: added + M2() +} + +// old +type I3 interface { + io.Reader + M() +} + +// new +// OK: what matters is the method set; the name of the embedded +// interface isn't important. +type I3 interface { + M() + Read([]byte) (int, error) +} + +// old +type I4 io.Writer + +// new +// OK: in both, I4 is a distinct type from io.Writer, and +// the old and new I4s have the same method set. +type I4 interface { + Write([]byte) (int, error) +} + +// old +type I5 = io.Writer + +// new +// i I5: changed from io.Writer to I5 +// In old, I5 and io.Writer are the same type; in new, +// they are different. That can break something like: +// var _ func(io.Writer) = func(pkg.I6) {} +type I5 io.Writer + +// old +type I6 interface{ Write([]byte) (int, error) } + +// new +// i I6: changed from I6 to io.Writer +// Similar to the above. +type I6 = io.Writer + +//// correspondence with a basic type +// Basic types are technically defined types, but they aren't +// represented that way in go/types, so the cases below are special. + +// both +type T1 int + +// old +var VT1 T1 + +// new +// i VT1: changed from T1 to int +// This fails because old T1 corresponds to both int and new T1. +var VT1 int + +// old +type t2 int + +var VT2 t2 + +// new +// OK: t2 corresponds to int. It's fine that old t2 +// doesn't exist in new. +var VT2 int + +// both +type t3 int + +func (t3) M() {} + +// old +var VT3 t3 + +// new +// i t3.M: removed +// Here the change from t3 to int is incompatible +// because old t3 has an exported method. +var VT3 int + +// old +var VT4 int + +// new +type t4 int + +// i VT4: changed from int to t4 +// This is incompatible because of code like +// VT4 + int(1) +// which works in old but fails in new. +// The difference from the above cases is that +// in those, we were merging two types into one; +// here, we are splitting int into t4 and int. +var VT4 t4 + +//////////////// Functions + +// old +func F1(a int, b string) map[u1]A { return nil } +func F2(int) {} +func F3(int) {} +func F4(int) int { return 0 } +func F5(int) int { return 0 } +func F6(int) {} +func F7(interface{}) {} + +// new +func F1(c int, d string) map[u2]AA { return nil } //OK: same (since u1 corresponds to u2) + +// i F2: changed from func(int) to func(int) bool +func F2(int) bool { return true } + +// i F3: changed from func(int) to func(int, int) +func F3(int, int) {} + +// i F4: changed from func(int) int to func(bool) int +func F4(bool) int { return 0 } + +// i F5: changed from func(int) int to func(int) string +func F5(int) string { return "" } + +// i F6: changed from func(int) to func(...int) +func F6(...int) {} + +// i F7: changed from func(interface{}) to func(interface{x()}) +func F7(a interface{ x() }) {} + +// old +func F8(bool) {} + +// new +// c F8: changed from func to var +var F8 func(bool) + +// old +var F9 func(int) + +// new +// i F9: changed from var to func +func F9(int) {} + +// both +// OK, even though new S1 is incompatible with old S1 (see below) +func F10(S1) {} + +//////////////// Structs + +// old +type S1 struct { + A int + B string + C bool + d float32 +} + +// new +type S1 = s1 + +type s1 struct { + C chan int + // i S1.C: changed from bool to chan int + A int + // i S1.B: removed + // i S1: old is comparable, new is not + x []int + d float32 + E bool + // c S1.E: added +} + +// old +type embed struct { + E string +} + +type S2 struct { + A int + embed +} + +// new +type embedx struct { + E string +} + +type S2 struct { + embedx // OK: the unexported embedded field changed names, but the exported field didn't + A int +} + +// both +type F int + +// old +type S3 struct { + A int + embed +} + +// new +type embed struct{ F int } + +type S3 struct { + // i S3.E: removed + embed + // c S3.F: added + A int +} + +// old +type embed2 struct { + embed3 + F // shadows embed3.F +} + +type embed3 struct { + F bool +} + +type alias = struct{ D bool } + +type S4 struct { + int + *embed2 + embed + E int // shadows embed.E + alias + A1 + *S4 +} + +// new +type S4 struct { + // OK: removed unexported fields + // D and F marked as added because they are now part of the immediate fields + D bool + // c S4.D: added + E int // OK: same as in old + F F + // c S4.F: added + A1 // OK: same + *S4 // OK: same (recursive embedding) +} + +//// Difference between exported selectable fields and exported immediate fields. +// both +type S5 struct{ A int } + +// old +// Exported immediate fields: A, S5 +// Exported selectable fields: A int, S5 S5 +type S6 struct { + S5 S5 + A int +} + +// new +// Exported immediate fields: S5 +// Exported selectable fields: A int, S5 S5. + +// i S6.A: removed +type S6 struct { + S5 +} + +//// Ambiguous fields can exist; they just can't be selected. +// both +type ( + embed7a struct{ E int } + embed7b struct{ E bool } +) + +// old +type S7 struct { // legal, but no selectable fields + embed7a + embed7b +} + +// new +type S7 struct { + embed7a + embed7b + // c S7.E: added + E string +} + +//////////////// Method sets + +// old +type SM struct { + embedm + Embedm +} + +func (SM) V1() {} +func (SM) V2() {} +func (SM) V3() {} +func (SM) V4() {} +func (SM) v() {} + +func (*SM) P1() {} +func (*SM) P2() {} +func (*SM) P3() {} +func (*SM) P4() {} +func (*SM) p() {} + +type embedm int + +func (embedm) EV1() {} +func (embedm) EV2() {} +func (embedm) EV3() {} +func (*embedm) EP1() {} +func (*embedm) EP2() {} +func (*embedm) EP3() {} + +type Embedm struct { + A int +} + +func (Embedm) FV() {} +func (*Embedm) FP() {} + +type RepeatEmbedm struct { + Embedm +} + +// new +type SM struct { + embedm2 + embedm3 + Embedm + // i SM.A: changed from int to bool +} + +// c SMa: added +type SMa = SM + +func (SM) V1() {} // OK: same + +// func (SM) V2() {} +// i SM.V2: removed + +// i SM.V3: changed from func() to func(int) +func (SM) V3(int) {} + +// c SM.V5: added +func (SM) V5() {} + +func (SM) v(int) {} // OK: unexported method change +func (SM) v2() {} // OK: unexported method added + +func (*SM) P1() {} // OK: same +//func (*SM) P2() {} +// i (*SM).P2: removed + +// i (*SM).P3: changed from func() to func(int) +func (*SMa) P3(int) {} + +// c (*SM).P5: added +func (*SM) P5() {} + +// func (*SM) p() {} // OK: unexported method removed + +// Changing from a value to a pointer receiver or vice versa +// just looks like adding and removing a method. + +// i SM.V4: removed +// i (*SM).V4: changed from func() to func(int) +func (*SM) V4(int) {} + +// c SM.P4: added +// P4 is not removed from (*SM) because value methods +// are in the pointer method set. +func (SM) P4() {} + +type embedm2 int + +// i embedm.EV1: changed from func() to func(int) +func (embedm2) EV1(int) {} + +// i embedm.EV2, method set of SM: removed +// i embedm.EV2, method set of *SM: removed + +// i (*embedm).EP2, method set of *SM: removed +func (*embedm2) EP1() {} + +type embedm3 int + +func (embedm3) EV3() {} // OK: compatible with old embedm.EV3 +func (*embedm3) EP3() {} // OK: compatible with old (*embedm).EP3 + +type Embedm struct { + // i Embedm.A: changed from int to bool + A bool +} + +// i Embedm.FV: changed from func() to func(int) +func (Embedm) FV(int) {} +func (*Embedm) FP() {} + +type RepeatEmbedm struct { + // i RepeatEmbedm.A: changed from int to bool + Embedm +} + +//////////////// Whole-package interface satisfaction + +// old +type WI1 interface { + M1() + m1() +} + +type WI2 interface { + M2() + m2() +} + +type WS1 int + +func (WS1) M1() {} +func (WS1) m1() {} + +type WS2 int + +func (WS2) M2() {} +func (WS2) m2() {} + +// new +type WI1 interface { + M1() + m() +} + +type WS1 int + +func (WS1) M1() {} + +// i WS1: no longer implements WI1 +//func (WS1) m1() {} + +type WI2 interface { + M2() + m2() + // i WS2: no longer implements WI2 + m3() +} + +type WS2 int + +func (WS2) M2() {} +func (WS2) m2() {} + +//////////////// Miscellany + +// This verifies that the code works even through +// multiple levels of unexported typed. + +// old +var Z w + +type w []x +type x []z +type z int + +// new +var Z w + +type w []x +type x []z + +// i z: changed from int to bool +type z bool + +// old +type H struct{} + +func (H) M() {} + +// new +// i H: changed from struct{} to interface{M()} +type H interface { + M() +} + +//// Splitting types + +//// OK: in both old and new, {J1, K1, L1} name the same type. +// old +type ( + J1 = K1 + K1 = L1 + L1 int +) + +// new +type ( + J1 = K1 + K1 int + L1 = J1 +) + +//// Old has one type, K2; new has J2 and K2. +// both +type K2 int + +// old +type J2 = K2 + +// new +// i K2: changed from K2 to K2 +type J2 K2 // old K2 corresponds with new J2 +// old K2 also corresponds with new K2: problem + +// both +type k3 int + +var Vj3 j3 // expose j3 + +// old +type j3 = k3 + +// new +// OK: k3 isn't exposed +type j3 k3 + +// both +type k4 int + +var Vj4 j4 // expose j4 +var VK4 k4 // expose k4 + +// old +type j4 = k4 + +// new +// i Vj4: changed from k4 to j4 +// e.g. p.Vj4 = p.Vk4 +type j4 k4