CBG-1966 enable staticcheck

.golangci-strict.yml

@@ -24,7 +24,7 @@ linters:
     #- nakedret # Finds naked returns in functions greater than a specified function length
     #- prealloc # Finds slice declarations that could potentially be preallocated
     #- revive # Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes
-    #- staticcheck # (megacheck) Staticcheck is a go vet on steroids, applying a ton of static analysis checks
+    - staticcheck # (megacheck) Staticcheck is a go vet on steroids, applying a ton of static analysis checks
     - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code
     - unconvert # Remove unnecessary type conversions
     #- unparam # Reports unused function parameters
@@ -67,7 +67,6 @@ linters:
     - nakedret # Finds naked returns in functions greater than a specified function length
     - prealloc # Finds slice declarations that could potentially be preallocated
     - revive # Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes
-    - staticcheck # (megacheck) Staticcheck is a go vet on steroids, applying a ton of static analysis checks
     - structcheck # Finds unused struct fields
     - unparam # Reports unused function parameters
     - varcheck # Finds unused global variables and constants

auth/auth_test.go

@@ -721,7 +721,7 @@ func TestConcurrentUserWrites(t *testing.T) {
 	}
 
 	// Retrieve user to trigger initial calculation of roles, channels
-	user, getErr := auth.GetUser(username)
+	_, getErr := auth.GetUser(username)
 	require.NoError(t, getErr, "Error retrieving user")
 
 	require.NoError(t, auth.SetBcryptCost(DefaultBcryptCost))

auth/oidc_test.go

@@ -1255,7 +1255,6 @@ func TestJWTRolesChannels(t *testing.T) {
 				require.Equal(t, base.SetFromArray(login.expectedChannels), user.Channels().AsSet())
 
 				require.Greater(t, user.JWTLastUpdated(), lastUpdateTime)
-				lastUpdateTime = user.JWTLastUpdated()
 			}
 		})
 	}

auth/password_hash_test.go

