chore: Enable golangci-lint and fix errors (#554)

Enable golangci-lint checking in travis and fix errors identified by it.

This mainly involves removing dead code and improving error checking
such as:
* activeConn.Close now returns any error from operations it performs
  instead of always returning nil.

.github/workflows/golangci-lint.yml

@@ -0,0 +1,31 @@
+name: golangci-lint
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+  pull_request:
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v2
+        with:
+          # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
+          version: v1.36
+
+          # Optional: working directory, useful for monorepos
+          # working-directory: somedir
+
+          # Optional: golangci-lint command line arguments.
+          # args: --issues-exit-code=0
+
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          # only-new-issues: true
+
+          # Optional: if set to true then the action will use pre-installed Go.
+          # skip-go-installation: true

redis/conn.go

@@ -432,15 +432,23 @@ func (c *conn) writeLen(prefix byte, n int) error {
 }
 
 func (c *conn) writeString(s string) error {
-	c.writeLen('$', len(s))
-	c.bw.WriteString(s)
+	if err := c.writeLen('$', len(s)); err != nil {
+		return err
+	}
+	if _, err := c.bw.WriteString(s); err != nil {
+		return err
+	}
 	_, err := c.bw.WriteString("\r\n")
 	return err
 }
 
 func (c *conn) writeBytes(p []byte) error {
-	c.writeLen('$', len(p))
-	c.bw.Write(p)
+	if err := c.writeLen('$', len(p)); err != nil {
+		return err
+	}
+	if _, err := c.bw.Write(p); err != nil {
+		return err
+	}
 	_, err := c.bw.WriteString("\r\n")
 	return err
 }
@@ -454,7 +462,9 @@ func (c *conn) writeFloat64(n float64) error {
 }
 
 func (c *conn) writeCommand(cmd string, args []interface{}) error {
-	c.writeLen('*', 1+len(args))
+	if err := c.writeLen('*', 1+len(args)); err != nil {
+		return err
+	}
 	if err := c.writeString(cmd); err != nil {
 		return err
 	}
@@ -656,7 +666,9 @@ func (c *conn) Send(cmd string, args ...interface{}) error {
 	c.pending += 1
 	c.mu.Unlock()
 	if c.writeTimeout != 0 {
-		c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout))
+		if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)); err != nil {
+			return c.fatal(err)
+		}
 	}
 	if err := c.writeCommand(cmd, args); err != nil {
 		return c.fatal(err)
@@ -666,7 +678,9 @@ func (c *conn) Send(cmd string, args ...interface{}) error {
 
 func (c *conn) Flush() error {
 	if c.writeTimeout != 0 {
-		c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout))
+		if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)); err != nil {
+			return c.fatal(err)
+		}
 	}
 	if err := c.bw.Flush(); err != nil {
 		return c.fatal(err)
@@ -683,7 +697,9 @@ func (c *conn) ReceiveWithTimeout(timeout time.Duration) (reply interface{}, err
 	if timeout != 0 {
 		deadline = time.Now().Add(timeout)
 	}
-	c.conn.SetReadDeadline(deadline)
+	if err := c.conn.SetReadDeadline(deadline); err != nil {
+		return nil, c.fatal(err)
+	}
 
 	if reply, err = c.readReply(); err != nil {
 		return nil, c.fatal(err)
@@ -721,7 +737,9 @@ func (c *conn) DoWithTimeout(readTimeout time.Duration, cmd string, args ...inte
 	}
 
 	if c.writeTimeout != 0 {
-		c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout))
+		if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeTimeout)); err != nil {
+			return nil, c.fatal(err)
+		}
 	}
 
 	if cmd != "" {
@@ -738,7 +756,9 @@ func (c *conn) DoWithTimeout(readTimeout time.Duration, cmd string, args ...inte
 	if readTimeout != 0 {
 		deadline = time.Now().Add(readTimeout)
 	}
-	c.conn.SetReadDeadline(deadline)
+	if err := c.conn.SetReadDeadline(deadline); err != nil {
+		return nil, c.fatal(err)
+	}
 
 	if cmd == "" {
 		reply := make([]interface{}, pending)

redis/conn_test.go

@@ -30,6 +30,7 @@ import (
 	"time"
 
 	"github.com/gomodule/redigo/redis"
+	"github.com/stretchr/testify/require"
 )
 
 type testConn struct {
@@ -71,10 +72,10 @@ func dialTestConnTLS(r string, w io.Writer) redis.DialOption {
 	return redis.DialNetDial(func(network, addr string) (net.Conn, error) {
 		client, server := net.Pipe()
 		tlsServer := tls.Server(server, &serverTLSConfig)
-		go io.Copy(tlsServer, strings.NewReader(r))
+		go io.Copy(tlsServer, strings.NewReader(r)) // nolint: errcheck
 		done := make(chan struct{})
 		go func() {
-			io.Copy(w, tlsServer)
+			io.Copy(w, tlsServer) // nolint: errcheck
 			close(done)
 		}()
 		return &tlsTestConn{Conn: client, done: done}, nil
@@ -442,12 +443,12 @@ func TestRecvBeforeSend(t *testing.T) {
 	defer c.Close()
 	done := make(chan struct{})
 	go func() {
-		c.Receive()
+		c.Receive() // nolint: errcheck
 		close(done)
 	}()
 	time.Sleep(time.Millisecond)
-	c.Send("PING")
-	c.Flush()
+	require.NoError(t, c.Send("PING"))
+	require.NoError(t, c.Flush())
 	<-done
 	_, err = c.Do("")
 	if err != nil {
@@ -462,7 +463,8 @@ func TestError(t *testing.T) {
 	}
 	defer c.Close()
 
-	c.Do("SET", "key", "val")
+	_, err = c.Do("SET", "key", "val")
+	require.NoError(t, err)
 	_, err = c.Do("HSET", "key", "fld", "val")
 	if err == nil {
 		t.Errorf("Expected err for HSET on string key.")
@@ -491,7 +493,8 @@ func TestReadTimeout(t *testing.T) {
 			}
 			go func() {
 				time.Sleep(time.Second)
-				c.Write([]byte("+OK\r\n"))
+				_, err := c.Write([]byte("+OK\r\n"))
+				require.NoError(t, err)
 				c.Close()
 			}()
 		}
@@ -521,8 +524,8 @@ func TestReadTimeout(t *testing.T) {
 	}
 	defer c2.Close()
 
-	c2.Send("PING")
-	c2.Flush()
+	require.NoError(t, c2.Send("PING"))
+	require.NoError(t, c2.Flush())
 	_, err = c2.Receive()
 	if err == nil {
 		t.Fatalf("c2.Receive() returned nil, expect error")
@@ -859,11 +862,13 @@ func TestExecError(t *testing.T) {
 
 	// Execute commands that fail before EXEC is called.
 
-	c.Do("DEL", "k0")
-	c.Do("ZADD", "k0", 0, 0)
-	c.Send("MULTI")
-	c.Send("NOTACOMMAND", "k0", 0, 0)
-	c.Send("ZINCRBY", "k0", 0, 0)
+	_, err = c.Do("DEL", "k0")
+	require.NoError(t, err)
+	_, err = c.Do("ZADD", "k0", 0, 0)
+	require.NoError(t, err)
+	require.NoError(t, c.Send("MULTI"))
+	require.NoError(t, c.Send("NOTACOMMAND", "k0", 0, 0))
+	require.NoError(t, c.Send("ZINCRBY", "k0", 0, 0))
 	v, err := c.Do("EXEC")
 	if err == nil {
 		t.Fatalf("EXEC returned values %v, expected error", v)
@@ -872,11 +877,13 @@ func TestExecError(t *testing.T) {
 	// Execute commands that fail after EXEC is called. The first command
 	// returns an error.
 
-	c.Do("DEL", "k1")
-	c.Do("ZADD", "k1", 0, 0)
-	c.Send("MULTI")
-	c.Send("HSET", "k1", 0, 0)
-	c.Send("ZINCRBY", "k1", 0, 0)
+	_, err = c.Do("DEL", "k1")
+	require.NoError(t, err)
+	_, err = c.Do("ZADD", "k1", 0, 0)
+	require.NoError(t, err)
+	require.NoError(t, c.Send("MULTI"))
+	require.NoError(t, c.Send("HSET", "k1", 0, 0))
+	require.NoError(t, c.Send("ZINCRBY", "k1", 0, 0))
 	v, err = c.Do("EXEC")
 	if err != nil {
 		t.Fatalf("EXEC returned error %v", err)
@@ -902,10 +909,11 @@ func TestExecError(t *testing.T) {
 	// Execute commands that fail after EXEC is called. The second command
 	// returns an error.
 
-	c.Do("ZADD", "k2", 0, 0)
-	c.Send("MULTI")
-	c.Send("ZINCRBY", "k2", 0, 0)
-	c.Send("HSET", "k2", 0, 0)
+	_, err = c.Do("ZADD", "k2", 0, 0)
+	require.NoError(t, err)
+	require.NoError(t, c.Send("MULTI"))
+	require.NoError(t, c.Send("ZINCRBY", "k2", 0, 0))
+	require.NoError(t, c.Send("HSET", "k2", 0, 0))
 	v, err = c.Do("EXEC")
 	if err != nil {
 		t.Fatalf("EXEC returned error %v", err)
@@ -1030,26 +1038,29 @@ func TestWithTimeout(t *testing.T) {
 				var minDeadline, maxDeadline time.Time
 
 				// Alternate between default and specified timeout.
+				var err error
 				if i%2 == 0 {
 					if defaultTimout != 0 {
 						minDeadline = time.Now().Add(defaultTimout)
 					}
 					if recv {
-						c.Receive()
+						_, err = c.Receive()
 					} else {
-						c.Do("PING")
+						_, err = c.Do("PING")
 					}
+					require.NoError(t, err)
 					if defaultTimout != 0 {
 						maxDeadline = time.Now().Add(defaultTimout)
 					}
 				} else {
 					timeout := 10 * time.Minute
 					minDeadline = time.Now().Add(timeout)
 					if recv {
-						redis.ReceiveWithTimeout(c, timeout)
+						_, err = redis.ReceiveWithTimeout(c, timeout)
 					} else {
-						redis.DoWithTimeout(c, timeout, "PING")
+						_, err = redis.DoWithTimeout(c, timeout, "PING")
 					}
+					require.NoError(t, err)
 					maxDeadline = time.Now().Add(timeout)
 				}
 

redis/log.go

@@ -52,7 +52,7 @@ func (c *loggingConn) Close() error {
 	err := c.Conn.Close()
 	var buf bytes.Buffer
 	fmt.Fprintf(&buf, "%sClose() -> (%v)", c.prefix, err)
-	c.logger.Output(2, buf.String())
+	c.logger.Output(2, buf.String()) // nolint: errcheck
 	return err
 }
 
@@ -112,7 +112,7 @@ func (c *loggingConn) print(method, commandName string, args []interface{}, repl
 		buf.WriteString(", ")
 	}
 	fmt.Fprintf(&buf, "%v)", err)
-	c.logger.Output(3, buf.String())
+	c.logger.Output(3, buf.String()) // nolint: errcheck
 }
 
 func (c *loggingConn) Do(commandName string, args ...interface{}) (interface{}, error) {