GOCBC-902: Improve linting abilities

Motivation
----------
We've been using the same linting for quite some time now and IDEs
are beginning to flag up issues that our CI isn't. We should improve
our linting.

Changes
-------
Replace all existing linting with golangci-lint which covers the
linters we were using and many more.
Pick a sensible set of linters for now, allowing us to add more in
the future.
Make various code fixes to satisfy the linters.

Change-Id: Ic74071f24975c9ba756713051647e6355c779ae6
Reviewed-on: http://review.couchbase.org/c/gocb/+/129730
Reviewed-by: Brett Lawson <brett19@gmail.com>
Tested-by: Charles Dixon <chvckd@gmail.com>

.golangci.yml

@@ -0,0 +1,18 @@
+run:
+  modules-download-mode: readonly
+  tests: false
+  skip-files:
+    - logging.go  # Logging has some utility functions that are useful to have around which get flagged up
+linters:
+  enable:
+    - bodyclose
+    - golint
+    - gosec
+    - unconvert
+linters-settings:
+  golint:
+    set-exit-status: true
+    min-confidence: 0.81
+  errcheck:
+    check-type-assertions: true
+    check-blank: true

Makefile

@@ -1,8 +1,5 @@
 devsetup:
-	go get "github.com/kisielk/errcheck"
-	go get "golang.org/x/lint/golint"
-	go get "github.com/gordonklaus/ineffassign"
-	go get "github.com/client9/misspell/cmd/misspell"
+	go get github.com/golangci/golangci-lint/cmd/golangci-lint
 	go get github.com/vektra/mockery/.../
 	git submodule update --remote --init --recursive
 
@@ -14,23 +11,8 @@ fasttest:
 cover:
 	go test -coverprofile=cover.out ./
 
-checkerrs:
-	errcheck -blank -asserts -ignoretests *.go
-
-checkfmt:
-	! gofmt -l -d *.go 2>&1 | read
-
-checkvet:
-	go vet *.go
-
-checkiea:
-	ineffassign -n .
-
-checkspell:
-	misspell -error *.*
-
-lint: checkfmt checkerrs checkvet checkiea checkspell
-	golint -set_exit_status -min_confidence 0.81 *.go
+lint:
+	golangci-lint run -v
 
 check: lint
 	go test -short -cover -race ./
@@ -54,4 +36,4 @@ updatemocks:
 	mockery -name waitUntilReadyProvider -output . -testonly -inpkg
 	# pendingOp is manually mocked
 
-.PHONY: all test devsetup fasttest lint cover checkerrs checkfmt checkvet checkiea checkspell check bench updatetestcases updatemocks
+.PHONY: all test devsetup fasttest lint cover check bench updatetestcases updatemocks

auth.go

@@ -107,19 +107,6 @@ func (ca CertificateAuthenticator) Credentials(req AuthCredsRequest) ([]UserPass
 	}}, nil
 }
 
-func getSingleCredential(auth Authenticator, req AuthCredsRequest) (UserPassPair, error) {
-	creds, err := auth.Credentials(req)
-	if err != nil {
-		return UserPassPair{}, err
-	}
-
-	if len(creds) != 1 {
-		return UserPassPair{}, gocbcore.ErrInvalidCredentials
-	}
-
-	return creds[0], nil
-}
-
 type coreAuthWrapper struct {
 	auth Authenticator
 }

cluster_analyticsindexes.go

