Improve Argument implementation (#281)

- Encode result of RedisArg() using same conversion as other arguments. This
  change makes []byte return values useful.
- Improve documentation.

redis/conn.go

@@ -346,43 +346,55 @@ func (c *conn) writeFloat64(n float64) error {
 	return c.writeBytes(strconv.AppendFloat(c.numScratch[:0], n, 'g', -1, 64))
 }
 
-func (c *conn) writeCommand(cmd string, args []interface{}) (err error) {
+func (c *conn) writeCommand(cmd string, args []interface{}) error {
 	c.writeLen('*', 1+len(args))
-	err = c.writeString(cmd)
+	if err := c.writeString(cmd); err != nil {
+		return err
+	}
 	for _, arg := range args {
-		if err != nil {
-			break
+		if err := c.writeArg(arg, true); err != nil {
+			return err
 		}
-		switch arg := arg.(type) {
-		case string:
-			err = c.writeString(arg)
-		case []byte:
-			err = c.writeBytes(arg)
-		case int:
-			err = c.writeInt64(int64(arg))
-		case int64:
-			err = c.writeInt64(arg)
-		case float64:
-			err = c.writeFloat64(arg)
-		case bool:
-			if arg {
-				err = c.writeString("1")
-			} else {
-				err = c.writeString("0")
-			}
-		case nil:
-			err = c.writeString("")
-		case Argument:
-			var buf bytes.Buffer
-			fmt.Fprint(&buf, arg.RedisArg())
-			err = c.writeBytes(buf.Bytes())
-		default:
-			var buf bytes.Buffer
-			fmt.Fprint(&buf, arg)
-			err = c.writeBytes(buf.Bytes())
+	}
+	return nil
+}
+
+func (c *conn) writeArg(arg interface{}, argumentTypeOK bool) (err error) {
+	switch arg := arg.(type) {
+	case string:
+		return c.writeString(arg)
+	case []byte:
+		return c.writeBytes(arg)
+	case int:
+		return c.writeInt64(int64(arg))
+	case int64:
+		return c.writeInt64(arg)
+	case float64:
+		return c.writeFloat64(arg)
+	case bool:
+		if arg {
+			return c.writeString("1")
+		} else {
+			return c.writeString("0")
 		}
+	case nil:
+		return c.writeString("")
+	case Argument:
+		if argumentTypeOK {
+			return c.writeArg(arg.RedisArg(), false)
+		}
+		// See comment in default clause below.
+		var buf bytes.Buffer
+		fmt.Fprint(&buf, arg)
+		return c.writeBytes(buf.Bytes())
+	default:
+		// This default clause is intended to handle builtin numeric types.
+		// The function should return an error for other types, but this is not
+		// done for compatibility with previous versions of the package.
+		var buf bytes.Buffer
+		fmt.Fprint(&buf, arg)
+		return c.writeBytes(buf.Bytes())
 	}
-	return err
 }
 
 type protocolError string

redis/conn_test.go

@@ -82,6 +82,10 @@ func (t durationArg) RedisArg() interface{} {
 	return t.Seconds()
 }
 
+type recursiveArg int
+
+func (v recursiveArg) RedisArg() interface{} { return v }
+
 var writeTests = []struct {
 	args     []interface{}
 	expected string
@@ -122,6 +126,10 @@ var writeTests = []struct {
 		[]interface{}{"SET", "key", durationArg{time.Minute}},
 		"*3\r\n$3\r\nSET\r\n$3\r\nkey\r\n$2\r\n60\r\n",
 	},
+	{
+		[]interface{}{"SET", "key", recursiveArg(123)},
+		"*3\r\n$3\r\nSET\r\n$3\r\nkey\r\n$3\r\n123\r\n",
+	},
 	{
 		[]interface{}{"ECHO", true, false},
 		"*3\r\n$4\r\nECHO\r\n$1\r\n1\r\n$1\r\n0\r\n",

redis/doc.go

@@ -48,7 +48,7 @@
 //  float64                 strconv.FormatFloat(v, 'g', -1, 64)
 //  bool                    true -> "1", false -> "0"
 //  nil                     ""
-//  all other types         fmt.Print(v)
+//  all other types         fmt.Fprint(w, v)
 //
 // Redis command reply types are represented using the following Go types:
 //

redis/redis.go

@@ -40,18 +40,20 @@ type Conn interface {
 	Receive() (reply interface{}, err error)
 }
 
-// Argument is implemented by types which want to control how their value is
-// interpreted when used as an argument to a redis command.
+// Argument is the interface implemented by an object which wants to control how
+// the object is converted to Redis bulk strings.
 type Argument interface {
-	// RedisArg returns the interface that represents the value to be used
-	// in redis commands.
+	// RedisArg returns a value to be encoded as a bulk string per the
+	// conversions listed in the section 'Executing Commands'.
+	// Implementations should typically return a []byte or string.
 	RedisArg() interface{}
 }
 
-// Scanner is implemented by types which want to control how their value is
-// interpreted when read from redis.
+// Scanner is implemented by an object which wants to control it's value is
+// interpreted when read from Redis.
 type Scanner interface {
-	// RedisScan assigns a value from a redis value.
+	// RedisScan assigns a value from a Redis value. The argument src is one of
+	// the reply types listed in the section `Executing Commands`.
 	//
 	// An error should be returned if the value cannot be stored without
 	// loss of information.