gps: Make PackageTree.Copy more careful, efficient

We only perform a single allocation for all []string, and are careful to
copy all possible error values via reflection.

internal/gps/pkgtree/pkgtree.go

@@ -14,6 +14,7 @@ import (
 	"go/token"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strconv"
 	"strings"
@@ -608,21 +609,33 @@ func (t PackageTree) ToReachMap(main, tests, backprop bool, ignore *IgnoredRules
 func (t PackageTree) Copy() PackageTree {
 	t2 := PackageTree{
 		ImportRoot: t.ImportRoot,
-		Packages:   map[string]PackageOrErr{},
+		Packages:   make(map[string]PackageOrErr, len(t.Packages)),
 	}
 
+	// Walk through and count up the total number of string slice elements we'll
+	// need, then allocate them all at once.
+	strcount := 0
+	for _, poe := range t.Packages {
+		strcount = strcount + len(poe.P.Imports) + len(poe.P.TestImports)
+	}
+	pool := make([]string, strcount)
+
 	for path, poe := range t.Packages {
-		poe2 := PackageOrErr{
-			Err: poe.Err,
-			P:   poe.P,
-		}
-		if len(poe.P.Imports) > 0 {
-			poe2.P.Imports = make([]string, len(poe.P.Imports))
-			copy(poe2.P.Imports, poe.P.Imports)
-		}
-		if len(poe.P.TestImports) > 0 {
-			poe2.P.TestImports = make([]string, len(poe.P.TestImports))
-			copy(poe2.P.TestImports, poe.P.TestImports)
+		var poe2 PackageOrErr
+
+		if poe.Err != nil {
+			poe2.Err = reflect.New(reflect.ValueOf(poe.Err).Elem().Type()).Interface().(error)
+		} else {
+			poe2.P = poe.P
+			il, til := len(poe.P.Imports), len(poe.P.TestImports)
+			if il > 0 {
+				poe2.P.Imports, pool = pool[:il], pool[il:]
+				copy(poe2.P.Imports, poe.P.Imports)
+			}
+			if til > 0 {
+				poe2.P.TestImports, pool = pool[:til], pool[til:]
+				copy(poe2.P.TestImports, poe.P.TestImports)
+			}
 		}
 
 		t2.Packages[path] = poe2

internal/gps/pkgtree/pkgtree_test.go

@@ -1409,7 +1409,7 @@ func TestListPackagesNoPerms(t *testing.T) {
 		// It's not a big deal, though, because the os.IsPermission() call we
 		// use in the real code is effectively what's being tested here, and
 		// that's designed to be cross-platform. So, if the unix tests pass, we
-		// have every reason to believe windows tests would to, if the situation
+		// have every reason to believe windows tests would too, if the situation
 		// arises.
 		t.Skip()
 	}
@@ -2019,3 +2019,38 @@ func getTestdataRootDir(t *testing.T) string {
 	}
 	return filepath.Join(cwd, "..", "_testdata")
 }
+
+// Canary regression test to make sure that if PackageTree ever gains new
+// fields, we update the Copy method accordingly.
+func TestCanaryPackageTreeCopy(t *testing.T) {
+	ptreeFields := []string{
+		"ImportRoot",
+		"Packages",
+	}
+	packageFields := []string{
+		"Name",
+		"ImportPath",
+		"CommentPath",
+		"Imports",
+		"TestImports",
+	}
+
+	fieldNames := func(typ reflect.Type) []string {
+		var names []string
+		for i := 0; i < typ.NumField(); i++ {
+			names = append(names, typ.Field(i).Name)
+		}
+		return names
+	}
+
+	ptreeRefl := fieldNames(reflect.TypeOf(PackageTree{}))
+	packageRefl := fieldNames(reflect.TypeOf(Package{}))
+
+	if !reflect.DeepEqual(ptreeFields, ptreeRefl) {
+		t.Errorf("PackageTree.Copy is designed to work with an exact set of fields in the PackageTree struct - make sure it (and this test) have been updated!\n\t(GOT):%s\n\t(WNT):%s", ptreeFields, ptreeRefl)
+	}
+
+	if !reflect.DeepEqual(packageFields, packageRefl) {
+		t.Errorf("PackageTree.Copy is designed to work with an exact set of fields in the Package struct - make sure it (and this test) have been updated!\n\t(GOT):%s\n\t(WNT):%s", packageFields, packageRefl)
+	}
+}