@@ -566,7 +566,7 @@ func (am *AnalyticsIndexManager) GetPendingMutations(opts *GetPendingMutationsAn
 	req := mgmtRequest{
 		Service:       ServiceTypeAnalytics,
 		Method:        "GET",
-		Path:          fmt.Sprintf("/analytics/node/agg/stats/remaining"),
+		Path:          "/analytics/node/agg/stats/remaining",
 		IsIdempotent:  true,
 		RetryStrategy: opts.RetryStrategy,
 		Timeout:       timeout,

cluster_analyticsquery.go

@@ -90,8 +90,6 @@ type AnalyticsMetaData struct {
 	Metrics         AnalyticsMetrics
 	Signature       interface{}
 	Warnings        []AnalyticsWarning
-
-	preparedName string
 }
 
 func (meta *AnalyticsMetaData) fromData(data jsonAnalyticsResponse) error {

cluster_diag.go

@@ -63,7 +63,7 @@ func (report *DiagnosticsResult) MarshalJSON() ([]byte, error) {
 
 			jsonReport.Services[serviceStr] = append(jsonReport.Services[serviceStr], jsonDiagnosticEntry{
 				ID:             service.ID,
-				LastActivityUs: uint64(time.Now().Sub(service.LastActivity).Nanoseconds()),
+				LastActivityUs: uint64(time.Since(service.LastActivity).Nanoseconds()),
 				Remote:         service.Remote,
 				Local:          service.Local,
 				State:          stateStr,

cluster_query.go

@@ -7,12 +7,6 @@ import (
 	gocbcore "github.com/couchbase/gocbcore/v9"
 )
 
-type queryCacheEntry struct {
-	enhanced    bool
-	name        string
-	encodedPlan string
-}
-
 type jsonQueryMetrics struct {
 	ElapsedTime   string `json:"elapsedTime"`
 	ExecutionTime string `json:"executionTime"`
@@ -40,11 +34,6 @@ type jsonQueryResponse struct {
 	Prepared        string             `json:"prepared"`
 }
 
-type jsonQueryPrepData struct {
-	EncodedPlan string `json:"encoded_plan"`
-	Name        string `json:"name"`
-}
-
 // QueryMetrics encapsulates various metrics gathered during a queries execution.
 type QueryMetrics struct {
 	ElapsedTime   time.Duration

cluster_query_test.go

@@ -131,11 +131,6 @@ type mockQueryRowReader struct {
 	mockQueryRowReaderBase
 }
 
-type mockPreparedQueryRowReader struct {
-	Dataset []jsonQueryPrepData
-	mockQueryRowReaderBase
-}
-
 type mockQueryRowReaderBase struct {
 	Meta     []byte
 	MetaErr  error
@@ -159,17 +154,6 @@ func (arr *mockQueryRowReader) NextRow() []byte {
 	return arr.Suite.mustConvertToBytes(arr.Dataset[idx])
 }
 
-func (arr *mockPreparedQueryRowReader) NextRow() []byte {
-	if arr.idx == len(arr.Dataset) {
-		return nil
-	}
-
-	idx := arr.idx
-	arr.idx++
-
-	return arr.Suite.mustConvertToBytes(arr.Dataset[idx])
-}
-
 func (arr *mockQueryRowReaderBase) MetaData() ([]byte, error) {
 	return arr.Meta, arr.MetaErr
 }

cluster_queryindexes.go

@@ -522,7 +522,7 @@ func (qm *QueryIndexManager) WatchIndexes(bucketName string, watchList []string,
 			span.Context(),
 			bucketName,
 			&GetAllQueryIndexesOptions{
-				Timeout:       deadline.Sub(time.Now()),
+				Timeout:       time.Until(deadline),
 				RetryStrategy: opts.RetryStrategy,
 			})
 		if err != nil {
@@ -551,7 +551,7 @@ func (qm *QueryIndexManager) WatchIndexes(bucketName string, watchList []string,
 		}
 
 		// wait till our next poll interval
-		time.Sleep(sleepDeadline.Sub(time.Now()))
+		time.Sleep(time.Until(sleepDeadline))
 	}
 
 	return nil

cluster_usermgr.go

@@ -410,7 +410,7 @@ func (um *UserManager) UpsertUser(user User, opts *UpsertUserOptions) error {
 	var reqRoleStrs []string
 	for _, roleData := range user.Roles {
 		if roleData.Bucket == "" {
-			reqRoleStrs = append(reqRoleStrs, fmt.Sprintf("%s", roleData.Name))
+			reqRoleStrs = append(reqRoleStrs, roleData.Name)
 		} else {
 			reqRoleStrs = append(reqRoleStrs, fmt.Sprintf("%s[%s]", roleData.Name, roleData.Bucket))
 		}
@@ -704,7 +704,7 @@ func (um *UserManager) UpsertGroup(group Group, opts *UpsertGroupOptions) error
 	var reqRoleStrs []string
 	for _, roleData := range group.Roles {
 		if roleData.Bucket == "" {
-			reqRoleStrs = append(reqRoleStrs, fmt.Sprintf("%s", roleData.Name))
+			reqRoleStrs = append(reqRoleStrs, roleData.Name)
 		} else {
 			reqRoleStrs = append(reqRoleStrs, fmt.Sprintf("%s[%s]", roleData.Name, roleData.Bucket))
 		}

collection.go

@@ -50,11 +50,6 @@ func (c *Collection) scopeName() string {
 	return c.scope
 }
 
-func (c *Collection) clone() *Collection {
-	newC := *c
-	return &newC
-}
-
 // Name returns the name of the collection.
 func (c *Collection) Name() string {
 	return c.collectionName

collection_bulk.go

@@ -75,12 +75,10 @@ func (c *Collection) Do(ops []BulkOp, opts *BulkOpOptions) error {
 	}
 
 	for range ops {
-		select {
-		case item := <-signal:
-			// We're really just clearing the pendop from this thread,
-			//   since it already completed, no cancel actually occurs
-			item.finish()
-		}
+		item := <-signal
+		// We're really just clearing the pendop from this thread,
+		//   since it already completed, no cancel actually occurs
+		item.finish()
 	}
 	return nil
 }

collection_crud.go

@@ -674,7 +674,7 @@ func (c *Collection) GetAllReplicas(id string, opts *GetAllReplicaOptions) (docO
 	// Start a timer to close it after the deadline
 	go func() {
 		select {
-		case <-time.After(deadline.Sub(time.Now())):
+		case <-time.After(time.Until(deadline)):
 			// If we timeout, we should close the result
 			err := repRes.Close()
 			if err != nil {

collection_dura.go

@@ -144,7 +144,7 @@ func (c *Collection) waitForDurability(
 	persistCh := make(chan struct{}, numServers)
 
 	for replicaIdx := 0; replicaIdx < numServers; replicaIdx++ {
-		go c.observeOne(opm.TraceSpan(), docID, mt, replicaIdx, replicaCh, persistCh, subOpCancelCh, deadline.Sub(time.Now()))
+		go c.observeOne(opm.TraceSpan(), docID, mt, replicaIdx, replicaCh, persistCh, subOpCancelCh, time.Until(deadline))
 	}
 
 	numReplicated := uint(0)
@@ -156,7 +156,7 @@ func (c *Collection) waitForDurability(
 			numReplicated++
 		case <-persistCh:
 			numPersisted++
-		case <-time.After(deadline.Sub(time.Now())):
+		case <-time.After(time.Until(deadline)):
 			// deadline exceeded
 			close(subOpCancelCh)
 			return opm.EnhanceErr(ErrAmbiguousTimeout)

collection_subdoc.go

@@ -43,7 +43,7 @@ func (c *Collection) internalLookupIn(
 	var subdocs []gocbcore.SubDocOp
 	for _, op := range ops {
 		if op.op == memd.SubDocOpGet && op.path == "" {
-			if op.isXattr != false {
+			if op.isXattr {
 				return nil, errors.New("invalid xattr fetch with no path")
 			}
 
@@ -53,7 +53,7 @@ func (c *Collection) internalLookupIn(
 			})
 			continue
 		} else if op.op == memd.SubDocOpDictSet && op.path == "" {
-			if op.isXattr != false {
+			if op.isXattr {
 				return nil, errors.New("invalid xattr set with no path")
 			}
 

constants.go

@@ -7,8 +7,6 @@ import (
 
 const (
 	goCbVersionStr = "v2.1.1"
-
-	persistenceTimeoutFloor = 1500
 )
 
 // QueryIndexType provides information on the type of indexer used for an index.

constants_str.go

@@ -18,24 +18,6 @@ func serviceTypeToString(service ServiceType) string {
 	return ""
 }
 
-func serviceTypeFromString(service string) ServiceType {
-	switch service {
-	case "mgmt":
-		return ServiceTypeManagement
-	case "kv":
-		return ServiceTypeKeyValue
-	case "views":
-		return ServiceTypeViews
-	case "query":
-		return ServiceTypeQuery
-	case "search":
-		return ServiceTypeSearch
-	case "analytics":
-		return ServiceTypeAnalytics
-	}
-	return ServiceType(0)
-}
-
 func clusterStateToString(state ClusterState) string {
 	switch state {
 	case ClusterStateOnline:

error.go

@@ -7,10 +7,6 @@ import (
 	gocbcore "github.com/couchbase/gocbcore/v9"
 )
 
-func newCliInternalError(message string) error {
-	return errors.New(message)
-}
-
 type wrappedError struct {
 	Message    string
 	InnerError error

error_http.go

@@ -46,10 +46,6 @@ func makeGenericHTTPError(baseErr error, req *gocbcore.HTTPRequest, resp *gocbco
 	return err
 }
 
-func makeHTTPBadStatusError(message string, req *gocbcore.HTTPRequest, resp *gocbcore.HTTPResponse) error {
-	return makeGenericHTTPError(errors.New(message), req, resp)
-}
-
 func makeGenericMgmtError(baseErr error, req *mgmtRequest, resp *mgmtResponse) error {
 	if baseErr == nil {
 		logErrorf("makeGenericMgmtError got an empty error")

results.go

@@ -39,7 +39,7 @@ func (d *GetResult) Expiry() *time.Duration {
 }
 
 func (d *GetResult) fromFullProjection(ops []LookupInSpec, result *LookupInResult, fields []string) error {
-	if fields == nil || len(fields) == 0 {
+	if len(fields) == 0 {
 		// This is a special case where user specified a full doc fetch with expiration.
 		d.contents = result.contents[0].data
 		return nil
@@ -101,7 +101,6 @@ func (d *GetResult) fromSubDoc(ops []LookupInSpec, result *LookupInResult) error
 
 type subdocPath struct {
 	path    string
-	elem    int
 	isArray bool
 }
 
@@ -207,6 +206,7 @@ func (d *GetResult) set(paths []subdocPath, content interface{}, value interface
 		cMap, ok := content.(map[string]interface{})
 		if !ok {
 			// this isn't possible but the linter won't play nice without it
+			logErrorf("Failed to assert projection content to a map")
 		}
 		cMap[path.path] = make(map[string]interface{})
 		return d.set(paths[1:], cMap[path.path], value)
@@ -219,7 +219,6 @@ func (d *GetResult) set(paths []subdocPath, content interface{}, value interface
 type LookupInResult struct {
 	Result
 	contents []lookupInPartial
-	pathMap  map[string]int
 }
 
 type lookupInPartial struct {