go-redis icon indicating copy to clipboard operation
go-redis copied to clipboard

Use `...string` instead of `...interface{}` as values parameter type for Push methods

Open kenzoi opened this issue 2 years ago • 1 comments

Redis.client RPush, LPush... methods are expecting ...interface{} values which may be too open and sometimes accepts arguments that could result in runtime errors.

It's true that now is accepting either multiple strings as RPush(ctx, 'key', 'v1', 'v2'...) or a slice of strings as RPush(ctx, 'key', []string{'v1', 'v2'}), but I think that unifying to one signature (either []string or ...string ) would bring more API safety.

Expected Behavior

  • I think we should be able to explode a slice of strings as argument since it's a variadic function.
  • It should not accept invalid combination of arguments.

Current Behavior

  • Not possible to explode a slice of strings as argument (but can pass a (one) slice of strings directly)
  • It accepts two or more slices of strings, but results in a error during runtime

Possible Solution

Use ...string instead of ...interface{} as values parameter type on Push methods.

Steps to Reproduce

  • If we try to explode a slice of strings as argument:
	slice := []string{"s1", "s2", "s3"}

	rdb.RPush(context.TODO(), "list", slice...)

We receive a IncompatibleAssign: cannot use slice (variable of type []string) as []interface{} value in argument to RPush

  • Function signature doesn't complain if we pass two or more []string:
	slice := []string{"s1", "s2", "s3"}

	result, err := rdb.RPush(context.TODO(), "list", slice, slice).Result()

But at runtime we got an error: redis: can't marshal []string (implement encoding.BinaryMarshaler)

Context (Environment)

Pushing multiple items to list once.

kenzoi avatar Jul 26 '23 11:07 kenzoi