diff --git a/internal/gps/pkgtree/pkgtree.go b/internal/gps/pkgtree/pkgtree.go index 37239818..2ad9215b 100644 --- a/internal/gps/pkgtree/pkgtree.go +++ b/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 diff --git a/internal/gps/pkgtree/pkgtree_test.go b/internal/gps/pkgtree/pkgtree_test.go index 30860329..d70cab75 100644 --- a/internal/gps/pkgtree/pkgtree_test.go +++ b/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) + } +}