apidiff: add module comparison support

Change-Id: I11fa1aac30c8e2131bd0dd948fdcaf15332f8deb
Reviewed-on: https://go-review.googlesource.com/c/exp/+/495635
Reviewed-by: Jonathan Amsterdam <[email protected]>
Run-TryBot: Jonathan Amsterdam <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: noahdietz

apidiff/README.md

@@ -1,21 +1,22 @@
-# Checking Go Package API Compatibility
+# Checking Go 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.
+The `apidiff` tool in this directory determines whether two examples of a
+package or module 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.
-
+`apidiff` may be used to display API differences between any two packages or
+modules, not just different versions of the same thing. It does this by ignoring
+the package import paths when directly comparing two packages, and
+by ignoring module paths when comparing two modules. That is to say, when
+comparing two modules, the package import paths **do** matter, but are compared
+_relative_ to their respective module root.
 
 ## Compatibility Desiderata
 
@@ -222,6 +223,111 @@ element types correspond.
 
 We can now present the definition of compatibility used by `apidiff`.
 
+### Module Compatibility
+
+> A new module is compatible with an old one if:
+>1. Each package present in the old module also appears in the new module,
+> with matching import paths relative to their respective module root, and
+>2. Each package present in both modules fulfills Package Compatibility as
+> defined below.
+>
+>Otherwise the modules are incompatible.
+
+If a package is converted into a nested module of the original module then
+comparing two versions of the module, before and after nested module creation,
+will produce an incompatible package removal message. This removal message does
+not necessarily mean that client code will need to change. If the package API
+retains Package Compatibility after nested module creation, then only the
+`go.mod` of the client code will need to change. Take the following example:
+
+```
+./
+  go.mod
+  go.sum
+  foo.go
+  bar/bar.go
+```
+
+Where `go.mod` is:
+
+```
+module example.com/foo
+
+go 1.20
+```
+
+Where `bar/bar.go` is:
+
+```
+package bar
+
+var V int
+```
+
+And `foo.go` is:
+
+```
+package foo
+
+import "example.com/foo/bar"
+
+_ = bar.V
+```
+
+Creating a nested module with the package `bar` while retaining Package
+Compatibility is _code_ compatible, because the import path of the package does
+not change:
+
+```
+./
+  go.mod
+  go.sum
+  foo.go
+  bar/
+    bar.go
+    go.mod
+    go.sum
+```
+
+Where `bar/go.mod` is:
+```
+module example.com/foo/bar
+
+go 1.20
+```
+
+And the top-level `go.mod` becomes:
+```
+module example.com/foo
+
+go 1.20
+
+// New dependency on nested module.
+require example.com/foo/bar v1.0.0
+```
+
+If during nested module creation either Package Compatibility is broken, like so
+in `bar/bar.go`:
+
+```
+package bar
+
+// Changed from V to T.
+var T int
+```
+
+Or the nested module uses a name other than the original package's import path,
+like so in `bar/go.mod`:
+
+```
+// Completely different module name
+module example.com/qux
+
+go 1.20
+```
+
+Then the move is backwards incompatible for client code.
+
 ### Package Compatibility
 
 > A new package is compatible with an old one if:

apidiff/apidiff.go

@@ -15,12 +15,12 @@ import (
 	"go/constant"
 	"go/token"
 	"go/types"
+	"strings"
 )
 
 // 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.