@@ -35,7 +35,6 @@ func BenchmarkBcryptCostTimes(b *testing.B) {
 
 	for i := minCostToTest; i < maxCostToTest; i++ {
 		b.Run(fmt.Sprintf("cost%d", i), func(bn *testing.B) {
-			bn.N = 1
 			_, err := bcrypt.GenerateFromPassword([]byte("hunter2"), i)
 			assert.NoError(bn, err)
 		})

base/collection.go

@@ -63,10 +63,6 @@ func GetGoCBv2Bucket(ctx context.Context, spec BucketSpec) (*GocbV2Bucket, error
 		RetryStrategy:  gocb.NewBestEffortRetryStrategy(nil),
 	}
 
-	if spec.KvPoolSize > 0 {
-		// TODO: Equivalent of kvPoolSize in gocb v2?
-	}
-
 	cluster, err := gocb.Connect(connString, clusterOptions)
 	if err != nil {
 		InfofCtx(ctx, KeyAuth, "Unable to connect to cluster: %v", err)

base/dcp_sharded.go

@@ -678,11 +678,11 @@ func (meh *sgMgrEventHandlers) OnUnregisterPIndex(pindex *cbgt.PIndex) {
 // OnFeedError is required to trigger reconnection to a feed on a closed connection (EOF).
 // NotifyMgrOnClose will trigger cbgt closing and then attempt to reconnect to the feed, if the manager hasn't
 // been stopped.
-func (meh *sgMgrEventHandlers) OnFeedError(srcType string, r cbgt.Feed, feedErr error) {
+func (meh *sgMgrEventHandlers) OnFeedError(_ string, r cbgt.Feed, feedErr error) {
 
 	// cbgt always passes srcType = SOURCE_GOCBCORE, but we have a wrapped type associated with our indexes - use that instead
 	// for our logging
-	srcType = SOURCE_DCP_SG
+	srcType := SOURCE_DCP_SG
 	var bucketName, bucketUUID string
 	dcpFeed, ok := r.(cbgt.FeedEx)
 	if ok {

base/leaky_bucket.go

@@ -358,8 +358,7 @@ func dedupeTapEvents(tapEvents []sgbucket.FeedEvent) []sgbucket.FeedEvent {
 	// sequence order as read off the feed.
 	deduped := []sgbucket.FeedEvent{}
 	for _, tapEvent := range tapEvents {
-		key := string(tapEvent.Key)
-		latestTapEventForKey := latestTapEventPerKey[key]
+		latestTapEventForKey := latestTapEventPerKey[string(tapEvent.Key)]
 		if tapEvent.Cas == latestTapEventForKey.Cas {
 			deduped = append(deduped, tapEvent)
 		}

base/logger_external.go

@@ -46,7 +46,8 @@ func initExternalLoggers() {
 }
 
 func updateExternalLoggers() {
-	if consoleLogger != nil && consoleLogger.shouldLog(nil, LevelDebug, KeyWalrus) {
+	// use context.Background() since this is called from init or to reset test logging
+	if consoleLogger != nil && consoleLogger.shouldLog(context.Background(), LevelDebug, KeyWalrus) {
 		rosmar.SetLogLevel(rosmar.LevelDebug)
 	} else {
 		rosmar.SetLogLevel(rosmar.LevelInfo)

base/logging_test.go

@@ -10,8 +10,8 @@ package base
 
 import (
 	"bytes"
+	"crypto/rand"
 	"fmt"
-	"math/rand"
 	"os"
 	"path/filepath"
 	"runtime"

base/util_testing.go

@@ -18,7 +18,6 @@ import (
 	"fmt"
 	"io"
 	"log"
-	"math/rand"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -42,12 +41,6 @@ import (
 
 var TestExternalRevStorage = false
 
-func init() {
-
-	// Prevent https://issues.couchbase.com/browse/MB-24237
-	rand.Seed(time.Now().UTC().UnixNano())
-}
-
 type TestBucket struct {
 	Bucket
 	BucketSpec BucketSpec

db/active_replicator_pull.go

@@ -40,10 +40,6 @@ func (apr *ActivePullReplicator) Start(ctx context.Context) error {
 	apr.lock.Lock()
 	defer apr.lock.Unlock()
 
-	if apr == nil {
-		return fmt.Errorf("nil ActivePullReplicator, can't start")
-	}
-
 	if apr.ctx != nil && apr.ctx.Err() == nil {
 		return fmt.Errorf("ActivePullReplicator already running")
 	}
@@ -158,10 +154,6 @@ func (apr *ActivePullReplicator) _subChanges(collectionIdx *int, since string) e
 func (apr *ActivePullReplicator) Complete() {
 	base.TracefCtx(apr.ctx, base.KeyReplicate, "ActivePullReplicator.Complete()")
 	apr.lock.Lock()
-	if apr == nil {
-		apr.lock.Unlock()
-		return
-	}
 	_ = apr.forEachCollection(func(c *activeReplicatorCollection) error {
 		base.TracefCtx(apr.ctx, base.KeyReplicate, "Before calling waitForExpectedSequences in Complete()")
 		if err := c.Checkpointer.waitForExpectedSequences(); err != nil {

db/active_replicator_push.go

@@ -44,10 +44,6 @@ func (apr *ActivePushReplicator) Start(ctx context.Context) error {
 	apr.lock.Lock()
 	defer apr.lock.Unlock()
 
-	if apr == nil {
-		return fmt.Errorf("nil ActivePushReplicator, can't start")
-	}
-
 	if apr.ctx != nil && apr.ctx.Err() == nil {
 		return fmt.Errorf("ActivePushReplicator already running")
 	}
@@ -111,10 +107,6 @@ func (apr *ActivePushReplicator) _connect() error {
 func (apr *ActivePushReplicator) Complete() {
 	base.TracefCtx(apr.ctx, base.KeyReplicate, "ActivePushReplicator.Complete()")
 	apr.lock.Lock()
-	if apr == nil {
-		apr.lock.Unlock()
-		return
-	}
 
 	// Wait for any pending changes responses to arrive and be processed
 	err := apr._waitForPendingChangesResponse()

db/attachment_test.go

@@ -10,12 +10,12 @@ package db
 
 import (
 	"context"
+	"crypto/rand"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"log"
-	"math/rand"
 	"net/http"
 	"strconv"
 	"strings"

db/crud.go

@@ -413,10 +413,10 @@ func (db *DatabaseCollectionWithUser) GetDelta(ctx context.Context, docID, fromR
 			// db.DbStats.StatsDeltaSync().Add(base.StatKeyDeltaCacheHits, 1)
 			db.dbStats().DeltaSync().DeltaCacheHit.Add(1)
 			return fromRevision.Delta, nil, nil
-		} else {
-			// TODO: Recurse and merge deltas when gen(revCacheDelta.toRevID) < gen(toRevId)
-			// until then, fall through to generating delta for given rev pair
-		}
+		} // else {
+		// TODO: Recurse and merge deltas when gen(revCacheDelta.toRevID) < gen(toRevId)
+		// until then, fall through to generating delta for given rev pair
+		//	}
 	}
 
 	// Delta is unavailable, but the body is available.

db/crud_test.go

@@ -1360,6 +1360,7 @@ func TestGet1xRevFromDoc(t *testing.T) {
 	// Get the document body bytes with the tombstone revision rev3, with listRevisions=true
 	// Also validate that the BodyRevisions property is present and correct.
 	bodyBytes, removed, err = collection.get1xRevFromDoc(ctx, doc, rev3, true)
+	require.NoError(t, err)
 	assert.NotEmpty(t, bodyBytes, "Document body bytes should be returned")
 	assert.False(t, removed, "This shouldn't be a removed document")
 	assert.NoError(t, response.Unmarshal(bodyBytes))

db/database_test.go

@@ -3063,8 +3063,8 @@ func TestGetDatabaseCollectionWithUserDefaultCollection(t *testing.T) {
 			require.NoError(t, err)
 
 			db, err := GetDatabase(dbCtx, nil)
-			defer db.Close(ctx)
 			require.NoError(t, err)
+			defer db.Close(ctx)
 			col, err := db.GetDatabaseCollectionWithUser(testCase.scope, testCase.collection)
 			if testCase.err {
 				require.Error(t, err)

db/sg_replicate_cfg.go

@@ -803,10 +803,9 @@ func (m *sgReplicateManager) RefreshReplicationCfg(ctx context.Context) error {
 	// Check for replications newly assigned to this node
 	for replicationID, replicationCfg := range configReplications {
 		if replicationCfg.AssignedNode == m.localNodeUUID {
-			replicator, exists := m.activeReplicators[replicationID]
+			_, exists := m.activeReplicators[replicationID]
 			if !exists {
-				var initError error
-				replicator, initError = m.InitializeReplication(replicationCfg)
+				replicator, initError := m.InitializeReplication(replicationCfg)
 				if initError != nil {
 					base.WarnfCtx(m.loggingCtx, "Error initializing replication %s: %v", initError)
 					continue

db/sg_replicate_cfg_test.go

@@ -43,7 +43,7 @@ func TestReplicateManagerReplications(t *testing.T) {
 	assert.Equal(t, replication1_id, r.ID)
 
 	// Request non-existent replication
-	r, err = manager.GetReplication("dne")
+	_, err = manager.GetReplication("dne")
 	require.Error(t, err, base.ErrNotFound)
 
 	// Attempt to add existing replication

main.go

@@ -9,16 +9,9 @@
 package main
 
 import (
-	"math/rand"
-	"time"
-
 	"github.com/couchbase/sync_gateway/rest"
 )
 
-func init() {
-	rand.Seed(time.Now().UTC().UnixNano())
-}
-
 // Simple Sync Gateway launcher tool.
 func main() {
 	rest.ServerMain()

rest/api_test.go

@@ -1700,8 +1700,8 @@ func TestLongpollWithWildcard(t *testing.T) {
 	// has a wait counter of zero (no documents writted since the listener was restarted).
 	wg := sync.WaitGroup{}
 	// start changes request
+	wg.Add(1)
 	go func() {
-		wg.Add(1)
 		defer wg.Done()
 		changesJSON := `{"style":"all_docs", "heartbeat":300000, "feed":"longpoll", "limit":50, "since":"0"}`
 		changesResponse := rt.SendUserRequest("POST", "/{{.keyspace}}/_changes", changesJSON, "bernard")

rest/bulk_api.go

@@ -264,9 +264,7 @@ func (h *handler) handleDump() error {
 func (h *handler) handleRepair() error {
 
 	// TODO: If repair is re-enabled, it may need to be modified to support xattrs and GSI
-	if true == true {
-		return errors.New("_repair endpoint disabled")
-	}
+	return errors.New("_repair endpoint disabled")
 
 	/*base.InfofCtx(h.ctx(), base.KeyHTTP, "Repair bucket")
 
@@ -305,8 +303,6 @@ func (h *handler) handleRepair() error {
 	return err
 
 	*/
-
-	return nil
 }
 
 // HTTP handler for _dumpchannel

rest/config_test.go

@@ -1431,8 +1431,8 @@ func TestSetupServerContext(t *testing.T) {
 		config.Bootstrap.Password = base.TestClusterPassword()
 		ctx := base.TestCtx(t)
 		sc, err := SetupServerContext(ctx, &config, false)
-		defer sc.Close(ctx)
 		require.NoError(t, err)
+		defer sc.Close(ctx)
 		require.NotNil(t, sc)
 	})
 }

rest/multipart_test.go

@@ -13,10 +13,10 @@ package rest
 import (
 	"bytes"
 	"compress/gzip"
+	"crypto/rand"
 	"fmt"
 	"io"
 	"log"
-	"math/rand"
 	"mime/multipart"
 	"net/http"
 	"strconv"
@@ -26,6 +26,7 @@ import (
 	"github.com/couchbase/sync_gateway/base"
 	"github.com/couchbase/sync_gateway/db"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestWriteMultipartDocument(t *testing.T) {
@@ -163,7 +164,8 @@ func TestWriteJSONPart(t *testing.T) {
 	// a body larger than 300 bytes to test writeJSONPart with compression=true and compression=false
 	mockFakeBody := func() db.Body {
 		bytes := make([]byte, 139)
-		rand.Read(bytes)
+		_, err := rand.Read(bytes)
+		require.NoError(t, err)
 		value := fmt.Sprintf("%x", bytes)
 		return db.Body{"key": "foo", "value": value}
 	}