bytes: ReplaceAll returns nil slice when empty slice is given

[KaitoHH]

What version of Go are you using (go version)?

$ go version
go version go1.14.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

package main

import (
	"bytes"
	"fmt"
)

func main() {
	old := []byte("")
	b := bytes.ReplaceAll(old, []byte("oink"), []byte("moo"))
	fmt.Println(old == nil)
	fmt.Println(b == nil)
}

What did you expect to see?

false
true

the call of bytes.ReplaceAll on an empty slice should also return an empty slice. However, it returns nil

What did you see instead?

false
false

[mvdan]

Can I ask why you want them to be equally nil or not nil? As far as I know, that's not a guarantee of any of Go's APIs by default.

[KaitoHH]

Can I ask why you want them to be equally nil or not nil? As far as I know, that's not a guarantee of any of Go's APIs by default.

The inconsistent type of slice (empty slice or nil slice) looks confusing. To me: Empty slice in, empty slice out. Nil slice in, nil slice out.

[mknyszek]

FWIW, I think generally the pattern here is to check length instead of nil-ness. The length of a nil slice is also zero.

CC @ianlancetaylor via https://dev.golang.org/owners

[ianlancetaylor]

The bytes package makes no promise to preserve the nil status of an argument slice. The general guideline of Go style is to treat nil slices and empty slices the same, which means to always use len(s) == 0 rather than s == nil. While there are packages that intentionally distinguish an empty slice and a nil slice, those packages are always clearly documented. The bytes package is not one of them.

[novns]

Please fix this issue.

Using bytes.ReplaceAll corrupts empty slices when they are used as arguments of database queries.
It turns empty string arguments '' to NULL values.

Currently additional checks are required for special cases of empty slices and a lot of unnecessary debugging.

[novns]

The documentation clearly says "ReplaceAll returns a copy of the slice..."

If the slice is empty it should return the empty slice as it was exactly before without any changes.
Because it breaks things otherwise, and introduces inconsistency.

[ianlancetaylor]

The byte.Replace function has worked as it does today since the Go 1.0 release, and the bytes.ReplaceAll function has worked this way since it was introduced in the Go 1.12 release. The behavior is not buggy and it matches the documentation. We aren't going to change it now. Sorry.