+// a detailed discussion of what constitutes an incompatible change, see the README.
 func Changes(old, new *types.Package) Report {
 	d := newDiffer(old, new)
 	d.checkPackage()
@@ -34,6 +34,63 @@ func Changes(old, new *types.Package) Report {
 	return r
 }
 
+// ModuleChanges reports on the differences between the APIs of the old and new
+// modules. It classifies each difference as either compatible or incompatible
+// (breaking). This includes the addition and removal of entire packages. For a
+// detailed discussion of what constitutes an incompatible change, see the README.
+func ModuleChanges(old, new *Module) Report {
+	var r Report
+
+	oldPkgs := make(map[string]*types.Package)
+	for _, p := range old.Packages {
+		oldPkgs[old.relativePath(p)] = p
+	}
+
+	newPkgs := make(map[string]*types.Package)
+	for _, p := range new.Packages {
+		newPkgs[new.relativePath(p)] = p
+	}
+
+	for n, op := range oldPkgs {
+		if np, ok := newPkgs[n]; ok {
+			// shared package, compare surfaces
+			rr := Changes(op, np)
+			r.Changes = append(r.Changes, rr.Changes...)
+		} else {
+			// old package was removed
+			r.Changes = append(r.Changes, packageChange(op, "removed", false))
+		}
+	}
+
+	for n, np := range newPkgs {
+		if _, ok := oldPkgs[n]; !ok {
+			// new package was added
+			r.Changes = append(r.Changes, packageChange(np, "added", true))
+		}
+	}
+
+	return r
+}
+
+func packageChange(p *types.Package, change string, compatible bool) Change {
+	return Change{
+		Message:    fmt.Sprintf("package %s: %s", p.Path(), change),
+		Compatible: compatible,
+	}
+}
+
+// Module is a convenience type for representing a Go module with a path and a
+// slice of Packages contained within.
+type Module struct {
+	Path     string
+	Packages []*types.Package
+}
+
+// relativePath computes the module-relative package path of the given Package.
+func (m *Module) relativePath(p *types.Package) string {
+	return strings.TrimPrefix(p.Path(), m.Path)
+}
+
 type differ struct {
 	old, new *types.Package
 	// Correspondences between named types.

apidiff/apidiff_test.go

@@ -14,8 +14,71 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/go/packages/packagestest"
 )
 
+func TestModuleChanges(t *testing.T) {
+	packagestest.TestAll(t, testModuleChanges)
+}
+
+func testModuleChanges(t *testing.T, x packagestest.Exporter) {
+	e := packagestest.Export(t, x, []packagestest.Module{
+		{
+			Name: "example.com/moda",
+			Files: map[string]any{
+				"foo/foo.go":     "package foo\n\nconst Version = 1",
+				"foo/baz/baz.go": "package baz",
+			},
+		},
+		{
+			Name: "example.com/modb",
+			Files: map[string]any{
+				"foo/foo.go": "package foo\n\nconst Version = 2\nconst Other = 1",
+				"bar/bar.go": "package bar",
+			},
+		},
+	})
+	defer e.Cleanup()
+
+	a, err := loadModule(t, e.Config, "example.com/moda")
+	if err != nil {
+		t.Fatal(err)
+	}
+	b, err := loadModule(t, e.Config, "example.com/modb")
+	if err != nil {
+		t.Fatal(err)
+	}
+	report := ModuleChanges(a, b)
+	if len(report.Changes) == 0 {
+		t.Fatal("expected some changes, but got none")
+	}
+	wanti := []string{
+		"Version: value changed from 1 to 2",
+		"package example.com/moda/foo/baz: removed",
+	}
+	sort.Strings(wanti)
+
+	got := report.messages(false)
+	sort.Strings(got)
+
+	if diff := cmp.Diff(wanti, got); diff != "" {
+		t.Errorf("incompatibles: mismatch (-want, +got)\n%s", diff)
+	}
+
+	wantc := []string{
+		"Other: added",
+		"package example.com/modb/bar: added",
+	}
+	sort.Strings(wantc)
+
+	got = report.messages(true)
+	sort.Strings(got)
+
+	if diff := cmp.Diff(wantc, got); diff != "" {
+		t.Errorf("compatibles: mismatch (-want, +got)\n%s", diff)
+	}
+}
+
 func TestChanges(t *testing.T) {
 	dir, err := os.MkdirTemp("", "apidiff_test")
 	if err != nil {
@@ -27,11 +90,11 @@ func TestChanges(t *testing.T) {
 	sort.Strings(wanti)
 	sort.Strings(wantc)
 
-	oldpkg, err := load(t, "apidiff/old", dir)
+	oldpkg, err := loadPackage(t, "apidiff/old", dir)
 	if err != nil {
 		t.Fatal(err)
 	}
-	newpkg, err := load(t, "apidiff/new", dir)
+	newpkg, err := loadPackage(t, "apidiff/new", dir)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -116,7 +179,31 @@ func splitIntoPackages(t *testing.T, dir string) (incompatibles, compatibles []s
 	return
 }
 
-func load(t *testing.T, importPath, goPath string) (*packages.Package, error) {
+// Copied from cmd/apidiff/main.go.
+func loadModule(t *testing.T, cfg *packages.Config, modulePath string) (*Module, error) {
+	needsGoPackages(t)
+
+	cfg.Mode = cfg.Mode | packages.LoadTypes
+	loaded, err := packages.Load(cfg, fmt.Sprintf("%s/...", modulePath))
+	if err != nil {
+		return nil, err
+	}
+	if len(loaded) == 0 {
+		return nil, fmt.Errorf("found no packages for module %s", modulePath)
+	}
+	var tpkgs []*types.Package
+	for _, p := range loaded {
+		if len(p.Errors) > 0 {
+			// TODO: use errors.Join once Go 1.21 is released.
+			return nil, p.Errors[0]
+		}
+		tpkgs = append(tpkgs, p.Types)
+	}
+
+	return &Module{Path: modulePath, Packages: tpkgs}, nil
+}
+
+func loadPackage(t *testing.T, importPath, goPath string) (*packages.Package, error) {
 	needsGoPackages(t)
 
 	cfg := &packages.Config{
@@ -137,7 +224,7 @@ func load(t *testing.T, importPath, goPath string) (*packages.Package, error) {
 }
 
 func TestExportedFields(t *testing.T) {
-	pkg, err := load(t, "golang.org/x/exp/apidiff/testdata/exported_fields", "")
+	pkg, err := loadPackage(t, "golang.org/x/exp/apidiff/testdata/exported_fields", "")
 	if err != nil {
 		t.Fatal(err)
 	}