Optimizations based on commutativity of identical

cmd/modver/compare.go

@@ -5,8 +5,8 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/bobg/errors"
 	"github.com/google/go-github/v50/github"
-	"github.com/pkg/errors"
 
 	"github.com/bobg/modver/v2"
 	"github.com/bobg/modver/v2/internal"

cmd/modver/options.go

@@ -6,7 +6,7 @@ import (
 	"os"
 	"strings"
 
-	"github.com/pkg/errors"
+	"github.com/bobg/errors"
 	"golang.org/x/mod/semver"
 )
 

cmd/modver/tags.go

@@ -6,11 +6,11 @@ import (
 	"os"
 	"strings"
 
+	"github.com/bobg/errors"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/go-git/go-git/v5/plumbing/storer"
-	"github.com/pkg/errors"
 	"golang.org/x/mod/semver"
 
 	"github.com/bobg/modver/v2"

embed_test.go

@@ -8,7 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/pkg/errors"
+	"github.com/bobg/errors"
 	"golang.org/x/tools/go/packages"
 )
 

go.mod

@@ -3,9 +3,9 @@ module github.com/bobg/modver/v2
 go 1.22
 
 require (
+	github.com/bobg/errors v1.1.0
 	github.com/go-git/go-git/v5 v5.6.1
 	github.com/google/go-github/v50 v50.2.0
-	github.com/pkg/errors v0.9.1
 	golang.org/x/mod v0.15.0
 	golang.org/x/oauth2 v0.16.0
 	golang.org/x/tools v0.17.0

go.sum

@@ -10,6 +10,8 @@ github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFI
 github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
+github.com/bobg/errors v1.1.0 h1:gsVanPzJMpZQpwY+27/GQYElZez5CuMYwiIpk2A3RGw=
+github.com/bobg/errors v1.1.0/go.mod h1:Q4775qBZpnte7EGFJqmvnlB1U4pkI1XmU3qxqdp7Zcc=
 github.com/bwesterb/go-ristretto v1.2.0/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0=
 github.com/cloudflare/circl v1.1.0/go.mod h1:prBCrKB9DV4poKZY1l9zBXg2QJY7mvgRvtMxxK7fi4I=
 github.com/cloudflare/circl v1.3.2 h1:VWp8dY3yH69fdM7lM6A1+NhhVoDu9vqK0jOgmkQHFWk=

identical.go

@@ -8,6 +8,11 @@ func (c *comparer) identical(a, b types.Type) (res bool) {
 	if res, ok := c.identicache[tp]; ok {
 		return res
 	}
+	// identical(a, b) = identical(b, a)
+	tp.a, tp.b = b, a
+	if res, ok := c.identicache[tp]; ok {
+		return res
+	}
 	defer func() { c.identicache[tp] = res }()
 
 	if types.Identical(a, b) {
@@ -20,8 +25,11 @@ func (c *comparer) identical(a, b types.Type) (res bool) {
 		if a == pair.a && b == pair.b {
 			return true
 		}
+		if a == pair.b && b == pair.a {
+			return true
+		}
 	}
-	c.stack = append(c.stack, typePair{a: a, b: b})
+	c.stack = append(c.stack, tp)
 	defer func() { c.stack = c.stack[:len(c.stack)-1] }()
 
 	if na, ok := a.(*types.Named); ok {

internal/github.go

@@ -7,8 +7,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/bobg/errors"
 	"github.com/google/go-github/v50/github"
-	"github.com/pkg/errors"
 	"golang.org/x/oauth2"
 )
 

internal/pr.go

@@ -10,8 +10,8 @@ import (
 	"strings"
 	"text/template"
 
+	"github.com/bobg/errors"
 	"github.com/google/go-github/v50/github"
-	"github.com/pkg/errors"
 
 	"github.com/bobg/modver/v2"
 )

modver_test.go

@@ -4,7 +4,6 @@ import (
 	"bufio"
 	"bytes"
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -13,9 +12,19 @@ import (
 	"strings"
 	"testing"
 	"text/template"
+
+	"github.com/bobg/errors"
 )
 
 func TestCompare(t *testing.T) {
+	tbCompare(t)
+}
+
+func BenchmarkCompare(b *testing.B) {
+	tbCompare(b)
+}
+
+func tbCompare(tb testing.TB) {
 	cases := []struct {
 		dir  string
 		want ResultCode
@@ -30,15 +39,26 @@ func TestCompare(t *testing.T) {
 	}}
 
 	for _, c := range cases {
-		runtest(t, c.dir, c.want)
+		runtest(tb, c.dir, c.want)
+	}
+}
+
+func tbRun(tb testing.TB, name string, f func(testing.TB)) {
+	switch tb := tb.(type) {
+	case *testing.T:
+		tb.Run(name, func(t *testing.T) { f(tb) })
+	case *testing.B:
+		tb.Run(name, func(b *testing.B) { f(tb) })
 	}
 }
 
-func runtest(t *testing.T, typ string, want ResultCode) {
+func runtest(tb testing.TB, typ string, want ResultCode) {
+	b, _ := tb.(*testing.B)
+
 	tree := filepath.Join("testdata", typ)
 	entries, err := os.ReadDir(tree)
 	if err != nil {
-		t.Fatal(err)
+		tb.Fatal(err)
 	}
 	for _, entry := range entries {
 		if entry.IsDir() {
@@ -48,103 +68,117 @@ func runtest(t *testing.T, typ string, want ResultCode) {
 			continue
 		}
 		name := strings.TrimSuffix(entry.Name(), ".tmpl")
-		t.Run(fmt.Sprintf("%s/%s", typ, name), func(t *testing.T) {
-			tmpls, err := template.ParseFiles(filepath.Join(tree, entry.Name()))
-			if err != nil {
-				t.Fatal(err)
-			}
+		tbRun(tb, fmt.Sprintf("%s/%s", typ, name), func(tb testing.TB) {
+			err := withTestDirs(tree, name, func(olderTestDir, newerTestDir string) {
+				if b != nil {
+					b.ResetTimer()
+					for i := 0; i < b.N; i++ {
+						_, err := CompareDirs(olderTestDir, newerTestDir)
+						if err != nil {
+							b.Fatal(err)
+						}
+					}
+					return
+				}
 
-			tmpdir, err := os.MkdirTemp("", "modver")
+				got, err := CompareDirs(olderTestDir, newerTestDir)
+				if err != nil {
+					tb.Fatal(err)
+				}
+				if got.Code() != want {
+					tb.Errorf("want %s, got %s", want, got)
+				} else {
+					tb.Log(got)
+				}
+			})
 			if err != nil {
-				t.Fatal(err)
+				tb.Fatal(err)
 			}
-			defer os.RemoveAll(tmpdir)
+		})
+	}
+}
 
-			var (
-				olderTestDir = filepath.Join(tmpdir, "older")
-				newerTestDir = filepath.Join(tmpdir, "newer")
-			)
+func withTestDirs(tree, name string, f func(olderTestDir, newerTestDir string)) error {
+	tmpls, err := template.ParseFiles(filepath.Join(tree, name+".tmpl"))
+	if err != nil {
+		return errors.Wrap(err, "parsing templates")
+	}
 
-			err = os.Mkdir(olderTestDir, 0755)
-			if err != nil {
-				t.Fatal(err)
-			}
-			err = os.Mkdir(newerTestDir, 0755)
-			if err != nil {
-				t.Fatal(err)
-			}
+	tmpdir, err := os.MkdirTemp("", "modver")
+	if err != nil {
+		return errors.Wrap(err, "creating temp dir")
+	}
+	defer os.RemoveAll(tmpdir)
 
-			var sawGomod bool
-			for _, tmpl := range tmpls.Templates() {
-				// Skip the top-level template
-				if strings.HasSuffix(tmpl.Name(), ".tmpl") {
-					continue
-				}
+	var (
+		olderTestDir = filepath.Join(tmpdir, "older")
+		newerTestDir = filepath.Join(tmpdir, "newer")
+	)
 
-				sawGomod = sawGomod || (filepath.Base(tmpl.Name()) == "go.mod")
+	if err = os.Mkdir(olderTestDir, 0755); err != nil {
+		return errors.Wrap(err, "creating older test dir")
+	}
+	if err = os.Mkdir(newerTestDir, 0755); err != nil {
+		return errors.Wrap(err, "creating newer test dir")
+	}
 
-				parts := strings.Split(tmpl.Name(), "/")
-				if !strings.Contains(parts[len(parts)-1], ".") {
-					parts = append(parts, "x.go")
-				}
+	var sawGomod bool
+	for _, tmpl := range tmpls.Templates() {
+		// Skip the top-level template
+		if strings.HasSuffix(tmpl.Name(), ".tmpl") {
+			continue
+		}
 
-				if len(parts) == 1 {
-					// Only a filename is given.
-					// Write it to both older and newer dirs.
-					buf := new(bytes.Buffer)
-					err = executeTmpl(tmpl, buf)
-					if err != nil {
-						t.Fatal(err)
-					}
-					for _, subdir := range []string{olderTestDir, newerTestDir} {
-						filename := filepath.Join(subdir, parts[0])
-						err = os.WriteFile(filename, buf.Bytes(), 0644)
-						if err != nil {
-							t.Fatal(err)
-						}
-					}
-					continue
-				}
+		sawGomod = sawGomod || (filepath.Base(tmpl.Name()) == "go.mod")
 
-				if len(parts) > 1 {
-					dirparts := append([]string{tmpdir}, parts[:len(parts)-1]...)
-					dirname := filepath.Join(dirparts...)
-					err = os.MkdirAll(dirname, 0755)
-					if err != nil {
-						t.Fatal(err)
-					}
-				}
-				fileparts := append([]string{tmpdir}, parts...)
-				filename := filepath.Join(fileparts...)
-				err = executeTmplToFile(tmpl, filename)
-				if err != nil {
-					t.Fatal(err)
-				}
+		parts := strings.Split(tmpl.Name(), "/")
+		if !strings.Contains(parts[len(parts)-1], ".") {
+			parts = append(parts, "x.go")
+		}
+
+		if len(parts) == 1 {
+			// Only a filename is given.
+			// Write it to both older and newer dirs.
+			buf := new(bytes.Buffer)
+			if err = executeTmpl(tmpl, buf); err != nil {
+				return errors.Wrap(err, "executing template")
 			}
-			if !sawGomod {
-				buf := new(bytes.Buffer)
-				fmt.Fprintf(buf, "module %s\n\ngo 1.18\n", name)
-				err = os.WriteFile(filepath.Join(olderTestDir, "go.mod"), buf.Bytes(), 0644)
-				if err != nil {
-					t.Fatal(err)
-				}
-				err = os.WriteFile(filepath.Join(newerTestDir, "go.mod"), buf.Bytes(), 0644)
-				if err != nil {
-					t.Fatal(err)
+			for _, subdir := range []string{olderTestDir, newerTestDir} {
+				filename := filepath.Join(subdir, parts[0])
+				if err = os.WriteFile(filename, buf.Bytes(), 0644); err != nil {
+					return errors.Wrapf(err, "writing file %s", filename)
 				}
 			}
+			continue
+		}
 
-			got, err := CompareDirs(olderTestDir, newerTestDir)
-			if err != nil {
-				t.Fatal(err)
+		if len(parts) > 1 {
+			dirparts := append([]string{tmpdir}, parts[:len(parts)-1]...)
+			dirname := filepath.Join(dirparts...)
+			if err = os.MkdirAll(dirname, 0755); err != nil {
+				return errors.Wrapf(err, "creating dir %s", dirname)
 			}
-			if got.Code() != want {
-				t.Errorf("want %s, got %s", want, got)
-			} else {
-				t.Log(got)
-			}
-		})
+		}
+		fileparts := append([]string{tmpdir}, parts...)
+		filename := filepath.Join(fileparts...)
+		if err = executeTmplToFile(tmpl, filename); err != nil {
+			return errors.Wrapf(err, "executing template to file %s", filename)
+		}
 	}
+	if !sawGomod {
+		buf := new(bytes.Buffer)
+		fmt.Fprintf(buf, "module %s\n\ngo 1.18\n", name)
+		if err = os.WriteFile(filepath.Join(olderTestDir, "go.mod"), buf.Bytes(), 0644); err != nil {
+			return errors.Wrap(err, "writing older go.mod")
+		}
+		if err = os.WriteFile(filepath.Join(newerTestDir, "go.mod"), buf.Bytes(), 0644); err != nil {
+			return errors.Wrap(err, "writing newer go.mod")
+		}
+	}
+
+	f(olderTestDir, newerTestDir)
+
+	return nil
 }
 
 func executeTmpl(tmpl *template.Template, w io.Writer) error {