Skip to content

Commit

Permalink
Fix StrListDelete side effect. (#118)
Browse files Browse the repository at this point in the history
  • Loading branch information
ncabatoff committed Feb 9, 2024
1 parent b4a7f3e commit 4321d38
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
16 changes: 10 additions & 6 deletions strutil/strutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ func ParseKeyValues(input string, out map[string]string, sep string) error {

// ParseArbitraryKeyValues parses arbitrary <key,value> tuples. The input
// can be one of the following:
// * JSON string
// * Base64 encoded JSON string
// * Comma separated list of `<key>=<value>` pairs
// * Base64 encoded string containing comma separated list of
// `<key>=<value>` pairs
// - JSON string
// - Base64 encoded JSON string
// - Comma separated list of `<key>=<value>` pairs
// - Base64 encoded string containing comma separated list of
// `<key>=<value>` pairs
//
// Input will be parsed into the output parameter, which should
// be a non-nil map[string]string.
Expand Down Expand Up @@ -367,7 +367,11 @@ func StrListDelete(s []string, d string) []string {

for index, element := range s {
if element == d {
return append(s[:index], s[index+1:]...)
// Using the provided slice as the basis for the return value can
// result in confusing behaviour, see https://go.dev/play/p/EAtNw4lLugu
c := make([]string, len(s))
copy(c, s)
return append(c[:index], c[index+1:]...)
}
}

Expand Down
7 changes: 7 additions & 0 deletions strutil/strutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ func TestStrListDelete(t *testing.T) {
if StrListContains(output, "item3") {
t.Fatal("bad: 'item3' should not have been present")
}

// Test that StrListDelete doesn't modify its input
input := []string{"a", "b", "c"}
output = StrListDelete(input, "b")
if !reflect.DeepEqual(input, []string{"a", "b", "c"}) {
t.Fatal("bad: input modified")
}
}

func TestEquivalentSlices(t *testing.T) {
Expand Down

0 comments on commit 4321d38

Please sign in to comment.