From 4cc5290aace2c46f211a3ccbbd41afdce925fc8f Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 28 Mar 2024 12:37:22 -0700 Subject: [PATCH] Use maps and slices packages Use slices.Clone, slices.Contains, maps.Clone and maps.Equal. Test: go test ./... Change-Id: I96596f157dec2558007da4917c998a64d0cc4d79 --- bootstrap/bpdoc/properties.go | 5 +++-- context.go | 31 ++++++++----------------------- glob.go | 9 +++++---- ninja_strings_test.go | 3 ++- pathtools/fs_test.go | 5 +++-- proptools/escape.go | 7 ++++--- proptools/escape_test.go | 3 ++- proptools/extend.go | 3 ++- 8 files changed, 29 insertions(+), 37 deletions(-) diff --git a/bootstrap/bpdoc/properties.go b/bootstrap/bpdoc/properties.go index 31b93b1..29e6c56 100644 --- a/bootstrap/bpdoc/properties.go +++ b/bootstrap/bpdoc/properties.go @@ -20,6 +20,7 @@ import ( "go/doc" "html/template" "reflect" + "slices" "strconv" "strings" "unicode" @@ -34,7 +35,7 @@ import ( func (ps *PropertyStruct) Clone() *PropertyStruct { ret := *ps - ret.Properties = append([]Property(nil), ret.Properties...) + ret.Properties = slices.Clone(ret.Properties) for i, prop := range ret.Properties { ret.Properties[i] = prop.Clone() } @@ -44,7 +45,7 @@ func (ps *PropertyStruct) Clone() *PropertyStruct { func (p *Property) Clone() Property { ret := *p - ret.Properties = append([]Property(nil), ret.Properties...) + ret.Properties = slices.Clone(ret.Properties) for i, prop := range ret.Properties { ret.Properties[i] = prop.Clone() } diff --git a/context.go b/context.go index 6ec1641..a64250d 100644 --- a/context.go +++ b/context.go @@ -24,6 +24,7 @@ import ( "hash/fnv" "io" "io/ioutil" + "maps" "os" "path/filepath" "reflect" @@ -418,15 +419,7 @@ type Variation struct { type variationMap map[string]string func (vm variationMap) clone() variationMap { - if vm == nil { - return nil - } - newVm := make(variationMap) - for k, v := range vm { - newVm[k] = v - } - - return newVm + return maps.Clone(vm) } // Compare this variationMap to another one. Returns true if the every entry in this map @@ -441,10 +434,7 @@ func (vm variationMap) subsetOf(other variationMap) bool { } func (vm variationMap) equal(other variationMap) bool { - if len(vm) != len(other) { - return false - } - return vm.subsetOf(other) + return maps.Equal(vm, other) } type singletonInfo struct { @@ -800,15 +790,10 @@ type transitionMutatorImpl struct { // Adds each argument in items to l if it's not already there. func addToStringListIfNotPresent(l []string, items ...string) []string { -OUTER: for _, i := range items { - for _, existing := range l { - if existing == i { - continue OUTER - } + if !slices.Contains(l, i) { + l = append(l, i) } - - l = append(l, i) } return l @@ -1746,14 +1731,14 @@ func (c *Context) createVariations(origModule *moduleInfo, mutatorName string, m := *origModule newModule := &m - newModule.directDeps = append([]depInfo(nil), origModule.directDeps...) + newModule.directDeps = slices.Clone(origModule.directDeps) newModule.reverseDeps = nil newModule.forwardDeps = nil newModule.logicModule = newLogicModule newModule.variant = newVariant(origModule, mutatorName, variationName, local) newModule.properties = newProperties - newModule.providers = append([]interface{}(nil), origModule.providers...) - newModule.providerInitialValueHashes = append([]uint64(nil), origModule.providerInitialValueHashes...) + newModule.providers = slices.Clone(origModule.providers) + newModule.providerInitialValueHashes = slices.Clone(origModule.providerInitialValueHashes) newModules = append(newModules, newModule) diff --git a/glob.go b/glob.go index 91ae723..8d0748a 100644 --- a/glob.go +++ b/glob.go @@ -16,6 +16,7 @@ package blueprint import ( "fmt" + "slices" "sort" "strings" @@ -40,7 +41,7 @@ func verifyGlob(key globKey, pattern string, excludes []string, g pathtools.Glob func (c *Context) glob(pattern string, excludes []string) ([]string, error) { // Sort excludes so that two globs with the same excludes in a different order reuse the same // key. Make a copy first to avoid modifying the caller's version. - excludes = append([]string(nil), excludes...) + excludes = slices.Clone(excludes) sort.Strings(excludes) key := globToKey(pattern, excludes) @@ -54,7 +55,7 @@ func (c *Context) glob(pattern string, excludes []string) ([]string, error) { // Glob has already been done, double check it is identical verifyGlob(key, pattern, excludes, g) // Return a copy so that modifications don't affect the cached value. - return append([]string(nil), g.Matches...), nil + return slices.Clone(g.Matches), nil } // Get a globbed file list @@ -74,11 +75,11 @@ func (c *Context) glob(pattern string, excludes []string) ([]string, error) { // Getting the list raced with another goroutine, throw away the results and use theirs verifyGlob(key, pattern, excludes, g) // Return a copy so that modifications don't affect the cached value. - return append([]string(nil), g.Matches...), nil + return slices.Clone(g.Matches), nil } // Return a copy so that modifications don't affect the cached value. - return append([]string(nil), result.Matches...), nil + return slices.Clone(result.Matches), nil } func (c *Context) Globs() pathtools.MultipleGlobResults { diff --git a/ninja_strings_test.go b/ninja_strings_test.go index 50a80fa..2789b7a 100644 --- a/ninja_strings_test.go +++ b/ninja_strings_test.go @@ -16,6 +16,7 @@ package blueprint import ( "reflect" + "slices" "strconv" "strings" "testing" @@ -271,7 +272,7 @@ func Test_parseNinjaOrSimpleStrings(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - inCopy := append([]string(nil), tt.in...) + inCopy := slices.Clone(tt.in) scope := newLocalScope(nil, "") scope.AddLocalVariable("abc", "abc") diff --git a/pathtools/fs_test.go b/pathtools/fs_test.go index 3b4d4d0..8826c7c 100644 --- a/pathtools/fs_test.go +++ b/pathtools/fs_test.go @@ -18,6 +18,7 @@ import ( "os" "path/filepath" "reflect" + "slices" "syscall" "testing" ) @@ -233,7 +234,7 @@ func TestFs_ListDirsRecursiveFollowSymlinks(t *testing.T) { t.Run(test.name, func(t *testing.T) { got, err := fs.ListDirsRecursive(filepath.Join(dir, test.name), FollowSymlinks) checkErr(t, test.err, err) - want := append([]string(nil), test.dirs...) + want := slices.Clone(test.dirs) for i := range want { want[i] = filepath.Join(dir, want[i]) } @@ -279,7 +280,7 @@ func TestFs_ListDirsRecursiveDontFollowSymlinks(t *testing.T) { t.Run(test.name, func(t *testing.T) { got, err := fs.ListDirsRecursive(filepath.Join(dir, test.name), DontFollowSymlinks) checkErr(t, test.err, err) - want := append([]string(nil), test.dirs...) + want := slices.Clone(test.dirs) for i := range want { want[i] = filepath.Join(dir, want[i]) } diff --git a/proptools/escape.go b/proptools/escape.go index 9792444..8fcf82e 100644 --- a/proptools/escape.go +++ b/proptools/escape.go @@ -15,6 +15,7 @@ package proptools import ( + "slices" "strings" "unsafe" ) @@ -33,7 +34,7 @@ func NinjaEscapeList(slice []string) []string { if !sliceCopied { // If this was the first string that was modified by escaping then make a copy of the // input slice to use as the output slice. - slice = append([]string(nil), slice...) + slice = slices.Clone(slice) sliceCopied = true } slice[i] = escaped @@ -66,7 +67,7 @@ func ShellEscapeList(slice []string) []string { if !sliceCopied { // If this was the first string that was modified by escaping then make a copy of the // input slice to use as the output slice. - slice = append([]string(nil), slice...) + slice = slices.Clone(slice) sliceCopied = true } slice[i] = escaped @@ -83,7 +84,7 @@ func ShellEscapeListIncludingSpaces(slice []string) []string { if !sliceCopied { // If this was the first string that was modified by escaping then make a copy of the // input slice to use as the output slice. - slice = append([]string(nil), slice...) + slice = slices.Clone(slice) sliceCopied = true } slice[i] = escaped diff --git a/proptools/escape_test.go b/proptools/escape_test.go index 2937058..91c9e76 100644 --- a/proptools/escape_test.go +++ b/proptools/escape_test.go @@ -18,6 +18,7 @@ import ( "bytes" "os/exec" "reflect" + "slices" "testing" "unsafe" ) @@ -235,7 +236,7 @@ func TestNinjaEscapeList(t *testing.T) { t.Run(tf.name, func(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - inCopy := append([]string(nil), tt.in...) + inCopy := slices.Clone(tt.in) got := tf.f(tt.in) diff --git a/proptools/extend.go b/proptools/extend.go index eec5d43..af4e185 100644 --- a/proptools/extend.go +++ b/proptools/extend.go @@ -17,6 +17,7 @@ package proptools import ( "fmt" "reflect" + "slices" "strings" ) @@ -317,7 +318,7 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value // of destinations to consider. Make a copy of dstValues if necessary // to avoid modifying the backing array of an input parameter. if !dstValuesCopied { - dstValues = append([]reflect.Value(nil), dstValues...) + dstValues = slices.Clone(dstValues) dstValuesCopied = true } dstValues = append(dstValues, embeddedDstValue)